Skip to content

feat: Allow a returned claim to have a list of alternative fully qualified user ids to check on login - #107

Open
jason-famedly wants to merge 7 commits into
mainfrom
jason/exp-oauth-with-multiple-userids
Open

feat: Allow a returned claim to have a list of alternative fully qualified user ids to check on login#107
jason-famedly wants to merge 7 commits into
mainfrom
jason/exp-oauth-with-multiple-userids

Conversation

@jason-famedly

Copy link
Copy Markdown
Member

SYN-126

Add support for alternative_fq_uids_path option in both jwt_validation and introspection_validation. This takes a Path that can point into either claim to resolve to a list of fully qualified user ids.

If the claim does not contain a list, there will be a failure.

If using alternative_fq_uids_path in either validation section, then localpart_path and fq_uid_path can not be used anywhere as that is also a failure.

See README file updates for full details.

@jason-famedly
jason-famedly requested a review from a team as a code owner September 11, 2026 14:53
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.41270% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.51%. Comparing base (7d40c7d) to head (80c8097).

Files with missing lines Patch % Lines
synapse_token_authenticator/token_authenticator.py 95.23% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #107      +/-   ##
==========================================
+ Coverage   84.55%   85.51%   +0.96%     
==========================================
  Files          13       13              
  Lines         900      953      +53     
  Branches      163      176      +13     
==========================================
+ Hits          761      815      +54     
+ Misses         85       84       -1     
  Partials       54       54              
Files with missing lines Coverage Δ
synapse_token_authenticator/config/oauth.py 94.15% <100.00%> (+2.19%) ⬆️
synapse_token_authenticator/token_authenticator.py 72.49% <95.23%> (+1.06%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d40c7d...80c8097. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread synapse_token_authenticator/token_authenticator.py Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 599638e. Configure here.

)
elif jwt_fq_uid is None or intro_fq_uid is None:
# Let whichever is not None win
fully_qualified_uid = jwt_fq_uid or intro_fq_uid

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

One list can authorize both sources

Medium Severity

When both JWT and introspection set alternative_fq_uids_path, a username missing from one list still logs in if the other list contains it. Helpers return None for a non-matching list, and the caller then lets the other source win, so the two lists are not required to agree.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 599638e. Configure here.

the things we do sometimes, I swear :rolling_eyes:
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.

1 participant