0
0
mirror of https://github.com/go-gitea/gitea.git synced 2026-06-08 10:45:25 +02:00

fix(auth): do not auto-reactivate disabled users on OAuth2 callback (#38009)

The OAuth2 sign-in callback unconditionally set IsActive=true on the
local user row whenever the IdP authenticated them, silently undoing an
administrator's "Disable Account" action and granting the user a fresh
session in the same response. Treat the local IsActive flag as an
authoritative admin override: inactive users get a session and are
routed through the existing activate / prohibit-login pages by
verifyAuthWithOptions, matching the local-credentials sign-in path.

Adds an integration regression test that disables a linked local user
and asserts the row stays IsActive=false after a full OIDC callback.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
bircni 2026-06-07 00:07:47 +02:00 committed by GitHub
parent 42513398c0
commit c43eb7c33a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 71 additions and 5 deletions

View File

@ -47,7 +47,7 @@ func OrderBy(orderBy string) any {
}
func whereOrderConditions(e db.Engine, conditions []any) db.Engine {
orderBy := "id" // query must have the "ORDER BY", otherwise the result is not deterministic
orderBy := "id" // query must have the "ORDER BY", otherwise the result is not deterministic. FIXME: some tables do not have "id" column
for _, condition := range conditions {
switch cond := condition.(type) {
case *testCond:

View File

@ -364,9 +364,21 @@ func handleOAuth2SignIn(ctx *context.Context, authSource *auth.Source, u *user_m
opts := &user_service.UpdateOptions{}
// Reactivate user if they are deactivated
// HINT: OAUTH-AUTO-SYNC-USER-ACTIVATION: see services/auth/source/oauth2/source_sync.go
// Reactivate user only if they were disabled by the OAuth2 auto sync cron (invalid_grant),
// which clears AccessToken/RefreshToken/ExpiresAt on the ExternalLoginUser row
// An admin-disabled user has no such signature, so we leave IsActive alone
// and let verifyAuthWithOptions route them through the prohibit-login / activate page.
if !u.IsActive {
opts.IsActive = optional.Some(true)
extLogin, hasExt, err := user_model.GetExternalLogin(ctx, authSource.ID, gothUser.UserID)
if err != nil {
ctx.ServerError("GetExternalLogin", err)
return
}
isDisabledByAutoSync := hasExt && extLogin.RefreshToken == ""
if isDisabledByAutoSync {
opts.IsActive = optional.Some(true)
}
}
if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval {

View File

@ -88,8 +88,8 @@ func (source *Source) refresh(ctx context.Context, provider goth.Provider, u *us
}
}
// Delete stored tokens, since they are invalid. This
// also provents us from checking this in subsequent runs.
// HINT: OAUTH-AUTO-SYNC-USER-ACTIVATION
// Delete stored tokens, since they are invalid. This also prevents us from checking this in subsequent runs.
u.AccessToken = ""
u.RefreshToken = ""
u.ExpiresAt = time.Time{}

View File

@ -13,6 +13,7 @@ import (
"time"
auth_model "gitea.dev/models/auth"
"gitea.dev/models/db"
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
"gitea.dev/modules/json"
@ -23,6 +24,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"xorm.io/builder"
)
// TestMigrateAzureADV2ToOIDC simulates a login source migration from the Azure AD V2 OAuth2 provider to the OpenID Connect provider,
@ -191,6 +193,58 @@ func TestOIDCIgnoresStaleExternalLoginLinks(t *testing.T) {
})
}
// TestOAuth2CallbackReactivationGating exercises the gate in handleOAuth2SignIn:
// an inactive user can only be reactivated when who was disabled by auto-sync
func TestOAuth2CallbackReactivationGating(t *testing.T) {
defer tests.PrepareTestEnv(t)()
defer test.MockVariableValue(&setting.OAuth2Client.EnableAutoRegistration, true)()
defer test.MockVariableValue(&setting.OAuth2Client.Username, setting.OAuth2UsernameUserid)()
srv := newFakeOIDCServer(t, FakeOIDCConfig{Sub: "test-sub", Email: "test@example.com", Name: "Test User"})
addOAuth2Source(t, "test-oauth-source", 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(), "test-oauth-source")
require.NoError(t, err)
u := &user_model.User{Name: "test-user", Email: "test@example.com"}
require.NoError(t, user_model.CreateUser(t.Context(), u, &user_model.Meta{}))
extLink := &user_model.ExternalLoginUser{
UserID: u.ID,
LoginSourceID: authSource.ID,
Provider: authSource.Name,
ExternalID: "test-sub",
}
require.NoError(t, user_model.LinkExternalToUser(t.Context(), u, extLink))
prepareUserExternalLink := func(t *testing.T, refreshToken string) {
err := user_model.UpdateUserCols(t.Context(), &user_model.User{ID: u.ID, IsActive: false}, "is_active")
require.NoError(t, err)
_, err = db.GetEngine(t.Context()).Where(builder.Eq{"user_id": u.ID}).Cols("refresh_token").
Update(&user_model.ExternalLoginUser{RefreshToken: refreshToken})
require.NoError(t, err)
require.False(t, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: u.ID}).IsActive)
}
t.Run("admin-disabled user is not reactivated", func(t *testing.T) {
prepareUserExternalLink(t, "non-empty-refresh-token")
doOIDCSignIn(t, authSource.Name)
after := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: u.ID})
assert.False(t, after.IsActive, "OAuth callback must not re-enable an administrator-disabled account")
})
t.Run("auto-sync-disabled user is reactivated", func(t *testing.T) {
prepareUserExternalLink(t, "" /* empty refresh token */)
doOIDCSignIn(t, authSource.Name)
after := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: u.ID})
assert.True(t, after.IsActive, "OAuth callback must reactivate a sync-disabled account on successful login")
})
}
// FakeOIDCConfig holds configuration for the fake OIDC server used in tests.
type FakeOIDCConfig struct {
Sub string