mirror of
https://github.com/oauth2-proxy/oauth2-proxy.git
synced 2026-10-05 22:21:18 +02:00
fix: surface Microsoft Graph errors during Entra group overage instead of logging in with an incomplete group set (#3535)
* Propagate Microsoft Graph errors during Entra group overage When a session hit AAD group overage, addGraphGroupsToSession logged a failed Graph request and returned nil, so EnrichSession's error handling never ran and the session kept the overage placeholder instead of the real groups. Return the error like the rest of the file does, and add a regression test using the existing 401 Graph mock. Signed-off-by: Ishan Shaurya Jaiswal <19599684+no-hup@users.noreply.github.com> * Tighten the Graph-error test and wrap the error, add a changelog entry Wrap the Graph error with %w so callers can errors.Is through EnrichSession. Pin the test to the Graph path with ErrorContains and assert the session's groups are left untouched after the failed call, so the test fails if the fix regresses rather than on any error. Add the behaviour change to the changelog since a Graph outage during overage now fails the login instead of proceeding with the overage placeholder. Signed-off-by: Ishan Shaurya Jaiswal <19599684+no-hup@users.noreply.github.com> --------- Signed-off-by: Ishan Shaurya Jaiswal <19599684+no-hup@users.noreply.github.com> Co-authored-by: Ishan Shaurya Jaiswal <19599684+no-hup@users.noreply.github.com>
This commit is contained in:
co-authored by
Ishan Shaurya Jaiswal
parent
bde7f9eee1
commit
4b94ed869b
@@ -14,6 +14,7 @@ Additionally refer to OAuth client configuration for Bitbucket provider in the [
|
||||
- [#3546](https://github.com/oauth2-proxy/oauth2-proxy/pull/3546) fix: strip the port from the request host when matching cookie domains @kirilju
|
||||
- [#3547](https://github.com/oauth2-proxy/oauth2-proxy/pull/3547) fix: refresh additional claims for OIDC and MS Entra ID providers and properly populate additional claims during login (@Apollo3zehn)
|
||||
- [#3477](https://github.com/oauth2-proxy/oauth2-proxy/pull/3477) fix(bitbucket): auth failure due to Bitbucket OAuth 2.0 [changes on May 4th 2026](https://developer.atlassian.com/cloud/bitbucket/changelog/#CHANGE-3052) @mfouad
|
||||
- [#3535](https://github.com/oauth2-proxy/oauth2-proxy/pull/3535) fix: surface Microsoft Graph errors during Entra group overage instead of logging in with an incomplete group set @no-hup
|
||||
|
||||
# V7.15.4
|
||||
|
||||
|
||||
@@ -248,8 +248,7 @@ func (p *MicrosoftEntraIDProvider) addGraphGroupsToSession(ctx context.Context,
|
||||
UnmarshalSimpleJSON()
|
||||
|
||||
if err != nil {
|
||||
logger.Errorf("invalid response from microsoft graph, no groups added to session: %v", err)
|
||||
return nil
|
||||
return fmt.Errorf("invalid response from microsoft graph: %w", err)
|
||||
}
|
||||
reqGroups := response.Get("value").MustArray()
|
||||
|
||||
|
||||
@@ -82,6 +82,44 @@ func TestAzureEntraOIDCProviderEnrichSessionGroupOverage(t *testing.T) {
|
||||
assert.Contains(t, session.Groups, "b1aef995-6b55-4ac6-bbfe-e829810e9352", "Pagination using $skiptoken failed")
|
||||
}
|
||||
|
||||
func TestAzureEntraOIDCProviderEnrichSessionGraphError(t *testing.T) {
|
||||
// Create ID Token that indicates group overage with _claim_names
|
||||
key, _ := rsa.GenerateKey(rand.Reader, 2048)
|
||||
claimsWithGroupOverage := &claimsWithGroupOverage{
|
||||
jwt.RegisteredClaims{
|
||||
Issuer: "https://login.microsoftonline.com/18014347-dd57-41a1-8191-7a1f734ea457/v2.0",
|
||||
},
|
||||
map[string]string{"groups": "src1"},
|
||||
}
|
||||
|
||||
jwtWithClaims := jwt.NewWithClaims(jwt.SigningMethodRS256, claimsWithGroupOverage)
|
||||
signedJWT, err := jwtWithClaims.SignedString(key)
|
||||
assert.NoError(t, err)
|
||||
|
||||
session := CreateAuthorizedSession()
|
||||
session.IDToken = signedJWT
|
||||
session.Email = "mock@example.com"
|
||||
|
||||
provider := NewMicrosoftEntraIDProvider(&ProviderData{},
|
||||
options.Provider{OIDCConfig: options.OIDCOptions{
|
||||
IssuerURL: "https://login.microsoftonline.com/18014347-dd57-41a1-8191-7a1f734ea457/v2.0",
|
||||
}},
|
||||
)
|
||||
|
||||
// Mock a Graph server that rejects the groups request
|
||||
mockedGraph := mockGraphAPI(true)
|
||||
mockedGraphURL, _ := url.Parse(mockedGraph.URL)
|
||||
updateURL(provider.microsoftGraphURL, mockedGraphURL.Host)
|
||||
|
||||
// A failed Graph lookup during overage must surface as an error, not a
|
||||
// silently under-populated session.
|
||||
groupsBefore := session.Groups
|
||||
err = provider.EnrichSession(context.Background(), session)
|
||||
assert.ErrorContains(t, err, "invalid response from microsoft graph")
|
||||
// The session must be left exactly as it was, not half-populated.
|
||||
assert.Equal(t, groupsBefore, session.Groups)
|
||||
}
|
||||
|
||||
func TestAzureEntraOIDCProviderValidateSessionAllowedTenants(t *testing.T) {
|
||||
// Create multi-tenant Azure Entra provider with allowed tenants
|
||||
provider := NewMicrosoftEntraIDProvider(
|
||||
|
||||
Reference in New Issue
Block a user