Skip to content

feat(app): add OIDC federated app sign-in (start/exchange_token) - #1676

Open
mrunankpawar wants to merge 8 commits into
mainfrom
feat/oidc-federated-app-signin
Open

feat(app): add OIDC federated app sign-in (start/exchange_token)#1676
mrunankpawar wants to merge 8 commits into
mainfrom
feat/oidc-federated-app-signin

Conversation

@mrunankpawar

Copy link
Copy Markdown

Related Issues

Fixes https://github.com/descope/etc/issues/10672

Description

Adds descope_client.app.start()/exchange_token() for OIDC Federated Apps configured in the Descope Console, with Descope acting as the IDP.

  • start() builds the authorize URL (and a fresh PKCE pair) locally, with no network call
  • exchange_token() does the real code->token exchange as a standard OAuth2 client, returning the raw OAuth2/OIDC token shape.
  • flow + login_hint support a "homegrown first factor, Descope for MFA only" integration: run only an MFA-only Descope Flow for an already-identified user. See samples/app_oidc_mfa_sample_app.py for a full runnable example.

Verified end-to-end against a live confidential-client test app, including a full round trip through two real logins (default flow and a console-edited magic-link MFA flow) and real token exchange - see AppBase's docstring for exactly what was confirmed live vs. what's still open (public-client/PKCE-only path, redirect_uri validation timing, non-email login_hint).

Only OIDC federated apps are supported; SAML/WS-Fed federated apps are IDP-initiated with no code/token/exchange_token step at all and are out of scope here.

Adds descope_client.app.start()/exchange_token() for OIDC Federated Apps
configured in the Descope Console, with Descope acting as the IDP.

- start() builds the authorize URL (and a fresh PKCE pair) locally, with
  no network call - the authorize endpoint 303-redirects rather than
  returning JSON.
- exchange_token() does the real code->token exchange as a standard OAuth2
  client (form-encoded body, no Descope bearer header), returning the raw
  OAuth2/OIDC token shape.
- flow + login_hint support a "homegrown first factor, Descope for MFA
  only" integration: run only an MFA-only Descope Flow for an
  already-identified user. See samples/app_oidc_mfa_sample_app.py for a
  full runnable example.

Verified end-to-end against a live confidential-client test app,
including a full round trip through two real logins (default flow and a
console-edited magic-link MFA flow) and real token exchange - see
AppBase's docstring for exactly what was confirmed live vs. what's
still open (public-client/PKCE-only path, redirect_uri validation
timing, non-email login_hint).

Only OIDC federated apps are supported; SAML/WS-Fed federated apps are
IDP-initiated with no code/token/exchange_token step at all and are out
of scope here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shuni-bot

shuni-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot

shuni-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🐕 Suggested Reviewers

This PR adds OIDC federated app sign-in functionality with two primary components: OAuth2/OIDC token exchange. The key changed areas are: (1) Core client files (descope_client.py, descope_client_async.py) where the new app methods integrate, (2) Authentication method modules (app.py, app_async.py, _app_base.py) implementing the feature logic, (3) Common utilities (common.py) likely supporting PKCE and token handling, and (4) Documentation and samples. To ensure quality coverage, reviewers should span expertise in the client layer, async patterns, common utilities, and OIDC/OAuth2 flows.

Reviewer Reason
LioriE LioriE has 6 commits across 3 critical files including descope_client.py and descope_client_async.py—essential for reviewing the new start()/exchange_token() integration points and async implementation.
dorsha dorsha has 10 commits across README.md and descope/common.py, providing expertise in both documentation updates and common utilities (likely PKCE, token handling, and shared utilities needed by the new feature).
omercnet omercnet has 5 commits across descope/common.py and descope/descope_client.py, offering insight into both utility layer and client integration patterns, complementing LioriE's async coverage.
guyp-descope guyp-descope has 2 commits in descope_client.py with focused expertise in the client layer, useful for reviewing how the new app methods integrate into the main client structure.

Suggested by Shuni based on git history and PR context. Names are not @-mentioned to avoid notifying anyone — request a review from whoever fits best.

Comment thread samples/app_oidc_mfa_sample_app.py Fixed
Comment thread samples/app_oidc_mfa_sample_app.py Fixed
Comment thread samples/app_oidc_mfa_sample_app.py Fixed
mrunankpawar and others added 4 commits August 24, 2026 09:51
…e cross-site scripting'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… through an exception'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
… through an exception'

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

