Open Redrield opened 1 year ago
I'm sympathetic to Microsoft's customers' frustrations here while also being annoyed that Microsoft decided to ignore the spec and push this problem to both customers and library maintainers. Since this library focuses on standardized protocols, I think a reasonable goal here is to have escape hatches (but not foot guns) available that allow users to interoperate with Microsoft, but without adding any Microsoft-specific functionality to this crate. A PR adding a working Microsoft code example would be welcomed, though.
This means that Microsoft customers should be able to use this crate, but their code may be more verbose than for users of standards-compliant OIDC implementations. Making Microsoft OIDC integrations work out of the box without any extra code is a non-goal of this crate, although community members should feel free to publish a separate crate built on top of this one to improve the ergonomics.
The two checks that fail are in verification.rs:284 (JWT issuer validation), and discovery.rs:386 (Provider metadata issuer validation).
For JWT issuer validation, there are already two escape hatches available:
IdTokenVerifier
, an IssuerUrl
should be specified that matches the one that will be present in the ID tokens. This is the recommended approach.require_issuer_match
method that can be used to disable this check altogether.Discovery validation is a bit trickier, but the http_client
argument provides an escape hatch. Since the OIDC discovery document isn't signed, Microsoft users can pass an http_client
shim to discover
/discover_async
that replaces the invalid issuer
field in the response with the one that will be in the ID tokens. That way, by the time the response reaches this crate, it'll be compliant with the spec, and the rest of the library should just work. This is the functionality that I could see living in a separate, Microsoft-specific crate that wraps around this one.
Thanks for the guidance, I did manage to put together a shim http client as you suggested to fix the issue with discovery validation. As for JWT validation, I'm not sure how the "recommended" escape hatch could be implemented, even by a library that wraps this one, because the client id/secret gets moved into the oidc Client type, and is held there opaque to any API consumers. Since the instantiation of an IdTokenVerifier happens here, there doesn't seem to be a lot that can be done to get around that.
As for a Microsoft code example, after circumventing the failing validation checks, I was able to get this crate working with the same overall structure as the Google example, albeit with URLs changed.
As for JWT validation, I'm not sure how the "recommended" escape hatch could be implemented, even by a library that wraps this one, because the client id/secret gets moved into the oidc Client type, and is held there opaque to any API consumers. Since the instantiation of an IdTokenVerifier happens here, there doesn't seem to be a lot that can be done to get around that.
The automatic instantiation happens within the Client
, but Microsoft users can instantiate their own using similar code to: https://github.com/ramosbugs/openidconnect-rs/blob/e9b1c53aa7f97cc9524ce5c8cb58c356b238ca07/src/lib.rs#L986-L1005
All of these values are available outside the client, either from users knowing their own client ID and client secret (cloning rather than moving them into the Client
) or from the OIDC discovery document (making a copy of provider_metadata.jwks()
before moving the metadata into Client::from_provider_metadata
).
LinkedIn's new OIDC implementations seems to adopt a lot of Microsoft's poor behavior regarding OIDC standards, but they also:
email_verified
field is encoded as a string literal "true"
, not true
, which causes openidconnect-rs to fail when deserializing the payload in question. Scope
s). It's probably possible to go and work around this using the http shim method, and I've been able to do so to at least reconstruct the id_token and token response to be spec-compliant, but because the incorrect format of email_verified
causes IDToken
deserialization to fail, it ends up being a bit of an awkward dance (though until I go and try to validate signatures here, I cannot say how awkward). Needless to say, I don't think this is the concern of this library necessarily, except perhaps in the capacity to go and workaround non-compliant OIDC implementors in a natural way.
Hey,
For information it appears there is now a Microsoft Entra v2
endpoint which should be compliant.
You should be able to find it on https://entra.microsoft.com following Identity | Applications | App registrations | Endpoints
. And it should look something like this : https://login.microsoftonline.com/${tenantguid}/v2.0
Edit: Looking at the Microsoft issue the endpoint is mentioned as not compliant in Feb (https://github.com/MicrosoftDocs/azure-docs/issues/38427#issuecomment-1430186584) but 2 peoples mentioned it as now working in : https://github.com/Timshel/vaultwarden/issues/8.
This library strictly validates responses from the server according to spec in ways that make it incompatible with Microsoft's implementation of OIDC in certain situations due to their refusal to make their implementation spec compliant. Currently the library doesn't allow users to configure the validation strictness, meaning the only way to work around this is to copy the repository locally to selectively ignore validation failures caused by this. It would be helpful if users could configure this to disable the validation to support Microsoft OIDC (Or alternatively, to specify alternate valid fields to provide some level of certainty that the payload is mostly correct).
The two checks that fail are in verification.rs:284 (JWT issuer validation), and discovery.rs:386 (Provider metadata issuer validation).
Based on the responses to the above issue (as well as the age of the issue), I don't think a spec compliant implementation of OIDC should be expected from Microsoft any time soon, and as such it would be helpful to allow for exceptions in this case to be compatible.