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 {