Skip to content
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

Unable to use different issuerurl than in metadata. #75

Closed
MarcelCoding opened this issue Jun 18, 2022 · 9 comments
Closed

Unable to use different issuerurl than in metadata. #75

MarcelCoding opened this issue Jun 18, 2022 · 9 comments

Comments

@MarcelCoding
Copy link
Contributor

Keycloak has a feature that you can statically define a public URL. This URL is then being used in all user-facing (issuer_url, auth_url, end_session_url) endpoints in the autodiscovery. All other URLs are prefixed with the current connecting URL. In my example, I have an internal network and a publicly routed subdomain to keycloak. All token, user info, ... requests should be routed internally and the user-facing request should be obviously routed externally.

Here you can see this behavior:
intern:
image
extern:
image

Unfourtly this library is not allowing that a different URL is used for the autodiscovery, that the actual issuer URL provided in the autodiscovery:

Error: Validation error: unexpected issuer URI `https://id.admin-guide.com/realms/main` (expected `https://keycloak.general.pve3.secshell.net/realms/main`)

Would it be possible to fully remove this behavior or add an option it at least disable it? Or is this kind of forced by the opened connect spec?
Depending on what you find better, I am willing to implement it in your library.

@ramosbugs
Copy link
Owner

This is related to #38 and is due to the requirement in Section 4.3 of the OpenID Connect Discovery spec:

The issuer value returned MUST be identical to the Issuer URL that was directly used to retrieve the configuration information. This MUST also be identical to the iss Claim value in ID Tokens issued from this Issuer.

It sounds like this Keycloak setup is functioning as a sort of proxy to the OP, which the spec does not seem to allow based on the requirement cited above.

Which iss claim ends up being provided in the ID tokens returned by the token endpoint?

@MarcelCoding
Copy link
Contributor Author

In the jwt itself is the same iss as provided using autodiscovery in issuer. Maybe it would be possible to provide a statically configured alternative URL to support setups like this. Or even setups that don't conform with the spec. One who is using one of my projects encounters the same problem because his IDP does not conform with that. (MarcelCoding/jitsi-openid#105 (comment)) I would be up to implement this? What do you think?

@DanielMalmgren
Copy link

Just kicking in (I'm the uer @MarcelCoding is referring to above). I just found out that the issuer in my IDP is actually configurable (problem was just me leaving that setting blank making it default to a global setting), so for me this is actually fixable on the IDP side of things so it can conform to the spec. So I'm not dependent on a change in this behaviour :-)

@ramosbugs
Copy link
Owner

so for me this is actually fixable on the IDP side of things so it can conform to the spec. So I'm not dependent on a change in this behaviour :-)

Awesome! I think I'd prefer to leave the current behavior as-is since the spec uses pretty strong language around this (right above the snippet from Section 4.3 quoted above):

If any of the validation procedures defined in this specification fail, any operations requiring the information that failed to correctly validate MUST be aborted and the information that failed to validate MUST NOT be used.

@ramosbugs ramosbugs closed this as not planned Won't fix, can't repro, duplicate, stale Jul 4, 2022
@TKFRvisionOfficial
Copy link

Please reopen. Mfs are still not happy.

@MarcelCoding MarcelCoding changed the title Unable to use different issuerurl that in metadata. Unable to use different issuerurl than in metadata. Jul 5, 2022
@MarcelCoding MarcelCoding changed the title Unable to use different issuerurl than in metadata. Unable to use different issuerurl then in metadata. Jul 5, 2022
@MarcelCoding MarcelCoding changed the title Unable to use different issuerurl then in metadata. Unable to use different issuerurl than in metadata. Jul 5, 2022
@MarcelCoding
Copy link
Contributor Author

@ramosbugs what do you think of this idea: #75 (comment) - not only has faced the issue, I am still facing this issue

@ramosbugs
Copy link
Owner

Given the strong language and lack of ambiguity in the spec, I don't think it makes sense to add potential foot guns to this crate to work around misconfigured IDPs. The solution here is to fix the IDP configuration.

@MarcelCoding
Copy link
Contributor Author

But it should be, that external request are route different than internal ones. It isn't really a missconfiguration? And if the alternative URL is ataticly defined there alps would be and possible security flaws. Or am I missing something here? Sorry if this sound a little harsh, this is absolutely not my intention. I am just not that good in englisch. :)

@ramosbugs
Copy link
Owner

The concept of external vs. internal requests does not exist in OpenID Connect; it's purely an artifact of how this IdP presents itself to the world. IdPs are free to be creative in the functionality they provide, but they must still adhere to the spec. If they return a different issuer URL than the one used to fetch the provider metadata via .well-known/openid-configuration, that's a violation of the spec that should be fixed at the IdP, not worked around by every client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants