diff --git a/.github/workflows/elixir.yml b/.github/workflows/elixir.yml index c91e059e3..6cba41145 100644 --- a/.github/workflows/elixir.yml +++ b/.github/workflows/elixir.yml @@ -38,7 +38,7 @@ jobs: - uses: erlef/setup-beam@v1 with: otp-version: "25" - elixir-version: "1.14" + elixir-version: "1.15" - uses: actions/checkout@v3 - uses: actions/cache@v3 name: Elixir Deps Cache @@ -46,7 +46,7 @@ jobs: cache-name: cache-elixir-deps-${{ env.MIX_ENV }} with: path: elixir/deps - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} restore-keys: | ${{ runner.os }}-${{ env.cache-name }}- - uses: actions/cache@v3 @@ -55,7 +55,7 @@ jobs: cache-name: cache-elixir-build-${{ env.MIX_ENV }} with: path: elixir/_build - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} - name: Install Dependencies run: mix deps.get --only $MIX_ENV - name: Compile Dependencies @@ -66,11 +66,10 @@ jobs: run: | mix ecto.create mix ecto.migrate - - name: Run Tests and Upload Coverage Report + - name: Run Tests env: E2E_MAX_WAIT_SECONDS: 20 run: | - # XXX: This can fail when coveralls is down mix test --warnings-as-errors - name: Test Report uses: dorny/test-reporter@v1 @@ -92,7 +91,7 @@ jobs: id: setup-beam with: otp-version: "25" - elixir-version: "1.14" + elixir-version: "1.15" - uses: actions/checkout@v3 - uses: actions/cache@v3 name: Elixir Deps Cache @@ -100,7 +99,7 @@ jobs: cache-name: cache-elixir-deps-${{ env.MIX_ENV }} with: path: elixir/deps - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} restore-keys: | ${{ runner.os }}-${{ env.cache-name }}- - uses: actions/cache@v3 @@ -109,7 +108,7 @@ jobs: cache-name: cache-elixir-build-${{ env.MIX_ENV }} with: path: elixir/_build - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} - name: Install Dependencies run: mix deps.get --only $MIX_ENV - name: Compile Dependencies @@ -145,7 +144,7 @@ jobs: - uses: erlef/setup-beam@v1 with: otp-version: "25" - elixir-version: "1.14" + elixir-version: "1.15" - uses: actions/checkout@v3 - uses: actions/cache@v3 name: Elixir Deps Cache @@ -153,7 +152,7 @@ jobs: cache-name: cache-elixir-deps-${{ env.MIX_ENV }} with: path: elixir/deps - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} restore-keys: | ${{ runner.os }}-${{ env.cache-name }}- - uses: actions/cache@v3 @@ -162,7 +161,7 @@ jobs: cache-name: cache-elixir-build-${{ env.MIX_ENV }} with: path: elixir/_build - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} - name: Install Dependencies run: mix deps.get --only $MIX_ENV - name: Compile Dependencies @@ -214,14 +213,14 @@ jobs: - uses: erlef/setup-beam@v1 with: otp-version: "25" - elixir-version: "1.14" + elixir-version: "1.15" - uses: actions/cache@v3 name: Elixir Deps Cache env: cache-name: cache-elixir-deps-${{ env.MIX_ENV }}-${{ env.MIX_ENV }} with: path: elixir/deps - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} restore-keys: | ${{ runner.os }}-${{ env.cache-name }}- - uses: actions/cache@v3 @@ -230,7 +229,7 @@ jobs: cache-name: cache-elixir-build-${{ env.MIX_ENV }} with: path: elixir/_build - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} - name: Install Dependencies run: mix deps.get --only $MIX_ENV - name: Compile @@ -316,7 +315,7 @@ jobs: - uses: erlef/setup-beam@v1 with: otp-version: "25" - elixir-version: "1.14" + elixir-version: "1.15" - uses: actions/setup-node@v3 with: node-version: 18 @@ -330,7 +329,7 @@ jobs: cache-name: cache-elixir-deps-${{ env.MIX_ENV }}-${{ env.MIX_ENV }} with: path: elixir/deps - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} restore-keys: | ${{ runner.os }}-${{ env.cache-name }}- - uses: actions/cache@v3 @@ -339,7 +338,7 @@ jobs: cache-name: cache-elixir-build-${{ env.MIX_ENV }} with: path: elixir/_build - key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} + key: ${{ runner.os }}-${{ steps.setup-beam.outputs.elixir-version }}-${{ steps.setup-beam.outputs.otp-version }}-${{ env.cache-name }}-${{ hashFiles('**/elixir/mix.lock') }} - uses: actions/cache@v3 name: pnpm Deps Cache env: @@ -356,6 +355,7 @@ jobs: key: ${{ runner.os }}-${{ env.cache-name }}-${{ hashFiles('**/pnpm-lock.yaml') }} - run: | export DISPLAY=:99 + chromedriver --url-base=/wd/hub & sudo Xvfb -ac :99 -screen 0 1280x1024x24 > /dev/null 2>&1 & - name: Install Dependencies run: mix deps.get --only $MIX_ENV diff --git a/.tool-versions b/.tool-versions index aba54f7d9..0d075abc2 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,8 +1,8 @@ # These are used for the dev environment. # This should match the versions used in the built product. nodejs 18.16.0 -elixir 1.14.4-otp-25 -erlang 25.3.2 +elixir 1.15.0-otp-25 +erlang 25.3.2.2 terraform 1.5.0 # Used for static analysis diff --git a/docker-compose.yml b/docker-compose.yml index 22b84f6bb..5a1f462d7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -26,16 +26,31 @@ services: - app vault: - image: vault + image: vault:1.13.3 environment: VAULT_ADDR: 'http://127.0.0.1:8200' VAULT_DEV_ROOT_TOKEN_ID: 'firezone' + VAULT_LOG_LEVEL: 'debug' ports: - 8200:8200/tcp cap_add: - IPC_LOCK networks: - app + healthcheck: + test: + [ + "CMD", + "wget", + "--spider", + "--proxy", + "off", + "http://localhost:8200/v1/sys/health?standbyok=true" + ] + interval: 10s + timeout: 3s + retries: 10 + start_period: 5s # Firezone Components web: @@ -93,6 +108,8 @@ services: # Client info USER_AGENT: "iOS/12.5 (iPhone) connlib/0.7.412" depends_on: + vault: + condition: 'service_healthy' postgres: condition: 'service_healthy' networks: @@ -218,6 +235,8 @@ services: # Seeds STATIC_SEEDS: "true" depends_on: + vault: + condition: 'service_healthy' postgres: condition: 'service_healthy' networks: diff --git a/elixir/Dockerfile b/elixir/Dockerfile index 6cd1ea722..91155e4dc 100644 --- a/elixir/Dockerfile +++ b/elixir/Dockerfile @@ -1,6 +1,6 @@ -ARG ELIXIR_VERSION=1.14.3 -ARG OTP_VERSION=25.2.1 -ARG ALPINE_VERSION=3.16.3 +ARG ALPINE_VERSION=3.18.2 +ARG OTP_VERSION=26.0.1 +ARG ELIXIR_VERSION=1.15.0 ARG BUILDER_IMAGE="firezone/elixir:${ELIXIR_VERSION}-otp-${OTP_VERSION}" ARG RUNNER_IMAGE="alpine:${ALPINE_VERSION}" diff --git a/elixir/apps/api/mix.exs b/elixir/apps/api/mix.exs index 94278c3a4..8d64eb090 100644 --- a/elixir/apps/api/mix.exs +++ b/elixir/apps/api/mix.exs @@ -56,9 +56,14 @@ defmodule API.MixProject do # Other deps {:jason, "~> 1.2"}, - {:remote_ip, "~> 1.1"} + {:remote_ip, "~> 1.1"}, # Test deps + {:credo, "~> 1.5", only: [:dev, :test], runtime: false}, + {:dialyxir, "~> 1.1", only: [:dev], runtime: false}, + {:junit_formatter, "~> 3.3", only: [:test]}, + {:mix_audit, "~> 2.1", only: [:dev, :test]}, + {:sobelow, "~> 0.12", only: [:dev, :test]} ] end diff --git a/elixir/apps/api/test/api/device/socket_test.exs b/elixir/apps/api/test/api/device/socket_test.exs index fb51d7e79..42d3a76f8 100644 --- a/elixir/apps/api/test/api/device/socket_test.exs +++ b/elixir/apps/api/test/api/device/socket_test.exs @@ -12,12 +12,12 @@ defmodule API.Device.SocketTest do describe "connect/3" do test "returns error when token is missing" do - assert connect(Socket, %{}, @connect_info) == {:error, :missing_token} + assert connect(Socket, %{}, connect_info: @connect_info) == {:error, :missing_token} end test "returns error when token is invalid" do attrs = connect_attrs(token: "foo") - assert connect(Socket, attrs, @connect_info) == {:error, :invalid_token} + assert connect(Socket, attrs, connect_info: @connect_info) == {:error, :invalid_token} end test "creates a new device" do @@ -26,7 +26,7 @@ defmodule API.Device.SocketTest do attrs = connect_attrs(token: token) - assert {:ok, socket} = connect(Socket, attrs, connect_info(subject)) + assert {:ok, socket} = connect(Socket, attrs, connect_info: connect_info(subject)) assert device = Map.fetch!(socket.assigns, :device) assert device.external_id == attrs["external_id"] @@ -43,7 +43,7 @@ defmodule API.Device.SocketTest do attrs = connect_attrs(token: token, external_id: existing_device.external_id) - assert {:ok, socket} = connect(Socket, attrs, connect_info(subject)) + assert {:ok, socket} = connect(Socket, attrs, connect_info: connect_info(subject)) assert device = Repo.one(Domain.Devices.Device) assert device.id == socket.assigns.device.id end diff --git a/elixir/apps/api/test/api/gateway/socket_test.exs b/elixir/apps/api/test/api/gateway/socket_test.exs index 4a37a790f..4e64d1644 100644 --- a/elixir/apps/api/test/api/gateway/socket_test.exs +++ b/elixir/apps/api/test/api/gateway/socket_test.exs @@ -14,7 +14,7 @@ defmodule API.Gateway.SocketTest do describe "connect/3" do test "returns error when token is missing" do - assert connect(Socket, %{}, @connect_info) == {:error, :missing_token} + assert connect(Socket, %{}, connect_info: @connect_info) == {:error, :missing_token} end test "creates a new gateway" do @@ -23,7 +23,7 @@ defmodule API.Gateway.SocketTest do attrs = connect_attrs(token: encrypted_secret) - assert {:ok, socket} = connect(Socket, attrs, @connect_info) + assert {:ok, socket} = connect(Socket, attrs, connect_info: @connect_info) assert gateway = Map.fetch!(socket.assigns, :gateway) assert gateway.external_id == attrs["external_id"] @@ -40,14 +40,14 @@ defmodule API.Gateway.SocketTest do attrs = connect_attrs(token: encrypted_secret, external_id: existing_gateway.external_id) - assert {:ok, socket} = connect(Socket, attrs, @connect_info) + assert {:ok, socket} = connect(Socket, attrs, connect_info: @connect_info) assert gateway = Repo.one(Domain.Gateways.Gateway) assert gateway.id == socket.assigns.gateway.id end test "returns error when token is invalid" do attrs = connect_attrs(token: "foo") - assert connect(Socket, attrs, @connect_info) == {:error, :invalid_token} + assert connect(Socket, attrs, connect_info: @connect_info) == {:error, :invalid_token} end end diff --git a/elixir/apps/api/test/api/relay/socket_test.exs b/elixir/apps/api/test/api/relay/socket_test.exs index 1043b001f..69291d01b 100644 --- a/elixir/apps/api/test/api/relay/socket_test.exs +++ b/elixir/apps/api/test/api/relay/socket_test.exs @@ -14,7 +14,7 @@ defmodule API.Relay.SocketTest do describe "connect/3" do test "returns error when token is missing" do - assert connect(Socket, %{}, @connect_info) == {:error, :missing_token} + assert connect(Socket, %{}, connect_info: @connect_info) == {:error, :missing_token} end test "creates a new relay" do @@ -23,7 +23,7 @@ defmodule API.Relay.SocketTest do attrs = connect_attrs(token: encrypted_secret) - assert {:ok, socket} = connect(Socket, attrs, @connect_info) + assert {:ok, socket} = connect(Socket, attrs, connect_info: @connect_info) assert relay = Map.fetch!(socket.assigns, :relay) assert relay.ipv4.address == attrs["ipv4"] @@ -40,14 +40,14 @@ defmodule API.Relay.SocketTest do attrs = connect_attrs(token: encrypted_secret, ipv4: existing_relay.ipv4) - assert {:ok, socket} = connect(Socket, attrs, @connect_info) + assert {:ok, socket} = connect(Socket, attrs, connect_info: @connect_info) assert relay = Repo.one(Domain.Relays.Relay) assert relay.id == socket.assigns.relay.id end test "returns error when token is invalid" do attrs = connect_attrs(token: "foo") - assert connect(Socket, attrs, @connect_info) == {:error, :invalid_token} + assert connect(Socket, attrs, connect_info: @connect_info) == {:error, :invalid_token} end end diff --git a/elixir/apps/domain/lib/domain/accounts.ex b/elixir/apps/domain/lib/domain/accounts.ex index 572f95405..6f067ef16 100644 --- a/elixir/apps/domain/lib/domain/accounts.ex +++ b/elixir/apps/domain/lib/domain/accounts.ex @@ -1,8 +1,17 @@ defmodule Domain.Accounts do - alias Domain.Repo + alias Domain.{Repo, Validator} alias Domain.Auth alias Domain.Accounts.Account + def fetch_account_by_id(id) do + if Validator.valid_uuid?(id) do + Account.Query.by_id(id) + |> Repo.fetch() + else + {:error, :not_found} + end + end + def create_account(attrs) do Account.Changeset.create_changeset(attrs) |> Repo.insert() diff --git a/elixir/apps/domain/lib/domain/accounts/account/query.ex b/elixir/apps/domain/lib/domain/accounts/account/query.ex new file mode 100644 index 000000000..ddb4185d9 --- /dev/null +++ b/elixir/apps/domain/lib/domain/accounts/account/query.ex @@ -0,0 +1,12 @@ +defmodule Domain.Accounts.Account.Query do + use Domain, :query + + def all do + from(account in Domain.Accounts.Account, as: :account) + # |> where([account: account], is_nil(account.deleted_at)) + end + + def by_id(queryable \\ all(), id) do + where(queryable, [account: account], account.id == ^id) + end +end diff --git a/elixir/apps/domain/lib/domain/actors/actor.ex b/elixir/apps/domain/lib/domain/actors/actor.ex index 4e009b4b3..180783d23 100644 --- a/elixir/apps/domain/lib/domain/actors/actor.ex +++ b/elixir/apps/domain/lib/domain/actors/actor.ex @@ -4,9 +4,7 @@ defmodule Domain.Actors.Actor do schema "actors" do field :type, Ecto.Enum, values: [:account_user, :account_admin_user, :service_account] - # TODO: - # field :first_name, :string - # field :last_name, :string + field :name, :string has_many :identities, Domain.Auth.Identity diff --git a/elixir/apps/domain/lib/domain/actors/actor/changeset.ex b/elixir/apps/domain/lib/domain/actors/actor/changeset.ex index 3a1d8b3ea..f63837ceb 100644 --- a/elixir/apps/domain/lib/domain/actors/actor/changeset.ex +++ b/elixir/apps/domain/lib/domain/actors/actor/changeset.ex @@ -5,8 +5,8 @@ defmodule Domain.Actors.Actor.Changeset do def create_changeset(%Auth.Provider{} = provider, attrs) do %Actors.Actor{} - |> cast(attrs, ~w[type]a) - |> validate_required(~w[type]a) + |> cast(attrs, ~w[type name]a) + |> validate_required(~w[type name]a) |> put_change(:account_id, provider.account_id) end diff --git a/elixir/apps/domain/lib/domain/auth.ex b/elixir/apps/domain/lib/domain/auth.ex index 7daaab5db..87a75dfd0 100644 --- a/elixir/apps/domain/lib/domain/auth.ex +++ b/elixir/apps/domain/lib/domain/auth.ex @@ -1,7 +1,6 @@ defmodule Domain.Auth do use Supervisor - alias Domain.Repo - alias Domain.Config + alias Domain.{Repo, Config, Validator} alias Domain.{Accounts, Actors} alias Domain.Auth.{Authorizer, Subject, Context, Permission, Roles, Role, Identity} alias Domain.Auth.{Adapters, Provider} @@ -25,9 +24,20 @@ defmodule Domain.Auth do # Providers - def fetch_provider_by_id(id) do - Provider.Query.by_id(id) - |> Repo.fetch() + def fetch_active_provider_by_id(id) do + if Validator.valid_uuid?(id) do + Provider.Query.by_id(id) + |> Provider.Query.not_disabled() + |> Repo.fetch() + else + {:error, :not_found} + end + end + + def list_active_providers_for_account(%Accounts.Account{} = account) do + Provider.Query.by_account_id(account.id) + |> Provider.Query.not_disabled() + |> Repo.list() end def create_provider(%Accounts.Account{} = account, attrs, %Subject{} = subject) do @@ -173,11 +183,13 @@ defmodule Domain.Auth do # Sign Up / In / Off - def sign_in(%Provider{} = provider, provider_identifier, secret, user_agent, remote_ip) do - with {:ok, identity} <- - fetch_identity_by_provider_and_identifier(provider, provider_identifier), - {:ok, identity, expires_at} <- - Adapters.verify_secret(provider, identity, secret) do + def sign_in(%Provider{} = provider, id_or_provider_identifier, secret, user_agent, remote_ip) do + identity_queryable = + Identity.Query.by_provider_id(provider.id) + |> Identity.Query.by_id_or_provider_identifier(id_or_provider_identifier) + + with {:ok, identity} <- Repo.fetch(identity_queryable), + {:ok, identity, expires_at} <- Adapters.verify_secret(provider, identity, secret) do {:ok, build_subject(identity, expires_at, user_agent, remote_ip)} else {:error, :not_found} -> {:error, :unauthorized} @@ -186,7 +198,18 @@ defmodule Domain.Auth do end end - def sign_in(session_token, user_agent, remote_ip) do + def sign_in(%Provider{} = provider, payload, user_agent, remote_ip) do + with {:ok, identity, expires_at} <- + Adapters.verify_identity(provider, payload) do + {:ok, build_subject(identity, expires_at, user_agent, remote_ip)} + else + {:error, :not_found} -> {:error, :unauthorized} + {:error, :invalid} -> {:error, :unauthorized} + {:error, :expired} -> {:error, :unauthorized} + end + end + + def sign_in(session_token, user_agent, remote_ip) when is_binary(session_token) do with {:ok, identity, expires_at} <- verify_session_token(session_token, user_agent, remote_ip) do {:ok, build_subject(identity, expires_at, user_agent, remote_ip)} @@ -198,7 +221,7 @@ defmodule Domain.Auth do end end - defp fetch_identity_by_provider_and_identifier(%Provider{} = provider, provider_identifier) do + def fetch_identity_by_provider_and_identifier(%Provider{} = provider, provider_identifier) do Identity.Query.by_provider_id(provider.id) |> Identity.Query.by_provider_identifier(provider_identifier) |> Repo.fetch() @@ -298,7 +321,7 @@ defmodule Domain.Auth do _remote_ip ) do with {:ok, identity} <- fetch_identity_by_id(identity_id), - {:ok, provider} <- fetch_provider_by_id(identity.provider_id), + {:ok, provider} <- fetch_active_provider_by_id(identity.provider_id), {:ok, identity, expires_at} <- Adapters.verify_secret(provider, identity, secret) do {:ok, identity, expires_at} diff --git a/elixir/apps/domain/lib/domain/auth/adapter.ex b/elixir/apps/domain/lib/domain/auth/adapter.ex index 9574984ec..70ccbdcaf 100644 --- a/elixir/apps/domain/lib/domain/auth/adapter.ex +++ b/elixir/apps/domain/lib/domain/auth/adapter.ex @@ -25,13 +25,28 @@ defmodule Domain.Auth.Adapter do @callback ensure_deprovisioned(%Ecto.Changeset{data: %Provider{}}) :: %Ecto.Changeset{data: %Provider{}} - @doc """ - A callback invoked during sign-in, should verify the secret and return the identity - if it's valid, or an error otherwise. - """ - @callback verify_secret(%Identity{}, secret :: term()) :: - {:ok, %Identity{}, expires_at :: %DateTime{} | nil} - | {:error, :invalid_secret} - | {:error, :expired_secret} - | {:error, :internal_error} + defmodule Local do + @doc """ + A callback invoked during sign-in, should verify the secret and return the identity + if it's valid, or an error otherwise. + + Used by secret-based providers, eg.: UserPass, Email. + """ + @callback verify_secret(%Identity{}, secret :: term()) :: + {:ok, %Identity{}, expires_at :: %DateTime{} | nil} + | {:error, :invalid_secret} + | {:error, :expired_secret} + | {:error, :internal_error} + end + + defmodule IdP do + @doc """ + Used for adapters that are not secret-based, eg. OpenID Connect. + """ + @callback verify_identity(%Provider{}, payload :: term()) :: + {:ok, %Identity{}, expires_at :: %DateTime{} | nil} + | {:error, :invalid_secret} + | {:error, :expired_secret} + | {:error, :internal_error} + end end diff --git a/elixir/apps/domain/lib/domain/auth/adapters.ex b/elixir/apps/domain/lib/domain/auth/adapters.ex index 2f9c16cce..c1507f363 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters.ex @@ -52,6 +52,18 @@ defmodule Domain.Auth.Adapters do end end + def verify_identity(%Provider{} = provider, payload) do + adapter = fetch_adapter!(provider) + + case adapter.verify_identity(provider, payload) do + {:ok, %Identity{} = identity, expires_at} -> {:ok, identity, expires_at} + {:error, :not_found} -> {:error, :not_found} + {:error, :invalid} -> {:error, :invalid} + {:error, :expired} -> {:error, :expired} + {:error, :internal_error} -> {:error, :internal_error} + end + end + defp fetch_adapter!(provider) do Map.fetch!(@adapters, provider.adapter) end diff --git a/elixir/apps/domain/lib/domain/auth/adapters/email.ex b/elixir/apps/domain/lib/domain/auth/adapters/email.ex index 4026a2ef0..7f5d5cf0d 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/email.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/email.ex @@ -4,6 +4,7 @@ defmodule Domain.Auth.Adapters.Email do alias Domain.Auth.{Identity, Provider, Adapter} @behaviour Adapter + @behaviour Adapter.Local @sign_in_token_expiration_seconds 15 * 60 diff --git a/elixir/apps/domain/lib/domain/auth/adapters/openid_connect.ex b/elixir/apps/domain/lib/domain/auth/adapters/openid_connect.ex index 3ad12ad67..a2476a61e 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/openid_connect.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/openid_connect.ex @@ -6,6 +6,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do require Logger @behaviour Adapter + @behaviour Adapter.IdP def start_link(_init_arg) do Supervisor.start_link(__MODULE__, nil, name: __MODULE__) @@ -69,8 +70,8 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do end @impl true - def verify_secret(%Identity{} = identity, {redirect_uri, code_verifier, code}) do - sync_identity(identity, %{ + def verify_identity(%Provider{} = provider, {redirect_uri, code_verifier, code}) do + sync_identity(provider, %{ grant_type: "authorization_code", redirect_uri: redirect_uri, code: code, @@ -78,25 +79,28 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do }) |> case do {:ok, identity, expires_at} -> {:ok, identity, expires_at} - {:error, :expired_token} -> {:error, :expired_secret} - {:error, :invalid_token} -> {:error, :invalid_secret} + {:error, :not_found} -> {:error, :not_found} + {:error, :expired_token} -> {:error, :expired} + {:error, :invalid_token} -> {:error, :invalid} {:error, :internal_error} -> {:error, :internal_error} end end def refresh_token(%Identity{} = identity) do - sync_identity(identity, %{ + identity = Repo.preload(identity, :provider) + + sync_identity(identity.provider, %{ grant_type: "refresh_token", refresh_token: identity.provider_state["refresh_token"] }) end - defp sync_identity(%Identity{} = identity, token_params) do - {config, identity} = config_for_identity(identity) + defp sync_identity(%Provider{} = provider, token_params) do + config = config_for_provider(provider) with {:ok, tokens} <- OpenIDConnect.fetch_tokens(config, token_params), {:ok, claims} <- OpenIDConnect.verify(config, tokens["id_token"]), - {:ok, userinfo} <- OpenIDConnect.fetch_userinfo(config, tokens["id_token"]) do + {:ok, userinfo} <- OpenIDConnect.fetch_userinfo(config, tokens["access_token"]) do # TODO: sync groups expires_at = cond do @@ -110,17 +114,22 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do nil end - Identity.Query.by_id(identity.id) + provider_identifier = claims["sub"] + + Identity.Query.by_provider_id(provider.id) + |> Identity.Query.by_provider_identifier(provider_identifier) |> Repo.fetch_and_update( with: fn identity -> - Identity.Changeset.update_provider_state(identity, %{ - id_token: tokens["id_token"], - access_token: tokens["access_token"], - refresh_token: tokens["refresh_token"], - expires_at: expires_at, - userinfo: userinfo, - claims: claims - }) + Identity.Changeset.update_provider_state( + identity, + %{ + access_token: tokens["access_token"], + refresh_token: tokens["refresh_token"], + expires_at: expires_at, + userinfo: userinfo, + claims: claims + } + ) end ) |> case do @@ -136,8 +145,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do {:error, other} -> Logger.error("Failed to connect OpenID Connect provider", - provider_id: identity.provider_id, - identity_id: identity.id, + provider_id: provider.id, reason: inspect(other) ) @@ -145,11 +153,6 @@ defmodule Domain.Auth.Adapters.OpenIDConnect do end end - defp config_for_identity(%Identity{} = identity) do - identity = Repo.preload(identity, :provider) - {config_for_provider(identity.provider), identity} - end - defp config_for_provider(%Provider{} = provider) do Ecto.embedded_load(Settings, provider.adapter_config, :json) |> Map.from_struct() diff --git a/elixir/apps/domain/lib/domain/auth/adapters/token.ex b/elixir/apps/domain/lib/domain/auth/adapters/token.ex index 59b5070f8..7a29591c8 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/token.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/token.ex @@ -9,6 +9,7 @@ defmodule Domain.Auth.Adapters.Token do alias Domain.Auth.Adapters.Token.State @behaviour Adapter + @behaviour Adapter.Local def start_link(_init_arg) do Supervisor.start_link(__MODULE__, nil, name: __MODULE__) diff --git a/elixir/apps/domain/lib/domain/auth/adapters/userpass.ex b/elixir/apps/domain/lib/domain/auth/adapters/userpass.ex index 87d01004c..a6f6d145c 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/userpass.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/userpass.ex @@ -9,6 +9,7 @@ defmodule Domain.Auth.Adapters.UserPass do alias Domain.Auth.Adapters.UserPass.Password @behaviour Adapter + @behaviour Adapter.Local def start_link(_init_arg) do Supervisor.start_link(__MODULE__, nil, name: __MODULE__) diff --git a/elixir/apps/domain/lib/domain/auth/adapters/userpass/password/changeset.ex b/elixir/apps/domain/lib/domain/auth/adapters/userpass/password/changeset.ex index a8a245dd8..f7123895e 100644 --- a/elixir/apps/domain/lib/domain/auth/adapters/userpass/password/changeset.ex +++ b/elixir/apps/domain/lib/domain/auth/adapters/userpass/password/changeset.ex @@ -4,7 +4,7 @@ defmodule Domain.Auth.Adapters.UserPass.Password.Changeset do @fields ~w[password]a @min_password_length 12 - @max_password_length 64 + @max_password_length 72 def create_changeset(attrs) do changeset(%Password{}, attrs) @@ -15,7 +15,18 @@ defmodule Domain.Auth.Adapters.UserPass.Password.Changeset do |> cast(attrs, @fields) |> validate_required(@fields) |> validate_confirmation(:password, required: true) - |> validate_length(:password, min: @min_password_length, max: @max_password_length) + |> validate_length(:password, + min: @min_password_length, + max: @max_password_length, + count: :bytes + ) + # We can improve password strength checks later if we decide to run this provider in production. + # |> validate_format(:password, ~r/[a-z]/, message: "at least one lower case character") + # |> validate_format(:password, ~r/[A-Z]/, message: "at least one upper case character") + # |> validate_format(:password, ~r/[!?@#$%^&*_0-9]/, message: "at least one digit or punctuation character") + # |> validate_no_repetitive_characters(:password) + # |> validate_no_sequential_characters(:password) + # |> validate_no_public_context(:password) |> put_hash(:password, to: :password_hash) |> redact_field(:password) |> redact_field(:password_confirmation) diff --git a/elixir/apps/domain/lib/domain/auth/identity/query.ex b/elixir/apps/domain/lib/domain/auth/identity/query.ex index 1b6534b94..410fd2233 100644 --- a/elixir/apps/domain/lib/domain/auth/identity/query.ex +++ b/elixir/apps/domain/lib/domain/auth/identity/query.ex @@ -4,6 +4,9 @@ defmodule Domain.Auth.Identity.Query do def all do from(identities in Domain.Auth.Identity, as: :identities) |> where([identities: identities], is_nil(identities.deleted_at)) + |> join(:inner, [identities: identities], actors in assoc(identities, :actor), as: :actors) + |> where([actors: actors], is_nil(actors.deleted_at)) + |> where([actors: actors], is_nil(actors.disabled_at)) end def by_id(queryable \\ all(), id) @@ -43,6 +46,19 @@ defmodule Domain.Auth.Identity.Query do ) end + def by_id_or_provider_identifier(queryable \\ all(), id_or_provider_identifier) do + if Domain.Validator.valid_uuid?(id_or_provider_identifier) do + where( + queryable, + [identities: identities], + identities.provider_identifier == ^id_or_provider_identifier or + identities.id == ^id_or_provider_identifier + ) + else + by_provider_identifier(queryable, id_or_provider_identifier) + end + end + def lock(queryable \\ all()) do lock(queryable, "FOR UPDATE") end diff --git a/elixir/apps/domain/lib/domain/auth/provider/changeset.ex b/elixir/apps/domain/lib/domain/auth/provider/changeset.ex index 0699d2ed8..59314f066 100644 --- a/elixir/apps/domain/lib/domain/auth/provider/changeset.ex +++ b/elixir/apps/domain/lib/domain/auth/provider/changeset.ex @@ -12,6 +12,14 @@ defmodule Domain.Auth.Provider.Changeset do |> put_change(:account_id, account.id) |> validate_length(:name, min: 1, max: 255) |> validate_required(@required_fields) + |> unique_constraint(:adapter, + name: :auth_providers_account_id_adapter_index, + message: "this provider is already enabled" + ) + |> unique_constraint(:adapter, + name: :auth_providers_account_id_oidc_adapter_index, + message: "this provider is already connected" + ) end def disable_provider(%Provider{} = provider) do diff --git a/elixir/apps/domain/lib/domain/config/errors.ex b/elixir/apps/domain/lib/domain/config/errors.ex index 2fda26c4b..cf04a8950 100644 --- a/elixir/apps/domain/lib/domain/config/errors.ex +++ b/elixir/apps/domain/lib/domain/config/errors.ex @@ -91,7 +91,7 @@ defmodule Domain.Config.Errors do end def legacy_key_used(key, legacy_key, removed_at) do - Logger.warn( + Logger.warning( "A legacy configuration option '#{legacy_key}' is used and it will be removed in v#{removed_at}. " <> "Please use '#{Domain.Config.Resolver.env_key(key)}' configuration option instead." ) diff --git a/elixir/apps/domain/lib/domain/validator.ex b/elixir/apps/domain/lib/domain/validator.ex index caca8f4dd..ef35ba7a5 100644 --- a/elixir/apps/domain/lib/domain/validator.ex +++ b/elixir/apps/domain/lib/domain/validator.ex @@ -21,7 +21,9 @@ defmodule Domain.Validator do end def validate_email(changeset, field) do - validate_format(changeset, field, ~r/@/, message: "is invalid email address") + changeset + |> validate_format(field, ~r/^[^\s]+@[^\s]+$/, message: "is an invalid email address") + |> validate_length(field, max: 160) end def validate_uri(changeset, field, opts \\ []) when is_atom(field) do diff --git a/elixir/apps/domain/mix.exs b/elixir/apps/domain/mix.exs index 233ccd8c3..29259544e 100644 --- a/elixir/apps/domain/mix.exs +++ b/elixir/apps/domain/mix.exs @@ -35,7 +35,8 @@ defmodule Domain.MixProject do extra_applications: [ :logger, :runtime_tools, - :crypto + :crypto, + :dialyzer ] ] end @@ -77,7 +78,12 @@ defmodule Domain.MixProject do {:opentelemetry_finch, "~> 0.2.0"}, # Test and dev deps - {:bypass, "~> 2.1", only: :test} + {:bypass, "~> 2.1", only: :test}, + {:credo, "~> 1.5", only: [:dev, :test], runtime: false}, + {:dialyxir, "~> 1.1", only: [:dev], runtime: false}, + {:junit_formatter, "~> 3.3", only: [:test]}, + {:mix_audit, "~> 2.1", only: [:dev, :test]}, + {:sobelow, "~> 0.12", only: [:dev, :test]} ] end diff --git a/elixir/apps/domain/priv/repo/migrations/20230616162535_make_auth_providers_unique.exs b/elixir/apps/domain/priv/repo/migrations/20230616162535_make_auth_providers_unique.exs new file mode 100644 index 000000000..ea76428e5 --- /dev/null +++ b/elixir/apps/domain/priv/repo/migrations/20230616162535_make_auth_providers_unique.exs @@ -0,0 +1,17 @@ +defmodule Domain.Repo.Migrations.MakeAuthProvidersUnique do + use Ecto.Migration + + def change do + create( + index(:auth_providers, [:account_id, :adapter], + unique: true, + where: "deleted_at IS NULL AND adapter in ('email', 'userpass', 'token')" + ) + ) + + execute(""" + CREATE UNIQUE INDEX auth_providers_account_id_oidc_adapter_index ON auth_providers (account_id, adapter, (adapter_config->>'client_id')) + WHERE deleted_at IS NULL AND adapter = 'openid_connect'; + """) + end +end diff --git a/elixir/apps/domain/priv/repo/migrations/20230619192420_add_actor_name.exs b/elixir/apps/domain/priv/repo/migrations/20230619192420_add_actor_name.exs new file mode 100644 index 000000000..3834b73b0 --- /dev/null +++ b/elixir/apps/domain/priv/repo/migrations/20230619192420_add_actor_name.exs @@ -0,0 +1,9 @@ +defmodule Domain.Repo.Migrations.AddActorName do + use Ecto.Migration + + def change do + alter table(:actors) do + add(:name, :string, null: false) + end + end +end diff --git a/elixir/apps/domain/test/domain/accounts_test.exs b/elixir/apps/domain/test/domain/accounts_test.exs new file mode 100644 index 000000000..508d1c52a --- /dev/null +++ b/elixir/apps/domain/test/domain/accounts_test.exs @@ -0,0 +1,36 @@ +defmodule Domain.AccountsTest do + use Domain.DataCase, async: true + import Domain.Accounts + alias Domain.{AccountsFixtures, AuthFixtures} + + describe "fetch_account_by_id/1" do + test "returns error when account is not found" do + assert fetch_account_by_id(Ecto.UUID.generate()) == {:error, :not_found} + end + + test "returns error when id is not a valid UUIDv4" do + assert fetch_account_by_id("foo") == {:error, :not_found} + end + + test "returns account" do + account = AccountsFixtures.create_account() + assert {:ok, returned_account} = fetch_account_by_id(account.id) + assert returned_account.id == account.id + end + end + + describe "ensure_has_access_to/2" do + test "returns :ok if subject has access to the account" do + subject = AuthFixtures.create_subject() + + assert ensure_has_access_to(subject, subject.account) == :ok + end + + test "returns :error if subject has no access to the account" do + account = AccountsFixtures.create_account() + subject = AuthFixtures.create_subject() + + assert ensure_has_access_to(subject, account) == {:error, :unauthorized} + end + end +end diff --git a/elixir/apps/domain/test/domain/actors_test.exs b/elixir/apps/domain/test/domain/actors_test.exs index 854dbb57c..9610676e2 100644 --- a/elixir/apps/domain/test/domain/actors_test.exs +++ b/elixir/apps/domain/test/domain/actors_test.exs @@ -212,7 +212,8 @@ defmodule Domain.ActorsTest do refute changeset.valid? assert errors_on(changeset) == %{ - type: ["can't be blank"] + type: ["can't be blank"], + name: ["can't be blank"] } end @@ -341,12 +342,12 @@ defmodule Domain.ActorsTest do MapSet.difference(admin_permissions, MapSet.new(required_permissions)) |> MapSet.to_list() - attrs = %{type: :account_admin_user} + attrs = %{type: :account_admin_user, name: "John Smith"} assert create_actor(provider, provider_identifier, attrs, subject) == {:error, {:unauthorized, privilege_escalation: missing_permissions}} - attrs = %{"type" => "account_admin_user"} + attrs = %{"type" => "account_admin_user", "name" => "John Smith"} assert create_actor(provider, provider_identifier, attrs, subject) == {:error, {:unauthorized, privilege_escalation: missing_permissions}} @@ -452,12 +453,35 @@ defmodule Domain.ActorsTest do allow_child_sandbox_access(test_pid) account = AccountsFixtures.create_account() + provider = AuthFixtures.create_email_provider(account: account) - actor_one = ActorsFixtures.create_actor(type: :account_admin_user, account: account) - actor_two = ActorsFixtures.create_actor(type: :account_admin_user, account: account) + actor_one = + ActorsFixtures.create_actor( + type: :account_admin_user, + account: account, + provider: provider + ) - identity_one = AuthFixtures.create_identity(account: account, actor: actor_one) - identity_two = AuthFixtures.create_identity(account: account, actor: actor_two) + actor_two = + ActorsFixtures.create_actor( + type: :account_admin_user, + account: account, + provider: provider + ) + + identity_one = + AuthFixtures.create_identity( + account: account, + actor: actor_one, + provider: provider + ) + + identity_two = + AuthFixtures.create_identity( + account: account, + actor: actor_two, + provider: provider + ) subject_one = AuthFixtures.create_subject(identity_one) subject_two = AuthFixtures.create_subject(identity_two) @@ -634,12 +658,35 @@ defmodule Domain.ActorsTest do allow_child_sandbox_access(test_pid) account = AccountsFixtures.create_account() + provider = AuthFixtures.create_email_provider(account: account) - actor_one = ActorsFixtures.create_actor(type: :account_admin_user, account: account) - actor_two = ActorsFixtures.create_actor(type: :account_admin_user, account: account) + actor_one = + ActorsFixtures.create_actor( + type: :account_admin_user, + account: account, + provider: provider + ) - identity_one = AuthFixtures.create_identity(account: account, actor: actor_one) - identity_two = AuthFixtures.create_identity(account: account, actor: actor_two) + actor_two = + ActorsFixtures.create_actor( + type: :account_admin_user, + account: account, + provider: provider + ) + + identity_one = + AuthFixtures.create_identity( + account: account, + actor: actor_one, + provider: provider + ) + + identity_two = + AuthFixtures.create_identity( + account: account, + actor: actor_two, + provider: provider + ) subject_one = AuthFixtures.create_subject(identity_one) subject_two = AuthFixtures.create_subject(identity_two) diff --git a/elixir/apps/domain/test/domain/auth/adapters/openid_connect_test.exs b/elixir/apps/domain/test/domain/auth/adapters/openid_connect_test.exs index cc8801a75..225732056 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/openid_connect_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/openid_connect_test.exs @@ -107,7 +107,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do assert authorization_uri == "http://localhost:#{bypass.port}/authorize" <> "?access_type=offline" <> - "&client_id=google-client-id" <> + "&client_id=#{provider.adapter_config["client_id"]}" <> "&code_challenge=#{Domain.Auth.Adapters.OpenIDConnect.PKCE.code_challenge(verifier)}" <> "&code_challenge_method=S256" <> "&redirect_uri=https%3A%2F%2Fexample.com%2F" <> @@ -133,7 +133,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do end end - describe "verify_secret/2" do + describe "verify_identity/2" do setup do account = AccountsFixtures.create_account() @@ -151,7 +151,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do identity: identity, bypass: bypass } do - {token, claims} = generate_token(provider, identity) + {token, claims} = AuthFixtures.generate_openid_connect_token(provider, identity) AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) AuthFixtures.expect_userinfo(bypass) @@ -160,13 +160,12 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do redirect_uri = "https://example.com/" payload = {redirect_uri, code_verifier, "MyFakeCode"} - assert {:ok, identity, expires_at} = verify_secret(identity, payload) + assert {:ok, identity, expires_at} = verify_identity(provider, payload) assert identity.provider_state == %{ access_token: nil, claims: claims, expires_at: expires_at, - id_token: token, refresh_token: nil, userinfo: %{ "email" => "ada@example.com", @@ -187,7 +186,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do identity: identity, bypass: bypass } do - {token, _claims} = generate_token(provider, identity) + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) AuthFixtures.expect_refresh_token(bypass, %{ "token_type" => "Bearer", @@ -203,9 +202,8 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do redirect_uri = "https://example.com/" payload = {redirect_uri, code_verifier, "MyFakeCode"} - assert {:ok, identity, _expires_at} = verify_secret(identity, payload) + assert {:ok, identity, _expires_at} = verify_identity(provider, payload) - assert identity.provider_state.id_token == token assert identity.provider_state.access_token == "MY_ACCESS_TOKEN" assert identity.provider_state.refresh_token == "MY_REFRESH_TOKEN" assert DateTime.diff(identity.provider_state.expires_at, DateTime.utc_now()) in 3595..3605 @@ -218,7 +216,10 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do } do forty_seconds_ago = DateTime.utc_now() |> DateTime.add(-40, :second) |> DateTime.to_unix() - {token, _claims} = generate_token(provider, identity, %{"exp" => forty_seconds_ago}) + {token, _claims} = + AuthFixtures.generate_openid_connect_token(provider, identity, %{ + "exp" => forty_seconds_ago + }) AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) @@ -226,11 +227,11 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do redirect_uri = "https://example.com/" payload = {redirect_uri, code_verifier, "MyFakeCode"} - assert verify_secret(identity, payload) == {:error, :expired_secret} + assert verify_identity(provider, payload) == {:error, :expired} end test "returns error when token is invalid", %{ - identity: identity, + provider: provider, bypass: bypass } do token = "foo" @@ -241,11 +242,61 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do redirect_uri = "https://example.com/" payload = {redirect_uri, code_verifier, "MyFakeCode"} - assert verify_secret(identity, payload) == {:error, :invalid_secret} + assert verify_identity(provider, payload) == {:error, :invalid} + end + + test "returns error when identity does not exist", %{ + identity: identity, + provider: provider, + bypass: bypass + } do + {token, _claims} = + AuthFixtures.generate_openid_connect_token(provider, identity, %{"sub" => "foo@bar.com"}) + + AuthFixtures.expect_refresh_token(bypass, %{ + "token_type" => "Bearer", + "id_token" => token, + "access_token" => "MY_ACCESS_TOKEN", + "refresh_token" => "MY_REFRESH_TOKEN", + "expires_in" => 3600 + }) + + AuthFixtures.expect_userinfo(bypass) + + code_verifier = PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert verify_identity(provider, payload) == {:error, :not_found} + end + + test "returns error when identity does not belong to provider", %{ + account: account, + provider: provider, + bypass: bypass + } do + identity = AuthFixtures.create_identity(account: account) + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + + AuthFixtures.expect_refresh_token(bypass, %{ + "token_type" => "Bearer", + "id_token" => token, + "access_token" => "MY_ACCESS_TOKEN", + "refresh_token" => "MY_REFRESH_TOKEN", + "expires_in" => 3600 + }) + + AuthFixtures.expect_userinfo(bypass) + + code_verifier = PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert verify_identity(provider, payload) == {:error, :not_found} end test "returns error when provider is down", %{ - identity: identity, + provider: provider, bypass: bypass } do Bypass.down(bypass) @@ -254,7 +305,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do redirect_uri = "https://example.com/" payload = {redirect_uri, code_verifier, "MyFakeCode"} - assert verify_secret(identity, payload) == {:error, :internal_error} + assert verify_identity(provider, payload) == {:error, :internal_error} end end @@ -276,7 +327,7 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do identity: identity, bypass: bypass } do - {token, claims} = generate_token(provider, identity) + {token, claims} = AuthFixtures.generate_openid_connect_token(provider, identity) AuthFixtures.expect_refresh_token(bypass, %{ "token_type" => "Bearer", @@ -294,7 +345,6 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do access_token: "MY_ACCESS_TOKEN", claims: claims, expires_at: expires_at, - id_token: token, refresh_token: "MY_REFRESH_TOKEN", userinfo: %{ "email" => "ada@example.com", @@ -312,27 +362,4 @@ defmodule Domain.Auth.Adapters.OpenIDConnectTest do assert DateTime.diff(expires_at, DateTime.utc_now()) in 5..15 end end - - defp generate_token(provider, identity, claims \\ %{}) do - jwk = AuthFixtures.jwks_attrs() - - claims = - Map.merge( - %{ - "email" => "foo@example.com", - "sub" => identity.provider_identifier, - "aud" => provider.adapter_config["client_id"], - "exp" => DateTime.utc_now() |> DateTime.add(10, :second) |> DateTime.to_unix() - }, - claims - ) - - {_alg, token} = - jwk - |> JOSE.JWK.from() - |> JOSE.JWS.sign(Jason.encode!(claims), %{"alg" => "RS256"}) - |> JOSE.JWS.compact() - - {token, claims} - end end diff --git a/elixir/apps/domain/test/domain/auth/adapters/userpass_test.exs b/elixir/apps/domain/test/domain/auth/adapters/userpass_test.exs index 5d7060934..0b4a526ba 100644 --- a/elixir/apps/domain/test/domain/auth/adapters/userpass_test.exs +++ b/elixir/apps/domain/test/domain/auth/adapters/userpass_test.exs @@ -50,7 +50,7 @@ defmodule Domain.Auth.Adapters.UserPassTest do assert errors_on(changeset) == %{ provider_virtual_state: %{ - password: ["should be at least 12 character(s)"], + password: ["should be at least 12 byte(s)"], password_confirmation: ["does not match confirmation"] } } diff --git a/elixir/apps/domain/test/domain/auth/oidc/refresher_test.exs b/elixir/apps/domain/test/domain/auth/oidc/refresher_test.exs deleted file mode 100644 index ede48e383..000000000 --- a/elixir/apps/domain/test/domain/auth/oidc/refresher_test.exs +++ /dev/null @@ -1,45 +0,0 @@ -# defmodule Domain.Auth.OIDC.RefresherTest do -# use Domain.DataCase, async: true -# alias Domain.Auth.OIDC.Refresher -# alias Domain.UsersFixtures - -# setup do -# user = UsersFixtures.create_user_with_role(:account_admin_user) -# {bypass, [provider_attrs]} = Domain.AuthFixtures.start_openid_providers(["google"]) - -# conn = -# Repo.insert!(%Domain.Auth.OIDC.Connection{ -# user_id: user.id, -# provider: "google", -# refresh_token: "REFRESH_TOKEN" -# }) - -# %{user: user, conn: conn, bypass: bypass, provider_attrs: provider_attrs} -# end - -# describe "refresh failed" do -# test "disable user", %{user: user, conn: conn, bypass: bypass} do -# Domain.AuthFixtures.expect_refresh_token_failure(bypass) - -# assert Refresher.refresh(user.id) == {:stop, :shutdown, user.id} -# user = Repo.reload(user) -# assert user.disabled_at - -# conn = Repo.reload(conn) -# assert %{"error" => _} = conn.refresh_response -# end -# end - -# describe "refresh succeeded" do -# test "does not change user", %{user: user, conn: conn, bypass: bypass} do -# Domain.AuthFixtures.expect_refresh_token(bypass) - -# assert Refresher.refresh(user.id) == {:stop, :shutdown, user.id} -# user = Repo.reload(user) -# refute user.disabled_at - -# conn = Repo.reload(conn) -# refute match?(%{"error" => _}, conn.refresh_response) -# end -# end -# end diff --git a/elixir/apps/domain/test/domain/auth_test.exs b/elixir/apps/domain/test/domain/auth_test.exs index 5faebe7fa..23d8c3e79 100644 --- a/elixir/apps/domain/test/domain/auth_test.exs +++ b/elixir/apps/domain/test/domain/auth_test.exs @@ -6,6 +6,79 @@ defmodule Domain.AuthTest do alias Domain.Auth.Authorizer alias Domain.{AccountsFixtures, AuthFixtures} + describe "fetch_active_provider_by_id/1" do + test "returns error when provider does not exist" do + assert fetch_active_provider_by_id(Ecto.UUID.generate()) == {:error, :not_found} + end + + test "returns error when provider is disabled" do + account = AccountsFixtures.create_account() + AuthFixtures.create_userpass_provider(account: account) + provider = AuthFixtures.create_email_provider(account: account) + + identity = + AuthFixtures.create_identity( + actor_default_type: :account_admin_user, + account: account, + provider: provider + ) + + subject = AuthFixtures.create_subject(identity) + {:ok, _provider} = disable_provider(provider, subject) + + assert fetch_active_provider_by_id(provider.id) == {:error, :not_found} + end + + test "returns error when provider is deleted" do + account = AccountsFixtures.create_account() + AuthFixtures.create_userpass_provider(account: account) + provider = AuthFixtures.create_email_provider(account: account) + + identity = + AuthFixtures.create_identity( + actor_default_type: :account_admin_user, + account: account, + provider: provider + ) + + subject = AuthFixtures.create_subject(identity) + {:ok, _provider} = delete_provider(provider, subject) + + assert fetch_active_provider_by_id(provider.id) == {:error, :not_found} + end + + test "returns provider" do + provider = AuthFixtures.create_email_provider() + assert {:ok, fetched_provider} = fetch_active_provider_by_id(provider.id) + assert fetched_provider.id == provider.id + end + end + + describe "list_active_providers_for_account/1" do + test "returns active providers for a given account" do + account = AccountsFixtures.create_account() + + userpass_provider = AuthFixtures.create_userpass_provider(account: account) + email_provider = AuthFixtures.create_email_provider(account: account) + token_provider = AuthFixtures.create_token_provider(account: account) + + identity = + AuthFixtures.create_identity( + actor_default_type: :account_admin_user, + account: account, + provider: email_provider + ) + + subject = AuthFixtures.create_subject(identity) + + {:ok, _provider} = disable_provider(token_provider, subject) + {:ok, _provider} = delete_provider(email_provider, subject) + + assert {:ok, [provider]} = list_active_providers_for_account(account) + assert provider.id == userpass_provider.id + end + end + describe "create_provider/2" do setup do account = AccountsFixtures.create_account() @@ -48,6 +121,56 @@ defmodule Domain.AuthTest do } end + test "returns error if email provider is already enabled", %{ + account: account + } do + # email, userpass, token + AuthFixtures.create_email_provider(account: account) + attrs = AuthFixtures.provider_attrs(adapter: :email) + assert {:error, changeset} = create_provider(account, attrs) + refute changeset.valid? + assert errors_on(changeset) == %{adapter: ["this provider is already enabled"]} + end + + test "returns error if userpass provider is already enabled", %{ + account: account + } do + # userpass, userpass, token + AuthFixtures.create_userpass_provider(account: account) + attrs = AuthFixtures.provider_attrs(adapter: :userpass) + assert {:error, changeset} = create_provider(account, attrs) + refute changeset.valid? + assert errors_on(changeset) == %{adapter: ["this provider is already enabled"]} + end + + test "returns error if token provider is already enabled", %{ + account: account + } do + AuthFixtures.create_token_provider(account: account) + attrs = AuthFixtures.provider_attrs(adapter: :token) + assert {:error, changeset} = create_provider(account, attrs) + refute changeset.valid? + assert errors_on(changeset) == %{adapter: ["this provider is already enabled"]} + end + + test "returns error if openid connect provider is already enabled", %{ + account: account + } do + {provider, _bypass} = + AuthFixtures.start_openid_providers(["google"]) + |> AuthFixtures.create_openid_connect_provider(account: account) + + attrs = + AuthFixtures.provider_attrs( + adapter: :openid_connect, + adapter_config: provider.adapter_config + ) + + assert {:error, changeset} = create_provider(account, attrs) + refute changeset.valid? + assert errors_on(changeset) == %{adapter: ["this provider is already connected"]} + end + test "creates a provider", %{ account: account } do @@ -135,7 +258,7 @@ defmodule Domain.AuthTest do subject: subject, provider: provider } do - other_provider = AuthFixtures.create_email_provider(account: account) + other_provider = AuthFixtures.create_userpass_provider(account: account) assert {:ok, provider} = disable_provider(provider, subject) assert provider.disabled_at @@ -168,7 +291,7 @@ defmodule Domain.AuthTest do subject: subject, provider: provider } do - other_provider = AuthFixtures.create_email_provider(account: account) + other_provider = AuthFixtures.create_userpass_provider(account: account) {:ok, _other_provider} = disable_provider(other_provider, subject) assert disable_provider(provider, subject) == {:error, :cant_disable_the_last_provider} @@ -184,7 +307,7 @@ defmodule Domain.AuthTest do account = AccountsFixtures.create_account() provider_one = AuthFixtures.create_email_provider(account: account) - provider_two = AuthFixtures.create_email_provider(account: account) + provider_two = AuthFixtures.create_userpass_provider(account: account) actor = ActorsFixtures.create_actor( @@ -224,7 +347,7 @@ defmodule Domain.AuthTest do subject: subject, account: account } do - provider = AuthFixtures.create_email_provider(account: account) + provider = AuthFixtures.create_userpass_provider(account: account) assert {:ok, _provider} = disable_provider(provider, subject) assert {:ok, provider} = disable_provider(provider, subject) assert {:ok, _provider} = disable_provider(provider, subject) @@ -233,7 +356,7 @@ defmodule Domain.AuthTest do test "does not allow to disable providers in other accounts", %{ subject: subject } do - provider = AuthFixtures.create_email_provider() + provider = AuthFixtures.create_userpass_provider() assert disable_provider(provider, subject) == {:error, :not_found} end @@ -334,7 +457,7 @@ defmodule Domain.AuthTest do subject: subject, provider: provider } do - other_provider = AuthFixtures.create_email_provider(account: account) + other_provider = AuthFixtures.create_userpass_provider(account: account) assert {:ok, provider} = delete_provider(provider, subject) assert provider.deleted_at @@ -367,7 +490,7 @@ defmodule Domain.AuthTest do subject: subject, provider: provider } do - other_provider = AuthFixtures.create_email_provider(account: account) + other_provider = AuthFixtures.create_userpass_provider(account: account) {:ok, _other_provider} = delete_provider(other_provider, subject) assert delete_provider(provider, subject) == {:error, :cant_delete_the_last_provider} @@ -383,7 +506,7 @@ defmodule Domain.AuthTest do account = AccountsFixtures.create_account() provider_one = AuthFixtures.create_email_provider(account: account) - provider_two = AuthFixtures.create_email_provider(account: account) + provider_two = AuthFixtures.create_userpass_provider(account: account) actor = ActorsFixtures.create_actor( @@ -419,7 +542,7 @@ defmodule Domain.AuthTest do subject: subject, account: account } do - provider = AuthFixtures.create_email_provider(account: account) + provider = AuthFixtures.create_userpass_provider(account: account) assert {:ok, deleted_provider} = delete_provider(provider, subject) assert delete_provider(provider, subject) == {:error, :not_found} assert delete_provider(deleted_provider, subject) == {:error, :not_found} @@ -428,7 +551,7 @@ defmodule Domain.AuthTest do test "does not allow to delete providers in other accounts", %{ subject: subject } do - provider = AuthFixtures.create_email_provider() + provider = AuthFixtures.create_userpass_provider() assert delete_provider(provider, subject) == {:error, :not_found} end @@ -496,7 +619,7 @@ defmodule Domain.AuthTest do provider_identifier = Ecto.UUID.generate() assert {:error, changeset} = create_identity(actor, provider, provider_identifier) - assert errors_on(changeset) == %{provider_identifier: ["is invalid email address"]} + assert errors_on(changeset) == %{provider_identifier: ["is an invalid email address"]} provider_identifier = nil assert {:error, changeset} = create_identity(actor, provider, provider_identifier) @@ -540,7 +663,7 @@ defmodule Domain.AuthTest do } do provider_identifier = Ecto.UUID.generate() assert {:error, changeset} = replace_identity(identity, provider_identifier, subject) - assert errors_on(changeset) == %{provider_identifier: ["is invalid email address"]} + assert errors_on(changeset) == %{provider_identifier: ["is an invalid email address"]} provider_identifier = nil assert {:error, changeset} = replace_identity(identity, provider_identifier, subject) @@ -750,7 +873,7 @@ defmodule Domain.AuthTest do {:error, :unauthorized} end - test "returns subject on success", %{ + test "returns subject on success using provider identifier", %{ account: account, provider: provider, user_agent: user_agent, @@ -769,6 +892,25 @@ defmodule Domain.AuthTest do assert subject.context.user_agent == user_agent end + test "returns subject on success using identity id", %{ + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + identity = AuthFixtures.create_identity(account: account, provider: provider) + secret = identity.provider_virtual_state.sign_in_token + + assert {:ok, %Auth.Subject{} = subject} = + sign_in(provider, identity.id, secret, user_agent, remote_ip) + + assert subject.account.id == account.id + assert subject.actor.id == identity.actor_id + assert subject.identity.id == identity.id + assert subject.context.remote_ip == remote_ip + assert subject.context.user_agent == user_agent + end + test "returned subject expiration depends on user type", %{ account: account, provider: provider, @@ -814,6 +956,56 @@ defmodule Domain.AuthTest do {:error, :unauthorized} end + test "returns error when identity is disabled", %{ + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = ActorsFixtures.create_actor(type: :account_admin_user, account: account) + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + secret = identity.provider_virtual_state.sign_in_token + subject = AuthFixtures.create_subject(identity) + {:ok, identity} = delete_identity(identity, subject) + + assert sign_in(provider, identity.provider_identifier, secret, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns error when actor is disabled", %{ + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = + ActorsFixtures.create_actor(type: :account_admin_user, account: account) + |> ActorsFixtures.disable() + + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + secret = identity.provider_virtual_state.sign_in_token + + assert sign_in(provider, identity.provider_identifier, secret, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns error when actor is deleted", %{ + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = + ActorsFixtures.create_actor(type: :account_admin_user, account: account) + |> ActorsFixtures.delete() + + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + secret = identity.provider_virtual_state.sign_in_token + + assert sign_in(provider, identity.provider_identifier, secret, user_agent, remote_ip) == + {:error, :unauthorized} + end + test "returns error when provider is deleted", %{ account: account, provider: provider, @@ -851,6 +1043,278 @@ defmodule Domain.AuthTest do end end + describe "sign_in/4" do + setup do + account = AccountsFixtures.create_account() + + {provider, bypass} = + AuthFixtures.start_openid_providers(["google"]) + |> AuthFixtures.create_openid_connect_provider(account: account) + + user_agent = AuthFixtures.user_agent() + remote_ip = AuthFixtures.remote_ip() + + %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } + end + + test "returns error when provider_identifier does not exist", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + identity = AuthFixtures.create_identity(account: account, provider: provider) + + {token, _claims} = + AuthFixtures.generate_openid_connect_token(provider, identity, %{ + "sub" => "foo@bar.com" + }) + + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert sign_in(provider, payload, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns error when token is invalid", %{ + bypass: bypass, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => "foo"}) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert sign_in(provider, payload, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns subject on success using sub claim", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + identity = AuthFixtures.create_identity(account: account, provider: provider) + + token = + AuthFixtures.sign_openid_connect_token(%{ + "sub" => identity.provider_identifier, + "aud" => provider.adapter_config["client_id"], + "exp" => DateTime.utc_now() |> DateTime.add(10, :second) |> DateTime.to_unix() + }) + + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert {:ok, %Auth.Subject{} = subject} = + sign_in(provider, payload, user_agent, remote_ip) + + assert subject.account.id == account.id + assert subject.actor.id == identity.actor_id + assert subject.identity.id == identity.id + assert subject.context.remote_ip == remote_ip + assert subject.context.user_agent == user_agent + end + + # test "returned subject expiration depends on user type", %{ + # account: account, + # provider: provider, + # user_agent: user_agent, + # remote_ip: remote_ip + # } do + # actor = ActorsFixtures.create_actor(type: :account_admin_user, account: account) + # identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + + # code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + # redirect_uri = "https://example.com/" + # payload = {redirect_uri, code_verifier, "MyFakeCode"} + + # assert {:ok, %Auth.Subject{} = subject} = + # sign_in(provider, payload, user_agent, remote_ip) + + # three_hours = 3 * 60 * 60 + # assert_datetime_diff(subject.expires_at, DateTime.utc_now(), three_hours) + + # actor = ActorsFixtures.create_actor(type: :account_user, account: account) + # identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + + # assert {:ok, %Auth.Subject{} = subject} = + # sign_in(provider, payload, user_agent, remote_ip) + + # one_week = 7 * 24 * 60 * 60 + # assert_datetime_diff(subject.expires_at, DateTime.utc_now(), one_week) + # end + + test "returns error when provider is disabled", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = ActorsFixtures.create_actor(type: :account_admin_user, account: account) + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + subject = AuthFixtures.create_subject(identity) + {:ok, _provider} = disable_provider(provider, subject) + + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert sign_in(provider, payload, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns error when identity is disabled", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = ActorsFixtures.create_actor(type: :account_admin_user, account: account) + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + subject = AuthFixtures.create_subject(identity) + {:ok, identity} = delete_identity(identity, subject) + + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert sign_in(provider, payload, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns error when actor is disabled", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = + ActorsFixtures.create_actor(type: :account_admin_user, account: account) + |> ActorsFixtures.disable() + + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert sign_in(provider, payload, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns error when actor is deleted", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = + ActorsFixtures.create_actor(type: :account_admin_user, account: account) + |> ActorsFixtures.delete() + + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert sign_in(provider, payload, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "returns error when provider is deleted", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + actor = ActorsFixtures.create_actor(type: :account_admin_user, account: account) + identity = AuthFixtures.create_identity(account: account, provider: provider, actor: actor) + subject = AuthFixtures.create_subject(identity) + {:ok, _provider} = delete_provider(provider, subject) + + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert sign_in(provider, payload, user_agent, remote_ip) == + {:error, :unauthorized} + end + + test "updates last signed in fields for identity on success", %{ + bypass: bypass, + account: account, + provider: provider, + user_agent: user_agent, + remote_ip: remote_ip + } do + identity = AuthFixtures.create_identity(account: account, provider: provider) + + {token, _claims} = AuthFixtures.generate_openid_connect_token(provider, identity) + AuthFixtures.expect_refresh_token(bypass, %{"id_token" => token}) + AuthFixtures.expect_userinfo(bypass) + + code_verifier = Domain.Auth.Adapters.OpenIDConnect.PKCE.code_verifier() + redirect_uri = "https://example.com/" + payload = {redirect_uri, code_verifier, "MyFakeCode"} + + assert {:ok, _subject} = + sign_in(provider, payload, user_agent, remote_ip) + + assert updated_identity = Repo.one(Auth.Identity) + assert updated_identity.last_seen_at != identity.last_seen_at + assert updated_identity.last_seen_remote_ip != identity.last_seen_remote_ip + assert updated_identity.last_seen_user_agent != identity.last_seen_user_agent + end + end + describe "sign_in/3" do setup do account = AccountsFixtures.create_account() diff --git a/elixir/apps/domain/test/support/fixtures/actors_fixtures.ex b/elixir/apps/domain/test/support/fixtures/actors_fixtures.ex index 148be5946..3ad4c234f 100644 --- a/elixir/apps/domain/test/support/fixtures/actors_fixtures.ex +++ b/elixir/apps/domain/test/support/fixtures/actors_fixtures.ex @@ -4,7 +4,11 @@ defmodule Domain.ActorsFixtures do alias Domain.{AccountsFixtures, AuthFixtures} def actor_attrs(attrs \\ %{}) do + first_name = Enum.random(~w[Wade Dave Seth Riley Gilbert Jorge Dan Brian Roberto Ramon]) + last_name = Enum.random(~w[Robyn Traci Desiree Jon Bob Karl Joe Alberta Lynda Cara Brandi]) + Enum.into(attrs, %{ + name: "#{first_name} #{last_name}", type: :account_user }) end @@ -19,7 +23,11 @@ defmodule Domain.ActorsFixtures do {provider, attrs} = Map.pop_lazy(attrs, :provider, fn -> - AuthFixtures.create_email_provider(account: account) + {provider, _bypass} = + AuthFixtures.start_openid_providers(["google"]) + |> AuthFixtures.create_openid_connect_provider(account: account) + + provider end) attrs = actor_attrs(attrs) @@ -37,4 +45,8 @@ defmodule Domain.ActorsFixtures do def disable(actor) do update(actor, %{disabled_at: DateTime.utc_now()}) end + + def delete(actor) do + update(actor, %{deleted_at: DateTime.utc_now()}) + end end diff --git a/elixir/apps/domain/test/support/fixtures/auth_fixtures.ex b/elixir/apps/domain/test/support/fixtures/auth_fixtures.ex index 83adf4a7a..a789a5eac 100644 --- a/elixir/apps/domain/test/support/fixtures/auth_fixtures.ex +++ b/elixir/apps/domain/test/support/fixtures/auth_fixtures.ex @@ -4,8 +4,10 @@ defmodule Domain.AuthFixtures do alias Domain.AccountsFixtures alias Domain.ActorsFixtures + def user_password, do: "Hello w0rld!" def remote_ip, do: {100, 64, 100, 58} def user_agent, do: "iOS/12.5 (iPhone) connlib/0.7.412" + def email, do: "user-#{counter()}@example.com" def random_provider_identifier(%Domain.Auth.Provider{adapter: :email, name: name}) do "user-#{counter()}@#{String.downcase(name)}.com" @@ -54,7 +56,7 @@ defmodule Domain.AuthFixtures do end) attrs = - %{adapter_config: provider_attrs} + %{adapter: :openid_connect, adapter_config: provider_attrs} |> Map.merge(attrs) |> provider_attrs() @@ -100,7 +102,11 @@ defmodule Domain.AuthFixtures do {provider, attrs} = Map.pop_lazy(attrs, :provider, fn -> - create_email_provider(account: account) + {provider, _bypass} = + start_openid_providers(["google"]) + |> create_openid_connect_provider(account: account) + + provider end) {provider_identifier, attrs} = @@ -108,9 +114,13 @@ defmodule Domain.AuthFixtures do random_provider_identifier(provider) end) + {actor_default_type, attrs} = + Map.pop(attrs, :actor_default_type, :account_user) + {actor, _attrs} = Map.pop_lazy(attrs, :actor, fn -> ActorsFixtures.create_actor( + type: actor_default_type, account: account, provider: provider, provider_identifier: provider_identifier @@ -134,10 +144,27 @@ defmodule Domain.AuthFixtures do end end + def delete_identity(identity) do + identity + |> Ecto.Changeset.change(deleted_at: DateTime.utc_now()) + |> Repo.update!() + end + def create_subject do account = AccountsFixtures.create_account() - actor = ActorsFixtures.create_actor(type: :account_admin_user, account: account) - identity = create_identity(actor: actor, account: account) + + {provider, _bypass} = + start_openid_providers(["google"]) + |> create_openid_connect_provider(account: account) + + actor = + ActorsFixtures.create_actor( + type: :account_admin_user, + account: account, + provider: provider + ) + + identity = create_identity(actor: actor, account: account, provider: provider) create_subject(identity) end @@ -186,13 +213,12 @@ defmodule Domain.AuthFixtures do Enum.into(overrides, %{ "id" => "google", "discovery_document_uri" => "https://firezone.example.com/.well-known/openid-configuration", - "client_id" => "google-client-id", + "client_id" => "google-client-id-#{counter()}", "client_secret" => "google-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/google/callback/", "response_type" => "code", "scope" => "openid email profile", - "label" => "OIDC Google", - "auto_create_users" => false + "label" => "OIDC Google" }) end @@ -201,79 +227,72 @@ defmodule Domain.AuthFixtures do %{ "id" => "google", "discovery_document_uri" => discovery_document_url, - "client_id" => "google-client-id", + "client_id" => "google-client-id-#{counter()}", "client_secret" => "google-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/google/callback/", "response_type" => "code", "scope" => "openid email profile", - "label" => "OIDC Google", - "auto_create_users" => false + "label" => "OIDC Google" }, %{ "id" => "okta", "discovery_document_uri" => discovery_document_url, - "client_id" => "okta-client-id", + "client_id" => "okta-client-id-#{counter()}", "client_secret" => "okta-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/okta/callback/", "response_type" => "code", "scope" => "openid email profile offline_access", - "label" => "OIDC Okta", - "auto_create_users" => false + "label" => "OIDC Okta" }, %{ "id" => "auth0", "discovery_document_uri" => discovery_document_url, - "client_id" => "auth0-client-id", + "client_id" => "auth0-client-id-#{counter()}", "client_secret" => "auth0-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/auth0/callback/", "response_type" => "code", "scope" => "openid email profile", - "label" => "OIDC Auth0", - "auto_create_users" => false + "label" => "OIDC Auth0" }, %{ "id" => "azure", "discovery_document_uri" => discovery_document_url, - "client_id" => "azure-client-id", + "client_id" => "azure-client-id-#{counter()}", "client_secret" => "azure-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/azure/callback/", "response_type" => "code", "scope" => "openid email profile offline_access", - "label" => "OIDC Azure", - "auto_create_users" => false + "label" => "OIDC Azure" }, %{ "id" => "onelogin", "discovery_document_uri" => discovery_document_url, - "client_id" => "onelogin-client-id", + "client_id" => "onelogin-client-id-#{counter()}", "client_secret" => "onelogin-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/onelogin/callback/", "response_type" => "code", "scope" => "openid email profile offline_access", - "label" => "OIDC Onelogin", - "auto_create_users" => false + "label" => "OIDC Onelogin" }, %{ "id" => "keycloak", "discovery_document_uri" => discovery_document_url, - "client_id" => "keycloak-client-id", + "client_id" => "keycloak-client-id-#{counter()}", "client_secret" => "keycloak-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/keycloak/callback/", "response_type" => "code", "scope" => "openid email profile offline_access", - "label" => "OIDC Keycloak", - "auto_create_users" => false + "label" => "OIDC Keycloak" }, %{ "id" => "vault", "discovery_document_uri" => discovery_document_url, - "client_id" => "vault-client-id", + "client_id" => "vault-client-id-#{counter()}", "client_secret" => "vault-client-secret", "redirect_uri" => "https://firezone.example.com/auth/oidc/vault/callback/", "response_type" => "code", "scope" => "openid email profile offline_access", - "label" => "OIDC Vault", - "auto_create_users" => false + "label" => "OIDC Vault" } ] end @@ -444,7 +463,34 @@ defmodule Domain.AuthFixtures do {bypass, "#{endpoint}/.well-known/openid-configuration"} end - def fetch_conn_params(conn) do + def generate_openid_connect_token(provider, identity, claims \\ %{}) do + claims = + Map.merge( + %{ + "email" => identity.provider_identifier, + "sub" => identity.provider_identifier, + "aud" => provider.adapter_config["client_id"], + "exp" => DateTime.utc_now() |> DateTime.add(10, :second) |> DateTime.to_unix() + }, + claims + ) + + {sign_openid_connect_token(claims), claims} + end + + def sign_openid_connect_token(claims) do + jwk = jwks_attrs() + + {_alg, token} = + jwk + |> JOSE.JWK.from() + |> JOSE.JWS.sign(Jason.encode!(claims), %{"alg" => "RS256"}) + |> JOSE.JWS.compact() + + token + end + + defp fetch_conn_params(conn) do opts = Plug.Parsers.init(parsers: [:urlencoded, :json], pass: ["*/*"], json_decoder: Jason) conn diff --git a/elixir/apps/web/.formatter.exs b/elixir/apps/web/.formatter.exs index ce4ddab66..a04ffb66e 100644 --- a/elixir/apps/web/.formatter.exs +++ b/elixir/apps/web/.formatter.exs @@ -1,7 +1,7 @@ [ import_deps: [:phoenix, :phoenix_live_view], plugins: [Phoenix.LiveView.HTMLFormatter], - inputs: ["*.{heex,ex,exs}", "{config,lib,test}/**/*.{heex,ex,exs}"], + inputs: ["*.{xml.heex,html.heex,ex,exs}", "{config,lib,test}/**/*.{xml.heex,html.heex,ex,exs}"], locals_without_parens: [ assert_authenticated: 2, assert_unauthenticated: 1 diff --git a/elixir/apps/web/.gitignore b/elixir/apps/web/.gitignore index 5e7075aea..5b7571cc0 100644 --- a/elixir/apps/web/.gitignore +++ b/elixir/apps/web/.gitignore @@ -17,7 +17,7 @@ web-*.tar /priv/static/assets/ # Ignore Tailwind temporary files -assets/tmp +/assets/tmp # Ignore digested assets cache. /priv/static/cache_manifest.json diff --git a/elixir/apps/web/assets/tailwind.config.js b/elixir/apps/web/assets/tailwind.config.js index 9151aecbb..040715e44 100644 --- a/elixir/apps/web/assets/tailwind.config.js +++ b/elixir/apps/web/assets/tailwind.config.js @@ -7,7 +7,8 @@ const path = require("path") const defaultTheme = require("tailwindcss/defaultTheme") module.exports = { - darkMode: "media", + // Use "media" to synchronize dark mode with the OS, "class" to require manual toggle + darkMode: "class", content: [ "./node_modules/flowbite/**/*.js", "./js/**/*.js", diff --git a/elixir/apps/web/lib/web.ex b/elixir/apps/web/lib/web.ex index 76332aee1..ae2046523 100644 --- a/elixir/apps/web/lib/web.ex +++ b/elixir/apps/web/lib/web.ex @@ -50,10 +50,10 @@ defmodule Web do end end - def live_view do + def live_view(opts \\ []) do quote do use Phoenix.LiveView, - layout: {Web.Layouts, :app} + layout: Keyword.get(unquote(opts), :layout, {Web.Layouts, :app}) unquote(html_helpers()) end @@ -124,4 +124,8 @@ defmodule Web do defmacro __using__(which) when is_atom(which) do apply(__MODULE__, which, []) end + + defmacro __using__({which, opts}) when is_atom(which) do + apply(__MODULE__, which, [opts]) + end end diff --git a/elixir/apps/web/lib/web/auth.ex b/elixir/apps/web/lib/web/auth.ex new file mode 100644 index 000000000..16f178a93 --- /dev/null +++ b/elixir/apps/web/lib/web/auth.ex @@ -0,0 +1,221 @@ +defmodule Web.Auth do + use Web, :verified_routes + alias Domain.Auth + + def signed_in_path(%Auth.Subject{actor: %{type: :account_admin_user}} = subject), + do: ~p"/#{subject.account}/dashboard" + + def signed_in_path(%Auth.Subject{actor: %{type: :account_user}} = subject), + do: ~p"/#{subject.account}" + + def put_subject_in_session(conn, %Auth.Subject{} = subject) do + {:ok, session_token} = Domain.Auth.create_session_token_from_subject(subject) + + conn + |> Plug.Conn.put_session(:signed_in_at, DateTime.utc_now()) + |> Plug.Conn.put_session(:session_token, session_token) + |> Plug.Conn.put_session(:live_socket_id, "actors_sessions:#{subject.actor.id}") + end + + @doc """ + Logs the user out. + + It clears all session data for safety. See `renew_session/1`. + """ + def sign_out(%Plug.Conn{} = conn) do + # token = Plug.Conn.get_session(conn, :session_token) + # subject && Accounts.delete_user_session_token(subject) + + if live_socket_id = Plug.Conn.get_session(conn, :live_socket_id) do + conn.private.phoenix_endpoint.broadcast(live_socket_id, "disconnect", %{}) + end + + conn + |> renew_session() + end + + @doc """ + This function renews the session ID and erases the whole + session to avoid fixation attacks. + """ + def renew_session(%Plug.Conn{} = conn) do + preferred_locale = Plug.Conn.get_session(conn, :preferred_locale) + + conn + |> Plug.Conn.configure_session(renew: true) + |> Plug.Conn.clear_session() + |> Plug.Conn.put_session(:preferred_locale, preferred_locale) + end + + ########################### + ## Plugs + ########################### + + @doc """ + Fetches the user agent value from headers and assigns it the connection. + """ + def fetch_user_agent(%Plug.Conn{} = conn, _opts) do + case Plug.Conn.get_req_header(conn, "user-agent") do + [user_agent | _] -> Plug.Conn.assign(conn, :user_agent, user_agent) + _ -> conn + end + end + + @doc """ + Fetches the session token from the session and assigns the subject to the connection. + """ + def fetch_subject(%Plug.Conn{} = conn, _opts) do + with token when not is_nil(token) <- Plug.Conn.get_session(conn, :session_token), + {:ok, subject} <- + Domain.Auth.sign_in(token, conn.assigns.user_agent, conn.remote_ip), + true <- conn.path_params["account_id"] == subject.account.id do + Plug.Conn.assign(conn, :subject, subject) + else + _ -> conn + end + end + + @doc """ + Used for routes that require the user to not be authenticated. + """ + def redirect_if_user_is_authenticated(%Plug.Conn{} = conn, _opts) do + if conn.assigns[:subject] do + conn + |> Phoenix.Controller.redirect(to: signed_in_path(conn.assigns.subject)) + |> Plug.Conn.halt() + else + conn + end + end + + @doc """ + Used for routes that require the user to be authenticated. + + This plug will only work if there is an `account_id` in the path params. + """ + def ensure_authenticated(%Plug.Conn{} = conn, _opts) do + if conn.assigns[:subject] do + conn + else + conn + |> Phoenix.Controller.put_flash(:error, "You must log in to access this page.") + |> maybe_store_return_to() + |> Phoenix.Controller.redirect(to: ~p"/#{conn.path_params["account_id"]}/sign_in") + |> Plug.Conn.halt() + end + end + + @doc """ + Used for routes that require the user to be authenticated as a specific kind of actor. + + This plug will only work if there is an `account_id` in the path params. + """ + def ensure_authenticated_actor_type(%Plug.Conn{} = conn, type) do + if not is_nil(conn.assigns[:subject]) and conn.assigns[:subject].actor.type == type do + conn + else + conn + |> Web.FallbackController.call({:error, :not_found}) + |> Plug.Conn.halt() + end + end + + defp maybe_store_return_to(%{method: "GET"} = conn) do + Plug.Conn.put_session(conn, :user_return_to, Phoenix.Controller.current_path(conn)) + end + + defp maybe_store_return_to(conn), do: conn + + ########################### + ## LiveView + ########################### + + @doc """ + Handles mounting and authenticating the actor in LiveViews. + + Notice: every protected route should have `account_id` in the path params. + + ## `on_mount` arguments + + * `:mount_subject` - assigns user_agent and subject to the socket assigns based on + session_token, or nil if there's no session_token or no matching user. + + * `:ensure_authenticated` - authenticates the user from the session, + and assigns the subject to socket assigns based on session_token. + Redirects to login page if there's no logged user. + + * `:redirect_if_user_is_authenticated` - authenticates the user from the session. + Redirects to signed_in_path if there's a logged user. + + ## Examples + + Use the `on_mount` lifecycle macro in LiveViews to mount or authenticate + the subject: + + defmodule Web.PageLive do + use Web, :live_view + + on_mount {Web.UserAuth, :mount_subject} + ... + end + + Or use the `live_session` of your router to invoke the on_mount callback: + + live_session :authenticated, on_mount: [{Web.UserAuth, :ensure_authenticated}] do + live "/:account_id/profile", ProfileLive, :index + end + """ + def on_mount(:mount_subject, params, session, socket) do + {:cont, mount_subject(socket, params, session)} + end + + def on_mount(:ensure_authenticated, params, session, socket) do + socket = mount_subject(socket, params, session) + + if socket.assigns.subject do + {:cont, socket} + else + socket = + socket + |> Phoenix.LiveView.put_flash(:error, "You must log in to access this page.") + |> Phoenix.LiveView.redirect(to: ~p"/#{socket.assigns.account}/sign_in") + + {:halt, socket} + end + end + + def on_mount(:ensure_account_admin_user_actor, params, session, socket) do + socket = mount_subject(socket, params, session) + + if socket.assigns.subject.actor.type == :account_admin_user do + {:cont, socket} + else + raise Ecto.NoResultsError + end + end + + def on_mount(:redirect_if_user_is_authenticated, params, session, socket) do + socket = mount_subject(socket, params, session) + + if socket.assigns.subject do + {:halt, Phoenix.LiveView.redirect(socket, to: signed_in_path(socket.assigns.subject))} + else + {:cont, socket} + end + end + + defp mount_subject(socket, params, session) do + Phoenix.Component.assign_new(socket, :subject, fn -> + user_agent = Phoenix.LiveView.get_connect_info(socket, :user_agent) + remote_ip = Phoenix.LiveView.get_connect_info(socket, :peer_data).address + + with token when not is_nil(token) <- session["session_token"], + {:ok, subject} <- Domain.Auth.sign_in(token, user_agent, remote_ip), + true <- params["account_id"] == subject.account.id do + subject + else + _ -> nil + end + end) + end +end diff --git a/elixir/apps/web/lib/web/components/core_components.ex b/elixir/apps/web/lib/web/components/core_components.ex index d2884f0b8..3b4fed24c 100644 --- a/elixir/apps/web/lib/web/components/core_components.ex +++ b/elixir/apps/web/lib/web/components/core_components.ex @@ -11,9 +11,22 @@ defmodule Web.CoreComponents do """ use Phoenix.Component use Web, :verified_routes - - alias Phoenix.LiveView.JS import Web.Gettext + alias Phoenix.LiveView.JS + + def logo(assigns) do + ~H""" + + Firezone Logo + + firezone + + + """ + end @doc """ Renders a generic

