foster/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2024-12-21 12:44:49 -05:00
Code Issues Wiki Activity

fix: Revert "allow synchronizing user status from OAuth2 login providers (#31572)"

Browse source 
This commit has a fundamental flaw, in order to syncronize if external
users are still active the commit checks if the refresh token is
accepted by the OAuth provider, if that is not the case it sees that as
the user is disabled and sets the is active field to `false` to signal
that. Because it might be possible (this commit makes this a highly
likelyhood) that the OAuth provider still recognizes this user the
commit introduces code to allow users to re-active themselves via the
oauth flow if they were disabled because of this. However this code
makes no distinction in why the user was disabled and always re-actives
the user.

Thus the reactivation via the OAuth flow allows users to bypass the
manually activation setting (`[service].REGISTER_MANUAL_CONFIRM`) or if
the admin for other reasons disabled the user.

This reverts commit 21fdd28f08.
This commit is contained in:
Gusted 2024-12-09 06:58:58 +01:00 committed by Earl Warren
parent 80179a373d
commit 7f8f9b878f
No known key found for this signature in database
GPG key ID: 0579CB2928A78A00
13 changed files with 48 additions and 371 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
models/auth/source.go
View file

@ -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
}

41
models/user/external_login_user.go
View file

@ -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)
}

6
routers/web/auth/auth.go
View file

@ -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)
}
}
}

64
routers/web/auth/oauth.go
View file

@ -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,

14
services/auth/source/oauth2/main_test.go
View file

@ -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{})
}

62
services/auth/source/oauth2/providers_test.go
View file

@ -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{}
}))
}

2
services/auth/source/oauth2/source.go
View file

@ -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)
}

114
services/auth/source/oauth2/source_sync.go
View file

@ -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
}

101
services/auth/source/oauth2/source_sync_test.go
View file

@ -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)
})
})
}

6
services/externalaccount/user.go
View file

@ -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

2
templates/admin/auth/edit.tmpl
View file

@ -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>

2
templates/admin/auth/new.tmpl
View file

@ -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}}>

3
tests/integration/auth_ldap_test.go
View file

@ -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 {