diff --git a/apps/authenticator/lib/sign_in/commands/resource_owner.ex b/apps/authenticator/lib/sign_in/commands/resource_owner.ex index bfae29d..e2d6c3e 100644 --- a/apps/authenticator/lib/sign_in/commands/resource_owner.ex +++ b/apps/authenticator/lib/sign_in/commands/resource_owner.ex @@ -5,8 +5,9 @@ defmodule Authenticator.SignIn.Commands.ResourceOwner do With the resource owner password credentials grant type, the user provides their username and password directly and we uses it to authenticates then. - The Client application should pass their secret (or client assertion) in order to be authorized - to exchange the credentials for an access_token. + When dealing with public applications we cannot ensure that the secret is safe so + we diden't request it unlike confidential applications where we can request a secret + or an client_assertion. When a public key is registered for the client application this flow will require that an assertion is passed instead of the raw secret to avoid sending it on requests. @@ -31,8 +32,7 @@ defmodule Authenticator.SignIn.Commands.ResourceOwner do @doc """ Sign in an user identity by ResouceOnwer flow. - The application has to be active, using openid-connect protocol and with access_type - confidential in order to use this flow. + The application has to be active, using openid-connect in order to use this flow. When the client application has a public_key saved on database we force the use of client_assertions on input to avoid passing it's secret open on requests. @@ -41,20 +41,16 @@ defmodule Authenticator.SignIn.Commands.ResourceOwner do to avoid exposing identity existance and time attacks. """ @impl true - def execute(%Input{username: username, client_id: client_id, scope: scope} = input) do + def execute(%Input{username: username, client_id: client_id} = input) do with {:app, {:ok, app}} <- {:app, Port.get_identity(%{client_id: client_id})}, {:flow_enabled?, true} <- {:flow_enabled?, "resource_owner" in app.grant_flows}, {:app_active?, true} <- {:app_active?, app.status == "active"}, - {:secret_matches?, true} <- {:secret_matches?, secret_matches?(app, input)}, - {:confidential?, true} <- {:confidential?, app.access_type == "confidential"}, {:valid_protocol?, true} <- {:valid_protocol?, app.protocol == "openid-connect"}, {:user, {:ok, user}} <- {:user, Port.get_identity(%{username: username})}, {:user_active?, true} <- {:user_active?, user.status == "active"}, - {:pass_matches?, true} <- {:pass_matches?, VerifyHash.execute(user, input.password)}, - {:ok, access_token, claims} <- generate_access_token(user, app, scope), - {:ok, refresh_token, _} <- generate_refresh_token(app, claims), - {:ok, _session} <- generate_and_save(input, claims) do - {:ok, parse_response(access_token, refresh_token, claims)} + {:public?, true, _identities} <- {:public?, app.access_type == "public", {user, app}} do + Logger.info("Running public authentication flow for client #{client_id}") + run_public_authentication(user, app, input) else {:app, {:error, :not_found}} -> Logger.info("Client application #{client_id} not found") @@ -71,16 +67,6 @@ defmodule Authenticator.SignIn.Commands.ResourceOwner do FakeVerifyHash.execute(:argon2) {:error, :unauthenticated} - {:secret_matches?, false} -> - Logger.info("Client application #{client_id} credential didn't matches") - FakeVerifyHash.execute(:argon2) - {:error, :unauthenticated} - - {:confidential?, false} -> - Logger.info("Client application #{client_id} is not confidential") - FakeVerifyHash.execute(:argon2) - {:error, :unauthenticated} - {:valid_protocol?, false} -> Logger.info("Client application #{client_id} protocol is not openid-connect") FakeVerifyHash.execute(:argon2) @@ -96,10 +82,9 @@ defmodule Authenticator.SignIn.Commands.ResourceOwner do FakeVerifyHash.execute(:argon2) {:error, :unauthenticated} - {:pass_matches?, false} -> - Logger.info("User #{username} password do not match any credential") - generate_attempt(input, false) - {:error, :unauthenticated} + {:public?, false, {user, app}} -> + Logger.info("Running confidential authentication flow for client #{client_id}") + run_confidential_authentication(user, app, input) error -> Logger.error("Failed to run command becuase of unknow error", error: inspect(error)) @@ -125,7 +110,44 @@ defmodule Authenticator.SignIn.Commands.ResourceOwner do end end - def execute(_any), do: {:error, :invalid_params} + defp run_public_authentication(user, app, input) do + if VerifyHash.execute(user, input.password) do + user + |> generate_tokens(app, input) + |> parse_response() + else + Logger.info("User #{user.username} password do not match any credential") + generate_attempt(input, false) + {:error, :unauthenticated} + end + end + + defp run_confidential_authentication(user, app, input) do + with {:secret_matches?, true} <- {:secret_matches?, secret_matches?(app, input)}, + {:pass_matches?, true} <- {:pass_matches?, VerifyHash.execute(user, input.password)} do + user + |> generate_tokens(app, input) + |> parse_response() + else + {:secret_matches?, false} -> + Logger.info("Client application #{app.client_id} credential didn't matches") + FakeVerifyHash.execute(:argon2) + {:error, :unauthenticated} + + {:pass_matches?, false} -> + Logger.info("User #{user.username} password do not match any credential") + generate_attempt(input, false) + {:error, :unauthenticated} + end + end + + defp generate_tokens(user, app, input) do + with {:ok, access_token, claims} <- generate_access_token(user, app, input.scope), + {:ok, refresh_token, _} <- generate_refresh_token(app, claims), + {:ok, _session} <- generate_and_save(input, claims) do + {:ok, access_token, refresh_token, claims} + end + end defp secret_matches?(%{client_id: id, public_key: public_key}, %{client_assertion: assertion}) when is_binary(assertion) do @@ -223,12 +245,16 @@ defmodule Authenticator.SignIn.Commands.ResourceOwner do }) end - defp parse_response(access_token, refresh_token, %{"ttl" => ttl, "typ" => typ}) do - %{ + defp parse_response({:ok, access_token, refresh_token, %{"ttl" => ttl, "typ" => typ}}) do + payload = %{ access_token: access_token, refresh_token: refresh_token, expires_in: ttl, token_type: typ } + + {:ok, payload} end + + defp parse_response({:error, _reason} = error), do: error end diff --git a/apps/authenticator/test/authenticator/sign_in/commands/resource_owner_test.exs b/apps/authenticator/test/authenticator/sign_in/commands/resource_owner_test.exs index b016575..4358656 100644 --- a/apps/authenticator/test/authenticator/sign_in/commands/resource_owner_test.exs +++ b/apps/authenticator/test/authenticator/sign_in/commands/resource_owner_test.exs @@ -128,7 +128,7 @@ defmodule Authenticator.SignIn.Commands.ResourceOwnerTest do test "succeeds using client_assertions and generates an access_token" do scopes = RF.insert_list!(:scope, 3) user = RF.insert!(:user) - app = RF.insert!(:client_application) + app = RF.insert!(:client_application, access_type: "confidential") public_key = RF.insert!(:public_key, client_application: app, value: get_public_key()) hash = RF.gen_hashed_password("MyPassw@rd234") password = RF.insert!(:password, user: user, password_hash: hash) @@ -176,7 +176,13 @@ defmodule Authenticator.SignIn.Commands.ResourceOwnerTest do test "succeeds using client_assertions and generates a refresh_token" do scopes = RF.insert_list!(:scope, 3) user = RF.insert!(:user) - app = RF.insert!(:client_application, grant_flows: ["resource_owner", "refresh_token"]) + + app = + RF.insert!(:client_application, + access_type: "confidential", + grant_flows: ["resource_owner", "refresh_token"] + ) + public_key = RF.insert!(:public_key, client_application: app, value: get_public_key()) hash = RF.gen_hashed_password("MyPassw@rd234") password = RF.insert!(:password, user: user, password_hash: hash) @@ -222,10 +228,20 @@ defmodule Authenticator.SignIn.Commands.ResourceOwnerTest do end test "fails if params are invalid" do - assert {:error, :invalid_params} == Command.execute(%{}) - assert {:error, changeset} = Command.execute(%{grant_type: "password"}) + assert %{ + client_assertion_type: ["can't be blank"], + client_assertion: ["can't be blank"], + username: ["can't be blank"], + password: ["can't be blank"], + client_id: ["can't be blank"], + ip_address: ["can't be blank"], + scope: ["can't be blank"] + } = errors_on(changeset) + + assert {:error, changeset} = Command.execute(%{"grant_type" => "password"}) + assert %{ client_assertion_type: ["can't be blank"], client_assertion: ["can't be blank"], @@ -290,37 +306,21 @@ defmodule Authenticator.SignIn.Commands.ResourceOwnerTest do end test "fails if client application secret do not match credential" do - input = %{ - username: "my-username", - password: "MyPassw@rd234", - grant_type: "password", - scope: "admin:read", - ip_address: "45.232.192.12", - client_id: Ecto.UUID.generate(), - client_secret: Ecto.UUID.generate() - } - - expect(ResourceManagerMock, :get_identity, fn _ -> - {:ok, RF.insert!(:client_application, secret: "another-secret")} - end) - - assert {:error, :unauthenticated} == Command.execute(input) - end + user = RF.insert!(:user) + app = RF.insert!(:client_application, access_type: "confidential", secret: "another-secret") - test "fails if client application is not confidential" do input = %{ - username: "my-username", + username: user.username, password: "MyPassw@rd234", grant_type: "password", scope: "admin:read", ip_address: "45.232.192.12", client_id: Ecto.UUID.generate(), - client_secret: "my-secret" + client_secret: Ecto.UUID.generate() } - expect(ResourceManagerMock, :get_identity, fn _ -> - {:ok, RF.insert!(:client_application, access_type: "public")} - end) + expect(ResourceManagerMock, :get_identity, fn _ -> {:ok, app} end) + expect(ResourceManagerMock, :get_identity, fn _ -> {:ok, user} end) assert {:error, :unauthenticated} == Command.execute(input) end @@ -410,7 +410,7 @@ defmodule Authenticator.SignIn.Commands.ResourceOwnerTest do end) expect(ResourceManagerMock, :get_identity, fn %{username: _} -> - user = RF.insert!(:user, status: "blocked") + user = RF.insert!(:user) hash = RF.gen_hashed_password("AnotherPassword") password = RF.insert!(:password, user: user, password_hash: hash) {:ok, %{user | password: password}} diff --git a/apps/authorizer/coveralls.json b/apps/authorizer/coveralls.json index ce7927a..173a9c6 100644 --- a/apps/authorizer/coveralls.json +++ b/apps/authorizer/coveralls.json @@ -7,6 +7,7 @@ "skip_files": [ "test/support/data_case.ex", "test/support/factory.ex", - "lib/ports/resource_manager.ex" + "lib/ports/resource_manager.ex", + "lib/authorizer.ex" ] } \ No newline at end of file diff --git a/apps/resource_manager/lib/identities/schemas/client_application.ex b/apps/resource_manager/lib/identities/schemas/client_application.ex index 8c0d1f2..85147eb 100644 --- a/apps/resource_manager/lib/identities/schemas/client_application.ex +++ b/apps/resource_manager/lib/identities/schemas/client_application.ex @@ -44,7 +44,7 @@ defmodule ResourceManager.Identities.Schemas.ClientApplication do field :status, :string, default: "active" field :blocked_until, :naive_datetime field :protocol, :string, default: "openid-connect" - field :access_type, :string, default: "confidential" + field :access_type, :string, default: "public" field :is_admin, :boolean, default: false field :grant_flows, {:array, :string} field :redirect_uri, :string diff --git a/apps/resource_manager/test/support/factory.ex b/apps/resource_manager/test/support/factory.ex index d13ab9b..771f4fb 100644 --- a/apps/resource_manager/test/support/factory.ex +++ b/apps/resource_manager/test/support/factory.ex @@ -25,6 +25,8 @@ defmodule ResourceManager.Factory do description: "It's a test application", grant_flows: ["resource_owner"], status: "active", + protocol: "openid-connect", + access_type: "public", is_admin: false, secret: gen_hashed_password(Ecto.UUID.generate(), :bcrypt) } diff --git a/apps/rest_api/test/controllers/public/auth_test.exs b/apps/rest_api/test/controllers/public/auth_test.exs index 63c8562..4b9f2ce 100644 --- a/apps/rest_api/test/controllers/public/auth_test.exs +++ b/apps/rest_api/test/controllers/public/auth_test.exs @@ -4,6 +4,7 @@ defmodule RestAPI.Controllers.Public.AuthTest do alias Authenticator.SignIn.Inputs.{ClientCredentials, RefreshToken, ResourceOwner} alias RestAPI.Ports.AuthenticatorMock + @content_type "application/x-www-form-urlencoded" @token_endpoint "/api/v1/auth/protocol/openid-connect/token" @logout_endpoint "api/v1/auth/protocol/openid-connect/logout" @logout_all_endpoint "api/v1/auth/protocol/openid-connect/logout-all-sessions" @@ -25,6 +26,7 @@ defmodule RestAPI.Controllers.Public.AuthTest do assert %{"access_token" => _, "refresh_token" => _, "token_type" => _, "expires_in" => _} = conn + |> put_req_header("content-type", @content_type) |> post(@token_endpoint, params) |> json_response(200) end @@ -41,6 +43,7 @@ defmodule RestAPI.Controllers.Public.AuthTest do assert %{"access_token" => _, "refresh_token" => _, "token_type" => _, "expires_in" => _} = conn + |> put_req_header("content-type", @content_type) |> post(@token_endpoint, params) |> json_response(200) end @@ -59,6 +62,7 @@ defmodule RestAPI.Controllers.Public.AuthTest do assert %{"access_token" => _, "refresh_token" => _, "token_type" => _, "expires_in" => _} = conn + |> put_req_header("content-type", @content_type) |> post(@token_endpoint, params) |> json_response(200) end @@ -79,6 +83,7 @@ defmodule RestAPI.Controllers.Public.AuthTest do } } = conn + |> put_req_header("content-type", @content_type) |> post(@token_endpoint, %{"grant_type" => "password"}) |> json_response(400) end @@ -90,6 +95,7 @@ defmodule RestAPI.Controllers.Public.AuthTest do assert %{"response" => %{"refresh_token" => ["can't be blank"]}} = conn + |> put_req_header("content-type", @content_type) |> post(@token_endpoint, %{"grant_type" => "refresh_token"}) |> json_response(400) end @@ -108,6 +114,7 @@ defmodule RestAPI.Controllers.Public.AuthTest do } } = conn + |> put_req_header("content-type", @content_type) |> post(@token_endpoint, %{"grant_type" => "client_credentials"}) |> json_response(400) end