Fix various permission & login related bugs (#36002)

Permission & protection check:

- Fix Delete Release permission check
- Fix Update Pull Request with rebase branch protection check
- Fix Issue Dependency permission check
- Fix Delete Comment History ID check

Information leaking:

- Show unified message for non-existing user and invalid password
    - Fix #35984
- Don't expose release draft to non-writer users.
- Make API returns signature's email address instead of the user
profile's.

Auth & Login:

- Avoid GCM OAuth2 attempt when OAuth2 is disabled
    - Fix #35510

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

services/auth/auth.go

@@ -5,6 +5,7 @@
 package auth
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"regexp"
@@ -40,6 +41,20 @@ var globalVars = sync.OnceValue(func() *globalVarsStruct {
 	}
 })
 
+type ErrUserAuthMessage string
+
+func (e ErrUserAuthMessage) Error() string {
+	return string(e)
+}
+
+func ErrAsUserAuthMessage(err error) (string, bool) {
+	var msg ErrUserAuthMessage
+	if errors.As(err, &msg) {
+		return msg.Error(), true
+	}
+	return "", false
+}
+
 // Init should be called exactly once when the application starts to allow plugins
 // to allocate necessary resources
 func Init() {

services/auth/basic.go

@@ -5,7 +5,6 @@
 package auth
 
 import (
-	"errors"
 	"net/http"
 
 	actions_model "code.gitea.io/gitea/models/actions"
@@ -146,7 +145,7 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
 			return nil, err
 		}
 		if hasWebAuthn {
-			return nil, errors.New("basic authorization is not allowed while WebAuthn enrolled")
+			return nil, ErrUserAuthMessage("basic authorization is not allowed while WebAuthn enrolled")
 		}
 
 		if err := validateTOTP(req, u); err != nil {

services/convert/convert.go

@@ -542,8 +542,9 @@ func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerifi
 	}
 	if verif.SigningUser != nil {
 		commitVerification.Signer = &api.PayloadUser{
-			Name:  verif.SigningUser.Name,
-			Email: verif.SigningUser.Email,
+			UserName: verif.SigningUser.Name,
+			Name:     verif.SigningUser.DisplayName(),
+			Email:    verif.SigningEmail, // Use the email from the signature, not from the user profile
 		}
 	}
 	return commitVerification

services/pull/merge.go

@@ -547,11 +547,15 @@ var escapedSymbols = regexp.MustCompile(`([*[?! \\])`)
 
 // IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections
 func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p access_model.Permission, user *user_model.User) (bool, error) {
+	return isUserAllowedToMergeInRepoBranch(ctx, pr.BaseRepoID, pr.BaseBranch, p, user)
+}
+
+func isUserAllowedToMergeInRepoBranch(ctx context.Context, repoID int64, branch string, p access_model.Permission, user *user_model.User) (bool, error) {
 	if user == nil {
 		return false, nil
 	}
 
-	pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
+	pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repoID, branch)
 	if err != nil {
 		return false, err
 	}

services/pull/update.go

@@ -101,11 +101,11 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
 }
 
 // IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections
+// update PR means send new commits to PR head branch from base branch
 func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, user *user_model.User) (mergeAllowed, rebaseAllowed bool, err error) {
 	if pull.Flow == issues_model.PullRequestFlowAGit {
 		return false, false, nil
 	}
-
 	if user == nil {
 		return false, false, nil
 	}
@@ -121,54 +121,46 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest,
 		return false, false, err
 	}
 
-	pr := &issues_model.PullRequest{
-		HeadRepoID: pull.BaseRepoID,
-		HeadRepo:   pull.BaseRepo,
-		BaseRepoID: pull.HeadRepoID,
-		BaseRepo:   pull.HeadRepo,
-		HeadBranch: pull.BaseBranch,
-		BaseBranch: pull.HeadBranch,
-	}
-
-	pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
-	if err != nil {
-		return false, false, err
-	}
-
-	if err := pr.LoadBaseRepo(ctx); err != nil {
-		return false, false, err
-	}
-	prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
-	if err != nil {
+	// 1. check base repository's AllowRebaseUpdate configuration
+	// it is a config in base repo but controls the head (fork) repo's "Update" behavior
+	{
+		prBaseUnit, err := pull.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
 		if repo_model.IsErrUnitTypeNotExist(err) {
-			return false, false, nil
+			return false, false, nil // the PR unit is disabled in base repo
+		} else if err != nil {
+			return false, false, fmt.Errorf("get base repo unit: %v", err)
 		}
-		log.Error("pr.BaseRepo.GetUnit(unit.TypePullRequests): %v", err)
-		return false, false, err
+		rebaseAllowed = prBaseUnit.PullRequestsConfig().AllowRebaseUpdate
 	}
 
-	rebaseAllowed = prUnit.PullRequestsConfig().AllowRebaseUpdate
-
-	// If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push)
-	if pb != nil {
-		pb.Repo = pull.BaseRepo
-		if !pb.CanUserForcePush(ctx, user) {
-			rebaseAllowed = false
+	// 2. check head branch protection whether rebase is allowed, if pb not found then rebase depends on the above setting
+	{
+		pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.HeadRepoID, pull.HeadBranch)
+		if err != nil {
+			return false, false, err
+		}
+		// If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push)
+		if pb != nil {
+			pb.Repo = pull.HeadRepo
+			rebaseAllowed = rebaseAllowed && pb.CanUserForcePush(ctx, user)
 		}
 	}
 
+	// 3. check whether user has write access to head branch
 	baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, user)
 	if err != nil {
 		return false, false, err
 	}
 
-	mergeAllowed, err = IsUserAllowedToMerge(ctx, pr, headRepoPerm, user)
+	mergeAllowed, err = isUserAllowedToMergeInRepoBranch(ctx, pull.HeadRepoID, pull.HeadBranch, headRepoPerm, user)
 	if err != nil {
 		return false, false, err
 	}
 
+	// 4. if the pull creator allows maintainer to edit, it means the write permissions of the head branch has been
+	// granted to the user with write permission of the base repository
 	if pull.AllowMaintainerEdit {
-		mergeAllowedMaintainer, err := IsUserAllowedToMerge(ctx, pr, baseRepoPerm, user)
+		mergeAllowedMaintainer, err := isUserAllowedToMergeInRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user)
 		if err != nil {
 			return false, false, err
 		}
@@ -176,6 +168,9 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest,
 		mergeAllowed = mergeAllowed || mergeAllowedMaintainer
 	}
 
+	// if merge is not allowed, rebase is also not allowed
+	rebaseAllowed = rebaseAllowed && mergeAllowed
+
 	return mergeAllowed, rebaseAllowed, nil
 }
 

services/release/release.go

@@ -361,7 +361,7 @@ func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *re
 		if err != nil {
 			return fmt.Errorf("GetProtectedTags: %w", err)
 		}
-		isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, protectedTags, rel.TagName, rel.PublisherID)
+		isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, protectedTags, rel.TagName, doer.ID)
 		if err != nil {
 			return err
 		}