From d8f415f332367c300c7c50021859cc7a6edda3fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Armin=20Ahmetovi=C4=87?= Date: Mon, 13 Jan 2025 10:10:47 +0100 Subject: [PATCH] Handle API authentication errors for the Pairing API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Expand the Fallback controller with some failed auth functions - Add additional error views with useful feedback messages - Add the gen_jwt_token_with_wrong_signature function in the JWTTestHelper - Add the necessary auth tests and adapt existing ones - Add new errors to the OpenAPI schema Signed-off-by: Armin Ahmetović --- .../controllers/fallback_controller.ex | 37 ++++- .../views/error_view.ex | 26 ++++ .../priv/static/astarte_pairing_api.yaml | 127 +++++++++++++----- .../auth/auth_test.exs | 33 ++++- .../controllers/agent_controller_test.exs | 4 +- .../test/support/jwt_test_helper.ex | 10 ++ 6 files changed, 194 insertions(+), 43 deletions(-) diff --git a/apps/astarte_pairing_api/lib/astarte_pairing_api_web/controllers/fallback_controller.ex b/apps/astarte_pairing_api/lib/astarte_pairing_api_web/controllers/fallback_controller.ex index a669bfa2a..d73319416 100644 --- a/apps/astarte_pairing_api/lib/astarte_pairing_api_web/controllers/fallback_controller.ex +++ b/apps/astarte_pairing_api/lib/astarte_pairing_api_web/controllers/fallback_controller.ex @@ -61,17 +61,46 @@ defmodule Astarte.Pairing.APIWeb.FallbackController do |> render(:"403") end - # This is the final call made by EnsureAuthenticated - def auth_error(conn, {:unauthenticated, reason}, _opts) do - _ = - Logger.info("Refusing unauthenticated request: #{inspect(reason)}.", tag: "unauthenticated") + # Invalid authorized path + def call(conn, {:error, :invalid_auth_path}) do + conn + |> put_status(:unauthorized) + |> put_view(Astarte.Pairing.APIWeb.ErrorView) + |> render(:invalid_auth_path) + end + + # This is called when no JWT token is present + def auth_error(conn, {:unauthenticated, :unauthenticated}, _opts) do + conn + |> put_status(:unauthorized) + |> put_view(Astarte.Pairing.APIWeb.ErrorView) + |> render(:missing_token) + end + + # Invalid JWT token + def auth_error(conn, {:invalid_token, :invalid_token}, _opts) do + conn + |> put_status(:unauthorized) + |> put_view(Astarte.Pairing.APIWeb.ErrorView) + |> render(:invalid_token) + end + + # Path not authorized + def auth_error(conn, {:unauthorized, :authorization_path_not_matched}, _opts) do + conn + |> put_status(:forbidden) + |> put_view(Astarte.Pairing.APIWeb.ErrorView) + |> render(:authorization_path_not_matched, %{method: conn.method, path: conn.request_path}) + end + def auth_error(conn, {:unauthenticated, _reason}, _opts) do conn |> put_status(:unauthorized) |> put_view(Astarte.Pairing.APIWeb.ErrorView) |> render(:"401") end + # In all other cases, we reply with 403 def auth_error(conn, _reason, _opts) do conn |> put_status(:forbidden) diff --git a/apps/astarte_pairing_api/lib/astarte_pairing_api_web/views/error_view.ex b/apps/astarte_pairing_api/lib/astarte_pairing_api_web/views/error_view.ex index 35067c476..7e79ec5e7 100644 --- a/apps/astarte_pairing_api/lib/astarte_pairing_api_web/views/error_view.ex +++ b/apps/astarte_pairing_api/lib/astarte_pairing_api_web/views/error_view.ex @@ -31,6 +31,32 @@ defmodule Astarte.Pairing.APIWeb.ErrorView do %{errors: %{detail: "Forbidden"}} end + def render("missing_token.json", _assigns) do + %{errors: %{detail: "Missing authorization token"}} + end + + def render("invalid_token.json", _assigns) do + %{errors: %{detail: "Invalid JWT token"}} + end + + # TODO: add error message for invalid_token_signature + + def render("invalid_auth_path.json", _assigns) do + %{ + errors: %{ + detail: "Authorization failed due to an invalid path" + } + } + end + + def render("authorization_path_not_matched.json", %{method: method, path: path}) do + %{ + errors: %{ + detail: "Unauthorized access to #{method} #{path}. Please verify your permissions" + } + } + end + def render("404.json", _assigns) do %{errors: %{detail: "Page not found"}} end diff --git a/apps/astarte_pairing_api/priv/static/astarte_pairing_api.yaml b/apps/astarte_pairing_api/priv/static/astarte_pairing_api.yaml index f3fba45af..9ee29f8f5 100644 --- a/apps/astarte_pairing_api/priv/static/astarte_pairing_api.yaml +++ b/apps/astarte_pairing_api/priv/static/astarte_pairing_api.yaml @@ -63,17 +63,15 @@ paths: data: credentials_secret: TTkd5OgB13X/3qU0LXU7OCxyTXz5QHM2NY1IgidtPOs= '401': - description: Unauthorized - content: - application/json: - schema: - $ref: '#/components/schemas/UnauthorizedResponse' + $ref: '#/components/responses/Unauthorized' '403': - description: Forbidden + description: Forbidden or Authorization path not matched content: application/json: schema: - $ref: '#/components/schemas/ForbiddenResponse' + oneOf: + - $ref: '#/components/schemas/ForbiddenResponse' + - $ref: '#/components/schemas/AuthorizationPathNotMatchedResponse' '422': description: Unprocessable entity content: @@ -135,17 +133,15 @@ paths: '204': description: Device unregistered '401': - description: Unauthorized - content: - application/json: - schema: - $ref: '#/components/schemas/UnauthorizedResponse' + $ref: '#/components/responses/Unauthorized' '403': - description: Forbidden + description: Forbidden or Authorization path not matched content: application/json: schema: - $ref: '#/components/schemas/ForbiddenResponse' + oneOf: + - $ref: '#/components/schemas/ForbiddenResponse' + - $ref: '#/components/schemas/AuthorizationPathNotMatchedResponse' '404': description: Device not found content: @@ -198,17 +194,15 @@ paths: astarte_mqtt_v1: broker_url: 'ssl://broker.astarte.example.com:8883' '401': - description: Unauthorized - content: - application/json: - schema: - $ref: '#/components/schemas/UnauthorizedResponse' + $ref: '#/components/responses/Unauthorized' '403': - description: Forbidden + description: Forbidden or Authorization path not matched content: application/json: schema: - $ref: '#/components/schemas/ForbiddenResponse' + oneOf: + - $ref: '#/components/schemas/ForbiddenResponse' + - $ref: '#/components/schemas/AuthorizationPathNotMatchedResponse' '/{realm_name}/devices/{hw_id}/protocols/astarte_mqtt_v1/credentials': post: tags: @@ -278,17 +272,15 @@ paths: ySj0xif2Z8U7MTfhmZs1cyDA/A== -----END CERTIFICATE----- '401': - description: Unauthorized - content: - application/json: - schema: - $ref: '#/components/schemas/UnauthorizedResponse' + $ref: '#/components/responses/Unauthorized' '403': - description: Forbidden + description: Forbidden or Authorization path not matched content: application/json: schema: - $ref: '#/components/schemas/ForbiddenResponse' + oneOf: + - $ref: '#/components/schemas/ForbiddenResponse' + - $ref: '#/components/schemas/AuthorizationPathNotMatchedResponse' '422': description: Unprocessable entity content: @@ -375,17 +367,15 @@ paths: valid: false cause: INVALID_ISSUER '401': - description: Unauthorized - content: - application/json: - schema: - $ref: '#/components/schemas/UnauthorizedResponse' + $ref: '#/components/responses/Unauthorized' '403': - description: Forbidden + description: Forbidden or Authorization path not matched content: application/json: schema: - $ref: '#/components/schemas/ForbiddenResponse' + oneOf: + - $ref: '#/components/schemas/ForbiddenResponse' + - $ref: '#/components/schemas/AuthorizationPathNotMatchedResponse' '422': description: Unprocessable entity content: @@ -465,6 +455,26 @@ components: The following syntax must be used in the 'Authorization' header : Bearer xxxxxxxxxxxxxxxxxxxxx + responses: + Unauthorized: + description: Token/Realm doesn't exist or operation not allowed. + content: + application/json: + schema: + oneOf: + - $ref: '#/components/schemas/MissingTokenResponse' + - $ref: '#/components/schemas/InvalidTokenResponse' + - $ref: '#/components/schemas/InvalidAuthPathResponse' + - $ref: '#/components/schemas/UnauthorizedResponse' + AuthorizationPathNotMatched: + description: Authorization path not matched. + content: + application/json: + schema: + type: object + properties: + data: + $ref: '#/components/schemas/AuthorizationPathNotMatchedResponse' schemas: DeviceRegistrationRequest: type: object @@ -578,3 +588,50 @@ components: example: errors: detail: Unauthorized + MissingTokenResponse: + type: object + properties: + errors: + type: object + properties: + detail: + type: string + example: + errors: + detail: Missing authorization token + + InvalidTokenResponse: + type: object + properties: + errors: + type: object + properties: + detail: + type: string + example: + errors: + detail: Invalid JWT token + + InvalidAuthPathResponse: + type: object + properties: + errors: + type: object + properties: + detail: + type: string + example: + errors: + detail: Authorization failed due to an invalid path + + AuthorizationPathNotMatchedResponse: + type: object + properties: + errors: + type: object + properties: + detail: + type: string + example: + errors: + detail: Unauthorized access to GET /api/v1/some_path. Please verify your permissions diff --git a/apps/astarte_pairing_api/test/astarte_pairing_api_web/auth/auth_test.exs b/apps/astarte_pairing_api/test/astarte_pairing_api_web/auth/auth_test.exs index c30c9a401..a01b38aff 100644 --- a/apps/astarte_pairing_api/test/astarte_pairing_api_web/auth/auth_test.exs +++ b/apps/astarte_pairing_api/test/astarte_pairing_api_web/auth/auth_test.exs @@ -45,6 +45,11 @@ defmodule Astarte.Pairing.APIWeb.AuthTest do {:ok, conn: conn} end + test "no token returns 401", %{conn: conn} do + conn = post(conn, agent_path(conn, :create, @realm), data: @create_attrs) + assert json_response(conn, 401)["errors"]["detail"] == "Missing authorization token" + end + test "succeeds with specific authorizations", %{conn: conn} do register_authorizations = ["POST::agent/devices"] @@ -86,7 +91,8 @@ defmodule Astarte.Pairing.APIWeb.AuthTest do |> authorize_conn(register_authorizations) |> post(agent_path(conn, :create, @realm), data: @create_attrs) - assert json_response(conn, 403)["errors"]["detail"] == "Forbidden" + assert json_response(conn, 403)["errors"]["detail"] == + "Unauthorized access to #{conn.assigns.method} #{conn.assigns.path}. Please verify your permissions" end test "fails with authorization for different method", %{conn: conn} do @@ -97,7 +103,30 @@ defmodule Astarte.Pairing.APIWeb.AuthTest do |> authorize_conn(register_authorizations) |> post(agent_path(conn, :create, @realm), data: @create_attrs) - assert json_response(conn, 403)["errors"]["detail"] == "Forbidden" + assert json_response(conn, 403)["errors"]["detail"] == + "Unauthorized access to #{conn.assigns.method} #{conn.assigns.path}. Please verify your permissions" + end + + test "invalid JWT token returns 401", %{conn: conn} do + conn = + put_req_header( + conn, + "authorization", + "bearer invalid_token" + ) + |> post(agent_path(conn, :create, @realm), data: @create_attrs) + + assert json_response(conn, 401)["errors"]["detail"] == "Invalid JWT token" + end + + test "token with mismatched signature returns 401", %{conn: conn} do + token = JWTTestHelper.gen_jwt_token_with_wrong_signature(["^POST$::agent/devices"]) + + conn = + put_req_header(conn, "authorization", "bearer #{token}") + |> post(agent_path(conn, :create, @realm), data: @create_attrs) + + assert json_response(conn, 401)["errors"]["detail"] == "Invalid JWT token" end end diff --git a/apps/astarte_pairing_api/test/astarte_pairing_api_web/controllers/agent_controller_test.exs b/apps/astarte_pairing_api/test/astarte_pairing_api_web/controllers/agent_controller_test.exs index cdd8cc98b..dc24f57f5 100644 --- a/apps/astarte_pairing_api/test/astarte_pairing_api_web/controllers/agent_controller_test.exs +++ b/apps/astarte_pairing_api/test/astarte_pairing_api_web/controllers/agent_controller_test.exs @@ -199,7 +199,7 @@ defmodule Astarte.Pairing.APIWeb.AgentControllerTest do |> delete_req_header("authorization") |> post(agent_path(conn, :create, @realm), data: @create_attrs) - assert json_response(conn, 401)["errors"] == %{"detail" => "Unauthorized"} + assert json_response(conn, 401)["errors"] == %{"detail" => "Missing authorization token"} end end @@ -258,7 +258,7 @@ defmodule Astarte.Pairing.APIWeb.AgentControllerTest do |> delete_req_header("authorization") |> delete(agent_path(conn, :delete, @realm, @device_id)) - assert json_response(conn, 401)["errors"] == %{"detail" => "Unauthorized"} + assert json_response(conn, 401)["errors"] == %{"detail" => "Missing authorization token"} end end diff --git a/apps/astarte_pairing_api/test/support/jwt_test_helper.ex b/apps/astarte_pairing_api/test/support/jwt_test_helper.ex index fb1e25ce3..1e46d145c 100644 --- a/apps/astarte_pairing_api/test/support/jwt_test_helper.ex +++ b/apps/astarte_pairing_api/test/support/jwt_test_helper.ex @@ -40,6 +40,16 @@ defmodule Astarte.Pairing.APIWeb.JWTTestHelper do jwt end + def gen_jwt_token_with_wrong_signature(authorization_paths) do + valid_token = gen_jwt_token(authorization_paths) + + [header, payload, _signature] = String.split(valid_token, ".") + + fake_signature = "fake_signature" + + "#{header}.#{payload}.#{fake_signature}" + end + def gen_jwt_all_access_token do gen_jwt_token([".*::.*"]) end