New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inconsistent meaning of provider_id between saml and oidc #3477
Comments
There is an inherent difference between OIDC and SAML. The situation for OAuth/OIDC is as follows:
For SAML, we have the following situation:
The above should give some insights as to why things landed this way. Making |
Just to be clear, you can still lookup the provider -- it is not broken. But, the lookup is indeed by Perhaps this needs to be extended to support:
|
@ryanhiebert Would the above suit your needs? |
You're correct, this isn't broken. You just use the {% provider_login_url "my-provider-id" %} I was thinking that I should expect whatever I could imagine, if I can't convince you to adjust the way you're using Thank you for sharing more about how you're thinking about OIDC and SAML and these parameters. I see your motivation, and they make sense. It does seem to me that there is a stronger parallel we should be respecting between OIDC and SAML, and stronger separation between OIDC and OAuth. OAuth and OpenID Connect are notably different because OAuth isn't actually a provider. It's some generic tooling to make a provider easy to write. OpenID Connect is a provider itself. It makes sense that when you were writing OpenID connect initially you drew parallels to OAuth, but I think the parallels with SAML are actually stronger. To support my case that you should consider SAML and OIDC to be more strongly related, I'd like to point out that OIDC is commonly used as an SSO protocol just like SAML is. I have particular experience using OIDC for SSO with Amazon Cognito, and I don't think that Cognito is an anomaly among SSO implementation services. SAML is much more common for that purpose, but OIDC seems to be picking up some steam as a preferable protocol for SSO. If there was another provider that OpenID Connect would arguably be similarly closely related to, that would be the OpenID provider, but TBH I don't even know whether OpenID Connect supports a mode where all you do is enter an OpenID (often an email address) and it will take you to that domain's OpenID login. That was a thing with the original OpenID, and is how the OpenID provider works, but it is not how the current OpenID Connect provider is implemented, anyway. What is currently implemented is more closely related to SAML. Your concern about necessity for stability of the provider_id is a good one. For all the reasons that you've laid out, changing the provider ID for OpenID Connect is unwise. At minimum, to do it right would mean adjusting any other places in the database that are similarly relying on that provider id, and that is not easy or implemented. However, because I see that OpenID Connect and SAML are very similarly positioned, all the motivations you've mentioned for SAML seem to also apply to OIDC. To my thinking, this bolsters the case that we should pattern the SAML provider's use of the Based on your explanation, I would suggest that you document that the I'd still like the |
Also, and for what it's worth, I consider URLs for both OpenID Connect and SAML to be things that need to be basically immutable. Coordinating those kinds of changes with customers is a big pain, and I don't personally intend to give them the option to change their URL. |
The implementation and documentation have conflicting stories about what the
provider_id
is for.provider_id
as the URL slug.provider_login_url
template tag looks up the provider using theprovider_id
.provider_login_url
template tag is documented to be used to look up the saml provider login url.client_id
as the URL slug.provider_id
is a good place for the IdP entity ID.Looking at the semantics of these fields in OIDC, it seems to me that SAML is using these fields significantly differently from the original intent, and this may be at least partially due to the fact that the
client_id
is a required field on the model.The
provider_id
, in OIDC, is that identifier that is used in the slug, that is the short code to identify the provider, where theclient_id
is part of the identification of the service provider to the identity provider. In SAML there are some things that half-fit the intent ofclient_id
(IdP entity ID, SP entity ID, certificate), but none that really have the same semantics.My suggestion is that
client_id
should not be required, and that the SAML documentation, urls, and views should use theprovider_id
the same way that the OIDC provider does. This is a breaking change, but that concern may be mitigated somewhat by the fact that the inconsistency is breaking an existing feature (to look up theprovider_login_url
), and my hope is that using theprovider_id
consistently will give implementors an easier path to having a proper intuition about how these fields are used across providers.Because of these inconsistencies, we've opted to ensure that the
provider_id
and theclient_id
are always set the same in our system for now, so that theprovider_login_url
can work and get the correct URL./cc @atribed
The text was updated successfully, but these errors were encountered: