From d6a665b0a3a319655e44f285ec9cfc15a5e06717 Mon Sep 17 00:00:00 2001 From: Owais Jamil Date: Fri, 19 Jun 2026 01:41:20 -0500 Subject: [PATCH] feat: native OAuth redirect URI validation --- TODO.md | 5 - lib/tempest/oauth/client_metadata.ex | 62 ++++++++- test/tempest/oauth/client_metadata_test.exs | 141 ++++++++++++++++++-- 3 files changed, 187 insertions(+), 21 deletions(-) diff --git a/TODO.md b/TODO.md index 46af8f9..c70522c 100644 --- a/TODO.md +++ b/TODO.md @@ -1,10 +1,5 @@ # Parking Lot (TODO) -- Add OAuth private-use redirect scheme support for native clients: - - accept reverse-domain private-use schemes only for native clients - - reject credentials, hosts, ports, fragments, and local/reserved scheme roots - - keep HTTP redirect URI support limited to loopback clients - - Add OAuth token introspection: - expose `/oauth/introspect` - return inactive for missing, revoked, rotated, expired, or malformed tokens diff --git a/lib/tempest/oauth/client_metadata.ex b/lib/tempest/oauth/client_metadata.ex index 598c6ab..8b893c4 100644 --- a/lib/tempest/oauth/client_metadata.ex +++ b/lib/tempest/oauth/client_metadata.ex @@ -11,6 +11,7 @@ defmodule Tempest.OAuth.ClientMetadata do @max_body_bytes 64 * 1024 @default_loopback_redirect_uris ["http://127.0.0.1/", "http://[::1]/"] + @reserved_private_scheme_roots ~w(arpa example invalid local localhost onion test) @type t :: %__MODULE__{ client_id: String.t(), @@ -80,8 +81,9 @@ defmodule Tempest.OAuth.ClientMetadata do defp parse_metadata(metadata, client_id, client_type) do with ^client_id <- Map.get(metadata, "client_id"), + application_type when application_type in ["web", "native"] <- Map.get(metadata, "application_type", "web"), redirect_uris when is_list(redirect_uris) <- Map.get(metadata, "redirect_uris"), - true <- Enum.all?(redirect_uris, &valid_redirect_uri?(&1, client_type)), + true <- Enum.all?(redirect_uris, &valid_redirect_uri?(&1, client_type, application_type, client_id)), true <- contains_string?(Map.get(metadata, "response_types"), "code"), true <- contains_string?(Map.get(metadata, "grant_types"), "authorization_code"), auth_method when auth_method in ["none", "private_key_jwt"] <- @@ -265,8 +267,16 @@ defmodule Tempest.OAuth.ClientMetadata do defp default_if_empty([], default), do: default defp default_if_empty(values, _default), do: values - defp valid_redirect_uri?(uri, :https_metadata), do: valid_https_redirect_uri?(uri) - defp valid_redirect_uri?(uri, :localhost_development), do: valid_loopback_redirect_uri?(uri) + defp valid_redirect_uri?(uri, :localhost_development, _application_type, _client_id), + do: valid_loopback_redirect_uri?(uri) + + defp valid_redirect_uri?(uri, :https_metadata, "web", _client_id), do: valid_https_redirect_uri?(uri) + + defp valid_redirect_uri?(uri, :https_metadata, "native", client_id) do + valid_native_https_redirect_uri?(uri, client_id) or valid_private_use_redirect_uri?(uri, client_id) + end + + defp valid_redirect_uri?(_uri, _client_type, _application_type, _client_id), do: false defp valid_https_redirect_uri?(uri) when is_binary(uri) do case URI.parse(uri) do @@ -277,6 +287,52 @@ defmodule Tempest.OAuth.ClientMetadata do defp valid_https_redirect_uri?(_uri), do: false + defp valid_native_https_redirect_uri?(uri, client_id) when is_binary(uri) and is_binary(client_id) do + with %URI{scheme: "https", host: host, fragment: nil} = redirect_uri when is_binary(host) <- URI.parse(uri), + %URI{} = client_uri <- URI.parse(client_id) do + origin(redirect_uri) == origin(client_uri) + else + _reason -> false + end + end + + defp valid_native_https_redirect_uri?(_uri, _client_id), do: false + + defp valid_private_use_redirect_uri?(uri, client_id) when is_binary(uri) and is_binary(client_id) do + with %URI{scheme: scheme, userinfo: nil, host: nil, port: nil, path: "/" <> path, fragment: nil} <- URI.parse(uri), + true <- path != "", + true <- String.downcase(scheme) == expected_private_use_scheme(client_id), + true <- private_scheme_root_allowed?(scheme) do + true + else + _reason -> false + end + end + + defp valid_private_use_redirect_uri?(_uri, _client_id), do: false + + defp expected_private_use_scheme(client_id) do + client_id + |> URI.parse() + |> Map.get(:host, "") + |> String.downcase() + |> String.split(".", trim: true) + |> Enum.reverse() + |> Enum.join(".") + end + + defp private_scheme_root_allowed?(scheme) do + scheme + |> String.downcase() + |> String.split(".", parts: 2) + |> List.first() + |> then(&(&1 not in @reserved_private_scheme_roots)) + end + + defp origin(%URI{scheme: scheme, host: host, port: port}) do + {scheme, String.downcase(host), port} + end + defp valid_loopback_redirect_uri?(uri) when is_binary(uri) do case URI.parse(uri) do %URI{scheme: "http", host: host, userinfo: nil, fragment: nil} when host in ["localhost", "127.0.0.1", "::1"] -> diff --git a/test/tempest/oauth/client_metadata_test.exs b/test/tempest/oauth/client_metadata_test.exs index 0d5a2b3..e32c5ce 100644 --- a/test/tempest/oauth/client_metadata_test.exs +++ b/test/tempest/oauth/client_metadata_test.exs @@ -5,6 +5,7 @@ defmodule Tempest.OAuth.ClientMetadataTest do alias Tempest.Security.ExternalMetadataFetcher @client_id "https://client.example.com/oauth/client-metadata.json" + @reserved_root_client_id "https://client.example/oauth/client-metadata.json" @redirect_uri "https://client.example.com/callback" setup context do @@ -14,7 +15,10 @@ defmodule Tempest.OAuth.ClientMetadataTest do original_fetcher_config = Application.get_env(:tempest, ExternalMetadataFetcher, []) Application.put_env(:tempest, ExternalMetadataFetcher, - dns_lookup: fn "client.example.com" -> {:ok, [{93, 184, 216, 34}]} end, + dns_lookup: fn + "client.example.com" -> {:ok, [{93, 184, 216, 34}]} + "client.example" -> {:ok, [{93, 184, 216, 34}]} + end, req_options: [plug: {Req.Test, __MODULE__}] ) @@ -149,6 +153,103 @@ defmodule Tempest.OAuth.ClientMetadataTest do }) end + test "accepts reverse-domain private-use redirects for native clients" do + Req.Test.expect(__MODULE__, fn conn -> + Req.Test.json(conn, native_metadata(%{"redirect_uris" => ["com.example.client:/callback"]})) + end) + + assert {:ok, %ClientMetadata{} = client} = + ClientMetadata.fetch_for_par(%{ + "client_id" => @client_id, + "redirect_uri" => "com.example.client:/callback", + "scope" => "atproto" + }) + + assert client.redirect_uris == ["com.example.client:/callback"] + end + + test "accepts same-origin https redirects for native clients" do + Req.Test.expect(__MODULE__, fn conn -> + Req.Test.json(conn, native_metadata(%{"redirect_uris" => ["https://client.example.com/native/callback"]})) + end) + + assert {:ok, %ClientMetadata{}} = + ClientMetadata.fetch_for_par(%{ + "client_id" => @client_id, + "redirect_uri" => "https://client.example.com/native/callback", + "scope" => "atproto" + }) + end + + test "rejects private-use redirects for web clients" do + Req.Test.expect(__MODULE__, fn conn -> + Req.Test.json(conn, metadata(%{"redirect_uris" => ["com.example.client:/callback"]})) + end) + + assert {:error, :invalid_client} = + ClientMetadata.fetch_for_par(%{ + "client_id" => @client_id, + "redirect_uri" => "com.example.client:/callback", + "scope" => "atproto" + }) + end + + test "rejects malformed or reserved private-use redirect schemes for native clients" do + invalid_redirects = [ + "com.example.client://callback", + "com.example.client:callback", + "com.example.client:/callback#fragment", + "com.example.other:/callback" + ] + + for redirect_uri <- invalid_redirects do + Req.Test.expect(__MODULE__, fn conn -> + Req.Test.json(conn, native_metadata(%{"redirect_uris" => [redirect_uri]})) + end) + + assert {:error, :invalid_client} = + ClientMetadata.fetch_for_par(%{ + "client_id" => @client_id, + "redirect_uri" => redirect_uri, + "scope" => "atproto" + }) + end + end + + test "rejects reserved private-use redirect scheme roots even when reverse-domain matched" do + redirect_uri = "example.client:/callback" + + Req.Test.expect(__MODULE__, fn conn -> + Req.Test.json( + conn, + native_metadata(%{ + "client_id" => @reserved_root_client_id, + "redirect_uris" => [redirect_uri] + }) + ) + end) + + assert {:error, :invalid_client} = + ClientMetadata.fetch_for_par(%{ + "client_id" => @reserved_root_client_id, + "redirect_uri" => redirect_uri, + "scope" => "atproto" + }) + end + + test "rejects native https redirects on a different origin" do + Req.Test.expect(__MODULE__, fn conn -> + Req.Test.json(conn, native_metadata(%{"redirect_uris" => ["https://other.example.com/callback"]})) + end) + + assert {:error, :invalid_client} = + ClientMetadata.fetch_for_par(%{ + "client_id" => @client_id, + "redirect_uri" => "https://other.example.com/callback", + "scope" => "atproto" + }) + end + test "synthesizes metadata for localhost development clients" do client_id = "http://localhost?redirect_uri=http%3A%2F%2F127.0.0.1%2Fcallback&scope=atproto%20rpc%3A*" @@ -202,18 +303,32 @@ defmodule Tempest.OAuth.ClientMetadataTest do end end - defp metadata do - %{ - "client_id" => @client_id, - "client_name" => "Client Metadata Test", - "redirect_uris" => [@redirect_uri], - "grant_types" => ["authorization_code", "refresh_token"], - "response_types" => ["code"], - "scope" => "atproto rpc:*", - "token_endpoint_auth_method" => "none", - "application_type" => "web", - "dpop_bound_access_tokens" => true - } + defp metadata(overrides \\ %{}) do + Map.merge( + %{ + "client_id" => @client_id, + "client_name" => "Client Metadata Test", + "redirect_uris" => [@redirect_uri], + "grant_types" => ["authorization_code", "refresh_token"], + "response_types" => ["code"], + "scope" => "atproto rpc:*", + "token_endpoint_auth_method" => "none", + "application_type" => "web", + "dpop_bound_access_tokens" => true + }, + overrides + ) + end + + defp native_metadata(overrides) do + metadata( + Map.merge( + %{ + "application_type" => "native" + }, + overrides + ) + ) end defp private_key_jwt_metadata do -- 2.51.2