tag using our color scheme. @@ -507,13 +520,12 @@ defmodule Web.CoreComponents do

hide("##{@id}")} - role="alert" class={[ - "fixed top-2 right-2 w-80 sm:w-96 z-50 rounded-lg p-3 ring-1", - @kind == :info && "bg-emerald-50 text-emerald-800 ring-emerald-500 fill-cyan-900", - @kind == :error && "bg-rose-50 text-rose-900 shadow-md ring-rose-500 fill-rose-900" + "p-4 mb-4 text-sm rounded-lg flash-#{@kind}", + @kind == :info && "text-yellow-800 bg-yellow-50 dark:bg-gray-800 dark:text-yellow-300", + @kind == :error && "text-red-800 bg-red-50 dark:bg-gray-800 dark:text-red-400" ]} + role="alert" {@rest} >

@@ -521,10 +533,7 @@ defmodule Web.CoreComponents do <.icon :if={@kind == :error} name="hero-exclamation-circle-mini" class="h-4 w-4" /> <%= @title %>

-

<%= msg %>

- + <%= msg %>
""" end @@ -581,7 +590,7 @@ defmodule Web.CoreComponents do def simple_form(assigns) do ~H""" <.form :let={f} for={@for} as={@as} {@rest}> -
+
<%= render_slot(@inner_block, f) %>
<%= render_slot(action, f) %> @@ -610,8 +619,12 @@ defmodule Web.CoreComponents do
- Steve Johnson + <%= @subject.actor.name %> - steve@tesla.com + <%= @subject.identity.provider_identifier %>
    @@ -88,7 +84,7 @@
    • Sign out @@ -101,7 +97,7 @@