erlef
Improve error for trailing `/` in issuer url
oidcc version
3.3.0
Erlang version
27
Elixir version
1.18.1
Summary
Using an issuer of https://cognito-idp.eu-central-1.amazonaws.com/…/ causes an issuer mismatch error, while the same url without trailing / doesn't. I'm not sure if this is expected. It either could have a better error message / be detected earlier if not be silently handled by removing the trailing /.
Current behavior
See above.
How to reproduce
See above.
Expected behavior
See above.
@LostKobrakai This is on purpose. I would however welcome a specific and better error message.
Relevant Spec:
https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation
The Issuer Identifier for the OpenID Provider (which is typically obtained during Discovery) MUST exactly match the value of the iss (issuer) Claim.
https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderMetadata
REQUIRED. URL using the https scheme with no query or fragment components that the OP asserts as its Issuer Identifier. If Issuer discovery is supported (see Section 2), this value MUST be identical to the issuer value returned by WebFinger. This also MUST be identical to the iss Claim value in ID Tokens issued from this Issuer.
Not sure if there's string compare functions on erlang like there are on elixir, which could quantify how much the difference between the string is, but at least the case of a trailing / being the difference could be explicitly checked and result in an error about that.
The current error is hard to parse for such small differences:
[error] GenServer Dashboard.Provider.Cognito terminating
** (stop) {:configuration_load_failed, {:issuer_mismatch, "https://cognito-idp.eu-central-1.amazonaws.com/…"}}
Last message: {:continue, :load_configuration}
State: {:state, :undefined, :undefined, "https://cognito-idp.eu-central-1.amazonaws.com/…/", %{}, :undefined, :undefined, Dashboard.Provider.Cognito, 1000, 30000, :stop, :undefined}
The 99% of the url being the same will be enough for a user to go "but that's the same value".
I'm also not aware of any diff functions in erlang like a myers diff. We would have to do that by hand I think. But that also goes further than needed. Just comparing with / without trailing slash should be enough.
The question however is where to do this. We shouldn't touch the error itself since it is just a tuple and this does not warrant a breaking change. We don't have an error struct like in Elixir where we could easily override the message.
Hmm if the direct effect path is not to be changed maybe a side effect would work? Logger or telemetry based maybe?
Hm, If it is in telemetry, people will probably not see it. Also, I would like to not log in the core functions itself. But we could add some code to the provider GenServer that logs a custom message and that's probably the most common way to run oidcc anyways.