0
0
mirror of https://github.com/go-gitea/gitea.git synced 2026-06-01 23:32:34 +02:00

fix(auth): ignore stale OIDC external login links to organizations (#37875)

## Summary

This fixes an OIDC sign-in edge case where a stale `external_login_user`
record can still point to an organization or a deleted user.

In that situation, Gitea may keep resolving the external login to the
wrong account during sign-in. For affected instances, this matches the
behavior reported in #36439 and #37812, where a user signing in with
OIDC/Entra ID could appear as an organization, or hit a 404 after that
organization was removed.

## What changed

- validate the user resolved from `external_login_user` during
OAuth2/OIDC login
- ignore stale links when the linked user no longer exists
- ignore stale links when the linked user is not an individual user
- remove the stale external login row so the sign-in flow can relink the
external account to the correct user

## Related

- Fixes #37812
- Related to #36439

---------

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: Claude (Opus 4.8) <noreply@anthropic.com>
This commit is contained in:
Lunny Xiao 2026-05-30 13:37:09 -07:00 committed by GitHub
parent 28096162fa
commit 4e5f43896e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 103 additions and 23 deletions

View File

@ -80,8 +80,11 @@ func init() {
}
// GetExternalLogin checks if a externalID in loginSourceID scope already exists
func GetExternalLogin(ctx context.Context, externalLoginUser *ExternalLoginUser) (bool, error) {
return db.GetEngine(ctx).Get(externalLoginUser)
func GetExternalLogin(ctx context.Context, loginSourceID int64, externalID string) (*ExternalLoginUser, bool, error) {
return db.Get[ExternalLoginUser](ctx, builder.Eq{
"external_id": externalID,
"login_source_id": loginSourceID,
})
}
// LinkExternalToUser link the external user to the user
@ -118,6 +121,12 @@ func RemoveAllAccountLinks(ctx context.Context, user *User) error {
return err
}
// RemoveExternalLoginByExternalID removes a specific external login link by its provider-side identifier.
func RemoveExternalLoginByExternalID(ctx context.Context, loginSourceID int64, externalID string) error {
_, err := db.GetEngine(ctx).Where("external_id=? AND login_source_id=?", externalID, loginSourceID).Delete(new(ExternalLoginUser))
return err
}
// GetUserIDByExternalUserID get user id according to provider and userID
func GetUserIDByExternalUserID(ctx context.Context, provider, userID string) (int64, error) {
var id int64

View File

@ -514,17 +514,33 @@ func oAuth2UserLoginCallback(ctx *context.Context, authSource *auth.Source, requ
}
// search in external linked users
externalLoginUser := &user_model.ExternalLoginUser{
ExternalID: gothUser.UserID,
LoginSourceID: authSource.ID,
}
hasUser, err = user_model.GetExternalLogin(request.Context(), externalLoginUser)
externalLoginUser, hasUser, err := user_model.GetExternalLogin(ctx, authSource.ID, gothUser.UserID)
if err != nil {
return nil, goth.User{}, err
}
if hasUser {
user, err = user_model.GetUserByID(request.Context(), externalLoginUser.UserID)
return user, gothUser, err
user, err = user_model.GetUserByID(ctx, externalLoginUser.UserID)
if err != nil && !user_model.IsErrUserNotExist(err) {
return nil, goth.User{}, err
}
if err == nil && user.IsIndividual() {
return user, gothUser, nil
}
// The external login record is stale: the linked user no longer exists, or it exists but is
// not an individual user (only individual users can sign in, so a link pointing at an
// organization, bot or remote user can never resolve). Remove it so the next sign-in can
// relink the external account to the correct user. Nothing is lost, because the link is
// recreated automatically on the next sign-in.
reason := "linked user does not exist"
if err == nil {
reason = fmt.Sprintf("linked user type is %d", user.Type)
}
log.Warn("Ignoring stale external login link [external-id=%s login-source-id=%d user-id=%d]: %s", externalLoginUser.ExternalID, externalLoginUser.LoginSourceID, externalLoginUser.UserID, reason)
if err := user_model.RemoveExternalLoginByExternalID(ctx, externalLoginUser.LoginSourceID, externalLoginUser.ExternalID); err != nil {
return nil, goth.User{}, err
}
}
// no user found to login

View File

@ -57,12 +57,7 @@ func TestSource(t *testing.T) {
err := source.refresh(t.Context(), provider, e)
assert.NoError(t, err)
e := &user_model.ExternalLoginUser{
ExternalID: e.ExternalID,
LoginSourceID: e.LoginSourceID,
}
ok, err := user_model.GetExternalLogin(t.Context(), e)
e, ok, err := user_model.GetExternalLogin(t.Context(), e.LoginSourceID, e.ExternalID)
assert.NoError(t, err)
assert.True(t, ok)
assert.Equal(t, "refresh", e.RefreshToken)
@ -82,12 +77,7 @@ func TestSource(t *testing.T) {
})
assert.NoError(t, err)
e := &user_model.ExternalLoginUser{
ExternalID: e.ExternalID,
LoginSourceID: e.LoginSourceID,
}
ok, err := user_model.GetExternalLogin(t.Context(), e)
e, ok, err := user_model.GetExternalLogin(t.Context(), e.LoginSourceID, e.ExternalID)
assert.NoError(t, err)
assert.True(t, ok)
assert.Empty(t, e.RefreshToken)

View File

@ -130,8 +130,73 @@ func TestMigrateAzureADV2ToOIDC(t *testing.T) {
})
}
func TestOIDCIgnoresStaleExternalLoginLinks(t *testing.T) {
defer tests.PrepareTestEnv(t)()
defer test.MockVariableValue(&setting.OAuth2Client.EnableAutoRegistration, true)()
defer test.MockVariableValue(&setting.OAuth2Client.AccountLinking, setting.OAuth2AccountLinkingAuto)()
defer test.MockVariableValue(&setting.OAuth2Client.Username, setting.OAuth2UsernameEmail)()
setup := func(t *testing.T, sourceName, sub, userName, email string) (*auth_model.Source, *user_model.User) {
t.Helper()
srv := newFakeOIDCServerWithProfile(t, sub, sub+"-oid", email, "OIDC Test User")
addOAuth2Source(t, sourceName, oauth2.Source{
Provider: "openidConnect",
ClientID: "test-client-id",
ClientSecret: "test-client-secret",
OpenIDConnectAutoDiscoveryURL: srv.URL + "/.well-known/openid-configuration",
})
authSource, err := auth_model.GetActiveOAuth2SourceByAuthName(t.Context(), sourceName)
require.NoError(t, err)
correctUser := &user_model.User{Name: userName, Email: email}
require.NoError(t, user_model.CreateUser(t.Context(), correctUser, &user_model.Meta{}))
return authSource, correctUser
}
// assertRelinked signs in via OIDC and asserts the stale link now points at the correct individual user.
assertRelinked := func(t *testing.T, authSource *auth_model.Source, sub string, correctUser *user_model.User) {
t.Helper()
doOIDCSignIn(t, authSource.Name)
// external_login_user has no "id" column, so order by the primary key instead
externalLink := unittest.AssertExistsAndLoadBean(t, &user_model.ExternalLoginUser{ExternalID: sub, LoginSourceID: authSource.ID}, unittest.OrderBy("external_id ASC"))
assert.Equal(t, correctUser.ID, externalLink.UserID)
assert.Equal(t, correctUser.Email, externalLink.Email)
assert.Equal(t, "OIDC Test User", externalLink.Name)
}
t.Run("organization", func(t *testing.T) {
const sub, userName, email = "oidc-stale-org-link-sub", "guizar_m", "guizar_m@example.com"
authSource, correctUser := setup(t, "test-oidc-stale-org-link", sub, userName, email)
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3, Type: user_model.UserTypeOrganization})
require.NoError(t, user_model.LinkExternalToUser(t.Context(), org, &user_model.ExternalLoginUser{
ExternalID: sub,
UserID: org.ID,
LoginSourceID: authSource.ID,
Provider: authSource.Name,
}))
assertRelinked(t, authSource, sub, correctUser)
})
t.Run("deleted user", func(t *testing.T) {
const sub, userName, email = "oidc-stale-deleted-link-sub", "guizar_d", "guizar_d@example.com"
const deletedUserID = 999999
authSource, correctUser := setup(t, "test-oidc-stale-deleted", sub, userName, email)
// link the external account to a user id that does not exist, simulating a deleted user
require.NoError(t, user_model.LinkExternalToUser(t.Context(), &user_model.User{ID: deletedUserID}, &user_model.ExternalLoginUser{
ExternalID: sub,
UserID: deletedUserID,
LoginSourceID: authSource.ID,
Provider: authSource.Name,
}))
assertRelinked(t, authSource, sub, correctUser)
})
}
// 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 {
return newFakeOIDCServerWithProfile(t, sub, oid, sub+"@example.com", "OIDC Test User")
}
func newFakeOIDCServerWithProfile(t *testing.T, sub, oid, email, name string) *httptest.Server {
t.Helper()
var srv *httptest.Server
@ -169,8 +234,8 @@ func newFakeOIDCServer(t *testing.T, sub, oid string) *httptest.Server {
// 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",
"email": email,
"name": name,
})
default:
http.NotFound(w, r)