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

Fix bug with AadIssuerValidator #3042

Merged

Conversation

ssmelov
Copy link
Contributor

@ssmelov ssmelov commented Nov 28, 2024

Fix bug with AadIssuerValidator

#3047

Description

The current implementation of AadIssuerValidator always expects that token issuer length should be greater than templateTenantIdPosition + tenantId's length.
It's not true for the case when Issuer in token and Issuer template in OIDC configuration match, but don't have a trailing slash.

Example:

Issuer template in OIDC configuration: https://sts.windows.net/{tenantid}
Issuer in JWT token: https://sts.windows.net/f645ad92-e38d-4d1a-b510-d1b09a74a8ca

Condition tokenIssuer.Length <= templateTenantIdPosition + tenantId.Length of IsValidIssuer method results to true because tokenIssuer.Length equals to templateTenantIdPosition + tenantId.Length. As a result, the issuer is considered as invalid even when it matches to the issuer template from OIDC config.

@ssmelov ssmelov requested a review from a team as a code owner November 28, 2024 11:06
@jennyf19
Copy link
Collaborator

jennyf19 commented Dec 3, 2024

@ssmelov do you mind opening an issue for this and linking back to the PR? Thank you.

@brentschmaltz brentschmaltz merged commit 131a363 into AzureAD:dev Dec 3, 2024
4 of 5 checks passed
@jennyf19
Copy link
Collaborator

jennyf19 commented Dec 3, 2024

@ssmelov do you mind opening an issue for this and linking back to the PR? Thank you.

nevermind, opened one so we can track it more easily: #3047

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

Successfully merging this pull request may close these issues.

5 participants