fix: update collaborator access mode and httpsign (#38894, #38862) (#38895)

backport #38894, partially #38862

models/repo/collaboration.go

@@ -111,39 +111,6 @@ func IsCollaborator(ctx context.Context, repoID, userID int64) (bool, error) {
 	return db.Exist[Collaboration](ctx, builder.Eq{"repo_id": repoID, "user_id": userID})
 }
 
-// ChangeCollaborationAccessMode sets new access mode for the collaboration.
-func ChangeCollaborationAccessMode(ctx context.Context, repo *Repository, uid int64, mode perm.AccessMode) error {
-	// Discard invalid input
-	if mode <= perm.AccessModeNone || mode > perm.AccessModeOwner {
-		return nil
-	}
-
-	return db.WithTx(ctx, func(ctx context.Context) error {
-		collaboration, has, err := db.Get[Collaboration](ctx, builder.Eq{"repo_id": repo.ID, "user_id": uid})
-		if err != nil {
-			return fmt.Errorf("get collaboration: %w", err)
-		} else if !has {
-			return nil
-		}
-
-		if collaboration.Mode == mode {
-			return nil
-		}
-		collaboration.Mode = mode
-
-		if _, err = db.GetEngine(ctx).
-			ID(collaboration.ID).
-			Cols("mode").
-			Update(collaboration); err != nil {
-			return fmt.Errorf("update collaboration: %w", err)
-		} else if _, err = db.Exec(ctx, "UPDATE access SET mode = ? WHERE user_id = ? AND repo_id = ?", mode, uid, repo.ID); err != nil {
-			return fmt.Errorf("update access table: %w", err)
-		}
-
-		return nil
-	})
-}
-
 // IsOwnerMemberCollaborator checks if a provided user is the owner, a collaborator or a member of a team in a repository
 func IsOwnerMemberCollaborator(ctx context.Context, repo *Repository, userID int64) (bool, error) {
 	if repo.OwnerID == userID {

models/repo/collaboration_test.go

@@ -7,8 +7,6 @@ import (
 	"testing"
 
 	"gitea.dev/models/db"
-	"gitea.dev/models/perm"
-	access_model "gitea.dev/models/perm/access"
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unittest"
 
@@ -69,28 +67,6 @@ func TestRepository_IsCollaborator(t *testing.T) {
 	test(4, 4, true)
 }
 
-func TestRepository_ChangeCollaborationAccessMode(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
-	assert.NoError(t, repo_model.ChangeCollaborationAccessMode(t.Context(), repo, 4, perm.AccessModeAdmin))
-
-	collaboration := unittest.AssertExistsAndLoadBean(t, &repo_model.Collaboration{RepoID: repo.ID, UserID: 4})
-	assert.Equal(t, perm.AccessModeAdmin, collaboration.Mode)
-
-	access := unittest.AssertExistsAndLoadBean(t, &access_model.Access{UserID: 4, RepoID: repo.ID})
-	assert.Equal(t, perm.AccessModeAdmin, access.Mode)
-
-	assert.NoError(t, repo_model.ChangeCollaborationAccessMode(t.Context(), repo, 4, perm.AccessModeAdmin))
-
-	assert.NoError(t, repo_model.ChangeCollaborationAccessMode(t.Context(), repo, unittest.NonexistentID, perm.AccessModeAdmin))
-
-	// Discard invalid input.
-	assert.NoError(t, repo_model.ChangeCollaborationAccessMode(t.Context(), repo, 4, perm.AccessMode(-1)))
-
-	unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repo.ID})
-}
-
 func TestRepository_IsOwnerMemberCollaborator(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 

routers/web/repo/setting/collaboration.go

@@ -118,13 +118,19 @@ func CollaborationPost(ctx *context.Context) {
 
 // ChangeCollaborationAccessMode response for changing access of a collaboration
 func ChangeCollaborationAccessMode(ctx *context.Context) {
-	if err := repo_model.ChangeCollaborationAccessMode(
+	// the frontend initRepoSettingsCollaboration logic: it only checks "resp.ok"
-		ctx,
+	u, err := user_model.GetUserByID(ctx, ctx.FormInt64("uid"))
-		ctx.Repo.Repository,
+	if err != nil {
-		ctx.FormInt64("uid"),
+		ctx.Status(http.StatusBadRequest)
-		perm.AccessMode(ctx.FormInt("mode"))); err != nil {
+		return
-		log.Error("ChangeCollaborationAccessMode: %v", err)
 	}
+	mode := perm.AccessMode(ctx.FormInt("mode"))
+	if err := repo_service.AddOrUpdateCollaborator(ctx, ctx.Repo.Repository, u, mode); err != nil {
+		ctx.Status(http.StatusBadRequest)
+		log.Error("AddOrUpdateCollaborator: %v", err)
+		return
+	}
+	ctx.JSONOK()
 }
 
 // DeleteCollaboration delete a collaboration for a repository

services/auth/httpsign.go

@@ -78,8 +78,6 @@ func (h *HTTPSign) Verify(req *http.Request, w http.ResponseWriter, store DataSt
 		return nil, err
 	}
 
-	store.GetData()["IsApiToken"] = true
-
 	log.Trace("HTTP Sign: Logged in user %-v", u)
 
 	return u, nil

services/repository/collaboration.go

@@ -19,7 +19,9 @@ import (
 )
 
 func AddOrUpdateCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User, mode perm.AccessMode) error {
-	// only allow valid access modes, read, write and admin
+	// Only allow valid access modes, read, write and admin
+	// Keep in mind: do not allow "owner" here: because "admin" user can update collaborators but not make dangerous operations.
+	// If the "admin" user updates a user to "owner", then it means that the admin user can use owner permission, which is not expected.
 	if mode < perm.AccessModeRead || mode > perm.AccessModeAdmin {
 		return perm.ErrInvalidAccessMode
 	}

services/repository/collaboration_test.go

@@ -20,16 +20,20 @@ import (
 func TestRepository_AddCollaborator(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
-	testSuccess := func(repoID, userID int64) {
+	repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
-		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repoID})
+	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
+	user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+	testSuccess := func(repo *repo_model.Repository, user *user_model.User) {
 		assert.NoError(t, repo.LoadOwner(t.Context()))
-		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID})
 		assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo, user, perm.AccessModeWrite))
-		unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repoID}, &user_model.User{ID: userID})
+		unittest.CheckConsistencyFor(t, repo, user)
 	}
