Merge remote-tracking branch 'upstream/main'

* upstream/main:
  Fix edit topic UI (go-gitea#27925)
  Unify two factor check (go-gitea#27915)
  Revert go-gitea#27870 (go-gitea#27917)
  Fix JS NPE when viewing specific range of PR commits (go-gitea#27912)
  Install poetry dependencies with --no-root (go-gitea#27919)
  Show correct commit sha when viewing single commit diff (go-gitea#27916)
  Fix 500 when deleting a dismissed review (go-gitea#27903)
  Remove action runners on user deletion (go-gitea#27902)
  Remove SSH workaround (go-gitea#27893)
  Remove "tabindex" from some form buttons (go-gitea#27892)
  Refactor the function RemoveOrgUser (go-gitea#27582)
  Fix DownloadFunc when migrating releases (go-gitea#27887)

Makefile

@@ -875,7 +875,7 @@ node_modules: package-lock.json
 	@touch node_modules
 
 .venv: poetry.lock
-	poetry install
+	poetry install --no-root
 	@touch .venv
 
 .PHONY: update

go.mod

@@ -36,7 +36,7 @@ require (
 	github.com/ethantkoenig/rupture v1.0.1
 	github.com/felixge/fgprof v0.9.3
 	github.com/fsnotify/fsnotify v1.6.0
-	github.com/gliderlabs/ssh v0.3.5
+	github.com/gliderlabs/ssh v0.3.6-0.20230927171611-ece6c7995e46
 	github.com/go-ap/activitypub v0.0.0-20231003111253-1fba3772399b
 	github.com/go-ap/jsonld v0.0.0-20221030091449-f2a191312c73
 	github.com/go-chi/chi/v5 v5.0.10

go.sum

@@ -329,8 +329,8 @@ github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4
 github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw=
 github.com/fxamacker/cbor/v2 v2.5.0 h1:oHsG0V/Q6E/wqTS2O1Cozzsy69nqCiguo5Q1a1ADivE=
 github.com/fxamacker/cbor/v2 v2.5.0/go.mod h1:TA1xS00nchWmaBnEIxPSE5oHLuJBAVvqrtAnWBwBCVo=
-github.com/gliderlabs/ssh v0.3.5 h1:OcaySEmAQJgyYcArR+gGGTHCyE7nvhEMTlYY+Dp8CpY=
-github.com/gliderlabs/ssh v0.3.5/go.mod h1:8XB4KraRrX39qHhT6yxPsHedjA08I/uBVwj4xC+/+z4=
+github.com/gliderlabs/ssh v0.3.6-0.20230927171611-ece6c7995e46 h1:fYiA820jw7wmAvdXrHwMItxjJkra7dT9y8yiXhtzb94=
+github.com/gliderlabs/ssh v0.3.6-0.20230927171611-ece6c7995e46/go.mod h1:i/TCLcdiX9Up/vs+Rp8c3yMbqp2Y4Y7Nh9uzGFCa5pM=
 github.com/glycerine/go-unsnap-stream v0.0.0-20181221182339-f9677308dec2/go.mod h1:/20jfyN9Y5QPEAprSgKAUr+glWDY39ZiUEAYOEv5dsE=
 github.com/glycerine/goconvey v0.0.0-20190410193231-58a59202ab31/go.mod h1:Ogl1Tioa0aV7gstGFO7KhffUsb9M4ydbEbbxpcEDc24=
 github.com/go-ap/activitypub v0.0.0-20231003111253-1fba3772399b h1:VLD6IPBDkqEsOZ+EfLO6MayuHycZ0cv4BStTlRoZduo=
@@ -1237,7 +1237,6 @@ golang.org/x/net v0.0.0-20210916014120-12bc252f5db8/go.mod h1:9nx3DQGgdP8bBQD5qx
 golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
 golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
-golang.org/x/net v0.0.0-20220826154423-83b083e8dc8b/go.mod h1:YDH+HFinaLZZlnHAfSS6ZXJJ9M9t4Dl22yv3iI2vPwk=
 golang.org/x/net v0.2.0/go.mod h1:KqCZLdyyvdV855qA2rE3GC2aiw5xGR5TEjj8smXukLY=
 golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
 golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
@@ -1337,9 +1336,7 @@ golang.org/x/sys v0.0.0-20220310020820-b874c991c1a5/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220825204002-c680a09ffe64/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
@@ -1353,7 +1350,6 @@ golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
-golang.org/x/term v0.0.0-20220722155259-a9ba230a4035/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc=
 golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
 golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U=

models/actions/runner.go

@@ -266,3 +266,27 @@ func CreateRunner(ctx context.Context, t *ActionRunner) error {
 	_, err := db.GetEngine(ctx).Insert(t)
 	return err
 }
+
+func CountRunnersWithoutBelongingOwner(ctx context.Context) (int64, error) {
+	// Only affect action runners were a owner ID is set, as actions runners
+	// could also be created on a repository.
+	return db.GetEngine(ctx).Table("action_runner").
+		Join("LEFT", "user", "`action_runner`.owner_id = `user`.id").
+		Where("`action_runner`.owner_id != ?", 0).
+		And(builder.IsNull{"`user`.id"}).
+		Count(new(ActionRunner))
+}
+
+func FixRunnersWithoutBelongingOwner(ctx context.Context) (int64, error) {
+	subQuery := builder.Select("`action_runner`.id").
+		From("`action_runner`").
+		Join("LEFT", "user", "`action_runner`.owner_id = `user`.id").
+		Where(builder.Neq{"`action_runner`.owner_id": 0}).
+		And(builder.IsNull{"`user`.id"})
+	b := builder.Delete(builder.In("id", subQuery)).From("`action_runner`")
+	res, err := db.GetEngine(ctx).Exec(b)
+	if err != nil {
+		return 0, err
+	}
+	return res.RowsAffected()
+}

models/issues/review.go

@@ -897,6 +897,16 @@ func DeleteReview(ctx context.Context, r *Review) error {
 		return err
 	}
 
+	opts = FindCommentsOptions{
+		Type:     CommentTypeDismissReview,
+		IssueID:  r.IssueID,
+		ReviewID: r.ID,
+	}
+
+	if _, err := sess.Where(opts.ToConds()).Delete(new(Comment)); err != nil {
+		return err
+	}
+
 	if _, err := sess.ID(r.ID).Delete(new(Review)); err != nil {
 		return err
 	}

models/issues/review_test.go

@@ -8,6 +8,7 @@ import (
 
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
+	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 
@@ -258,3 +259,32 @@ func TestDeleteReview(t *testing.T) {
 	assert.NoError(t, err)
 	assert.True(t, review1.Official)
 }
+
+func TestDeleteDismissedReview(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
+	review, err := issues_model.CreateReview(db.DefaultContext, issues_model.CreateReviewOptions{
+		Content:  "reject",
+		Type:     issues_model.ReviewTypeReject,
+		Official: false,
+		Issue:    issue,
+		Reviewer: user,
+	})
+	assert.NoError(t, err)
+	assert.NoError(t, issues_model.DismissReview(db.DefaultContext, review, true))
+	comment, err := issues_model.CreateComment(db.DefaultContext, &issues_model.CreateCommentOptions{
+		Type:     issues_model.CommentTypeDismissReview,
+		Doer:     user,
+		Repo:     repo,
+		Issue:    issue,
+		ReviewID: review.ID,
+		Content:  "dismiss",
+	})
+	assert.NoError(t, err)
+	unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: comment.ID})
+	assert.NoError(t, issues_model.DeleteReview(db.DefaultContext, review))
+	unittest.AssertNotExistsBean(t, &issues_model.Comment{ID: comment.ID})
+}

models/org.go

@@ -14,12 +14,11 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 )
 
-func removeOrgUser(ctx context.Context, orgID, userID int64) error {
+// RemoveOrgUser removes user from given organization.
+func RemoveOrgUser(ctx context.Context, orgID, userID int64) error {
 	ou := new(organization.OrgUser)
 
-	sess := db.GetEngine(ctx)
-
-	has, err := sess.
+	has, err := db.GetEngine(ctx).
 		Where("uid=?", userID).
 		And("org_id=?", orgID).
 		Get(ou)
@@ -52,7 +51,13 @@ func removeOrgUser(ctx context.Context, orgID, userID int64) error {
 		}
 	}
 
-	if _, err := sess.ID(ou.ID).Delete(ou); err != nil {
+	ctx, committer, err := db.TxContext(ctx)
+	if err != nil {
+		return err
+	}
+	defer committer.Close()
+
+	if _, err := db.GetEngine(ctx).ID(ou.ID).Delete(ou); err != nil {
 		return err
 	} else if _, err = db.Exec(ctx, "UPDATE `user` SET num_members=num_members-1 WHERE id=?", orgID); err != nil {
 		return err
@@ -74,7 +79,7 @@ func removeOrgUser(ctx context.Context, orgID, userID int64) error {
 	}
 
 	if len(repoIDs) > 0 {
-		if _, err = sess.
+		if _, err = db.GetEngine(ctx).
 			Where("user_id = ?", userID).
 			In("repo_id", repoIDs).
 			Delete(new(access_model.Access)); err != nil {
@@ -93,18 +98,5 @@ func removeOrgUser(ctx context.Context, orgID, userID int64) error {
 		}
 	}
 
-	return nil
-}
-
-// RemoveOrgUser removes user from given organization.
-func RemoveOrgUser(ctx context.Context, orgID, userID int64) error {
-	ctx, committer, err := db.TxContext(ctx)
-	if err != nil {
-		return err
-	}
-	defer committer.Close()
-	if err := removeOrgUser(ctx, orgID, userID); err != nil {
-		return err
-	}
 	return committer.Commit()
 }

models/org_team.go

@@ -502,7 +502,7 @@ func removeInvalidOrgUser(ctx context.Context, userID, orgID int64) error {
 	}); err != nil {
 		return err
 	} else if count == 0 {
-		return removeOrgUser(ctx, orgID, userID)
+		return RemoveOrgUser(ctx, orgID, userID)
 	}
 	return nil
 }

modules/context/api.go

@@ -11,7 +11,6 @@ import (
 	"net/url"
 	"strings"
 
-	"code.gitea.io/gitea/models/auth"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
@@ -211,32 +210,6 @@ func (ctx *APIContext) SetLinkHeader(total, pageSize int) {
 	}
 }
 
-// CheckForOTP validates OTP
-func (ctx *APIContext) CheckForOTP() {
-	if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
-		return // Skip 2FA
-	}
-
-	otpHeader := ctx.Req.Header.Get("X-Gitea-OTP")
-	twofa, err := auth.GetTwoFactorByUID(ctx, ctx.Doer.ID)
-	if err != nil {
-		if auth.IsErrTwoFactorNotEnrolled(err) {
-			return // No 2FA enrollment for this user
-		}
-		ctx.Error(http.StatusInternalServerError, "GetTwoFactorByUID", err)
-		return
-	}
-	ok, err := twofa.ValidateTOTP(otpHeader)
-	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "ValidateTOTP", err)
-		return
-	}
-	if !ok {
-		ctx.Error(http.StatusUnauthorized, "", nil)
-		return
-	}
-}
-
 // APIContexter returns apicontext as middleware
 func APIContexter() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {

modules/doctor/dbconsistency.go

@@ -6,6 +6,7 @@ package doctor
 import (
 	"context"
 
+	actions_model "code.gitea.io/gitea/models/actions"
 	activities_model "code.gitea.io/gitea/models/activities"
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
@@ -151,6 +152,12 @@ func checkDBConsistency(ctx context.Context, logger log.Logger, autofix bool) er
 			Fixer:        activities_model.FixActionCreatedUnixString,
 			FixedMessage: "Set to zero",
 		},
+		{
+			Name:         "Action Runners without existing owner",
+			Counter:      actions_model.CountRunnersWithoutBelongingOwner,
+			Fixer:        actions_model.FixRunnersWithoutBelongingOwner,
+			FixedMessage: "Removed",
+		},
 	}
 
 	// TODO: function to recalc all counters

modules/ssh/ssh.go

@@ -17,7 +17,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"reflect"
 	"strconv"
 	"strings"
 	"sync"
@@ -165,10 +164,6 @@ func sessionHandler(session ssh.Session) {
 }
 
 func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
-	// FIXME: the "ssh.Context" is not thread-safe, so db operations should use the immutable parent "Context"
-	// TODO: Remove after https://github.com/gliderlabs/ssh/pull/211
-	parentCtx := reflect.ValueOf(ctx).Elem().FieldByName("Context").Interface().(context.Context)
-
 	if log.IsDebug() { // <- FingerprintSHA256 is kinda expensive so only calculate it if necessary
 		log.Debug("Handle Public Key: Fingerprint: %s from %s", gossh.FingerprintSHA256(key), ctx.RemoteAddr())
 	}
@@ -200,7 +195,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
 		// look for the exact principal
 	principalLoop:
 		for _, principal := range cert.ValidPrincipals {
-			pkey, err := asymkey_model.SearchPublicKeyByContentExact(parentCtx, principal)
+			pkey, err := asymkey_model.SearchPublicKeyByContentExact(ctx, principal)
 			if err != nil {
 				if asymkey_model.IsErrKeyNotExist(err) {
 					log.Debug("Principal Rejected: %s Unknown Principal: %s", ctx.RemoteAddr(), principal)
@@ -257,7 +252,7 @@ func publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
 		log.Debug("Handle Public Key: %s Fingerprint: %s is not a certificate", ctx.RemoteAddr(), gossh.FingerprintSHA256(key))
 	}
 
-	pkey, err := asymkey_model.SearchPublicKeyByContent(parentCtx, strings.TrimSpace(string(gossh.MarshalAuthorizedKey(key))))
+	pkey, err := asymkey_model.SearchPublicKeyByContent(ctx, strings.TrimSpace(string(gossh.MarshalAuthorizedKey(key))))
 	if err != nil {
 		if asymkey_model.IsErrKeyNotExist(err) {
 			log.Warn("Unknown public key: %s from %s", gossh.FingerprintSHA256(key), ctx.RemoteAddr())

routers/api/v1/api.go

@@ -316,10 +316,6 @@ func reqToken() func(ctx *context.APIContext) {
 			return
 		}
 
-		if ctx.IsBasicAuth {
-			ctx.CheckForOTP()
-			return
-		}
 		if ctx.IsSigned {
 			return
 		}
@@ -344,7 +340,6 @@ func reqBasicOrRevProxyAuth() func(ctx *context.APIContext) {
 			ctx.Error(http.StatusUnauthorized, "reqBasicAuth", "auth required")
 			return
 		}
-		ctx.CheckForOTP()
 	}
 }
 
@@ -701,12 +696,6 @@ func bind[T any](_ T) any {
 	}
 }
 
-// The OAuth2 plugin is expected to be executed first, as it must ignore the user id stored
-// in the session (if there is a user id stored in session other plugins might return the user
-// object for that id).
-//
-// The Session plugin is expected to be executed second, in order to skip authentication
-// for users that have already signed in.
 func buildAuthGroup() *auth.Group {
 	group := auth.NewGroup(
 		&auth.OAuth2{},
@@ -786,31 +775,6 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIC
 				})
 				return
 			}
-			if ctx.IsSigned && ctx.IsBasicAuth {
-				if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
-					return // Skip 2FA
-				}
-				twofa, err := auth_model.GetTwoFactorByUID(ctx, ctx.Doer.ID)
-				if err != nil {
-					if auth_model.IsErrTwoFactorNotEnrolled(err) {
-						return // No 2FA enrollment for this user
-					}
-					ctx.InternalServerError(err)
-					return
-				}
-				otpHeader := ctx.Req.Header.Get("X-Gitea-OTP")
-				ok, err := twofa.ValidateTOTP(otpHeader)
-				if err != nil {
-					ctx.InternalServerError(err)
-					return
-				}
-				if !ok {
-					ctx.JSON(http.StatusForbidden, map[string]string{
-						"message": "Only signed in user is allowed to call APIs.",
-					})
-					return
-				}
-			}
 		}
 
 		if options.AdminRequired {

routers/web/user/setting/security/security.go

@@ -84,7 +84,7 @@ func loadSecurityData(ctx *context.Context) {
 	// map the provider display name with the AuthSource
 	sources := make(map[*auth_model.Source]string)
 	for _, externalAccount := range accountLinks {
-		if authSource, err := auth_model.GetSourceByID(ctx, externalAccount.LoginSourceID); err == nil && authSource.IsActive {
+		if authSource, err := auth_model.GetSourceByID(ctx, externalAccount.LoginSourceID); err == nil {
 			var providerDisplayName string
 
 			type DisplayNamed interface {