fix: public applications and request content-type

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

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}}

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"
     ]
 }

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

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)
     }

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