Skip to content

Commit

Permalink
Only allow oauth login against current login method
Browse files Browse the repository at this point in the history
There's probably a needed frontend change here too, but can handle that after the fact
  • Loading branch information
michaeljguarino committed Aug 29, 2024
1 parent e551c8d commit 632d1d6
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 4 deletions.
2 changes: 2 additions & 0 deletions apps/core/lib/core/schema/account.ex
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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


Expand Down
16 changes: 16 additions & 0 deletions apps/core/lib/core/schema/validations.ex
Original file line number Diff line number Diff line change
@@ -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
8 changes: 7 additions & 1 deletion apps/core/lib/core/services/cloud.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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} ->
Expand Down
3 changes: 2 additions & 1 deletion apps/core/lib/core/services/users.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 5 additions & 1 deletion apps/core/test/services/accounts_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions apps/core/test/services/cloud_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 7 additions & 1 deletion apps/core/test/services/users_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 632d1d6

Please sign in to comment.