Skip to content

fix(auth): include retry interval in pending device authorization error #264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jskorlol wants to merge 1 commit into
base: main
Choose a base branch
from
+404 −60
Open

fix(auth): include retry interval in pending device authorization error #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions lib/codex_pooler/upstreams/auth/codex_auth.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
95 changes: 75 additions & 20 deletions lib/codex_pooler_web/live/admin/components/pages/upstreams/cockpit/dialogs.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
>
<a
id="oauth-relink-authorization-url"
href={@oauth_relink_authorization_url}
target="_blank"
rel="noopener noreferrer"
class="btn btn-primary w-full justify-start gap-2 text-left"
>
<.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" />
<span class="truncate">Open OpenAI authorization</span>
</a>
<div class="flex min-w-0 items-stretch gap-2">
<a
id="oauth-relink-authorization-url"
href={@oauth_relink_authorization_url}
target="_blank"
rel="noopener noreferrer"
class="btn btn-primary min-w-0 flex-1 justify-start gap-2 text-left"
>
<.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" />
<span class="truncate">Open OpenAI authorization</span>
</a>
<button
id="oauth-relink-authorization-url-copy"
type="button"
class="btn btn-square size-10 shrink-0"
phx-hook="ClipboardCopy"
data-copy-text={@oauth_relink_authorization_url}
data-copy-label="Copy"
data-copied-label="Copied"
aria-label="Copy OpenAI authorization URL"
>
<.icon name="hero-clipboard-document" class="copy-icon size-4" />
<span data-copy-label class="sr-only" aria-live="polite" aria-atomic="true">
Copy
</span>
</button>
</div>

<.form
id="oauth-relink-callback-form"
Expand Down Expand Up @@ -109,19 +126,57 @@ defmodule CodexPoolerWeb.Admin.UpstreamCockpitComponents.Dialogs do
<p class="text-xs font-semibold uppercase tracking-wide text-base-content/60">
Device code
</p>
<p class="font-mono text-2xl font-bold tracking-widest text-base-content">
{@oauth_relink_flow.device_user_code}
</p>
<div class="flex min-w-0 items-center gap-2">
<p class="min-w-0 flex-1 break-all font-mono text-2xl font-bold tracking-widest text-base-content">
{@oauth_relink_flow.device_user_code}
</p>
<button
id="oauth-relink-device-code-copy"
type="button"
class="btn btn-square size-10 shrink-0"
phx-hook="ClipboardCopy"
data-copy-text={@oauth_relink_flow.device_user_code}
data-copy-label="Copy"
data-copied-label="Copied"
aria-label="Copy device code"
>
<.icon name="hero-clipboard-document" class="copy-icon size-4" />
<span data-copy-label class="sr-only" aria-live="polite" aria-atomic="true">
Copy
</span>
</button>
</div>
</div>
<a
<div
:if={@oauth_relink_flow.verification_uri}
href={@oauth_relink_flow.verification_uri}
target="_blank"
rel="noopener noreferrer"
class="link link-primary break-all text-sm"
class="flex min-w-0 items-stretch gap-2"
>
{@oauth_relink_flow.verification_uri}
</a>
<a
id="oauth-relink-device-verification-url"
href={@oauth_relink_flow.verification_uri}
target="_blank"
rel="noopener noreferrer"
title={@oauth_relink_flow.verification_uri}
class="link link-primary min-w-0 flex-1 self-center truncate text-sm"
>
{@oauth_relink_flow.verification_uri}
</a>
<button
id="oauth-relink-device-verification-url-copy"
type="button"
class="btn btn-square size-10 shrink-0"
phx-hook="ClipboardCopy"
data-copy-text={@oauth_relink_flow.verification_uri}
data-copy-label="Copy"
data-copied-label="Copied"
aria-label="Copy device verification URL"
>
<.icon name="hero-clipboard-document" class="copy-icon size-4" />
<span data-copy-label class="sr-only" aria-live="polite" aria-atomic="true">
Copy
</span>
</button>
</div>
</section>
</div>

