Skip to content

feat: support for impersonation token exchange#88

Closed
seriousben wants to merge 1 commit intomainfrom
seriousben/user-identifier-token-exchange
Closed

feat: support for impersonation token exchange#88
seriousben wants to merge 1 commit intomainfrom
seriousben/user-identifier-token-exchange

Conversation

@seriousben
Copy link
Copy Markdown
Member

No description provided.

@seriousben seriousben force-pushed the seriousben/user-identifier-token-exchange branch 3 times, most recently from a5c5153 to 372415b Compare March 26, 2026 14:15
Copy link
Copy Markdown
Contributor

@Larry-Osakwe Larry-Osakwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. one thing - build_user_identifier_token validates empty string with a ValueError but there's no test covering that path. worth adding.

also just a heads up on versioning - this repo uses commitizen with conventional commits. when this merges the commit message should follow feat(keycardai-oauth): add user_identifier parameter to token exchange so the minor version bump and changelog get generated automatically. check DEVELOPER.md for the full format.

@seriousben seriousben force-pushed the seriousben/user-identifier-token-exchange branch from 372415b to 2feff31 Compare April 1, 2026 22:05
@seriousben seriousben changed the title Add user_identifier parameter to token exchange feat: support for impersonation token exchange Apr 1, 2026
@seriousben seriousben force-pushed the seriousben/user-identifier-token-exchange branch 5 times, most recently from 81fb093 to 21bf638 Compare April 2, 2026 12:48
Copy link
Copy Markdown
Contributor

@Larry-Osakwe Larry-Osakwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. impersonate() follows the same pattern as exchange_token() on both clients, JWT builder is in the right place, and the MCP grant decorator change is clean.

Few things worth noting but not blocking:

PR is bigger than the title suggests. Beyond impersonation, this also adds exchange_authorization_code(), build_authorize_url(), a full _authorize.py module, and implements the PKCE methods that were previously stubs. Might be worth calling that out in the description so it's clear what's shipping here.

landing_page.py reaches into private APIs (_ensure_initialized(), _get_current_endpoints()). Would be nice if exchange_authorization_code handled discovery internally like impersonate and exchange_token do, so the example doesn't need to touch underscored methods. Not a blocker for this PR though.

The code scanning alert on _send_redirect is a false positive. The CR/LF stripping is already there.

Example is set up well. pyproject.toml with editable path dep, uv.lock, .env.example, CLI flags. Someone with a zone configured can pull this down and run it.

@seriousben seriousben force-pushed the seriousben/user-identifier-token-exchange branch from 21bf638 to cecacc9 Compare April 2, 2026 16:29
@seriousben seriousben closed this Apr 2, 2026
@seriousben seriousben deleted the seriousben/user-identifier-token-exchange branch April 2, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants