Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the OAuth-denial detection story started in #614. When the OAuth provider returns
error=access_denied(e.g. the user clicked Cancel on the provider page), the backend now revokes the placeholderUserApiKeyinstead of leaving it inpending_authuntil TTL. The wizard's polling — which already recognizesrevokedas a terminal failure status (#614) — exits within ~2 seconds with the user-friendly "canceled or denied" message instead of waiting the full 5-minute deadline.closes #599
What changed
user_api_key_service::revoke_pending_placeholders_for_providerhelper that finds(user_id, provider_config_id, status: pending_auth, credential_type != "node_managed")placeholders and revokes each through the existing atomicrevoke_api_key_if_pending(race-safe against late OAuth-success callbacks).generic_oauth_callback_impl'serrorbranch now best-effort-peeks the OAuth state, resolves the effective owner (target_user_idfirst, thenuser_id), and revokes the matching placeholder. Every failure mode (missing state, invalid state, DB error) still returns the redirect — the audit log captures the failure for ops triage.Notes for reviewers
deniedstatus. The existingrevokedstatus keeps the enum lean and PR fix: detect OAuth/device-code denial in CLI and wizard polling #614 already wired the frontend to recognizerevoked/failed/expiredas terminal.node_managedplaceholders are excluded because they don't authorize via OAuth — they're configured on the node agent.oauth_state.target_user_idset) correctly revokes the placeholder under the SA / org user, not the initiating admin.Test plan
cargo test -p nyxid revoke_pending_placeholders_for_provider— 5 service-level tests (pending revoke, skip active, skip node_managed, empty DB, provider scoping)cargo test -p nyxid generic_oauth_callback— 3 handler tests (denial + state revokes, denial without state redirects only, denial + invalid state tolerated)cargo test(full suite)nyxid service add llm-anthropic), click Cancel on the provider page, confirm the wizard exits within ~2 seconds with "Authorization didn't complete (it may have been canceled or denied on the provider page). Cancel and re-run to try again."