From b1d8f13bd0ecd9c576ebf2ecbd9c7dbeb3f5254f Mon Sep 17 00:00:00 2001
From: KN4CK3R <admin@oldschoolhack.me>
Date: Tue, 14 May 2024 08:48:21 +0200
Subject: [PATCH] Protected tag is no internal server error (#30962)
Fixes #30959
Adds an API test for protected tags.
Fix existing tag in combination with fixtures.
---
models/fixtures/protected_tag.yml | 24 ++++++++++++++++++++++++
routers/api/v1/repo/release.go | 11 ++++++++---
routers/api/v1/repo/release_tags.go | 6 +++---
routers/api/v1/repo/tag.go | 8 ++++++--
templates/swagger/v1_json.tmpl | 17 +++++++++++++----
tests/integration/api_releases_test.go | 25 +++++++++++++++++++++++++
tests/integration/repo_tag_test.go | 21 ++++-----------------
7 files changed, 83 insertions(+), 29 deletions(-)
create mode 100644 models/fixtures/protected_tag.yml
diff --git a/models/fixtures/protected_tag.yml b/models/fixtures/protected_tag.yml
new file mode 100644
index 0000000000..dbec52c0c2
--- /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 f0f3c0bbc7..f92fb86f5c 100644
--- 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 fec91164a2..f845fad53b 100644
--- 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 a6908f3615..8577a0e896 100644
--- 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 b1255f1289..9ad0aa2ab6 100644
--- a/templates/swagger/v1_json.tmpl
+++ b/templates/swagger/v1_json.tmpl
@@ -12831,6 +12831,9 @@
},
"409": {
"$ref": "#/responses/error"
+ },
+ "422": {
+ "$ref": "#/responses/validationError"
}
}
}
@@ -12949,8 +12952,8 @@
"404": {
"$ref": "#/responses/notFound"
},
- "405": {
- "$ref": "#/responses/empty"
+ "422": {
+ "$ref": "#/responses/validationError"
}
}
}
@@ -13035,8 +13038,8 @@
"404": {
"$ref": "#/responses/notFound"
},
- "405": {
- "$ref": "#/responses/empty"
+ "422": {
+ "$ref": "#/responses/validationError"
}
}
},
@@ -13886,6 +13889,9 @@
"409": {
"$ref": "#/responses/conflict"
},
+ "422": {
+ "$ref": "#/responses/validationError"
+ },
"423": {
"$ref": "#/responses/repoArchivedError"
}
@@ -13979,6 +13985,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 73b371b2cb..0b336a90e2 100644
--- 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 7e77906473..6d3d85532c 100644
--- 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)