Expand Down
95 changes: 75 additions & 20 deletions lib/codex_pooler_web/live/admin/components/pages/upstreams/page_components.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
>
<a
id="oauth-link-authorization-url"
href={@oauth_link_authorization_url}
target="_blank"
rel="noopener noreferrer"
class="btn btn-primary w-full justify-start gap-2 text-left"
>
<.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" />
<span class="truncate">Open OpenAI authorization</span>
</a>
<div class="flex min-w-0 items-stretch gap-2">
<a
id="oauth-link-authorization-url"
href={@oauth_link_authorization_url}
target="_blank"
rel="noopener noreferrer"
class="btn btn-primary min-w-0 flex-1 justify-start gap-2 text-left"
>
<.icon name="hero-arrow-top-right-on-square" class="size-4 shrink-0" />
<span class="truncate">Open OpenAI authorization</span>
</a>
<button
id="oauth-link-authorization-url-copy"
type="button"
class="btn btn-square size-10 shrink-0"
phx-hook="ClipboardCopy"
data-copy-text={@oauth_link_authorization_url}
data-copy-label="Copy"
data-copied-label="Copied"
aria-label="Copy OpenAI authorization URL"
>
<.icon name="hero-clipboard-document" class="copy-icon size-4" />
<span data-copy-label class="sr-only" aria-live="polite" aria-atomic="true">
Copy
</span>
</button>
</div>

<.form
id="oauth-link-callback-form"
Expand Down Expand Up @@ -466,19 +483,57 @@ defmodule CodexPoolerWeb.Admin.UpstreamPageComponents do
<p class="text-xs font-semibold uppercase tracking-wide text-base-content/60">
Device code
</p>
<p class="font-mono text-2xl font-bold tracking-widest text-base-content">
{@oauth_link_flow.device_user_code}
</p>
<div class="flex min-w-0 items-center gap-2">
<p class="min-w-0 flex-1 break-all font-mono text-2xl font-bold tracking-widest text-base-content">
{@oauth_link_flow.device_user_code}
</p>
<button
id="oauth-link-device-code-copy"
type="button"
class="btn btn-square size-10 shrink-0"
phx-hook="ClipboardCopy"
data-copy-text={@oauth_link_flow.device_user_code}
data-copy-label="Copy"
data-copied-label="Copied"
aria-label="Copy device code"
>
<.icon name="hero-clipboard-document" class="copy-icon size-4" />
<span data-copy-label class="sr-only" aria-live="polite" aria-atomic="true">
Copy
</span>
</button>
</div>
</div>
<a
<div
:if={@oauth_link_flow.verification_uri}
href={@oauth_link_flow.verification_uri}
target="_blank"
rel="noopener noreferrer"
class="link link-primary break-all text-sm"
class="flex min-w-0 items-stretch gap-2"
>
{@oauth_link_flow.verification_uri}
</a>
<a
id="oauth-link-device-verification-url"
href={@oauth_link_flow.verification_uri}
target="_blank"
rel="noopener noreferrer"
title={@oauth_link_flow.verification_uri}
class="link link-primary min-w-0 flex-1 self-center truncate text-sm"
>
{@oauth_link_flow.verification_uri}
</a>
<button
id="oauth-link-device-verification-url-copy"
type="button"
class="btn btn-square size-10 shrink-0"
phx-hook="ClipboardCopy"
data-copy-text={@oauth_link_flow.verification_uri}
data-copy-label="Copy"
data-copied-label="Copied"
aria-label="Copy device verification URL"
>
<.icon name="hero-clipboard-document" class="copy-icon size-4" />
<span data-copy-label class="sr-only" aria-live="polite" aria-atomic="true">
Copy
</span>
</button>
</div>
</section>
</div>

Expand Down
41 changes: 41 additions & 0 deletions test/codex_pooler/upstreams/auth/codex_auth_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 21 additions & 1 deletion test/codex_pooler/upstreams/oauth_device_linking_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading