Merge branch 'main' into copilot/refactor-apierror-function

cmd/serv.go

@@ -113,23 +113,25 @@ func handleCliResponseExtra(extra private.ResponseExtra) error {
 	return nil
 }
 
-func getAccessMode(verb, lfsVerb string) perm.AccessMode {
+// getAccessMode maps an SSH git/LFS verb to the access mode it requires, with
+// ok=false for an unrecognised verb. Callers MUST reject the request when ok is
+// false: AccessModeNone would otherwise pass the `userMode < mode` permission
+// check in routers/private/serv.go and grant access.
+func getAccessMode(verb, lfsVerb string) (mode perm.AccessMode, ok bool) {
 	switch verb {
 	case git.CmdVerbUploadPack, git.CmdVerbUploadArchive:
-		return perm.AccessModeRead
+		return perm.AccessModeRead, true
 	case git.CmdVerbReceivePack:
-		return perm.AccessModeWrite
+		return perm.AccessModeWrite, true
 	case git.CmdVerbLfsAuthenticate, git.CmdVerbLfsTransfer:
 		switch lfsVerb {
 		case git.CmdSubVerbLfsUpload:
-			return perm.AccessModeWrite
+			return perm.AccessModeWrite, true
 		case git.CmdSubVerbLfsDownload:
-			return perm.AccessModeRead
+			return perm.AccessModeRead, true
 		}
 	}
-	// should be unreachable
-	setting.PanicInDevOrTesting("unknown verb: %s %s", verb, lfsVerb)
-	return perm.AccessModeNone
+	return perm.AccessModeNone, false
 }
 
 func runServ(ctx context.Context, c *cli.Command) error {
@@ -247,7 +249,10 @@ func runServ(ctx context.Context, c *cli.Command) error {
 		}
 	}
 
-	requestedMode := getAccessMode(verb, lfsVerb)
+	requestedMode, ok := getAccessMode(verb, lfsVerb)
+	if !ok {
+		return fail(ctx, "Unknown git command", "Unknown git command %s %s", verb, lfsVerb)
+	}
 
 	results, extra := private.ServCommand(ctx, keyID, username, reponame, requestedMode, verb, lfsVerb)
 	if extra.HasError() {

cmd/serv_test.go

@@ -0,0 +1,56 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package cmd
+
+import (
+	"testing"
+
+	"gitea.dev/models/perm"
+	"gitea.dev/modules/git"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetAccessMode(t *testing.T) {
+	cases := []struct {
+		verb, lfsVerb string
+		expected      perm.AccessMode
+	}{
+		{git.CmdVerbUploadPack, "", perm.AccessModeRead},
+		{git.CmdVerbUploadArchive, "", perm.AccessModeRead},
+		{git.CmdVerbReceivePack, "", perm.AccessModeWrite},
+		{git.CmdVerbLfsAuthenticate, git.CmdSubVerbLfsUpload, perm.AccessModeWrite},
+		{git.CmdVerbLfsAuthenticate, git.CmdSubVerbLfsDownload, perm.AccessModeRead},
+		{git.CmdVerbLfsTransfer, git.CmdSubVerbLfsUpload, perm.AccessModeWrite},
+		{git.CmdVerbLfsTransfer, git.CmdSubVerbLfsDownload, perm.AccessModeRead},
+	}
+	for _, tc := range cases {
+		t.Run(tc.verb+"/"+tc.lfsVerb, func(t *testing.T) {
+			mode, ok := getAccessMode(tc.verb, tc.lfsVerb)
+			assert.True(t, ok)
+			assert.Equal(t, tc.expected, mode)
+		})
+	}
+}
+
+// TestGetAccessModeUnknownVerb locks in the invariant that getAccessMode reports
+// ok=false for unrecognised verbs and LFS sub-verbs, so runServ rejects them. An
+// unknown verb has no valid access mode; if it were treated as AccessModeNone (0)
+// it would pass the `userMode < mode` permission check in routers/private/serv.go
+// and hand out valid LFS JWTs for any private repository.
+func TestGetAccessModeUnknownVerb(t *testing.T) {
+	cases := []struct{ verb, lfsVerb string }{
+		{git.CmdVerbLfsAuthenticate, ""},
+		{git.CmdVerbLfsAuthenticate, "badverb"},
+		{git.CmdVerbLfsTransfer, "badverb"},
+		{"git-unknown-verb", ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.verb+"/"+tc.lfsVerb, func(t *testing.T) {
+			mode, ok := getAccessMode(tc.verb, tc.lfsVerb)
+			assert.False(t, ok)
+			assert.Equal(t, perm.AccessModeNone, mode)
+		})
+	}
+}

models/unittest/unit_tests.go

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

routers/web/auth/oauth.go

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

services/auth/source/oauth2/source_sync.go

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

tests/integration/auth_oauth2_test.go

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