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 081e97d6e6..9649dddbd1 100644
--- a/routers/web/auth/auth.go
+++ b/routers/web/auth/auth.go
@@ -590,8 +590,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 651c9fb474..8705d1b205 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 @@
<p class="help">{{ctx.Locale.Tr "admin.auths.sspi_default_language_helper"}}</p>
</div>
{{end}}
- {{if (or .Source.IsLDAP .Source.IsOAuth2)}}
+ {{if .Source.IsLDAP}}
<div class="inline field">
<div class="ui checkbox">
<label><strong>{{ctx.Locale.Tr "admin.auths.syncenabled"}}</strong></label>
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 @@
<input name="attributes_in_bind" type="checkbox" {{if .attributes_in_bind}}checked{{end}}>
</div>
</div>
- <div class="oauth2 ldap inline field {{if not (or (eq .type 2) (eq .type 6))}}tw-hidden{{end}}">
+ <div class="ldap inline field {{if not (eq .type 2)}}tw-hidden{{end}}">
<div class="ui checkbox">
<label><strong>{{ctx.Locale.Tr "admin.auths.syncenabled"}}</strong></label>
<input name="is_sync_enabled" type="checkbox" {{if .is_sync_enabled}}checked{{end}}>
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 {