Skip to content

Conversation

@ostempel
Copy link
Contributor

@ostempel ostempel commented Dec 4, 2025

Description

And in our metal-apiserver:
https://github.com/metal-stack/metal-apiserver/blob/ae0b0ee8a5789d7012590b6cc61db445f130c073/pkg/service/auth/oidc.go#L42-L53

Problem is somehow the extraction and the fallback of goth, which uses the internal provider name and not our. It could arise in auth-service here:

	providerName, err := gothic.GetProviderName(req)

References:

@ostempel ostempel requested review from Gerrit91 and majst01 December 4, 2025 16:15
@metal-robot metal-robot bot added this to Development Dec 4, 2025
@majst01
Copy link
Contributor

majst01 commented Dec 5, 2025

I am biased to skip this possibility to change the authentication provider. This was required in the metalstack.cloud api-server because we need to support multiple auth provider there. OTOH in metal-stack.io i doubt we need anything different than a openid-connect provider.

If we agree on this, we can also remove the suffix openid-connect from the generated tenant id.

@Gerrit91
Copy link
Contributor

Gerrit91 commented Dec 8, 2025

Why should we not provide different providers for metal-stack, too? I think it's quite likely people would like to use it at some point in the future?

@majst01
Copy link
Contributor

majst01 commented Dec 8, 2025

Why should we not provide different providers for metal-stack, too? I think it's quite likely people would like to use it at some point in the future?

Because with the help of either zitadel or any other idp like keycloack, the authentication provider will stay the same

@Gerrit91
Copy link
Contributor

Gerrit91 commented Dec 8, 2025

I find these often hard to manage. I would prefer to let people decide what they want to use.

@majst01
Copy link
Contributor

majst01 commented Dec 8, 2025

I find these often hard to m

But then we need to implement specific auth providers as well, which is actually not the case

@Gerrit91
Copy link
Contributor

Gerrit91 commented Dec 8, 2025

We can do it over time. Adding them is easy but as soon as the database is messed up because user IDs are not unique anymore, it's hard to fix.

@majst01
Copy link
Contributor

majst01 commented Dec 8, 2025

We can do it over time. Adding them is easy but as soon as the database is messed up because user IDs are not unique anymore, it's hard to fix.

But if we force userids to be unique for example by using email (which is actually the case and enforced by masterdata-api), there is no need to splatter the userids with a meaningless suffix

@ostempel ostempel merged commit dfef4a5 into main Dec 12, 2025
5 checks passed
@ostempel ostempel deleted the default-provider-for-oidc branch December 12, 2025 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants