semaphoreui / semaphore

Modern UI for Ansible, Terraform, OpenTofu, Bash, Pulumi.
https://semaphoreui.com
MIT License
9.87k stars 1.01k forks source link

OIDC configuration username_claim don't work #1731

Open EHEX-schildt opened 5 months ago

EHEX-schildt commented 5 months ago

Hi,

i configured OIDC with this guide: https://docs.semui.co/administration-guide/openid

"oidc_providers": {
  "my-idp": {
    "display_name": "Sign in with my-idp",
    "provider_url": "https://my.idp/oauth2/openid/semaphore",
    "client_id": "semaphore",
    "client_secret": "$secret"
  }
}

According to the guide:

I would expet username and display name are both the same and provided by my IDP via the preferred_username claim.

If i login with a IDP Account the display name (name_claim) works as expected. But for the username (username_claim) i get family_name? extend with some random String.

Setting "name_claim": "name", via oidc_providers config works as expected. explicit Setting the default "username_claim": "preferred_username", via oidc_providers config don't work. Setting "username_claim": "name", via oidc_providers config don't work.

Semaphore Version: v2.9.45

fiftin commented 5 months ago

Hello, do you have email_cliam? Is it works?

EHEX-schildt commented 5 months ago

yes, email_cliam works also as expected.

here is a example of data provided by my IDP: preferred_username: user.name name: User Name email: name@example.de

In semaphore with default values for username_claim and name_claim i get: Username: name_JACQoZvkyyKe Name: user.name Email: name@example.de

fiftin commented 5 months ago

Problem:

We have login and email fields for each user. login and email must be unique. But if we get both login and email from OIDC we can have conflict if multiple OIDC providers available.

For example we have provider1 and provider2.

Provider1 returns username: john and email: john@example.com. Provider2 returns username: john and email: john@gmail.com. As result we have conflict. So, currently we use only email from claims.

In following version:

EHEX-schildt commented 5 months ago

Ok but two Providers will have always have potential for conflict: even with email: Provider1 returns username: joe and email: joe@example.com. Provider2 returns username: john and email: joe@example.com.

fiftin commented 5 months ago

Username will be ignored. And it will be the same account in Semaphore. The one email address cannot become two users, right? But username can.

fiftin commented 5 months ago

New release will be soon.

fiftin commented 5 months ago

I would completely remove the username from the Semaphore, but it is needed for backward compatibility :)

EHEX-schildt commented 5 months ago

A little bit Off-Topic: So it is more stable to search for email, instead of usernames. If i want to get the ID of a user (for User to Team Links via API Calls)

tumbl3w33d commented 5 months ago

image

(quick and dirty draft)

How about more decoupling of the several means of authentication from the actual user object? A user can have several ways to authenticate to end up in the same application identity.

Gitea does something similar when adding another identity provider. They call it "linking to an existing account" and the connection ("to which account do we want to link?") is established through a field like your login/email. I. e., when the existing user (e.g. from an LDAP source or local db of the application) has login set to john.doe and the newly connected identity provider has the login_claim configured to a field with an identical value, then on first login the user is offered to link to the existing john.doe, but to prove it's actually their account, they need to login with the existing means of authentication (e.g. using their LDAP credentials) during the flow. Once that is done, the user can login to the same application identity either with the old LDAP or the new identity provider (e.g. OIDC).

For semaphore the application identity should not be identified by an email or a login/username something, but a uuid. You can treat semaphore itself as a means of authentication (local db) in this picture. The application identity could have an own email field (which could be a federated list of all emails from associated user_authentication objects, which is read-only) + you could offer the user to manually extend this list with other e-mails they deem necessary.

It's a much more invasive approach, but I think it would provide more flexibility.

fiftin commented 5 months ago

@tumbl3w33d I like your approach.

tboerger commented 4 months ago

IMHO it should be an optional config value if users are merged to being able to authenticate from multiple providers or if it gets an automatic suffix. For the cloud environment it totally makes sense to have every auth source separated, but for on premise installations it could make more sense to merge the users.