Only allow oauth login against current login method (#1353)

Author: michaeljguarino

apps/core/lib/core/schema/account.ex

@@ -1,6 +1,7 @@
 defmodule Core.Schema.Account do
   use Piazza.Ecto.Schema
   use Arc.Ecto.Schema
+  import Core.Schema.Validations
   alias Core.Schema.{User, DomainMapping, PlatformSubscription, Address}
 
   schema "accounts" do
@@ -64,6 +65,7 @@ defmodule Core.Schema.Account do
     |> generate_uuid(:icon_id)
     |> cast_attachments(attrs, [:icon], allow_urls: true)
     |> set_address_updated()
+    |> reject_urls(:name)
   end
 
 

apps/core/lib/core/schema/validations.ex

@@ -0,0 +1,16 @@
+defmodule Core.Schema.Validations do
+  import Ecto.Changeset
+
+  @url_regex ~r/https?:\/\/(www\.)?[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6}\b([-a-zA-Z0-9@:%_\+.~#?&\/\/=]*)/
+
+  def reject_urls(cs, field) do
+    validate_change(cs, field, fn
+      _, val when is_binary(val) ->
+        case String.match?(val, @url_regex) do
+          true -> [{field, "cannot contain urls"}]
+          _ -> []
+        end
+      _, _ -> [{field, "must be a string"}]
+    end)
+  end
+end

apps/core/lib/core/services/cloud.ex

@@ -43,7 +43,13 @@ defmodule Core.Services.Cloud do
     |> add_operation(:cluster, fn _ -> select_cluster(attrs[:cloud], attrs[:region]) end)
     |> add_operation(:postgres, fn _ -> select_roach(attrs[:cloud]) end)
     |> add_operation(:sa, fn _ ->
-      Accounts.create_service_account(%{name: "#{name}-cloud-sa", email: "#{name}[email protected]"}, user)
+      Accounts.create_service_account(%{
+        name: "#{name}-cloud-sa",
+        email: "#{name}[email protected]",
+        impersonation_policy: %{
+          bindings: [%{user_id: user.id}]
+        }
+      }, user)
     end)
     |> add_operation(:token, fn %{sa: sa} -> Users.create_persisted_token(sa) end)
     |> add_operation(:install, fn %{sa: sa} ->

apps/core/lib/core/services/users.ex

@@ -373,8 +373,9 @@ defmodule Core.Services.Users do
         |> Map.merge(login_args(service))
         |> Map.put(:password, Ecto.UUID.generate())
         |> create_user()
-      %User{} = user ->
+      %User{login_method: ^service} = user ->
         update_user(login_args(service), user)
+      _ -> {:error, "you don't have login with #{service} enabled"}
     end
   end
 

apps/core/test/services/accounts_test.exs

@@ -147,6 +147,10 @@ defmodule Core.Services.AccountsTest do
       assert account.name == "updated"
     end
 
+    test "cannot put urls in names", %{user: user} do
+      {:error, _} = Accounts.update_account(%{name: "https://evil.com"}, user)
+    end
+
     test "if billing address is updated, it will update the stripe customer", %{user: user, account: account} do
       {:ok, _} = update_record(account, %{billing_customer_id: "strp"})
       me = self()
@@ -335,7 +339,7 @@ defmodule Core.Services.AccountsTest do
       assert invite.user_id == user.id
     end
 
-    test "nonroot users can create group members", %{account: account} do
+    test "nonroot users cannot create group members", %{account: account} do
       {:error, _} = Accounts.create_invite(%{email: "[email protected]"}, insert(:user, account: account))
     end
   end

apps/core/test/services/cloud_test.exs

@@ -32,6 +32,10 @@ defmodule Core.Services.CloudTest do
       assert refetch(cluster).count == 1
       assert refetch(postgres).count == 1
 
+      sa = Core.Services.Users.get_user_by_email("[email protected]")
+      %{impersonation_policy: %{bindings: [binding]}} = Core.Repo.preload(sa, [impersonation_policy: :bindings])
+      assert binding.user_id == user.id
+
       assert_receive {:event, %PubSub.ConsoleInstanceCreated{item: ^instance}}
     end
 

apps/core/test/services/users_test.exs

@@ -524,13 +524,19 @@ defmodule Core.Services.UsersTest do
     end
 
     test "it will update login method for existing users" do
-      user = insert(:user)
+      user = insert(:user, login_method: :google)
 
       {:ok, upd} = Users.bootstrap_user(:google, %{email: user.email})
 
       assert upd.id == user.id
       assert upd.login_method == :google
     end
+
+    test "it will not allow logins w/o login method set" do
+      user = insert(:user)
+
+      {:error, _} = Users.bootstrap_user(:google, %{email: user.email})
+    end
   end
 
   describe "#create_trust_relationship" do