Improve error for trailing `/` in issuer url

[LostKobrakai]

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.

[maennchen]

@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.

[LostKobrakai]

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".

[maennchen]

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.

[LostKobrakai]

Hmm if the direct effect path is not to be changed maybe a side effect would work? Logger or telemetry based maybe?

[maennchen]

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.

[LostKobrakai]

Yeah I think that would be fine as well.