Protected tag is no internal server error (#30962) (#30970)

Backport #30962 by @KN4CK3R

Fixes #30959

Adds an API test for protected tags.
Fix existing tag in combination with fixtures.

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>

diff --git a/models/fixtures/protected_tag.yml b/models/fixtures/protected_tag.yml
new file mode 100644 (file)
index 0000000..dbec52c
--- /dev/null
+++ b/models/fixtures/protected_tag.yml
@@ -0,0 +1,24 @@
+-
+  id: 1
+  repo_id: 4
+  name_pattern: /v.+/
+  allowlist_user_i_ds: []
+  allowlist_team_i_ds: []
+  created_unix: 1715596037
+  updated_unix: 1715596037
+-
+  id: 2
+  repo_id: 1
+  name_pattern: v-*
+  allowlist_user_i_ds: []
+  allowlist_team_i_ds: []
+  created_unix: 1715596037
+  updated_unix: 1715596037
+-
+  id: 3
+  repo_id: 1
+  name_pattern: v-1.1
+  allowlist_user_i_ds: [2]
+  allowlist_team_i_ds: []
+  created_unix: 1715596037
+  updated_unix: 1715596037

diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go
index f0f3c0bbc79a07c36955d559dc40c77bf597df82..f92fb86f5cb0b261daadc99b7ad322edbf1f5901 100644 (file)
--- a/routers/api/v1/repo/release.go
+++ b/routers/api/v1/repo/release.go
@@ -215,6 +215,9 @@ func CreateRelease(ctx *context.APIContext) {
        //     "$ref": "#/responses/notFound"
        //   "409":
        //     "$ref": "#/responses/error"
+       //   "422":
+       //     "$ref": "#/responses/validationError"
+
        form := web.GetForm(ctx).(*api.CreateReleaseOption)
        if ctx.Repo.Repository.IsEmpty {
                ctx.Error(http.StatusUnprocessableEntity, "RepoIsEmpty", fmt.Errorf("repo is empty"))
@@ -246,6 +249,8 @@ func CreateRelease(ctx *context.APIContext) {
                if err := release_service.CreateRelease(ctx.Repo.GitRepo, rel, nil, ""); err != nil {
                        if repo_model.IsErrReleaseAlreadyExist(err) {
                                ctx.Error(http.StatusConflict, "ReleaseAlreadyExist", err)
+                       } else if models.IsErrProtectedTagName(err) {
+                               ctx.Error(http.StatusUnprocessableEntity, "ProtectedTagName", err)
                        } else {
                                ctx.Error(http.StatusInternalServerError, "CreateRelease", err)
                        }
@@ -386,8 +391,8 @@ func DeleteRelease(ctx *context.APIContext) {
        //     "$ref": "#/responses/empty"
        //   "404":
        //     "$ref": "#/responses/notFound"
-       //   "405":
-       //     "$ref": "#/responses/empty"
+       //   "422":
+       //     "$ref": "#/responses/validationError"
 
        id := ctx.ParamsInt64(":id")
        rel, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
@@ -401,7 +406,7 @@ func DeleteRelease(ctx *context.APIContext) {
        }
        if err := release_service.DeleteReleaseByID(ctx, ctx.Repo.Repository, rel, ctx.Doer, false); err != nil {
                if models.IsErrProtectedTagName(err) {
-                       ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
+                       ctx.Error(http.StatusUnprocessableEntity, "delTag", "user not allowed to delete protected tag")
                        return
                }
                ctx.Error(http.StatusInternalServerError, "DeleteReleaseByID", err)

diff --git a/routers/api/v1/repo/release_tags.go b/routers/api/v1/repo/release_tags.go
index fec91164a29ac7957fc46eaebe1bebad3fb1ad45..f845fad53bdd1cfa4e29abd51b0221182c4f90d9 100644 (file)
--- a/routers/api/v1/repo/release_tags.go
+++ b/routers/api/v1/repo/release_tags.go
@@ -92,8 +92,8 @@ func DeleteReleaseByTag(ctx *context.APIContext) {
        //     "$ref": "#/responses/empty"
        //   "404":
        //     "$ref": "#/responses/notFound"
-       //   "405":
-       //     "$ref": "#/responses/empty"
+       //   "422":
+       //     "$ref": "#/responses/validationError"
 
        tag := ctx.Params(":tag")
 
@@ -114,7 +114,7 @@ func DeleteReleaseByTag(ctx *context.APIContext) {
 
        if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, release, ctx.Doer, false); err != nil {
                if models.IsErrProtectedTagName(err) {
-                       ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
+                       ctx.Error(http.StatusUnprocessableEntity, "delTag", "user not allowed to delete protected tag")
                        return
                }
                ctx.Error(http.StatusInternalServerError, "DeleteReleaseByID", err)

diff --git a/routers/api/v1/repo/tag.go b/routers/api/v1/repo/tag.go
index a6908f361572e5d4afc4b82caa7cc324bd177cb3..8577a0e8962914f78d41af426f240b899c3804f4 100644 (file)
--- a/routers/api/v1/repo/tag.go
+++ b/routers/api/v1/repo/tag.go
@@ -184,6 +184,8 @@ func CreateTag(ctx *context.APIContext) {
        //     "$ref": "#/responses/empty"
        //   "409":
        //     "$ref": "#/responses/conflict"
+       //   "422":
+       //     "$ref": "#/responses/validationError"
        //   "423":
        //     "$ref": "#/responses/repoArchivedError"
        form := web.GetForm(ctx).(*api.CreateTagOption)
@@ -205,7 +207,7 @@ func CreateTag(ctx *context.APIContext) {
                        return
                }
                if models.IsErrProtectedTagName(err) {
-                       ctx.Error(http.StatusMethodNotAllowed, "CreateNewTag", "user not allowed to create protected tag")
+                       ctx.Error(http.StatusUnprocessableEntity, "CreateNewTag", "user not allowed to create protected tag")
                        return
                }
 
@@ -253,6 +255,8 @@ func DeleteTag(ctx *context.APIContext) {
        //     "$ref": "#/responses/empty"
        //   "409":
        //     "$ref": "#/responses/conflict"
+       //   "422":
+       //     "$ref": "#/responses/validationError"
        //   "423":
        //     "$ref": "#/responses/repoArchivedError"
        tagName := ctx.Params("*")
@@ -274,7 +278,7 @@ func DeleteTag(ctx *context.APIContext) {
 
        if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, tag, ctx.Doer, true); err != nil {
                if models.IsErrProtectedTagName(err) {
-                       ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
+                       ctx.Error(http.StatusUnprocessableEntity, "delTag", "user not allowed to delete protected tag")
                        return
                }
                ctx.Error(http.StatusInternalServerError, "DeleteReleaseByID", err)

diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl
index 1634901086f4aeb5cb53c65bae25c48be84728bc..1c23c72ac23c651e762e758fb7629f9c354f5abd 100644 (file)
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -12771,6 +12771,9 @@
           },
           "409": {
             "$ref": "#/responses/error"
+          },
+          "422": {
+            "$ref": "#/responses/validationError"
           }
         }
       }
@@ -12889,8 +12892,8 @@
           "404": {
             "$ref": "#/responses/notFound"
           },
-          "405": {
-            "$ref": "#/responses/empty"
+          "422": {
+            "$ref": "#/responses/validationError"
           }
         }
       }
@@ -12975,8 +12978,8 @@
           "404": {
             "$ref": "#/responses/notFound"
           },
-          "405": {
-            "$ref": "#/responses/empty"
+          "422": {
+            "$ref": "#/responses/validationError"
           }
         }
       },
@@ -13826,6 +13829,9 @@
           "409": {
             "$ref": "#/responses/conflict"
           },
+          "422": {
+            "$ref": "#/responses/validationError"
+          },
           "423": {
             "$ref": "#/responses/repoArchivedError"
           }
@@ -13919,6 +13925,9 @@
           "409": {
             "$ref": "#/responses/conflict"
           },
+          "422": {
+            "$ref": "#/responses/validationError"
+          },
           "423": {
             "$ref": "#/responses/repoArchivedError"
           }

diff --git a/tests/integration/api_releases_test.go b/tests/integration/api_releases_test.go
index 49aa4c4e1bf636edc48e9fe6a04fe7d0c94d83af..8b7d674508458c1a301df0cf4ce608c26f74a72a 100644 (file)
--- a/tests/integration/api_releases_test.go
+++ b/tests/integration/api_releases_test.go
@@ -154,6 +154,31 @@ func TestAPICreateAndUpdateRelease(t *testing.T) {
        assert.EqualValues(t, rel.Note, newRelease.Note)
 }
 
+func TestAPICreateProtectedTagRelease(t *testing.T) {
+       defer tests.PrepareTestEnv(t)()
+
+       repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4})
+       writer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4})
+       session := loginUser(t, writer.LowerName)
+       token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+       gitRepo, err := gitrepo.OpenRepository(git.DefaultContext, repo)
+       assert.NoError(t, err)
+       defer gitRepo.Close()
+
+       commit, err := gitRepo.GetBranchCommit("master")
+       assert.NoError(t, err)
+
+       req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/releases", repo.OwnerName, repo.Name), &api.CreateReleaseOption{
+               TagName:      "v0.0.1",
+               Title:        "v0.0.1",
+               IsDraft:      false,
+               IsPrerelease: false,
+               Target:       commit.ID.String(),
+       }).AddTokenAuth(token)
+       MakeRequest(t, req, http.StatusUnprocessableEntity)
+}
+
 func TestAPICreateReleaseToDefaultBranch(t *testing.T) {
        defer tests.PrepareTestEnv(t)()
 

diff --git a/tests/integration/repo_tag_test.go b/tests/integration/repo_tag_test.go
index 7e779064733e3e6040ba639c680655380c447de9..6d3d85532cccae8147e918480aadf1405d47960b 100644 (file)
--- a/tests/integration/repo_tag_test.go
+++ b/tests/integration/repo_tag_test.go
@@ -26,22 +26,10 @@ func TestCreateNewTagProtected(t *testing.T) {
        repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
        owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
 
-       t.Run("API", func(t *testing.T) {
+       t.Run("Code", func(t *testing.T) {
                defer tests.PrintCurrentTest(t)()
 
-               err := release.CreateNewTag(git.DefaultContext, owner, repo, "master", "v-1", "first tag")
-               assert.NoError(t, err)
-
-               err = git_model.InsertProtectedTag(db.DefaultContext, &git_model.ProtectedTag{
-                       RepoID:      repo.ID,
-                       NamePattern: "v-*",
-               })
-               assert.NoError(t, err)
-               err = git_model.InsertProtectedTag(db.DefaultContext, &git_model.ProtectedTag{
-                       RepoID:           repo.ID,
-                       NamePattern:      "v-1.1",
-                       AllowlistUserIDs: []int64{repo.OwnerID},
-               })
+               err := release.CreateNewTag(git.DefaultContext, owner, repo, "master", "t-first", "first tag")
                assert.NoError(t, err)
 
                err = release.CreateNewTag(git.DefaultContext, owner, repo, "master", "v-2", "second tag")
@@ -54,13 +42,12 @@ func TestCreateNewTagProtected(t *testing.T) {
 
        t.Run("Git", func(t *testing.T) {
                onGiteaRun(t, func(t *testing.T, u *url.URL) {
-                       username := "user2"
-                       httpContext := NewAPITestContext(t, username, "repo1")
+                       httpContext := NewAPITestContext(t, owner.Name, repo.Name)
 
                        dstPath := t.TempDir()
 
                        u.Path = httpContext.GitPath()
-                       u.User = url.UserPassword(username, userPassword)
+                       u.User = url.UserPassword(owner.Name, userPassword)
 
                        doGitClone(dstPath, u)(t)