search

hashicorp / boundary

Boundary enables identity-based access management for dynamic infrastructure.
https://boundaryproject.io
Other
3.84k stars 287 forks source link

Adding OIDC Keycloak with .well-known/openid-configuration failed because of trailing slash #4710

Open hillout opened 5 months ago

hillout commented 5 months ago

Hi, here is another bug that I discovered while was trying to add OIDC Keycloak from UI Admin console in Boundary.

If you add issuer with .well-known/openid-configuration, in my example it is http://172.21.33.27:9999/realms/inventory/.well-known/openid-configuration: Screenshot from 2024-04-24 17-57-44

And fill out the rest, you'll be able to add the OIDC Provider as expected, but, once you'll try to set it's state for example Public, it will fail: Screenshot from 2024-04-24 17-58-48

Cuz the boundary set the issuer as - http://172.21.33.27:9999/realms/inventory/ - with the trailing slas in the end, and with "/" it does not match with the OIDC provider which is: Screenshot from 2024-04-24 18-22-06

It doesn't include **/**

To resolve this issue you just need to delete that slash and the OIDC can be set to Public and after been used without any problems:

Screenshot from 2024-04-24 18-10-19

Screenshot from 2024-04-24 18-10-10

davidsbond commented 3 months ago

This seems to be caused by the OIDC parsing code here:

https://github.com/hashicorp/boundary/blob/main/internal/daemon/controller/handlers/authmethods/oidc.go#L412

Looking at the discovery spec:

The issuer value returned MUST be identical to the Issuer URL that was used as the prefix to /.well-known/openid-configuration to retrieve the configuration information. This MUST also be identical to the iss Claim value in ID Tokens issued from this Issuer.

This sounds to me like we do not want to keep that trailing slash, but the comment calls out keeping it there explicitly. @jimlambrt might know why that is the case.

davidsbond commented 3 months ago

After chatting with folks about this internally. It seems a slightly trickier problem to solve. We validate the configuration against the oidc-configuration endpoint once the auth method is changed to public. Different OIDC providers present the iss field differently. Some prefer to keep the slash, some don't.

I'm not sure there's a stripping behaviour that Boundary could employ to solve this problem. It's possible we could remove all mention of oidc-configuration entirely and don't perform any stripping. Users will then need to make sure the issuer value they enter is the one that will match their OIDC provider.

Validation is still performed regardless of whether you provide the oidc-configuration endpoint initially.