@shuni-bot shuni-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🐕 Shuni Review

Adds descope_client.app.start()/exchange_token() for OIDC Federated App sign-in, building the authorize URL/PKCE pair locally and doing a raw OAuth2 code→token exchange.

Actionable comments posted: 1

  • 1 🟡 Minor: exchange_token loses retry/rate-limit handling that every other SDK call gets

Merge risk: 🟢 Low: implementation, docstrings, and tests are internally consistent (verified client_id padding, PKCE, URL/body composition, and mock wiring line-by-line against the diff); the one finding is a bounded reliability gap in a new, isolated endpoint.

See inline comments for details.


Declared coverage: FULL — 10/10 changed files reviewed.

Comment thread descope/authmethod/app.py Outdated
@shuni-bot

shuni-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot shuni-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🐕 Shuni Review

New commits fix two CodeQL findings (XSS + exception-message exposure) in samples/app_oidc_mfa_sample_app.py.

Actionable comments posted: 1

  • 1 🟠 Major: flask.escape import breaks the sample under the SDK's own locked Flask version

Merge risk: 🟡 Moderate: the XSS/info-exposure intent is right, but the escaping fix itself uses a name that doesn't exist in Flask 3.1.3 (this repo's locked version), so the sample would fail at import.

See inline comments for details.

⚠️ Review follow-up issue: Unresolved prior review threads: PRRT_kwDOHT_eGc6bx4Tt.


Declared coverage: PARTIAL — 4/10 changed files reviewed.

Comment thread samples/app_oidc_mfa_sample_app.py Outdated
Co-authored-by: shuni-bot[bot] <251468265+shuni-bot[bot]@users.noreply.github.com>
@shuni-bot

shuni-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot shuni-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🐕 Shuni Review

No new issues found.

⚠️ Review follow-up issue: Unresolved prior review threads: PRRT_kwDOHT_eGc6bx4Tt.


Declared coverage: PARTIAL — 1/10 changed files reviewed.

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

Coverage report

The coverage rate went from 98.26% to 98.28% ⬆️

100% of new lines are covered.

Diff Coverage details (click to unfold)

descope/descope_client_async.py

100% of new lines are covered (99.37% of the complete file).

descope/authmethod/_app_base.py

100% of new lines are covered (100% of the complete file).

descope/descope_client.py

100% of new lines are covered (97.7% of the complete file).

descope/authmethod/app.py

100% of new lines are covered (100% of the complete file).

descope/authmethod/app_async.py

100% of new lines are covered (100% of the complete file).

@mrunankpawar

Copy link
Copy Markdown
Author

@shuni can you please fix this:

@shuni-bot

shuni-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🐕 Done! Updated PR: #1676

View Shuni run

📋 Actions Taken

View AI response

Both fixes are committed on feat/oidc-federated-app-signin:

  1. F841samples/app_oidc_mfa_sample_app.py:85: dropped the unused as e binding on the except AuthException block.
  2. Review thread r3845625324App.exchange_token/AppAsync.exchange_token now route their raw httpx.post call through HTTPClient._execute_with_retry/HTTPClientAsync._async_execute_with_retry and _raise_from_response, so a transient 429/5xx from the token endpoint now retries and raises RateLimitException consistently with the rest of the SDK. Kept the form-encoded body/no-bearer-header bypass since that part was already justified. Updated the docstring accordingly, replied on the thread, and marked it resolved.

Verified with uv run pytest -q (1094 passed, 26 skipped) and uv run ruff check . (clean).


View session on Shuni Portal 🐾

Ruff F841 - the AuthException wasn't referenced in the except block.
exchange_token bypassed the SDK's normal HTTP layer entirely for its
one-shot httpx call, so a transient 429/5xx from Descope's own token
endpoint surfaced as a generic AuthException instead of being retried
or raised as RateLimitException like every other call in the SDK.

Keep the form-encoded body/headers bypass (justified - no bearer
token), but reuse HTTPClient._execute_with_retry (sync) /
_async_execute_with_retry (async) and _raise_from_response so
retry/rate-limit behavior stays consistent with the rest of the SDK.
@shuni-bot

shuni-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot shuni-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🐕 Shuni Review

No new issues found.


Declared coverage: PARTIAL — 3/10 changed files reviewed.

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.

2 participants