diff --git a/lib/codex_pooler/upstreams/auth/codex_auth.ex b/lib/codex_pooler/upstreams/auth/codex_auth.ex index dd85f2893..eb8a9f44d 100644 --- a/lib/codex_pooler/upstreams/auth/codex_auth.ex +++ b/lib/codex_pooler/upstreams/auth/codex_auth.ex @@ -536,13 +536,15 @@ defmodule CodexPooler.Upstreams.Auth.CodexAuth do 403 ) - defp poll_error(_body, _state, status) when status in [403, 404], - do: - auth_error( - :codex_device_authorization_pending, - "Codex device authorization is still pending", - 200 - ) + defp poll_error(_body, state, status) when status in [403, 404] do + {:error, + %{ + code: :codex_device_authorization_pending, + message: "Codex device authorization is still pending", + retry_after_seconds: parse_interval(state["poll_interval_seconds"]), + status: 200 + }} + end defp poll_error(_body, _state, status) when status >= 500, do: diff --git a/lib/codex_pooler_web/live/admin/components/pages/upstreams/cockpit/dialogs.ex b/lib/codex_pooler_web/live/admin/components/pages/upstreams/cockpit/dialogs.ex index 1aa839b89..955470315 100644 --- a/lib/codex_pooler_web/live/admin/components/pages/upstreams/cockpit/dialogs.ex +++ b/lib/codex_pooler_web/live/admin/components/pages/upstreams/cockpit/dialogs.ex @@ -55,16 +55,33 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitComponents.Dialogs do :if={oauth_relink_browser_flow?(@oauth_relink_flow, @oauth_relink_authorization_url)} class="grid gap-4 rounded-lg border border-base-300 bg-base-200/40 p-4" > - - <.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" /> - Open OpenAI authorization - +
+ + <.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" /> + Open OpenAI authorization + + +
<.form id="oauth-relink-callback-form" @@ -109,19 +126,57 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitComponents.Dialogs do

Device code

-

- {@oauth_relink_flow.device_user_code} -

+
+

+ {@oauth_relink_flow.device_user_code} +

+ +
- - {@oauth_relink_flow.verification_uri} - + + {@oauth_relink_flow.verification_uri} + + + diff --git a/lib/codex_pooler_web/live/admin/components/pages/upstreams/page_components.ex b/lib/codex_pooler_web/live/admin/components/pages/upstreams/page_components.ex index 2dc6585f4..559e6a751 100644 --- a/lib/codex_pooler_web/live/admin/components/pages/upstreams/page_components.ex +++ b/lib/codex_pooler_web/live/admin/components/pages/upstreams/page_components.ex @@ -412,16 +412,33 @@ defmodule CodexPoolerWeb.Admin.UpstreamPageComponents do :if={oauth_browser_flow?(@oauth_link_flow, @oauth_link_authorization_url)} class="grid gap-4 rounded-lg border border-base-300 bg-base-200/40 p-4" > - - <.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" /> - Open OpenAI authorization - +
+ + <.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" /> + Open OpenAI authorization + + +
<.form id="oauth-link-callback-form" @@ -466,19 +483,57 @@ defmodule CodexPoolerWeb.Admin.UpstreamPageComponents do

Device code

-

- {@oauth_link_flow.device_user_code} -

+
+

+ {@oauth_link_flow.device_user_code} +

+ +
- - {@oauth_link_flow.verification_uri} - + + {@oauth_link_flow.verification_uri} + + + diff --git a/test/codex_pooler/upstreams/auth/codex_auth_test.exs b/test/codex_pooler/upstreams/auth/codex_auth_test.exs index 100dacca7..cd2bf303e 100644 --- a/test/codex_pooler/upstreams/auth/codex_auth_test.exs +++ b/test/codex_pooler/upstreams/auth/codex_auth_test.exs @@ -324,6 +324,47 @@ defmodule CodexPooler.Upstreams.Auth.CodexAuthTest do assert [_request] = FakeOpenAIAuthProvider.requests(slow_down_provider) end + + test "nested pending responses over 403 include a sanitized retry interval" do + raw_provider_value = "raw-nested-device-error-must-not-leak" + + for {interval, expected_retry} <- [ + {"7", 7}, + {nil, 5}, + {"invalid", 5}, + {"5seconds", 5}, + {0, 5}, + {-1, 5} + ] do + provider = + start_provider!(%{ + "/api/accounts/deviceauth/token" => + {403, + %{ + "error" => %{ + "code" => "deviceauth_authorization_pending", + "message" => raw_provider_value + } + }} + }) + + assert {:error, + %{ + code: :codex_device_authorization_pending, + message: "Codex device authorization is still pending", + retry_after_seconds: ^expected_retry, + status: 200 + } = error} = + CodexAuth.poll_device_authorization(%{ + "device_auth_id" => "device-auth-nested-pending", + "user_code" => "NESTED-PENDING", + "poll_interval_seconds" => interval + }) + + refute inspect(error) =~ raw_provider_value + assert [_request] = FakeOpenAIAuthProvider.requests(provider) + end + end end describe "refresh-token OAuth protocol" do diff --git a/test/codex_pooler/upstreams/oauth_device_linking_test.exs b/test/codex_pooler/upstreams/oauth_device_linking_test.exs index b11ced36c..0af706c47 100644 --- a/test/codex_pooler/upstreams/oauth_device_linking_test.exs +++ b/test/codex_pooler/upstreams/oauth_device_linking_test.exs @@ -254,17 +254,37 @@ defmodule CodexPooler.Upstreams.OAuthDeviceLinkingTest do test "pending device flow can be resumed from database and completed by a later poll" do scope = fixture_owner_scope() pool = pool_fixture() + raw_provider_value = "raw-nested-device-pending-must-not-leak" provider = start_provider!( device_routes(%{ - "/api/accounts/deviceauth/token" => {400, %{"error" => "authorization_pending"}} + "/api/accounts/deviceauth/token" => + {403, + %{ + "error" => %{ + "code" => "deviceauth_authorization_pending", + "message" => raw_provider_value + } + }} }) ) assert {:ok, %{flow: flow}} = Upstreams.start_device_oauth(scope, pool) assert {:ok, %{status: :pending, flow: pending}} = Upstreams.poll_device_oauth(scope, flow.id) + assert pending.status == "pending" + assert pending.interval_seconds == 5 + assert %DateTime{} = pending.last_polled_at + assert DateTime.diff(pending.poll_after_at, pending.last_polled_at, :second) in 4..5 + assert pending.error_code == nil + assert pending.error_message == nil + assert pending.completed_at == nil + assert pending.result_upstream_identity_id == nil + refute inspect(pending) =~ raw_provider_value + assert Repo.aggregate(UpstreamIdentity, :count) == 0 + assert Repo.aggregate(PoolUpstreamAssignment, :count) == 0 + assert Repo.aggregate(EncryptedSecret, :count) == 0 FakeUpstream.set_mode( provider, diff --git a/test/codex_pooler_web/live/admin/pages/upstream_cockpit_live_test.exs b/test/codex_pooler_web/live/admin/pages/upstream_cockpit_live_test.exs index 6e8af192b..c195e8a6f 100644 --- a/test/codex_pooler_web/live/admin/pages/upstream_cockpit_live_test.exs +++ b/test/codex_pooler_web/live/admin/pages/upstream_cockpit_live_test.exs @@ -9,6 +9,7 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitLiveTest do alias CodexPooler.Audit alias CodexPooler.Events alias CodexPooler.FakeOpenAIAuthProvider + alias CodexPooler.FakeUpstream alias CodexPooler.Gateway.Persistence.RoutingCircuitState alias CodexPooler.Jobs.SavedResetRedemptionWorker alias CodexPooler.Pools @@ -728,6 +729,24 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitLiveTest do assert has_element?(view, "#oauth-relink-submit-callback") authorization_url = oauth_relink_authorization_url_from_view(view) + + assert has_element?( + view, + ~s(#oauth-relink-authorization-url[target="_blank"][rel="noopener noreferrer"]) + ) + + assert has_element?( + view, + ~s(#oauth-relink-authorization-url-copy[type="button"][phx-hook="ClipboardCopy"][data-copy-text="#{authorization_url}"][aria-label="Copy OpenAI authorization URL"]) + ) + + assert has_element?( + view, + "#oauth-relink-authorization-url-copy .copy-icon.hero-clipboard-document" + ) + + assert has_element?(view, "#oauth-relink-authorization-url-copy [data-copy-label].sr-only") + callback_url = callback_url(authorization_state(authorization_url), "cockpit-browser-code") view @@ -781,6 +800,7 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitLiveTest do access_token = runtime_secret("cockpit-oauth-device-access") refresh_token = runtime_secret("cockpit-oauth-device-refresh") id_token = oauth_id_token("acct_cockpit_device_ui", "workspace-cockpit-ui") + pending_provider_value = "raw-cockpit-device-pending-must-not-leak" provider = start_oauth_provider!( @@ -794,11 +814,13 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitLiveTest do expires_at: DateTime.add(DateTime.utc_now(), 600, :second) |> DateTime.to_iso8601() )}, "/api/accounts/deviceauth/token" => - {200, - FakeOpenAIAuthProvider.authorization_code_response( - authorization_code: authorization_code, - code_verifier: code_verifier - )}, + {403, + %{ + "error" => %{ + "code" => "deviceauth_authorization_pending", + "message" => pending_provider_value + } + }}, "/oauth/token" => {200, FakeOpenAIAuthProvider.token_response( @@ -819,16 +841,79 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitLiveTest do assert has_element?(view, "#oauth-relink-device-code", "COCKPIT-CODE") + assert has_element?( + view, + ~s(#oauth-relink-device-code-copy[type="button"][phx-hook="ClipboardCopy"][data-copy-text="COCKPIT-CODE"][aria-label="Copy device code"]) + ) + + assert has_element?( + view, + "#oauth-relink-device-code-copy .copy-icon.hero-clipboard-document" + ) + + assert has_element?(view, "#oauth-relink-device-code-copy [data-copy-label].sr-only") + assert has_element?( view, "#oauth-relink-device-code", FakeOpenAIAuthProvider.url(provider) <> "/codex/device" ) + verification_url = FakeOpenAIAuthProvider.url(provider) <> "/codex/device" + + assert has_element?( + view, + ~s(#oauth-relink-device-verification-url[href="#{verification_url}"][target="_blank"][rel="noopener noreferrer"]) + ) + + assert has_element?( + view, + ~s(#oauth-relink-device-verification-url-copy[type="button"][phx-hook="ClipboardCopy"][data-copy-text="#{verification_url}"][aria-label="Copy device verification URL"]) + ) + + assert has_element?( + view, + "#oauth-relink-device-verification-url-copy .copy-icon.hero-clipboard-document" + ) + + assert has_element?( + view, + "#oauth-relink-device-verification-url-copy [data-copy-label].sr-only" + ) + flow = Repo.one!(OAuthFlow) send(view.pid, {:poll_oauth_relink_device, flow.id}) _ = :sys.get_state(view.pid) + assert has_element?(view, "#oauth-relink-device-code", "COCKPIT-CODE") + refute has_element?(view, "#oauth-relink-error", "OAuth token exchange failed") + assert Repo.get!(OAuthFlow, flow.id).status == "pending" + assert Repo.get!(OAuthFlow, flow.id).error_code == nil + assert Repo.aggregate(UpstreamIdentity, :count) == 1 + + FakeUpstream.set_mode( + provider, + {:path_json, + device_routes(%{ + "/api/accounts/deviceauth/token" => + {200, + FakeOpenAIAuthProvider.authorization_code_response( + authorization_code: authorization_code, + code_verifier: code_verifier + )}, + "/oauth/token" => + {200, + FakeOpenAIAuthProvider.token_response( + access_token: access_token, + refresh_token: refresh_token, + id_token: id_token + )} + })} + ) + + send(view.pid, {:poll_oauth_relink_device, flow.id}) + _ = :sys.get_state(view.pid) + assert has_element?(view, "#oauth-relink-status", "OpenAI account relinked") assert has_element?(view, "#oauth-relink-cancel", "Close") assert Repo.aggregate(UpstreamIdentity, :count) == 1 @@ -844,7 +929,8 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitLiveTest do code_verifier, access_token, refresh_token, - id_token + id_token, + pending_provider_value ] do refute html =~ raw_value end diff --git a/test/codex_pooler_web/live/admin/pages/upstreams_live_test.exs b/test/codex_pooler_web/live/admin/pages/upstreams_live_test.exs index ec419711c..8c02928e8 100644 --- a/test/codex_pooler_web/live/admin/pages/upstreams_live_test.exs +++ b/test/codex_pooler_web/live/admin/pages/upstreams_live_test.exs @@ -13,6 +13,7 @@ defmodule CodexPoolerWeb.Admin.UpstreamsLiveTest do alias CodexPooler.Events.Event alias CodexPooler.Events.PostgresBridge alias CodexPooler.FakeOpenAIAuthProvider + alias CodexPooler.FakeUpstream alias CodexPooler.Gateway.Persistence.RoutingCircuitState alias CodexPooler.Jobs.SavedResetRedemptionWorker alias CodexPooler.Jobs.TokenRefreshWorker @@ -1007,6 +1008,23 @@ defmodule CodexPoolerWeb.Admin.UpstreamsLiveTest do authorization_url = authorization_url_from_view(view) + assert has_element?( + view, + ~s(#oauth-link-authorization-url[target="_blank"][rel="noopener noreferrer"]) + ) + + assert has_element?( + view, + ~s(#oauth-link-authorization-url-copy[type="button"][phx-hook="ClipboardCopy"][data-copy-text="#{authorization_url}"][aria-label="Copy OpenAI authorization URL"]) + ) + + assert has_element?( + view, + "#oauth-link-authorization-url-copy .copy-icon.hero-clipboard-document" + ) + + assert has_element?(view, "#oauth-link-authorization-url-copy [data-copy-label].sr-only") + callback_url = provider_callback_url(authorization_state(authorization_url), "browser-admin-ui-code") @@ -1231,6 +1249,7 @@ defmodule CodexPoolerWeb.Admin.UpstreamsLiveTest do access_token = runtime_secret("oauth-device-access") refresh_token = runtime_secret("oauth-device-refresh") id_token = oauth_id_token("acct_admin_device") + pending_provider_value = "raw-admin-device-pending-must-not-leak" provider = start_oauth_provider!( @@ -1244,11 +1263,13 @@ defmodule CodexPoolerWeb.Admin.UpstreamsLiveTest do expires_at: DateTime.add(DateTime.utc_now(), 600, :second) |> DateTime.to_iso8601() )}, "/api/accounts/deviceauth/token" => - {200, - FakeOpenAIAuthProvider.authorization_code_response( - authorization_code: authorization_code, - code_verifier: code_verifier - )}, + {403, + %{ + "error" => %{ + "code" => "deviceauth_authorization_pending", + "message" => pending_provider_value + } + }}, "/oauth/token" => {200, FakeOpenAIAuthProvider.token_response( @@ -1270,16 +1291,79 @@ defmodule CodexPoolerWeb.Admin.UpstreamsLiveTest do assert has_element?(view, "#oauth-link-device-code", "CODE-UI") + assert has_element?( + view, + ~s(#oauth-link-device-code-copy[type="button"][phx-hook="ClipboardCopy"][data-copy-text="CODE-UI"][aria-label="Copy device code"]) + ) + + assert has_element?( + view, + "#oauth-link-device-code-copy .copy-icon.hero-clipboard-document" + ) + + assert has_element?(view, "#oauth-link-device-code-copy [data-copy-label].sr-only") + assert has_element?( view, "#oauth-link-device-code", FakeOpenAIAuthProvider.url(provider) <> "/codex/device" ) + verification_url = FakeOpenAIAuthProvider.url(provider) <> "/codex/device" + + assert has_element?( + view, + ~s(#oauth-link-device-verification-url[href="#{verification_url}"][target="_blank"][rel="noopener noreferrer"]) + ) + + assert has_element?( + view, + ~s(#oauth-link-device-verification-url-copy[type="button"][phx-hook="ClipboardCopy"][data-copy-text="#{verification_url}"][aria-label="Copy device verification URL"]) + ) + + assert has_element?( + view, + "#oauth-link-device-verification-url-copy .copy-icon.hero-clipboard-document" + ) + + assert has_element?( + view, + "#oauth-link-device-verification-url-copy [data-copy-label].sr-only" + ) + flow = Repo.one!(OAuthFlow) send(view.pid, {:poll_oauth_device, flow.id}) _ = :sys.get_state(view.pid) + assert has_element?(view, "#oauth-link-device-code", "CODE-UI") + refute has_element?(view, "#oauth-link-error", "OAuth token exchange failed") + assert Repo.get!(OAuthFlow, flow.id).status == "pending" + assert Repo.get!(OAuthFlow, flow.id).error_code == nil + assert Repo.aggregate(UpstreamIdentity, :count) == 0 + + FakeUpstream.set_mode( + provider, + {:path_json, + device_routes(%{ + "/api/accounts/deviceauth/token" => + {200, + FakeOpenAIAuthProvider.authorization_code_response( + authorization_code: authorization_code, + code_verifier: code_verifier + )}, + "/oauth/token" => + {200, + FakeOpenAIAuthProvider.token_response( + access_token: access_token, + refresh_token: refresh_token, + id_token: id_token + )} + })} + ) + + send(view.pid, {:poll_oauth_device, flow.id}) + _ = :sys.get_state(view.pid) + assert has_element?(view, "#oauth-link-status", "OpenAI account linked") assert has_element?(view, "#oauth-link-cancel", "Close") @@ -1295,7 +1379,8 @@ defmodule CodexPoolerWeb.Admin.UpstreamsLiveTest do code_verifier, access_token, refresh_token, - id_token + id_token, + pending_provider_value ] do refute html =~ raw_value end