diff --git a/models/auth/source.go b/models/auth/source.go index 8f7c2a89db..d03d4975dc 100644 --- a/models/auth/source.go +++ b/models/auth/source.go @@ -216,7 +216,7 @@ func CreateSource(ctx context.Context, source *Source) error { return ErrSourceAlreadyExist{source.Name} } // Synchronization is only available with LDAP for now - if !source.IsLDAP() && !source.IsOAuth2() { + if !source.IsLDAP() { source.IsSyncEnabled = false } diff --git a/models/user/external_login_user.go b/models/user/external_login_user.go index 0e764efb9f..965b7a5ed1 100644 --- a/models/user/external_login_user.go +++ b/models/user/external_login_user.go @@ -160,34 +160,12 @@ func UpdateExternalUserByExternalID(ctx context.Context, external *ExternalLogin return err } -// EnsureLinkExternalToUser link the external user to the user -func EnsureLinkExternalToUser(ctx context.Context, external *ExternalLoginUser) error { - has, err := db.Exist[ExternalLoginUser](ctx, builder.Eq{ - "external_id": external.ExternalID, - "login_source_id": external.LoginSourceID, - }) - if err != nil { - return err - } - - if has { - _, err = db.GetEngine(ctx).Where("external_id=? AND login_source_id=?", external.ExternalID, external.LoginSourceID).AllCols().Update(external) - return err - } - - _, err = db.GetEngine(ctx).Insert(external) - return err -} - // FindExternalUserOptions represents an options to find external users type FindExternalUserOptions struct { db.ListOptions - Provider string - UserID int64 - LoginSourceID int64 - HasRefreshToken bool - Expired bool - OrderBy string + Provider string + UserID int64 + OrderBy string } func (opts FindExternalUserOptions) ToConds() builder.Cond { @@ -198,22 +176,9 @@ func (opts FindExternalUserOptions) ToConds() builder.Cond { if opts.UserID > 0 { cond = cond.And(builder.Eq{"user_id": opts.UserID}) } - if opts.Expired { - cond = cond.And(builder.Lt{"expires_at": time.Now()}) - } - if opts.HasRefreshToken { - cond = cond.And(builder.Neq{"refresh_token": ""}) - } - if opts.LoginSourceID != 0 { - cond = cond.And(builder.Eq{"login_source_id": opts.LoginSourceID}) - } return cond } func (opts FindExternalUserOptions) ToOrders() string { return opts.OrderBy } - -func IterateExternalLogin(ctx context.Context, opts FindExternalUserOptions, f func(ctx context.Context, u *ExternalLoginUser) error) error { - return db.Iterate(ctx, opts.ToConds(), f) -} diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 71d7b8ca11..ccab47a9a2 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -599,8 +599,10 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. notify_service.NewUserSignUp(ctx, u) // update external user information if gothUser != nil { - if err := externalaccount.EnsureLinkExternalToUser(ctx, u, *gothUser); err != nil { - log.Error("EnsureLinkExternalToUser failed: %v", err) + if err := externalaccount.UpdateExternalUser(ctx, u, *gothUser); err != nil { + if !errors.Is(err, util.ErrNotExist) { + log.Error("UpdateExternalUser failed: %v", err) + } } } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index e329729dcd..fbdd47479a 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1205,39 +1205,9 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model groups := getClaimedGroups(oauth2Source, &gothUser) - opts := &user_service.UpdateOptions{} - - // Reactivate user if they are deactivated - if !u.IsActive { - opts.IsActive = optional.Some(true) - } - - // Update GroupClaims - opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) - - if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval { - if err := source_service.SyncGroupsToTeams(ctx, u, groups, groupTeamMapping, oauth2Source.GroupTeamMapRemoval); err != nil { - ctx.ServerError("SyncGroupsToTeams", err) - return - } - } - - if err := externalaccount.EnsureLinkExternalToUser(ctx, u, gothUser); err != nil { - ctx.ServerError("EnsureLinkExternalToUser", err) - return - } - // If this user is enrolled in 2FA and this source doesn't override it, // we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page. if !needs2FA { - // Register last login - opts.SetLastLogin = true - - if err := user_service.UpdateUser(ctx, u, opts); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - if err := updateSession(ctx, nil, map[string]any{ "uid": u.ID, }); err != nil { @@ -1248,6 +1218,29 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model // Clear whatever CSRF cookie has right now, force to generate a new one ctx.Csrf.DeleteCookie(ctx) + opts := &user_service.UpdateOptions{ + SetLastLogin: true, + } + opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) + if err := user_service.UpdateUser(ctx, u, opts); err != nil { + ctx.ServerError("UpdateUser", err) + return + } + + if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval { + if err := source_service.SyncGroupsToTeams(ctx, u, groups, groupTeamMapping, oauth2Source.GroupTeamMapRemoval); err != nil { + ctx.ServerError("SyncGroupsToTeams", err) + return + } + } + + // update external user information + if err := externalaccount.UpdateExternalUser(ctx, u, gothUser); err != nil { + if !errors.Is(err, util.ErrNotExist) { + log.Error("UpdateExternalUser failed: %v", err) + } + } + if err := resetLocale(ctx, u); err != nil { ctx.ServerError("resetLocale", err) return @@ -1263,13 +1256,22 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model return } - if opts.IsActive.Has() || opts.IsAdmin.Has() || opts.IsRestricted.Has() { + opts := &user_service.UpdateOptions{} + opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) + if opts.IsAdmin.Has() || opts.IsRestricted.Has() { if err := user_service.UpdateUser(ctx, u, opts); err != nil { ctx.ServerError("UpdateUser", err) return } } + if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval { + if err := source_service.SyncGroupsToTeams(ctx, u, groups, groupTeamMapping, oauth2Source.GroupTeamMapRemoval); err != nil { + ctx.ServerError("SyncGroupsToTeams", err) + return + } + } + if err := updateSession(ctx, nil, map[string]any{ // User needs to use 2FA, save data and redirect to 2FA page. "twofaUid": u.ID, diff --git a/services/auth/source/oauth2/main_test.go b/services/auth/source/oauth2/main_test.go deleted file mode 100644 index 57c74fd3e7..0000000000 --- a/services/auth/source/oauth2/main_test.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package oauth2 - -import ( - "testing" - - "code.gitea.io/gitea/models/unittest" -) - -func TestMain(m *testing.M) { - unittest.MainTest(m, &unittest.TestOptions{}) -} diff --git a/services/auth/source/oauth2/providers_test.go b/services/auth/source/oauth2/providers_test.go deleted file mode 100644 index 353816c71e..0000000000 --- a/services/auth/source/oauth2/providers_test.go +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package oauth2 - -import ( - "time" - - "github.com/markbates/goth" - "golang.org/x/oauth2" -) - -type fakeProvider struct{} - -func (p *fakeProvider) Name() string { - return "fake" -} - -func (p *fakeProvider) SetName(name string) {} - -func (p *fakeProvider) BeginAuth(state string) (goth.Session, error) { - return nil, nil -} - -func (p *fakeProvider) UnmarshalSession(string) (goth.Session, error) { - return nil, nil -} - -func (p *fakeProvider) FetchUser(goth.Session) (goth.User, error) { - return goth.User{}, nil -} - -func (p *fakeProvider) Debug(bool) { -} - -func (p *fakeProvider) RefreshToken(refreshToken string) (*oauth2.Token, error) { - switch refreshToken { - case "expired": - return nil, &oauth2.RetrieveError{ - ErrorCode: "invalid_grant", - } - default: - return &oauth2.Token{ - AccessToken: "token", - TokenType: "Bearer", - RefreshToken: "refresh", - Expiry: time.Now().Add(time.Hour), - }, nil - } -} - -func (p *fakeProvider) RefreshTokenAvailable() bool { - return true -} - -func init() { - RegisterGothProvider( - NewSimpleProvider("fake", "Fake", []string{"account"}, - func(clientKey, secret, callbackURL string, scopes ...string) goth.Provider { - return &fakeProvider{} - })) -} diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 3454c9ad55..675005e55a 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -36,7 +36,7 @@ func (source *Source) FromDB(bs []byte) error { return json.UnmarshalHandleDoubleEncode(bs, &source) } -// ToDB exports an OAuth2Config to a serialized format. +// ToDB exports an SMTPConfig to a serialized format. func (source *Source) ToDB() ([]byte, error) { return json.Marshal(source) } diff --git a/services/auth/source/oauth2/source_sync.go b/services/auth/source/oauth2/source_sync.go deleted file mode 100644 index 5e30313c8f..0000000000 --- a/services/auth/source/oauth2/source_sync.go +++ /dev/null @@ -1,114 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package oauth2 - -import ( - "context" - "time" - - "code.gitea.io/gitea/models/auth" - "code.gitea.io/gitea/models/db" - user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/log" - - "github.com/markbates/goth" - "golang.org/x/oauth2" -) - -// Sync causes this OAuth2 source to synchronize its users with the db. -func (source *Source) Sync(ctx context.Context, updateExisting bool) error { - log.Trace("Doing: SyncExternalUsers[%s] %d", source.authSource.Name, source.authSource.ID) - - if !updateExisting { - log.Info("SyncExternalUsers[%s] not running since updateExisting is false", source.authSource.Name) - return nil - } - - provider, err := createProvider(source.authSource.Name, source) - if err != nil { - return err - } - - if !provider.RefreshTokenAvailable() { - log.Trace("SyncExternalUsers[%s] provider doesn't support refresh tokens, can't synchronize", source.authSource.Name) - return nil - } - - opts := user_model.FindExternalUserOptions{ - HasRefreshToken: true, - Expired: true, - LoginSourceID: source.authSource.ID, - } - - return user_model.IterateExternalLogin(ctx, opts, func(ctx context.Context, u *user_model.ExternalLoginUser) error { - return source.refresh(ctx, provider, u) - }) -} - -func (source *Source) refresh(ctx context.Context, provider goth.Provider, u *user_model.ExternalLoginUser) error { - log.Trace("Syncing login_source_id=%d external_id=%s expiration=%s", u.LoginSourceID, u.ExternalID, u.ExpiresAt) - - shouldDisable := false - - token, err := provider.RefreshToken(u.RefreshToken) - if err != nil { - if err, ok := err.(*oauth2.RetrieveError); ok && err.ErrorCode == "invalid_grant" { - // this signals that the token is not valid and the user should be disabled - shouldDisable = true - } else { - return err - } - } - - user := &user_model.User{ - LoginName: u.ExternalID, - LoginType: auth.OAuth2, - LoginSource: u.LoginSourceID, - } - - hasUser, err := user_model.GetUser(ctx, user) - if err != nil { - return err - } - - // If the grant is no longer valid, disable the user and - // delete local tokens. If the OAuth2 provider still - // recognizes them as a valid user, they will be able to login - // via their provider and reactivate their account. - if shouldDisable { - log.Info("SyncExternalUsers[%s] disabling user %d", source.authSource.Name, user.ID) - - return db.WithTx(ctx, func(ctx context.Context) error { - if hasUser { - user.IsActive = false - err := user_model.UpdateUserCols(ctx, user, "is_active") - if err != nil { - return err - } - } - - // Delete stored tokens, since they are invalid. This - // also provents us from checking this in subsequent runs. - u.AccessToken = "" - u.RefreshToken = "" - u.ExpiresAt = time.Time{} - - return user_model.UpdateExternalUserByExternalID(ctx, u) - }) - } - - // Otherwise, update the tokens - u.AccessToken = token.AccessToken - u.ExpiresAt = token.Expiry - - // Some providers only update access tokens provide a new - // refresh token, so avoid updating it if it's empty - if token.RefreshToken != "" { - u.RefreshToken = token.RefreshToken - } - - err = user_model.UpdateExternalUserByExternalID(ctx, u) - - return err -} diff --git a/services/auth/source/oauth2/source_sync_test.go b/services/auth/source/oauth2/source_sync_test.go deleted file mode 100644 index 746df82055..0000000000 --- a/services/auth/source/oauth2/source_sync_test.go +++ /dev/null @@ -1,101 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package oauth2 - -import ( - "context" - "testing" - - "code.gitea.io/gitea/models/auth" - "code.gitea.io/gitea/models/unittest" - user_model "code.gitea.io/gitea/models/user" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestSource(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) - - source := &Source{ - Provider: "fake", - authSource: &auth.Source{ - ID: 12, - Type: auth.OAuth2, - Name: "fake", - IsActive: true, - IsSyncEnabled: true, - }, - } - - user := &user_model.User{ - LoginName: "external", - LoginType: auth.OAuth2, - LoginSource: source.authSource.ID, - Name: "test", - Email: "external@example.com", - } - - err := user_model.CreateUser(context.Background(), user, &user_model.CreateUserOverwriteOptions{}) - require.NoError(t, err) - - e := &user_model.ExternalLoginUser{ - ExternalID: "external", - UserID: user.ID, - LoginSourceID: user.LoginSource, - RefreshToken: "valid", - } - err = user_model.LinkExternalToUser(context.Background(), user, e) - require.NoError(t, err) - - provider, err := createProvider(source.authSource.Name, source) - require.NoError(t, err) - - t.Run("refresh", func(t *testing.T) { - t.Run("valid", func(t *testing.T) { - err := source.refresh(context.Background(), provider, e) - require.NoError(t, err) - - e := &user_model.ExternalLoginUser{ - ExternalID: e.ExternalID, - LoginSourceID: e.LoginSourceID, - } - - ok, err := user_model.GetExternalLogin(context.Background(), e) - require.NoError(t, err) - assert.True(t, ok) - assert.Equal(t, "refresh", e.RefreshToken) - assert.Equal(t, "token", e.AccessToken) - - u, err := user_model.GetUserByID(context.Background(), user.ID) - require.NoError(t, err) - assert.True(t, u.IsActive) - }) - - t.Run("expired", func(t *testing.T) { - err := source.refresh(context.Background(), provider, &user_model.ExternalLoginUser{ - ExternalID: "external", - UserID: user.ID, - LoginSourceID: user.LoginSource, - RefreshToken: "expired", - }) - require.NoError(t, err) - - e := &user_model.ExternalLoginUser{ - ExternalID: e.ExternalID, - LoginSourceID: e.LoginSourceID, - } - - ok, err := user_model.GetExternalLogin(context.Background(), e) - require.NoError(t, err) - assert.True(t, ok) - assert.Equal(t, "", e.RefreshToken) - assert.Equal(t, "", e.AccessToken) - - u, err := user_model.GetUserByID(context.Background(), user.ID) - require.NoError(t, err) - assert.False(t, u.IsActive) - }) - }) -} diff --git a/services/externalaccount/user.go b/services/externalaccount/user.go index b53e33654a..3cfd8c81f9 100644 --- a/services/externalaccount/user.go +++ b/services/externalaccount/user.go @@ -71,14 +71,14 @@ func LinkAccountToUser(ctx context.Context, user *user_model.User, gothUser goth return nil } -// EnsureLinkExternalToUser link the gothUser to the user -func EnsureLinkExternalToUser(ctx context.Context, user *user_model.User, gothUser goth.User) error { +// UpdateExternalUser updates external user's information +func UpdateExternalUser(ctx context.Context, user *user_model.User, gothUser goth.User) error { externalLoginUser, err := toExternalLoginUser(ctx, user, gothUser) if err != nil { return err } - return user_model.EnsureLinkExternalToUser(ctx, externalLoginUser) + return user_model.UpdateExternalUserByExternalID(ctx, externalLoginUser) } // UpdateMigrationsByType updates all migrated repositories' posterid from gitServiceType to replace originalAuthorID to posterID diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index a8b2049f92..8a8bd61148 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -416,7 +416,7 @@

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

{{end}} - {{if (or .Source.IsLDAP .Source.IsOAuth2)}} + {{if .Source.IsLDAP}}
diff --git a/templates/admin/auth/new.tmpl b/templates/admin/auth/new.tmpl index 47fa82825c..c70bd3ba43 100644 --- a/templates/admin/auth/new.tmpl +++ b/templates/admin/auth/new.tmpl @@ -59,7 +59,7 @@
-
+
diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 04c8a4b614..9bcb532d75 100644 --- a/tests/integration/auth_ldap_test.go +++ b/tests/integration/auth_ldap_test.go @@ -245,8 +245,7 @@ func TestLDAPUserSync(t *testing.T) { } defer tests.PrepareTestEnv(t)() addAuthSourceLDAP(t, "", "", "", "") - err := auth.SyncExternalUsers(context.Background(), true) - require.NoError(t, err) + auth.SyncExternalUsers(context.Background(), true) // Check if users exists for _, gitLDAPUser := range gitLDAPUsers {