Skip to content

Conversation

@sam-muncke
Copy link

@sam-muncke sam-muncke commented Nov 6, 2025

Changes

Requires MRRT functionality in #58 to work

This change adds support for the new Connected Accounts flow.

  • Adds start_connect_account that calls MyAccountAPI to initiate the flow. This returns a url with a ticket that a user should be redirected to to authenticate with the 3rd Party IdP
  • Adds complete_connect_account which completes the flow following being redirected back from the 3rd Party IdP

References

https://auth0team.atlassian.net/browse/FGI-1573

Testing

Manual testing can be done for this flow as described in the example doc in the PR. This flow does rely on transaction/state storage so requires an implementation of those stores. Since this library is typically intended to be used as a library to build framework specific implementations, it is probably most easily tested via this accompanying PR that implements framework support for this feature in FastAPI.
Pre-requisites

  • My Account API must be enabled on your Auth0 tenant

  • Connected Accounts must be enabled on your Auth0 tenant

  • MRRT must be configured for the tenant

  • This change adds unit test coverage

  • This change adds integration test coverage

  • This change has been tested on the latest version of the platform/language or why not

Checklist

sam-muncke and others added 6 commits November 6, 2025 15:29
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
…/auth0-server-python into FGI-1573_connected-account-support
@sam-muncke sam-muncke marked this pull request as ready for review November 6, 2025 20:08
@sam-muncke sam-muncke requested a review from a team as a code owner November 6, 2025 20:08
@arpit-jn
Copy link
Contributor

arpit-jn commented Nov 7, 2025

@claude

@github-actions
Copy link

github-actions bot commented Nov 7, 2025

Claude finished @arpit-jn's task —— View job


Overall Assessment

❌ This PR is not ready to merge. Multiple security issues, logic bugs, and code quality problems must be addressed first.

Critical Issues Requiring Fixes

Security Vulnerabilities:

  • Missing response content-type validation in HTTP requests (can lead to JSON parsing vulnerabilities)
  • URL injection vulnerability in start_connect_account (line 1373 in server_client.py)
  • Missing KeyError protection when parsing API responses

Logic Bugs:

  • Critical MRRT validation bug in get_access_token (lines 618-624 in server_client.py) - will incorrectly fail when valid token exists
  • Inconsistent authorization_params filtering - filtered but unfiltered version passed to API

Code Quality Issues:

  • Multiple syntax errors in example documentation (JavaScript comments in Python, dictionary syntax errors, missing operators)
  • Unused imports flagged by security scanner
  • Missing HTTP timeout configurations
  • Missing type hints on new parameters
  • Style inconsistencies (whitespace, naming conventions)

Recommendations

  1. Fix the critical MRRT logic bug - this will cause runtime failures
  2. Add response validation for all HTTP responses from My Account API
  3. Use proper URL construction instead of string concatenation
  4. Fix the example documentation - it contains multiple syntax errors that will confuse users
  5. Remove unused imports from test files
  6. Add HTTP timeouts to prevent indefinite hangs
  7. Consider adding integration tests for the full connected accounts flow

The feature implementation follows good patterns (PKCE, state management, transaction storage), but needs these issues resolved before merge.


Job Run | Branch

sam-muncke and others added 3 commits November 7, 2025 10:56
…ith one matching the audience

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…/auth0-server-python into FGI-1573_connected-account-support
@adamjmcgrath
Copy link
Contributor

adamjmcgrath commented Nov 7, 2025

Quick smoke test using https://github.com/jtemporal/minimal-fastapi-webapp-auth0

Seems to work ok

image

Couldn't test with DPoP because auth0-fastapi doesn't appear to support that

@sam-muncke
Copy link
Author

@adamjmcgrath - DPoP is out of scope for this PR. My account supports DPoP but does not strictly require it (at least for now). DPoP support will be added in a future PR but given this SDK is for confidential (server-based) clients - it does not really add any additional security.

adamjmcgrath
adamjmcgrath previously approved these changes Nov 7, 2025
@sam-muncke sam-muncke changed the title feat: FGI-1573 add connected account support [DO NOT MERGE]feat: FGI-1573 add connected account support Nov 11, 2025
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.

5 participants