Skip to content

Conversation

mcalinghee
Copy link
Contributor

@mcalinghee mcalinghee commented Mar 11, 2025

Fixes Point 1 and 4 in #2089

  • do nothing, and fail the registration/login (corresponds to allow_existing_users: false in Synapse)
  • import the link without overriding existing links for this user (corresponds to allow_existing_users: true in Synapse)
    import the link if and only if there is no existing link for this user

Introducing new configuration on_conflict in section claims_imports.localpart to set the account linking strategy

upstream_oauth2:
  providers:
    - id: 
      claims_imports:
        localpart:
          action: require 
          on_conflict: add
  • fail (default value): do nothing, and fail the registration/login (corresponds to allow_existing_users: false in Synapse)
  • add : import the link without overriding existing links for this user (corresponds to allow_existing_users: true in Synapse)
  • replace : import the link and replace any existing link for this user => not implemented in this PR
  • set : import the link if and only if there is no existing link for this user => not implemented in this PR

confirmation page when configuration is on_conflict: add :
image

@mcalinghee mcalinghee changed the title Allow existing user wip: allow existing user Mar 11, 2025
@mcalinghee mcalinghee force-pushed the feat/allow_override_user branch 6 times, most recently from 9d35eeb to d50b6c0 Compare March 12, 2025 16:20
@mcalinghee mcalinghee changed the title wip: allow existing user allow importing existing users when the localpart matches in upstream OAuth 2.0 logins Mar 12, 2025
@mcalinghee mcalinghee marked this pull request as ready for review March 12, 2025 16:21
@mcalinghee mcalinghee force-pushed the feat/allow_override_user branch from d50b6c0 to 405abce Compare March 12, 2025 16:47
@mcalinghee mcalinghee marked this pull request as draft March 12, 2025 16:52
@mcalinghee mcalinghee force-pushed the feat/allow_override_user branch from 405abce to 11f17cc Compare March 12, 2025 17:18
@mcalinghee mcalinghee marked this pull request as ready for review March 12, 2025 17:20
@mcalinghee mcalinghee force-pushed the feat/allow_override_user branch 3 times, most recently from 8f7d65b to 0307f3c Compare March 17, 2025 10:32
@mcalinghee mcalinghee force-pushed the feat/allow_override_user branch from 0307f3c to 63f811a Compare March 25, 2025 15:27
@mcalinghee mcalinghee force-pushed the feat/allow_override_user branch 2 times, most recently from fa428d9 to 3d64262 Compare April 11, 2025 06:55
@mcalinghee mcalinghee changed the title allow importing existing users when the localpart matches in upstream OAuth 2.0 logins allow account linking for existing users when the localpart matches in upstream OAuth 2.0 logins Apr 14, 2025
@mcalinghee mcalinghee force-pushed the feat/allow_override_user branch from 3d64262 to f5a3404 Compare April 14, 2025 13:01
@odelcroi odelcroi force-pushed the feat/allow_override_user branch 2 times, most recently from e2b0077 to 4405e08 Compare May 14, 2025 09:29
@odelcroi
Copy link
Contributor

odelcroi commented May 15, 2025

Wrong configuration of provider

When activating the option allow_existing_users : true, the claim username is used to check if users exist or not. If the attribute mapper username is set to ignore or suggest , the user existence is not checked. Therefore to enable this option the username attribute mapper should be set to require or force.

  • documentation should mention it
  • an error should be raised at startup if the configuration does not respect this condition

@odelcroi

This comment was marked as resolved.

@mcalinghee mcalinghee marked this pull request as draft May 19, 2025 07:43
@odelcroi odelcroi force-pushed the feat/allow_override_user branch 4 times, most recently from e910c49 to 00d8171 Compare July 17, 2025 09:06
@odelcroi odelcroi force-pushed the feat/allow_override_user branch from b877b9c to ea8c8d2 Compare July 17, 2025 12:37
@odelcroi
Copy link
Contributor

@sandhose I'm done with the required modifications, I think we can consider updating the user flow by skipping the screen in a second step. If good with you, we can merge :)

Copy link
Member

@sandhose sandhose left a comment

Choose a reason for hiding this comment

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

Thanks! Almost there, just a few minor stylistic things, but I think this should make the cut for the next RC on Tuesday!

@odelcroi odelcroi force-pushed the feat/allow_override_user branch from 999dfe9 to 3e9d572 Compare July 21, 2025 07:52
Copy link
Member

@sandhose sandhose left a comment

Choose a reason for hiding this comment

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

Many thanks! It should be part of the 0.20.0-rc.0 tomorrow 💃

@sandhose sandhose merged commit 5cbe503 into element-hq:main Jul 21, 2025
19 checks passed
@sandhose sandhose changed the title allow account linking for existing users when the localpart matches in upstream OAuth 2.0 logins Allow linking upstream accounts for existing users when the localpart matches in upstream OAuth 2.0 logins Jul 22, 2025
@sandhose sandhose added A-Upstream-OAuth Related to login via upstream OAuth 2.0 providers T-Enhancement New feature of request labels Jul 22, 2025
@odelcroi odelcroi deleted the feat/allow_override_user branch September 15, 2025 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Upstream-OAuth Related to login via upstream OAuth 2.0 providers T-Enhancement New feature of request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants