Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only allow oauth login against current login method #1353

Merged
merged 1 commit into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading