[WIP] SSO proof of concept #12404

andrewshadura · 2022-04-05T08:24:21Z

A minimal working SSO based on OmniAuth.

To minimise the impact of this code being integrated with the rest of the codebase, it doesn’t support the identity concept, but instead matches the SSO users against those already known to OBS by their email addresses.

There are three issues that need to be solved properly:

When a new user logs in with SSO, they are not asked to set up a password upfront. This is to improve the user experience, since the user may never need to use the password if they don’t use the command-line client, so they’re allowed to configure it later when they choose to; if they instead opt to only ever log in with SSO, they won’t need to set up a password at all.
This is currently implemented by setting the "hash type" to invalid, but since the hash type has been deprecated and will go away at some point, marking the user as not having a password needs to be done differently.
Redirection of anonymous users needs to be smarter. For security reasons, some setups configure the lifetime of a session to a minimal value and force users to re-authenticate with the SSO to confirm their access has not been revoked. This is currently difficult to implement in a user-friendly way, as the user with an expired session is not automatically redirected to the SSO, which would confirm their session and transparently redirect back. Even further, it’s difficult to keep the original URL as it’s only stored in the Referer header instead of some argument e.g. ?redirect_to=.
During the initial login flow, the authentication hash is stored in the session between individual flow steps. For some reason when used e.g. with GitHub, it overflows the session (even though it’s not supposed to be stored in a cookie). The current code works around it by only storing a part of the hash, but this seriously limits what can be learned about the user during the registration process.

I will rebase to the latest master branch shortly.

This will fix #7709 and #5957

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Parse-SSO-authentication-config-on-startup.patch

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Add-an-SSO-login-page-so-that-users-can-choose-between-pr.patch

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Rename-create_ldap_user-to-create_external_user.patch

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name user-model-add-find_with_omniauth-create_with_omniauth.patch

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Add-SSO-callback-to-allow-existing-users-log-in-with-an-e.patch

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Implement-login-flow.patch

Some providers set username or nickname to an email address. For this reason, first collect the best possible user name we can find, and only then fix it to match our requirements. Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Try-harder-to-derive-the-username-from-email-addresses.patch

…ater Add a new "hash type" for invalid passwords, which is never equal to normal passwords, but nevertheless can be changed without being known by the user. This "invalid" password can only be set by directly setting the password hash type. When updating the password using update_password method, it will always be upgrade it to the strongest hash type, sha256crypt. To allow changing this "invalid" password to a normal one, stop requiring a non-empty current password in the password change dialog when changing a password from an "invalid" one. Don’t show the current password box either, as it is not used anyway in this case, making it better not to show it to avoid confusion. Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Mark-passwords-for-SSO-only-users-as-invalid-to-allow-cha.patch

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>

The auth hash can be quite large, and with session storage in cookies, the cookie can easily reach the 4 KB limit. Work around this issue by only storing the part of the hash we currently use. Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>

Conan-Kudo · 2022-04-20T13:03:56Z

@andrewshadura Can you please remove the Gbp-Pq goop from the commits? ~~Also, how will osc work with SSO accounts?~~

andrewshadura · 2022-04-20T13:47:59Z

@andrewshadura Can you please remove the Gbp-Pq goop from the commits?

Sure, I was going to remove them in the next iteration of the pull request.

andrewshadura · 2022-05-04T10:12:02Z

src/api/app/views/webui2/webui/session/sso_confirm.html.haml

+.card
+  .card-body#loginform
+    .col-lg-6.pl-0
+      - if can_register


While deploying this today on a new instance, we found out we actually want to disallow new registrations while allow new users log in with SSO.
The code doesn’t currently distinguish between these two different sign-ups (non-verified self-registration and a new user being created after confirmed by the SSO), so we ended up removing this condition here so that SSO can still allow new users in deny mode.
Ultimately it needs to be a separate setting.

hellcp-work · 2023-01-09T11:57:46Z

To minimise the impact of this code being integrated with the rest of the codebase, it doesn’t support the identity concept, but instead matches the SSO users against those already known to OBS by their email addresses.

Which users can change in most idps, that's probably not the best idea. Maybe it would be possible to start off with introducing some way to support identities first before getting sso into the codebase? Alternatively allow admins to choose which field is static and base the match on that? SUSE Community Accounts have static usernames for example, so it would be best if that was the way the users were matched with accounts.

andrewshadura · 2023-01-09T12:05:01Z

Identities would be great. I didn't explore that option as it would be too big a change, and I wanted to minimise DB schema changes just in case we have to maintain the patchset for a long time. We are actually moving away from emails as the only id since our customer wants usernames to be a key id (UPN more precisely), so I think we'll have to make it configurable.

dmarcoux · 2023-01-09T12:43:47Z

src/api/Gemfile

+gem 'omniauth'
+gem 'omniauth-gitlab'
+gem 'omniauth-github'
+gem 'omniauth_openid_connect', git: 'https://github.com/m0n9oose/omniauth_openid_connect', ref: '88ae46e968dec5e66d4f92773b52babbe72d41fe'


What is the reason for pinning this gem to a commit? Is it a lack of a recent release?

Yes, at the time when this was written, there was no compatible release.

I did some work with omniauth_openid_connect as a part of paste-o-o, and there is a compatible version now, so we should be good to use the latest version

Yes — after I kicked a bunch of people who did omniauth development, they’ve all got together, moved omniauth_openid_connect under the OmniAuth umbrella and made a new release 🙂

hellcp-work · 2023-01-10T08:35:13Z

src/api/config/auth.yml.example

@@ -0,0 +1,8 @@
+fdo-gitlab:


I think we should have example config for all the gems we have added here, as of now we have config just for gitlab. I'm also not sure if it's the best option to have a new config file for this, or if it wouldn't make sense to add into the existing one

andrewshadura · 2023-01-10T13:49:30Z

@hellcp-work, right, it seems that both we (and our customer) and SUSE want this to move roughly in the same direction, so it’d probably make sense to co-ordinate the next steps. Implementing identities properly would both cover our and your use-case, while making the system more flexible and future-proof, so it probably makes sense to do just that — and we can probably invest some time to make it happen.

We also need to decide on a way to represent passwordless (SSO-only) users, since this would be very typical in this workflow.

hellcp-work · 2023-01-12T15:02:57Z

We also need to decide on a way to represent passwordless (SSO-only) users, since this would be very typical in this workflow.

On the other hand we could also think about making that a feature, allowing authenticating with tokens instead of passwords, so that there is not necessarily an expectation of a password from the api side for authentication

andrewshadura added 15 commits January 31, 2022 18:36

Add omniauth dependency

8e0bc17

Add dependency on omniauth-gitlab

87ab1fb

Add omniauth-rails_csrf_protection

a0fa5cf

Parse SSO authentication config on startup

dd65c4c

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Parse-SSO-authentication-config-on-startup.patch

Add an SSO login page so that users can choose between providers

a226ccb

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Add-an-SSO-login-page-so-that-users-can-choose-between-pr.patch

Rename create_ldap_user to create_external_user

38cd6ff

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Rename-create_ldap_user-to-create_external_user.patch

user model: add find_with_omniauth/create_with_omniauth

d1eaa4d

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name user-model-add-find_with_omniauth-create_with_omniauth.patch

Add SSO callback to allow existing users log in with an external source

cec4685

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Add-SSO-callback-to-allow-existing-users-log-in-with-an-e.patch

Implement first login flow

f9d73c3

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> Gbp-Pq: Topic collabora/sso Gbp-Pq: Name Implement-login-flow.patch

REWORD: Redirect correctly: secure and take SSO into account

deae750

Add dependency on a Git snapshot of omniauth_openid_connect

1e2e590

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>

Add omniauth-github

4cdfc39

Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>

github-actions bot added Backend Things regarding the OBS backend Documentation 📖 Things regarding our documentation Frontend Things related to the OBS RoR app Test Suite / CI 💉 Things related to our tests/CI labels Apr 5, 2022

andrewshadura changed the base branch from master to 2.10 April 5, 2022 08:29

andrewshadura commented May 4, 2022

View reviewed changes

hennevogel mentioned this pull request Jun 23, 2022

add lost-password-recovery for user accounts #699

Open

dmarcoux reviewed Jan 9, 2023

View reviewed changes

hellcp-work reviewed Jan 10, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] SSO proof of concept #12404

[WIP] SSO proof of concept #12404

andrewshadura commented Apr 5, 2022 •

edited

Conan-Kudo commented Apr 20, 2022 •

edited

andrewshadura commented Apr 20, 2022

andrewshadura May 4, 2022

hellcp-work commented Jan 9, 2023

andrewshadura commented Jan 9, 2023 via email •

edited

dmarcoux Jan 9, 2023

andrewshadura Jan 9, 2023

hellcp-work Jan 10, 2023

andrewshadura Jan 10, 2023

hellcp-work Jan 10, 2023

andrewshadura commented Jan 10, 2023

hellcp-work commented Jan 12, 2023

[WIP] SSO proof of concept #12404

Are you sure you want to change the base?

[WIP] SSO proof of concept #12404

Conversation

andrewshadura commented Apr 5, 2022 • edited

Conan-Kudo commented Apr 20, 2022 • edited

andrewshadura commented Apr 20, 2022

andrewshadura May 4, 2022

Choose a reason for hiding this comment

hellcp-work commented Jan 9, 2023

andrewshadura commented Jan 9, 2023 via email • edited

dmarcoux Jan 9, 2023

Choose a reason for hiding this comment

andrewshadura Jan 9, 2023

Choose a reason for hiding this comment

hellcp-work Jan 10, 2023

Choose a reason for hiding this comment

andrewshadura Jan 10, 2023

Choose a reason for hiding this comment

hellcp-work Jan 10, 2023

Choose a reason for hiding this comment

andrewshadura commented Jan 10, 2023

hellcp-work commented Jan 12, 2023

andrewshadura commented Apr 5, 2022 •

edited

Conan-Kudo commented Apr 20, 2022 •

edited

andrewshadura commented Jan 9, 2023 via email •

edited