-	testSuccess(1, 4)
+	testSuccess(repo1, user4)
-	testSuccess(1, 4)
+	testSuccess(repo1, user4)
-	testSuccess(3, 4)
+	testSuccess(repo3, user4)
+
+	assert.Error(t, AddOrUpdateCollaborator(t.Context(), repo1, user4, perm.AccessModeOwner))
+	assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo1, user4, perm.AccessModeAdmin))
 }
 
 func TestRepository_DeleteCollaboration(t *testing.T) {

templates/shared/combomarkdowneditor.tmpl

@@ -17,6 +17,7 @@
 {{$previewContext := or $editorContext.PreviewContext ""}}
 {{$previewLink := or $editorContext.PreviewLink (print AppSubUrl "/-/markup")}}
 {{$mentionsLink := or $editorContext.MentionsLink ""}}
+{{/* don't try to remove it, there are still many users who like it: https://github.com/go-gitea/gitea/issues/38228#issuecomment-4809312561 */}}
 {{$supportEasyMDE := or (eq $previewMode "comment") (eq $previewMode "wiki")}}
 <div {{if .ContainerId}}id="{{.ContainerId}}"{{end}} class="combo-markdown-editor {{if .CustomInit}}custom-init{{end}} {{.ContainerClasses}}"
 		data-dropzone-parent-container="{{.DropzoneParentContainer}}"

web_src/js/features/repo-settings.ts

@@ -18,16 +18,21 @@ function initRepoSettingsCollaboration() {
         dropdownEl.classList.add('is-loading', 'loading-icon-2px');
         const lastValue = dropdownEl.getAttribute('data-last-value')!;
         $dropdown.dropdown('hide');
+        let respOk = false;
         try {
           const uid = dropdownEl.getAttribute('data-uid')!;
-          await POST(dropdownEl.getAttribute('data-url')!, {data: new URLSearchParams({uid, 'mode': value})});
+          const resp = await POST(dropdownEl.getAttribute('data-url')!, {data: new URLSearchParams({uid, 'mode': value})});
-          textEl.textContent = text;
+          respOk = resp.ok;
-          dropdownEl.setAttribute('data-last-value', value);
+          if (respOk) {
-        } catch {
+            textEl.textContent = text;
-          textEl.textContent = '(error)'; // prevent from misleading users when error occurs
+            dropdownEl.setAttribute('data-last-value', value);
-          dropdownEl.setAttribute('data-last-value', lastValue);
+          }
         } finally {
           dropdownEl.classList.remove('is-loading');
+          if (!respOk) {
+            textEl.textContent = '(error)'; // prevent from misleading users when error occurs
+            dropdownEl.setAttribute('data-last-value', lastValue);
+          }
         }
       },
       onHide() {