diff --git a/CHANGELOG.md b/CHANGELOG.md index c974da0d..4c240122 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/providers/ms_entra_id.go b/providers/ms_entra_id.go index 052d45a8..1229c0de 100644 --- a/providers/ms_entra_id.go +++ b/providers/ms_entra_id.go @@ -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() diff --git a/providers/ms_entra_id_test.go b/providers/ms_entra_id_test.go index b153006e..0cd807d1 100644 --- a/providers/ms_entra_id_test.go +++ b/providers/ms_entra_id_test.go @@ -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(