Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Need a way to allow non-SSO users to use Key Backup on SSO-using hosts #12844

Closed
tadzik opened this issue May 23, 2022 · 5 comments · Fixed by #12883
Closed

Need a way to allow non-SSO users to use Key Backup on SSO-using hosts #12844

tadzik opened this issue May 23, 2022 · 5 comments · Fixed by #12883
Assignees
Labels
A-SSO Single Sign-On (maybe OIDC) T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@tadzik
Copy link
Contributor

tadzik commented May 23, 2022

Description:

On a host with both SSO users and non-SSO (e.g. bots) coexisting, it's impossible to use Key Backup as the latter. If we disable password auth (which makes sense when using SSO), even if we use our non-SSO user with its access token, an auth check will still be performed when uploading keys to key backup – and that will fail due to no available auth flow (and a 401) since there is no auth flow that the bot user can use.

Code-wise we end up here when setting up Key Backup in Element, and then this gives us no available options.

@babolivier babolivier added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label May 23, 2022
@MadLittleMods MadLittleMods added the A-SSO Single Sign-On (maybe OIDC) label May 23, 2022
@richvdh
Copy link
Member

richvdh commented May 26, 2022

@tadzik: for context, how does a "non-SSO user" get an access token in the first place if password auth is disabled?

Possibly the solution here is just to relax the _password_enabled check here, to allow users to present a password for re-authentication even if passwords are disabled for login?

@erikjohnston
Copy link
Member

@tadzik do you actually mean this API? I don't think the one you linked actually requires UI auth?

@interactive_auth_handler
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)
user_id = requester.user.to_string()
body = parse_json_object_from_request(request)
await self.auth_handler.validate_user_via_ui_auth(
requester,
request,
body,
"add a device signing key to your account",
# Allow skipping of UI auth since this is frequently called directly
# after login and it is silly to ask users to re-auth immediately.
can_skip_ui_auth=True,
)
result = await self.e2e_keys_handler.upload_signing_keys_for_user(user_id, body)
return 200, result

@tadzik
Copy link
Contributor Author

tadzik commented May 26, 2022

for context, how does a "non-SSO user" get an access token in the first place if password auth is disabled?

@richvdh in this case the user is registered with an admin API, with shared secret registration – which gives us the access_token in the response.

@erikjohnston yeah, that looks about right – I may have mislinked things.

@erikjohnston
Copy link
Member

Possibly the solution here is just to relax the _password_enabled check here, to allow users to present a password for re-authentication even if passwords are disabled for login?

I think that is probably fine, the only concern would be for servers that have changed from password to SSO, where this change will potentially show the password box for users that had signed up with a password. I don't think that is a big deal though.

@richvdh
Copy link
Member

richvdh commented May 26, 2022

I think that is probably fine, the only concern would be for servers that have changed from password to SSO, where this change will potentially show the password box for users that had signed up with a password.

possibly the password_enabled setting needs more options: True, False, "only_for_reauth"?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-SSO Single Sign-On (maybe OIDC) T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
6 participants