From b1bfca39f15bc4c5456b6e23aee872486b67fd22 Mon Sep 17 00:00:00 2001 From: Zettat123 Date: Thu, 16 Apr 2026 11:30:46 -0600 Subject: [PATCH] Add `ExternalIDClaim` option for OAuth2 OIDC auth source (#37229) This PR adds an External ID Claim Name configuration field to the OIDC auth source. When set, Gitea uses the specified JWT claim as the user's `ExternalID` instead of the default `sub` claim. This PR fixes the bug when migrating from Azure AD V2 to OIDC. When an admin migrates the same auth source to OIDC, goth's `openidConnect` provider defaults to using the `sub` claim as `UserID`. However, Azure AD's `sub` is a pairwise identifier: > `sub`: The subject is a pairwise identifier and is unique to an application ID. If a single user signs into two different apps using two different client IDs, those apps receive two different values for the subject claim. https://learn.microsoft.com/en-us/entra/identity-platform/id-token-claims-reference#payload-claims As a result, every existing user appears as a new account after migration. To fix this issue, Gitea should use `oid` claim for `UserID`. > `oid`: This ID uniquely identifies the user across applications - two different applications signing in the same user receives the same value in the oid claim. Note: The `oid` claim is not included in Azure AD tokens by default. The `profile` scope must be added to the Scopes field of the auth source. --- options/locale/locale_en-US.json | 2 + routers/web/admin/auths.go | 1 + .../auth/source/oauth2/providers_openid.go | 8 +- services/auth/source/oauth2/source.go | 1 + services/forms/auth_form.go | 1 + templates/admin/auth/edit.tmpl | 5 + templates/admin/auth/source/oauth.tmpl | 5 + tests/integration/auth_oauth2_test.go | 201 ++++++++++++++++++ web_src/js/features/admin/common.ts | 3 +- 9 files changed, 225 insertions(+), 2 deletions(-) create mode 100644 tests/integration/auth_oauth2_test.go diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index b9d5247b3d4..7e3ace5be17 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -3180,6 +3180,8 @@ "admin.auths.oauth2_required_claim_name_helper": "Set this name to restrict login from this source to users with a claim with this name", "admin.auths.oauth2_required_claim_value": "Required Claim Value", "admin.auths.oauth2_required_claim_value_helper": "Set this value to restrict login from this source to users with a claim with this name and value", + "admin.auths.open_id_connect_external_id_claim": "External ID Claim Name (Optional)", + "admin.auths.open_id_connect_external_id_claim_helper": "Claim name to use as the user's external identity. Defaults to \"sub\". For Azure AD / Entra ID, set this to \"oid\" to maintain continuity when migrating from the Azure AD V2 provider. Note: the \"oid\" claim requires the \"profile\" scope to be included in the Scopes field above.", "admin.auths.oauth2_group_claim_name": "Claim name providing group names for this source. (Optional)", "admin.auths.oauth2_full_name_claim_name": "Full Name Claim Name. (Optional — if set, the user's full name will always be synchronized with this claim)", "admin.auths.oauth2_ssh_public_key_claim_name": "SSH Public Key Claim Name", diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index e29aca127c8..cc02ce99996 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -205,6 +205,7 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { SSHPublicKeyClaimName: form.Oauth2SSHPublicKeyClaimName, FullNameClaimName: form.Oauth2FullNameClaimName, + ExternalIDClaim: form.OpenIDConnectExternalIDClaim, } } diff --git a/services/auth/source/oauth2/providers_openid.go b/services/auth/source/oauth2/providers_openid.go index fc0d77a7e61..557fe6cb013 100644 --- a/services/auth/source/oauth2/providers_openid.go +++ b/services/auth/source/oauth2/providers_openid.go @@ -46,8 +46,14 @@ func (o *OpenIDProvider) CreateGothProvider(providerName, callbackURL string, so provider, err := openidConnect.New(source.ClientID, source.ClientSecret, callbackURL, source.OpenIDConnectAutoDiscoveryURL, scopes...) if err != nil { log.Warn("Failed to create OpenID Connect Provider with name '%s' with url '%s': %v", providerName, source.OpenIDConnectAutoDiscoveryURL, err) + return nil, err } - return provider, err + if source.ExternalIDClaim != "" { + // UserIdClaims is a fallback list; goth returns the first non-empty matching claim. + // A single entry is sufficient because the admin explicitly chooses one claim (e.g. "oid" for Azure AD). + provider.UserIdClaims = []string{source.ExternalIDClaim} + } + return provider, nil } // CustomURLSettings returns the custom url settings for this provider diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 00d89b3481b..3f69c08fab8 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -30,6 +30,7 @@ type Source struct { SSHPublicKeyClaimName string FullNameClaimName string + ExternalIDClaim string } // FromDB fills up an OAuth2Config from serialized format. diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index 95965b5f29a..ad2243be348 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -88,6 +88,7 @@ type AuthenticationForm struct { Oauth2GroupTeamMapRemoval bool Oauth2SSHPublicKeyClaimName string Oauth2FullNameClaimName string + OpenIDConnectExternalIDClaim string // SSPI SSPIAutoCreateUsers bool diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 56f9e1b9cdb..60480ce82f7 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -330,6 +330,11 @@ +
+ + +

{{ctx.Locale.Tr "admin.auths.open_id_connect_external_id_claim_helper"}}

+
diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index 69590635e4b..94664a38a58 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -88,6 +88,11 @@
+
+ + +

{{ctx.Locale.Tr "admin.auths.open_id_connect_external_id_claim_helper"}}

+
diff --git a/tests/integration/auth_oauth2_test.go b/tests/integration/auth_oauth2_test.go new file mode 100644 index 00000000000..e977a7dc489 --- /dev/null +++ b/tests/integration/auth_oauth2_test.go @@ -0,0 +1,201 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "encoding/base64" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "testing" + "time" + + auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/services/auth/source/oauth2" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMigrateAzureADV2ToOIDC simulates a login source migration from the Azure AD V2 OAuth2 provider to the OpenID Connect provider, +// and verifies that setting ExternalIDClaim = "oid" restores account continuity. +// +// Background: Azure AD V2 (goth's azureadv2 provider) fetches the user profile from Microsoft Graph API (/v1.0/me) +// and uses the "id" field - the stable Object ID (OID) - as gothUser.UserID. That OID is stored as ExternalID in external_login_user. +// +// When the admin migrates the same source to OpenID Connect, the goth openidConnect provider defaults to ["sub"] for UserIdClaims. +// Azure AD's "sub" is pairwise (unique per application), so it differs from the OID that was previously stored, +// causing every existing user to appear as a new account. +// +// Setting ExternalIDClaim = "oid" on the OIDC source overrides UserIdClaims to ["oid"], +// so the same OID is extracted and matched against the existing rows, restoring continuity. +func TestMigrateAzureADV2ToOIDC(t *testing.T) { + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.OAuth2Client.EnableAutoRegistration, true)() + // Use UserID (gothUser.UserID) as the Gitea username so that different ExternalID values produce different, non-conflicting usernames. + defer test.MockVariableValue(&setting.OAuth2Client.Username, setting.OAuth2UsernameUserid)() + + const ( + sourceName = "test-migrate-azure" + + // oidValue is the stable Azure AD Object ID, used as ExternalID by the Azure AD V2 provider. + oidValue = "oid-object-id-stable" + + // subValue is the pairwise sub issued by Azure AD for OpenID Connect; it differs from oidValue and would produce a separate account if used. + subValue = "sub-pairwise-value" + ) + + // The fake OIDC server issues tokens containing both sub and oid claims, mirroring what Azure AD v2.0 returns. + srv := newFakeOIDCServer(t, subValue, oidValue) + + // --- Step 1: Establish the legacy Azure AD V2 state --- + // Create an azureadv2 auth source. In production this would have been the source used before the migration. + addOAuth2Source(t, sourceName, oauth2.Source{ + Provider: "azureadv2", + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + CustomURLMapping: &oauth2.CustomURLMapping{ + Tenant: "test-tenant-id", + }, + }) + authSource, err := auth_model.GetActiveOAuth2SourceByAuthName(t.Context(), sourceName) + require.NoError(t, err) + + // Create a user to represent the "legacy" account that was originally registered through the Azure AD V2 provider. + legacyUser := &user_model.User{ + Name: "legacy-azure-user", + Email: "legacy-azure-user@example.com", + } + require.NoError(t, user_model.CreateUser(t.Context(), legacyUser, &user_model.Meta{})) + require.NoError(t, user_model.LinkExternalToUser(t.Context(), legacyUser, &user_model.ExternalLoginUser{ + ExternalID: oidValue, + UserID: legacyUser.ID, + LoginSourceID: authSource.ID, + Provider: authSource.Name, + })) + + // --- Step 2: Migrate the source to OIDC without ExternalIDClaim --- + // The provider type of the OAuth2 source is changed from azureadv2 to openidConnect. + // Without ExternalIDClaim the goth provider defaults to "sub", which does not match the stored OID, so every sign-in creates a fresh account. + authSource.Cfg = &oauth2.Source{ + Provider: "openidConnect", + ClientID: "test-client-id", + ClientSecret: "test-client-secret", + OpenIDConnectAutoDiscoveryURL: srv.URL + "/.well-known/openid-configuration", + // ExternalIDClaim intentionally not set; goth defaults to "sub". + } + err = auth_model.UpdateSource(t.Context(), authSource) + require.NoError(t, err) + + t.Run("without ExternalIDClaim: legacy user is NOT matched", func(t *testing.T) { + // Confirm the external user with ExternalID=subValue doesn't exist. + unittest.AssertNotExistsBean(t, &user_model.ExternalLoginUser{ExternalID: subValue, LoginSourceID: authSource.ID}, unittest.OrderBy("external_id ASC")) + + doOIDCSignIn(t, sourceName) + + // "sub" is now the ExternalID - a new user was auto-registered. + subEntry := unittest.AssertExistsAndLoadBean(t, &user_model.ExternalLoginUser{ExternalID: subValue, LoginSourceID: authSource.ID}, unittest.OrderBy("external_id ASC")) + // The auto-registered user is NOT the legacy user. + assert.NotEqual(t, legacyUser.ID, subEntry.UserID) + }) + + // --- Step 3: Set ExternalIDClaim = "oid" to restore account continuity --- + // Set ExternalIDClaim = "oid" so that the OIDC source extracts the same Object ID that the Azure AD V2 provider previously stored. + authSource.Cfg.(*oauth2.Source).ExternalIDClaim = "oid" + err = auth_model.UpdateSource(t.Context(), authSource) + require.NoError(t, err) + + t.Run("with ExternalIDClaim=oid: legacy user IS matched", func(t *testing.T) { + // Confirm the legacy oid row has no RawData yet - it was created directly via LinkExternalToUser in setup, without going through an OAuth flow. + oidEntry := unittest.AssertExistsAndLoadBean(t, &user_model.ExternalLoginUser{ExternalID: oidValue, LoginSourceID: authSource.ID}, unittest.OrderBy("external_id ASC")) + require.Nil(t, oidEntry.RawData) + + doOIDCSignIn(t, sourceName) + + // After sign-in, RawData should contain both "oid" and "name". + oidEntry = unittest.AssertExistsAndLoadBean(t, &user_model.ExternalLoginUser{ExternalID: oidValue, LoginSourceID: authSource.ID}, unittest.OrderBy("external_id ASC")) + assert.Equal(t, oidValue, oidEntry.RawData["oid"]) + assert.Equal(t, "OIDC Test User", oidEntry.RawData["name"]) + + // The matched user must still be the original legacy user. + assert.Equal(t, legacyUser.ID, oidEntry.UserID) + }) +} + +// newFakeOIDCServer starts an httptest.Server that implements the minimum OIDC endpoints needed to complete a sign-in flow: +func newFakeOIDCServer(t *testing.T, sub, oid string) *httptest.Server { + t.Helper() + + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/.well-known/openid-configuration": // discovery document + _ = json.NewEncoder(w).Encode(map[string]string{ + "issuer": srv.URL, + "authorization_endpoint": srv.URL + "/authorize", + "token_endpoint": srv.URL + "/token", + "userinfo_endpoint": srv.URL + "/userinfo", + }) + case "/token": // returns an ID token with both "sub" and "oid" claims so tests can verify which one ends up as ExternalID + claims := map[string]any{ + "iss": srv.URL, + "aud": "test-client-id", + "exp": time.Now().Add(time.Hour).Unix(), + "sub": sub, + "oid": oid, + } + payload, _ := json.Marshal(claims) + header := base64.RawURLEncoding.EncodeToString([]byte(`{"alg":"none"}`)) + + // build a JWT-shaped string whose payload encodes claims. + // goth's decodeJWT only base64-decodes the payload without verifying the signature, so no real signing infrastructure is needed. + idToken := header + "." + base64.RawURLEncoding.EncodeToString(payload) + ".fakesig" + + _ = json.NewEncoder(w).Encode(map[string]any{ + "access_token": "fake-access-token", + "token_type": "Bearer", + "id_token": idToken, + }) + case "/userinfo": + // sub MUST match the id_token sub; goth rejects mismatches. + _ = json.NewEncoder(w).Encode(map[string]any{ + "sub": sub, + "email": sub + "@example.com", + "name": "OIDC Test User", + }) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + return srv +} + +// doOIDCSignIn runs a mock OIDC sign-in flow for the given auth source. +func doOIDCSignIn(t *testing.T, sourceName string) { + t.Helper() + session := emptyTestSession(t) + + // Step 1: initiate login + resp := session.MakeRequest(t, NewRequest(t, "GET", "/user/oauth2/"+sourceName), http.StatusTemporaryRedirect) + + // Step 2: extract the UUID state that Gitea embedded in the redirect URL. + location := resp.Header().Get("Location") + u, err := url.Parse(location) + require.NoError(t, err) + state := u.Query().Get("state") + require.NotEmpty(t, state, "redirect to OIDC provider must include state") + + // Step 3: simulate the provider redirecting back. + callbackURL := fmt.Sprintf("/user/oauth2/%s/callback?code=test-code&state=%s", sourceName, url.QueryEscape(state)) + session.MakeRequest(t, NewRequest(t, "GET", callbackURL), http.StatusSeeOther) +} diff --git a/web_src/js/features/admin/common.ts b/web_src/js/features/admin/common.ts index 4ba74f1b08d..0f457e2e1ac 100644 --- a/web_src/js/features/admin/common.ts +++ b/web_src/js/features/admin/common.ts @@ -78,7 +78,7 @@ function initAdminAuthentication() { } function onOAuth2Change(applyDefaultValues: boolean) { - hideElem('.open_id_connect_auto_discovery_url, .oauth2_use_custom_url'); + hideElem('.open_id_connect_auto_discovery_url, .open_id_connect_external_id_claim, .oauth2_use_custom_url'); for (const input of document.querySelectorAll('.open_id_connect_auto_discovery_url input[required]')) { input.removeAttribute('required'); } @@ -88,6 +88,7 @@ function initAdminAuthentication() { case 'openidConnect': document.querySelector('.open_id_connect_auto_discovery_url input')!.setAttribute('required', 'required'); showElem('.open_id_connect_auto_discovery_url'); + showElem('.open_id_connect_external_id_claim'); break; default: { const elProviderCustomUrlSettings = document.querySelector(`#${provider}_customURLSettings`);