From ecd463c2f14a8bc1f0eb91d809bf504c99ebf183 Mon Sep 17 00:00:00 2001 From: Kemal Zebari <60799661+kemzeb@users.noreply.github.com> Date: Mon, 13 Jan 2025 17:27:35 -0800 Subject: [PATCH 1/8] Validate that the tag doesn't exist when creating a tag via the web (#33241) Found while investigating #33210. This line no longer makes sense because the form field "TagName" is required, so this would mean that this code path would never be covered. Because it isn't covered, we end up going down the "update release" logic where we eventually set `Release.IsTag` to false (meaning it will now be treated as a release instead of a tag). This snapshot rewrites the condition to ensure that we aren't trying to create a tag that already exists. --------- Co-authored-by: wxiaoguang --- routers/web/repo/release.go | 238 +++++++++++++++--------------- routers/web/repo/release_test.go | 155 ++++++++++++++----- services/forms/repo_form.go | 4 +- templates/repo/release/new.tmpl | 16 +- tests/integration/release_test.go | 2 +- 5 files changed, 243 insertions(+), 172 deletions(-) diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 53655703fc..8909dedbb1 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/setting" @@ -330,34 +329,17 @@ func LatestRelease(ctx *context.Context) { ctx.Redirect(release.Link()) } -// NewRelease render creating or edit release page -func NewRelease(ctx *context.Context) { +func newReleaseCommon(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.release.new_release") ctx.Data["PageIsReleaseList"] = true - ctx.Data["tag_target"] = ctx.Repo.Repository.DefaultBranch - if tagName := ctx.FormString("tag"); len(tagName) > 0 { - rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, tagName) - if err != nil && !repo_model.IsErrReleaseNotExist(err) { - ctx.ServerError("GetRelease", err) - return - } - if rel != nil { - rel.Repo = ctx.Repo.Repository - if err := rel.LoadAttributes(ctx); err != nil { - ctx.ServerError("LoadAttributes", err) - return - } - - ctx.Data["tag_name"] = rel.TagName - if rel.Target != "" { - ctx.Data["tag_target"] = rel.Target - } - ctx.Data["title"] = rel.Title - ctx.Data["content"] = rel.Note - ctx.Data["attachments"] = rel.Attachments - } + tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) + if err != nil { + ctx.ServerError("GetTagNamesByRepoID", err) + return } + ctx.Data["Tags"] = tags + ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled assigneeUsers, err := repo_model.GetRepoAssignees(ctx, ctx.Repo.Repository) if err != nil { @@ -368,35 +350,74 @@ func NewRelease(ctx *context.Context) { upload.AddUploadContext(ctx, "release") - // For New Release page - PrepareBranchList(ctx) + PrepareBranchList(ctx) // for New Release page +} + +// NewRelease render creating or edit release page +func NewRelease(ctx *context.Context) { + newReleaseCommon(ctx) if ctx.Written() { return } - tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) - if err != nil { - ctx.ServerError("GetTagNamesByRepoID", err) - return + ctx.Data["ShowCreateTagOnlyButton"] = true + + // pre-fill the form with the tag name, target branch and the existing release (if exists) + ctx.Data["tag_target"] = ctx.Repo.Repository.DefaultBranch + if tagName := ctx.FormString("tag"); tagName != "" { + rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, tagName) + if err != nil && !repo_model.IsErrReleaseNotExist(err) { + ctx.ServerError("GetRelease", err) + return + } + + if rel != nil { + rel.Repo = ctx.Repo.Repository + if err = rel.LoadAttributes(ctx); err != nil { + ctx.ServerError("LoadAttributes", err) + return + } + + ctx.Data["ShowCreateTagOnlyButton"] = false + ctx.Data["tag_name"] = rel.TagName + ctx.Data["tag_target"] = rel.Target + ctx.Data["title"] = rel.Title + ctx.Data["content"] = rel.Note + ctx.Data["attachments"] = rel.Attachments + } } - ctx.Data["Tags"] = tags ctx.HTML(http.StatusOK, tplReleaseNew) } // NewReleasePost response for creating a release func NewReleasePost(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.NewReleaseForm) - ctx.Data["Title"] = ctx.Tr("repo.release.new_release") - ctx.Data["PageIsReleaseList"] = true - - tags, err := repo_model.GetTagNamesByRepoID(ctx, ctx.Repo.Repository.ID) - if err != nil { - ctx.ServerError("GetTagNamesByRepoID", err) + newReleaseCommon(ctx) + if ctx.Written() { return } - ctx.Data["Tags"] = tags + form := web.GetForm(ctx).(*forms.NewReleaseForm) + + // first, check whether the release exists, and prepare "ShowCreateTagOnlyButton" + // the logic should be done before the form error check to make the tmpl has correct variables + rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, form.TagName) + if err != nil && !repo_model.IsErrReleaseNotExist(err) { + ctx.ServerError("GetRelease", err) + return + } + + // We should still show the "tag only" button if the user clicks it, no matter the release exists or not. + // Because if error occurs, end users need to have the chance to edit the name and submit the form with "tag-only" again. + // It is still not completely right, because there could still be cases like this: + // * user visit "new release" page, see the "tag only" button + // * input something, click other buttons but not "tag only" + // * error occurs, the "new release" page is rendered again, but the "tag only" button is gone + // Such cases are not able to be handled by current code, it needs frontend code to toggle the "tag-only" button if the input changes. + // Or another choice is "always show the tag-only button" if error occurs. + ctx.Data["ShowCreateTagOnlyButton"] = form.TagOnly || rel == nil + + // do some form checks if ctx.HasError() { ctx.HTML(http.StatusOK, tplReleaseNew) return @@ -407,59 +428,49 @@ func NewReleasePost(ctx *context.Context) { return } - // Title of release cannot be empty - if len(form.TagOnly) == 0 && len(form.Title) == 0 { + if !form.TagOnly && form.Title == "" { + // if not "tag only", then the title of the release cannot be empty ctx.RenderWithErr(ctx.Tr("repo.release.title_empty"), tplReleaseNew, &form) return } - var attachmentUUIDs []string - if setting.Attachment.Enabled { - attachmentUUIDs = form.Files + handleTagReleaseError := func(err error) { + ctx.Data["Err_TagName"] = true + switch { + case release_service.IsErrTagAlreadyExists(err): + ctx.RenderWithErr(ctx.Tr("repo.branch.tag_collision", form.TagName), tplReleaseNew, &form) + case repo_model.IsErrReleaseAlreadyExist(err): + ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) + case release_service.IsErrInvalidTagName(err): + ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_invalid"), tplReleaseNew, &form) + case release_service.IsErrProtectedTagName(err): + ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_protected"), tplReleaseNew, &form) + default: + ctx.ServerError("handleTagReleaseError", err) + } } - rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, form.TagName) - if err != nil { - if !repo_model.IsErrReleaseNotExist(err) { - ctx.ServerError("GetRelease", err) + // prepare the git message for creating a new tag + newTagMsg := "" + if form.Title != "" && form.AddTagMsg { + newTagMsg = form.Title + "\n\n" + form.Content + } + + // no release, and tag only + if rel == nil && form.TagOnly { + if err = release_service.CreateNewTag(ctx, ctx.Doer, ctx.Repo.Repository, form.Target, form.TagName, newTagMsg); err != nil { + handleTagReleaseError(err) return } + ctx.Flash.Success(ctx.Tr("repo.tag.create_success", form.TagName)) + ctx.Redirect(ctx.Repo.RepoLink + "/src/tag/" + util.PathEscapeSegments(form.TagName)) + return + } - msg := "" - if len(form.Title) > 0 && form.AddTagMsg { - msg = form.Title + "\n\n" + form.Content - } - - if len(form.TagOnly) > 0 { - if err = release_service.CreateNewTag(ctx, ctx.Doer, ctx.Repo.Repository, form.Target, form.TagName, msg); err != nil { - if release_service.IsErrTagAlreadyExists(err) { - e := err.(release_service.ErrTagAlreadyExists) - ctx.Flash.Error(ctx.Tr("repo.branch.tag_collision", e.TagName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.RefTypeNameSubURL()) - return - } - - if release_service.IsErrInvalidTagName(err) { - ctx.Flash.Error(ctx.Tr("repo.release.tag_name_invalid")) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.RefTypeNameSubURL()) - return - } - - if release_service.IsErrProtectedTagName(err) { - ctx.Flash.Error(ctx.Tr("repo.release.tag_name_protected")) - ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.RefTypeNameSubURL()) - return - } - - ctx.ServerError("release_service.CreateNewTag", err) - return - } - - ctx.Flash.Success(ctx.Tr("repo.tag.create_success", form.TagName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/tag/" + util.PathEscapeSegments(form.TagName)) - return - } + attachmentUUIDs := util.Iif(setting.Attachment.Enabled, form.Files, nil) + // no existing release, create a new release + if rel == nil { rel = &repo_model.Release{ RepoID: ctx.Repo.Repository.ID, Repo: ctx.Repo.Repository, @@ -469,48 +480,39 @@ func NewReleasePost(ctx *context.Context) { TagName: form.TagName, Target: form.Target, Note: form.Content, - IsDraft: len(form.Draft) > 0, + IsDraft: form.Draft, IsPrerelease: form.Prerelease, IsTag: false, } - - if err = release_service.CreateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs, msg); err != nil { - ctx.Data["Err_TagName"] = true - switch { - case repo_model.IsErrReleaseAlreadyExist(err): - ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) - case release_service.IsErrInvalidTagName(err): - ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_invalid"), tplReleaseNew, &form) - case release_service.IsErrProtectedTagName(err): - ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_protected"), tplReleaseNew, &form) - default: - ctx.ServerError("CreateRelease", err) - } - return - } - } else { - if !rel.IsTag { - ctx.Data["Err_TagName"] = true - ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) - return - } - - rel.Title = form.Title - rel.Note = form.Content - rel.Target = form.Target - rel.IsDraft = len(form.Draft) > 0 - rel.IsPrerelease = form.Prerelease - rel.PublisherID = ctx.Doer.ID - rel.IsTag = false - - if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil); err != nil { - ctx.Data["Err_TagName"] = true - ctx.ServerError("UpdateRelease", err) + if err = release_service.CreateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs, newTagMsg); err != nil { + handleTagReleaseError(err) return } + ctx.Redirect(ctx.Repo.RepoLink + "/releases") + return } - log.Trace("Release created: %s/%s:%s", ctx.Doer.LowerName, ctx.Repo.Repository.Name, form.TagName) + // tag exists, try to convert it to a real release + // old logic: if the release is not a tag (it is a real release), do not update it on the "new release" page + // add new logic: if tag-only, do not convert the tag to a release + if form.TagOnly || !rel.IsTag { + ctx.Data["Err_TagName"] = true + ctx.RenderWithErr(ctx.Tr("repo.release.tag_name_already_exist"), tplReleaseNew, &form) + return + } + + // convert a tag to a real release (set is_tag=false) + rel.Title = form.Title + rel.Note = form.Content + rel.Target = form.Target + rel.IsDraft = form.Draft + rel.IsPrerelease = form.Prerelease + rel.PublisherID = ctx.Doer.ID + rel.IsTag = false + if err = release_service.UpdateRelease(ctx, ctx.Doer, ctx.Repo.GitRepo, rel, attachmentUUIDs, nil, nil); err != nil { + handleTagReleaseError(err) + return + } ctx.Redirect(ctx.Repo.RepoLink + "/releases") } diff --git a/routers/web/repo/release_test.go b/routers/web/repo/release_test.go index 7ebea4c3fb..9f49fc7500 100644 --- a/routers/web/repo/release_test.go +++ b/routers/web/repo/release_test.go @@ -11,60 +11,135 @@ import ( "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/contexttest" "code.gitea.io/gitea/services/forms" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewReleasePost(t *testing.T) { - for _, testCase := range []struct { - RepoID int64 - UserID int64 - TagName string - Form forms.NewReleaseForm - }{ - { - RepoID: 1, - UserID: 2, - TagName: "v1.1", // pre-existing tag - Form: forms.NewReleaseForm{ - TagName: "newtag", - Target: "master", - Title: "title", - Content: "content", - }, - }, - { - RepoID: 1, - UserID: 2, - TagName: "newtag", - Form: forms.NewReleaseForm{ - TagName: "newtag", - Target: "master", - Title: "title", - Content: "content", - }, - }, - } { - unittest.PrepareTestEnv(t) + unittest.PrepareTestEnv(t) + get := func(t *testing.T, tagName string) *context.Context { + ctx, _ := contexttest.MockContext(t, "user2/repo1/releases/new?tag="+tagName) + contexttest.LoadUser(t, ctx, 2) + contexttest.LoadRepo(t, ctx, 1) + contexttest.LoadGitRepo(t, ctx) + defer ctx.Repo.GitRepo.Close() + NewRelease(ctx) + return ctx + } + + t.Run("NewReleasePage", func(t *testing.T) { + ctx := get(t, "v1.1") + assert.Empty(t, ctx.Data["ShowCreateTagOnlyButton"]) + ctx = get(t, "new-tag-name") + assert.NotEmpty(t, ctx.Data["ShowCreateTagOnlyButton"]) + }) + + post := func(t *testing.T, form forms.NewReleaseForm) *context.Context { ctx, _ := contexttest.MockContext(t, "user2/repo1/releases/new") contexttest.LoadUser(t, ctx, 2) contexttest.LoadRepo(t, ctx, 1) contexttest.LoadGitRepo(t, ctx) - web.SetForm(ctx, &testCase.Form) + defer ctx.Repo.GitRepo.Close() + web.SetForm(ctx, &form) NewReleasePost(ctx) - unittest.AssertExistsAndLoadBean(t, &repo_model.Release{ - RepoID: 1, - PublisherID: 2, - TagName: testCase.Form.TagName, - Target: testCase.Form.Target, - Title: testCase.Form.Title, - Note: testCase.Form.Content, - }, unittest.Cond("is_draft=?", len(testCase.Form.Draft) > 0)) - ctx.Repo.GitRepo.Close() + return ctx } + + loadRelease := func(t *testing.T, tagName string) *repo_model.Release { + return unittest.GetBean(t, &repo_model.Release{}, unittest.Cond("repo_id=1 AND tag_name=?", tagName)) + } + + t.Run("NewTagRelease", func(t *testing.T) { + post(t, forms.NewReleaseForm{ + TagName: "newtag", + Target: "master", + Title: "title", + Content: "content", + }) + rel := loadRelease(t, "newtag") + require.NotNil(t, rel) + assert.False(t, rel.IsTag) + assert.Equal(t, "master", rel.Target) + assert.Equal(t, "title", rel.Title) + assert.Equal(t, "content", rel.Note) + }) + + t.Run("ReleaseExistsDoUpdate(non-tag)", func(t *testing.T) { + ctx := post(t, forms.NewReleaseForm{ + TagName: "v1.1", + Target: "master", + Title: "updated-title", + Content: "updated-content", + }) + rel := loadRelease(t, "v1.1") + require.NotNil(t, rel) + assert.False(t, rel.IsTag) + assert.Equal(t, "testing-release", rel.Title) + assert.NotEmpty(t, ctx.Flash.ErrorMsg) + }) + + t.Run("ReleaseExistsDoUpdate(tag-only)", func(t *testing.T) { + ctx := post(t, forms.NewReleaseForm{ + TagName: "delete-tag", // a strange name, but it is the only "is_tag=true" fixture + Target: "master", + Title: "updated-title", + Content: "updated-content", + TagOnly: true, + }) + rel := loadRelease(t, "delete-tag") + require.NotNil(t, rel) + assert.True(t, rel.IsTag) // the record should not be updated because the request is "tag-only". TODO: need to improve the logic? + assert.Equal(t, "delete-tag", rel.Title) + assert.NotEmpty(t, ctx.Flash.ErrorMsg) + assert.NotEmpty(t, ctx.Data["ShowCreateTagOnlyButton"]) // still show the "tag-only" button + }) + + t.Run("ReleaseExistsDoUpdate(tag-release)", func(t *testing.T) { + ctx := post(t, forms.NewReleaseForm{ + TagName: "delete-tag", // a strange name, but it is the only "is_tag=true" fixture + Target: "master", + Title: "updated-title", + Content: "updated-content", + }) + rel := loadRelease(t, "delete-tag") + require.NotNil(t, rel) + assert.False(t, rel.IsTag) // the tag has been "updated" to be a real "release" + assert.Equal(t, "updated-title", rel.Title) + assert.Empty(t, ctx.Flash.ErrorMsg) + }) + + t.Run("TagOnly", func(t *testing.T) { + ctx := post(t, forms.NewReleaseForm{ + TagName: "new-tag-only", + Target: "master", + Title: "title", + Content: "content", + TagOnly: true, + }) + rel := loadRelease(t, "new-tag-only") + require.NotNil(t, rel) + assert.True(t, rel.IsTag) + assert.Empty(t, ctx.Flash.ErrorMsg) + }) + + t.Run("TagOnlyConflict", func(t *testing.T) { + ctx := post(t, forms.NewReleaseForm{ + TagName: "v1.1", + Target: "master", + Title: "title", + Content: "content", + TagOnly: true, + }) + rel := loadRelease(t, "v1.1") + require.NotNil(t, rel) + assert.False(t, rel.IsTag) + assert.NotEmpty(t, ctx.Flash.ErrorMsg) + }) } func TestCalReleaseNumCommitsBehind(t *testing.T) { diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index b14171787e..35ea5378d3 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -651,8 +651,8 @@ type NewReleaseForm struct { Target string `form:"tag_target" binding:"Required;MaxSize(255)"` Title string `binding:"MaxSize(255)"` Content string - Draft string - TagOnly string + Draft bool + TagOnly bool Prerelease bool AddTagMsg bool Files []string diff --git a/templates/repo/release/new.tmpl b/templates/repo/release/new.tmpl index 574b0d0311..8b6aa252af 100644 --- a/templates/repo/release/new.tmpl +++ b/templates/repo/release/new.tmpl @@ -109,23 +109,17 @@ {{ctx.Locale.Tr "repo.release.delete_release"}} {{if .IsDraft}} - - + + {{else}} - + {{end}} {{else}} - {{if not .tag_name}} + {{if .ShowCreateTagOnlyButton}} {{end}} - + {{end}} diff --git a/tests/integration/release_test.go b/tests/integration/release_test.go index 40bd798d16..d3c4ed6a83 100644 --- a/tests/integration/release_test.go +++ b/tests/integration/release_test.go @@ -39,7 +39,7 @@ func createNewRelease(t *testing.T, session *TestSession, repoURL, tag, title st postData["prerelease"] = "on" } if draft { - postData["draft"] = "Save Draft" + postData["draft"] = "1" } req = NewRequestWithValues(t, "POST", link, postData) From a98a836e76ce8c95a16c9e26065fd05384b67ce8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 14 Jan 2025 09:53:34 +0800 Subject: [PATCH 2/8] Support public code/issue access for private repositories (#33127) Close #8649, close #639 (will add "anonymous access" in following PRs) --- models/perm/access/repo_permission.go | 47 ++++-- models/perm/access/repo_permission_test.go | 12 +- options/locale/locale_en-US.ini | 2 +- routers/web/repo/issue_poster.go | 10 +- routers/web/repo/setting/setting.go | 17 +- routers/web/web.go | 187 +++++++++++---------- services/context/permission.go | 74 +------- services/forms/repo_form.go | 66 +++++--- templates/repo/settings/options.tmpl | 20 ++- 9 files changed, 220 insertions(+), 215 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 0ed116a132..e00b7c5320 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -175,10 +175,14 @@ func (p *Permission) LogString() string { return fmt.Sprintf(format, args...) } -func applyEveryoneRepoPermission(user *user_model.User, perm *Permission) { +func finalProcessRepoUnitPermission(user *user_model.User, perm *Permission) { if user == nil || user.ID <= 0 { + // for anonymous access, it could be: + // AccessMode is None or Read, units has repo units, unitModes is nil return } + + // apply everyone access permissions for _, u := range perm.units { if u.EveryoneAccessMode >= perm_model.AccessModeRead && u.EveryoneAccessMode > perm.everyoneAccessMode[u.Type] { if perm.everyoneAccessMode == nil { @@ -187,17 +191,40 @@ func applyEveryoneRepoPermission(user *user_model.User, perm *Permission) { perm.everyoneAccessMode[u.Type] = u.EveryoneAccessMode } } + + if perm.unitsMode == nil { + // if unitsMode is not set, then it means that the default p.AccessMode applies to all units + return + } + + // remove no permission units + origPermUnits := perm.units + perm.units = make([]*repo_model.RepoUnit, 0, len(perm.units)) + for _, u := range origPermUnits { + shouldKeep := false + for t := range perm.unitsMode { + if shouldKeep = u.Type == t; shouldKeep { + break + } + } + for t := range perm.everyoneAccessMode { + if shouldKeep = shouldKeep || u.Type == t; shouldKeep { + break + } + } + if shouldKeep { + perm.units = append(perm.units, u) + } + } } // GetUserRepoPermission returns the user permissions to the repository func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (perm Permission, err error) { defer func() { if err == nil { - applyEveryoneRepoPermission(user, &perm) - } - if log.IsTrace() { - log.Trace("Permission Loaded for user %-v in repo %-v, permissions: %-+v", user, repo, perm) + finalProcessRepoUnitPermission(user, &perm) } + log.Trace("Permission Loaded for user %-v in repo %-v, permissions: %-+v", user, repo, perm) }() if err = repo.LoadUnits(ctx); err != nil { @@ -294,16 +321,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } } - // remove no permission units - perm.units = make([]*repo_model.RepoUnit, 0, len(repo.Units)) - for t := range perm.unitsMode { - for _, u := range repo.Units { - if u.Type == t { - perm.units = append(perm.units, u) - } - } - } - return perm, err } diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go index 50070c4368..9862da0673 100644 --- a/models/perm/access/repo_permission_test.go +++ b/models/perm/access/repo_permission_test.go @@ -50,7 +50,7 @@ func TestApplyEveryoneRepoPermission(t *testing.T) { {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, }, } - applyEveryoneRepoPermission(nil, &perm) + finalProcessRepoUnitPermission(nil, &perm) assert.False(t, perm.CanRead(unit.TypeWiki)) perm = Permission{ @@ -59,7 +59,7 @@ func TestApplyEveryoneRepoPermission(t *testing.T) { {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, }, } - applyEveryoneRepoPermission(&user_model.User{ID: 0}, &perm) + finalProcessRepoUnitPermission(&user_model.User{ID: 0}, &perm) assert.False(t, perm.CanRead(unit.TypeWiki)) perm = Permission{ @@ -68,7 +68,7 @@ func TestApplyEveryoneRepoPermission(t *testing.T) { {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, }, } - applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm) + finalProcessRepoUnitPermission(&user_model.User{ID: 1}, &perm) assert.True(t, perm.CanRead(unit.TypeWiki)) perm = Permission{ @@ -77,20 +77,22 @@ func TestApplyEveryoneRepoPermission(t *testing.T) { {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, }, } - applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm) + finalProcessRepoUnitPermission(&user_model.User{ID: 1}, &perm) // it should work the same as "EveryoneAccessMode: none" because the default AccessMode should be applied to units assert.True(t, perm.CanWrite(unit.TypeWiki)) perm = Permission{ units: []*repo_model.RepoUnit{ + {Type: unit.TypeCode}, // will be removed {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, }, unitsMode: map[unit.Type]perm_model.AccessMode{ unit.TypeWiki: perm_model.AccessModeWrite, }, } - applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm) + finalProcessRepoUnitPermission(&user_model.User{ID: 1}, &perm) assert.True(t, perm.CanWrite(unit.TypeWiki)) + assert.Len(t, perm.units, 1) } func TestUnitAccessMode(t *testing.T) { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 93155caa10..d336b6a700 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2158,7 +2158,7 @@ settings.advanced_settings = Advanced Settings settings.wiki_desc = Enable Repository Wiki settings.use_internal_wiki = Use Built-In Wiki settings.default_wiki_branch_name = Default Wiki Branch Name -settings.default_wiki_everyone_access = Default Access Permission for signed-in users: +settings.default_permission_everyone_access = Default access permission for all signed-in users: settings.failed_to_change_default_wiki_branch = Failed to change the default wiki branch. settings.use_external_wiki = Use External Wiki settings.external_wiki_url = External Wiki URL diff --git a/routers/web/repo/issue_poster.go b/routers/web/repo/issue_poster.go index 91ef947cb4..07059b9b7b 100644 --- a/routers/web/repo/issue_poster.go +++ b/routers/web/repo/issue_poster.go @@ -26,13 +26,9 @@ type userSearchResponse struct { Results []*userSearchInfo `json:"results"` } -// IssuePosters get posters for current repo's issues/pull requests -func IssuePosters(ctx *context.Context) { - issuePosters(ctx, false) -} - -func PullPosters(ctx *context.Context) { - issuePosters(ctx, true) +func IssuePullPosters(ctx *context.Context) { + isPullList := ctx.PathParam("type") == "pulls" + issuePosters(ctx, isPullList) } func issuePosters(ctx *context.Context, isPullList bool) { diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 7399c681e2..5b34fc60da 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -49,6 +49,15 @@ const ( tplDeployKeys templates.TplName = "repo/settings/deploy_keys" ) +func parseEveryoneAccessMode(permission string, allowed ...perm.AccessMode) perm.AccessMode { + // if site admin forces repositories to be private, then do not allow any other access mode, + // otherwise the "force private" setting would be bypassed + if setting.Repository.ForcePrivate { + return perm.AccessModeNone + } + return perm.ParseAccessMode(permission, allowed...) +} + // SettingsCtxData is a middleware that sets all the general context data for the // settings template. func SettingsCtxData(ctx *context.Context) { @@ -447,8 +456,9 @@ func SettingsPost(ctx *context.Context) { if form.EnableCode && !unit_model.TypeCode.UnitGlobalDisabled() { units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: unit_model.TypeCode, + RepoID: repo.ID, + Type: unit_model.TypeCode, + EveryoneAccessMode: parseEveryoneAccessMode(form.DefaultCodeEveryoneAccess, perm.AccessModeNone, perm.AccessModeRead), }) } else if !unit_model.TypeCode.UnitGlobalDisabled() { deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeCode) @@ -474,7 +484,7 @@ func SettingsPost(ctx *context.Context) { RepoID: repo.ID, Type: unit_model.TypeWiki, Config: new(repo_model.UnitConfig), - EveryoneAccessMode: perm.ParseAccessMode(form.DefaultWikiEveryoneAccess, perm.AccessModeNone, perm.AccessModeRead, perm.AccessModeWrite), + EveryoneAccessMode: parseEveryoneAccessMode(form.DefaultWikiEveryoneAccess, perm.AccessModeNone, perm.AccessModeRead, perm.AccessModeWrite), }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) } else { @@ -524,6 +534,7 @@ func SettingsPost(ctx *context.Context) { AllowOnlyContributorsToTrackTime: form.AllowOnlyContributorsToTrackTime, EnableDependencies: form.EnableIssueDependencies, }, + EveryoneAccessMode: parseEveryoneAccessMode(form.DefaultIssuesEveryoneAccess, perm.AccessModeNone, perm.AccessModeRead), }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalTracker) } else { diff --git a/routers/web/web.go b/routers/web/web.go index 2f7ee8ade7..d3d0360396 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -101,7 +101,7 @@ func buildAuthGroup() *auth_service.Group { group.Add(&auth_service.Basic{}) // FIXME: this should be removed and only applied in download and git/lfs routers if setting.Service.EnableReverseProxyAuth { - group.Add(&auth_service.ReverseProxy{}) // reverseproxy should before Session, otherwise the header will be ignored if user has login + group.Add(&auth_service.ReverseProxy{}) // reverse-proxy should before Session, otherwise the header will be ignored if user has login } group.Add(&auth_service.Session{}) @@ -816,21 +816,24 @@ func registerRoutes(m *web.Router) { m.Post("/{username}", reqSignIn, context.UserAssignmentWeb(), user.Action) reqRepoAdmin := context.RequireRepoAdmin() - reqRepoCodeWriter := context.RequireRepoWriter(unit.TypeCode) + reqRepoCodeWriter := context.RequireUnitWriter(unit.TypeCode) canEnableEditor := context.CanEnableEditor() - reqRepoCodeReader := context.RequireRepoReader(unit.TypeCode) - reqRepoReleaseWriter := context.RequireRepoWriter(unit.TypeReleases) - reqRepoReleaseReader := context.RequireRepoReader(unit.TypeReleases) - reqRepoWikiReader := context.RequireRepoReader(unit.TypeWiki) - reqRepoWikiWriter := context.RequireRepoWriter(unit.TypeWiki) - reqRepoIssueReader := context.RequireRepoReader(unit.TypeIssues) - reqRepoPullsReader := context.RequireRepoReader(unit.TypePullRequests) - reqRepoIssuesOrPullsWriter := context.RequireRepoWriterOr(unit.TypeIssues, unit.TypePullRequests) - reqRepoIssuesOrPullsReader := context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests) - reqRepoProjectsReader := context.RequireRepoReader(unit.TypeProjects) - reqRepoProjectsWriter := context.RequireRepoWriter(unit.TypeProjects) - reqRepoActionsReader := context.RequireRepoReader(unit.TypeActions) - reqRepoActionsWriter := context.RequireRepoWriter(unit.TypeActions) + reqRepoReleaseWriter := context.RequireUnitWriter(unit.TypeReleases) + reqRepoReleaseReader := context.RequireUnitReader(unit.TypeReleases) + reqRepoIssuesOrPullsWriter := context.RequireUnitWriter(unit.TypeIssues, unit.TypePullRequests) + reqRepoIssuesOrPullsReader := context.RequireUnitReader(unit.TypeIssues, unit.TypePullRequests) + reqRepoProjectsReader := context.RequireUnitReader(unit.TypeProjects) + reqRepoProjectsWriter := context.RequireUnitWriter(unit.TypeProjects) + reqRepoActionsReader := context.RequireUnitReader(unit.TypeActions) + reqRepoActionsWriter := context.RequireUnitWriter(unit.TypeActions) + + // the legacy names "reqRepoXxx" should be renamed to the correct name "reqUnitXxx", these permissions are for units, not repos + reqUnitsWithMarkdown := context.RequireUnitReader(unit.TypeCode, unit.TypeIssues, unit.TypePullRequests, unit.TypeReleases, unit.TypeWiki) + reqUnitCodeReader := context.RequireUnitReader(unit.TypeCode) + reqUnitIssuesReader := context.RequireUnitReader(unit.TypeIssues) + reqUnitPullsReader := context.RequireUnitReader(unit.TypePullRequests) + reqUnitWikiReader := context.RequireUnitReader(unit.TypeWiki) + reqUnitWikiWriter := context.RequireUnitWriter(unit.TypeWiki) reqPackageAccess := func(accessMode perm.AccessMode) func(ctx *context.Context) { return func(ctx *context.Context) { @@ -1053,7 +1056,7 @@ func registerRoutes(m *web.Router) { m.Group("/migrate", func() { m.Get("/status", repo.MigrateStatus) }) - }, optSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqUnitCodeReader) // end "/{username}/{reponame}/-": migrate m.Group("/{username}/{reponame}/settings", func() { @@ -1151,8 +1154,7 @@ func registerRoutes(m *web.Router) { // user/org home, including rss feeds m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRef(), repo.SetEditorconfigIfExists, repo.Home) - // TODO: maybe it should relax the permission to allow "any access" - m.Post("/{username}/{reponame}/markup", optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeCode, unit.TypeIssues, unit.TypePullRequests, unit.TypeReleases, unit.TypeWiki), web.Bind(structs.MarkupOption{}), misc.Markup) + m.Post("/{username}/{reponame}/markup", optSignIn, context.RepoAssignment, reqUnitsWithMarkdown, web.Bind(structs.MarkupOption{}), misc.Markup) m.Group("/{username}/{reponame}", func() { m.Get("/find/*", repo.FindFiles) @@ -1164,41 +1166,40 @@ func registerRoutes(m *web.Router) { m.Get("/compare", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff) m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists). Get(repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff). - Post(reqSignIn, context.RepoMustNotBeArchived(), reqRepoPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost) - }, optSignIn, context.RepoAssignment, reqRepoCodeReader) + Post(reqSignIn, context.RepoMustNotBeArchived(), reqUnitPullsReader, repo.MustAllowPulls, web.Bind(forms.CreateIssueForm{}), repo.SetWhitespaceBehavior, repo.CompareAndPullRequestPost) + }, optSignIn, context.RepoAssignment, reqUnitCodeReader) // end "/{username}/{reponame}": repo code: find, compare, list + addIssuesPullsViewRoutes := func() { + // for /{username}/{reponame}/issues" or "/{username}/{reponame}/pulls" + m.Get("/posters", repo.IssuePullPosters) + m.Group("/{index}", func() { + m.Get("/info", repo.GetIssueInfo) + m.Get("/attachments", repo.GetIssueAttachments) + m.Get("/attachments/{uuid}", repo.GetAttachment) + m.Group("/content-history", func() { + m.Get("/overview", repo.GetContentHistoryOverview) + m.Get("/list", repo.GetContentHistoryList) + m.Get("/detail", repo.GetContentHistoryDetail) + }) + }) + } m.Group("/{username}/{reponame}", func() { - m.Get("/issues/posters", repo.IssuePosters) // it can't use {type:issues|pulls} because it would conflict with other routes like "/pulls/{index}" - m.Get("/pulls/posters", repo.PullPosters) m.Get("/comments/{id}/attachments", repo.GetCommentAttachments) m.Get("/labels", repo.RetrieveLabelsForList, repo.Labels) m.Get("/milestones", repo.Milestones) m.Get("/milestone/{id}", context.RepoRef(), repo.MilestoneIssuesAndPulls) - m.Group("/{type:issues|pulls}", func() { - m.Group("/{index}", func() { - m.Get("/info", repo.GetIssueInfo) - m.Get("/attachments", repo.GetIssueAttachments) - m.Get("/attachments/{uuid}", repo.GetAttachment) - m.Group("/content-history", func() { - m.Get("/overview", repo.GetContentHistoryOverview) - m.Get("/list", repo.GetContentHistoryList) - m.Get("/detail", repo.GetContentHistoryDetail) - }) - }) - }, context.RepoRef()) m.Get("/issues/suggestions", repo.IssueSuggestions) - }, optSignIn, context.RepoAssignment, reqRepoIssuesOrPullsReader) + }, optSignIn, context.RepoAssignment, reqRepoIssuesOrPullsReader) // issue/pull attachments, labels, milestones + + m.Group("/{username}/{reponame}/{type:issues}", addIssuesPullsViewRoutes, optSignIn, context.RepoAssignment, reqUnitIssuesReader) + m.Group("/{username}/{reponame}/{type:pulls}", addIssuesPullsViewRoutes, optSignIn, context.RepoAssignment, reqUnitPullsReader) // end "/{username}/{reponame}": view milestone, label, issue, pull, etc - m.Group("/{username}/{reponame}", func() { - m.Group("/{type:issues|pulls}", func() { - m.Get("", repo.Issues) - m.Group("/{index}", func() { - m.Get("", repo.ViewIssue) - }) - }) - }, optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests, unit.TypeExternalTracker)) + m.Group("/{username}/{reponame}/{type:issues}", func() { + m.Get("", repo.Issues) + m.Get("/{index}", repo.ViewIssue) + }, optSignIn, context.RepoAssignment, context.RequireUnitReader(unit.TypeIssues, unit.TypeExternalTracker)) // end "/{username}/{reponame}": issue/pull list, issue/pull view, external tracker m.Group("/{username}/{reponame}", func() { // edit issues, pulls, labels, milestones, etc @@ -1209,11 +1210,10 @@ func registerRoutes(m *web.Router) { m.Get("/choose", context.RepoRef(), repo.NewIssueChooseTemplate) }) m.Get("/search", repo.SearchRepoIssuesJSON) - }, context.RepoMustNotBeArchived(), reqRepoIssueReader) + }, reqUnitIssuesReader) - // FIXME: should use different URLs but mostly same logic for comments of issue and pull request. - // So they can apply their own enable/disable logic on routers. - m.Group("/{type:issues|pulls}", func() { + addIssuesPullsRoutes := func() { + // for "/{username}/{reponame}/issues" or "/{username}/{reponame}/pulls" m.Group("/{index}", func() { m.Post("/title", repo.UpdateIssueTitle) m.Post("/content", repo.UpdateIssueContent) @@ -1240,39 +1240,37 @@ func registerRoutes(m *web.Router) { m.Post("/lock", reqRepoIssuesOrPullsWriter, web.Bind(forms.IssueLockForm{}), repo.LockIssue) m.Post("/unlock", reqRepoIssuesOrPullsWriter, repo.UnlockIssue) m.Post("/delete", reqRepoAdmin, repo.DeleteIssue) - }, context.RepoMustNotBeArchived()) - - m.Group("/{index}", func() { m.Post("/content-history/soft-delete", repo.SoftDeleteContentHistory) }) - m.Post("/labels", reqRepoIssuesOrPullsWriter, repo.UpdateIssueLabel) - m.Post("/milestone", reqRepoIssuesOrPullsWriter, repo.UpdateIssueMilestone) - m.Post("/projects", reqRepoIssuesOrPullsWriter, reqRepoProjectsReader, repo.UpdateIssueProject) - m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee) - m.Post("/request_review", repo.UpdatePullReviewRequest) - m.Post("/dismiss_review", reqRepoAdmin, web.Bind(forms.DismissReviewForm{}), repo.DismissReview) - m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus) - m.Post("/delete", reqRepoAdmin, repo.BatchDeleteIssues) - m.Post("/resolve_conversation", repo.SetShowOutdatedComments, repo.UpdateResolveConversation) m.Post("/attachments", repo.UploadIssueAttachment) m.Post("/attachments/remove", repo.DeleteAttachment) + + m.Post("/projects", reqRepoIssuesOrPullsWriter, reqRepoProjectsReader, repo.UpdateIssueProject) + m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee) + m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus) + m.Post("/delete", reqRepoAdmin, repo.BatchDeleteIssues) m.Delete("/unpin/{index}", reqRepoAdmin, repo.IssueUnpin) m.Post("/move_pin", reqRepoAdmin, repo.IssuePinMove) - }, context.RepoMustNotBeArchived()) + } + m.Group("/{type:issues}", addIssuesPullsRoutes, reqUnitIssuesReader, context.RepoMustNotBeArchived()) + m.Group("/{type:pulls}", addIssuesPullsRoutes, reqUnitPullsReader, context.RepoMustNotBeArchived()) m.Group("/comments/{id}", func() { m.Post("", repo.UpdateCommentContent) m.Post("/delete", repo.DeleteComment) m.Post("/reactions/{action}", web.Bind(forms.ReactionForm{}), repo.ChangeCommentReaction) - }, context.RepoMustNotBeArchived()) + }, reqRepoIssuesOrPullsReader) // edit issue/pull comment + m.Post("/labels", reqRepoIssuesOrPullsWriter, repo.UpdateIssueLabel) m.Group("/labels", func() { m.Post("/new", web.Bind(forms.CreateLabelForm{}), repo.NewLabel) m.Post("/edit", web.Bind(forms.CreateLabelForm{}), repo.UpdateLabel) m.Post("/delete", repo.DeleteLabel) m.Post("/initialize", web.Bind(forms.InitializeLabelsForm{}), repo.InitializeLabels) - }, context.RepoMustNotBeArchived(), reqRepoIssuesOrPullsWriter, context.RepoRef()) + }, reqRepoIssuesOrPullsWriter, context.RepoRef()) + + m.Post("/milestone", reqRepoIssuesOrPullsWriter, repo.UpdateIssueMilestone) m.Group("/milestones", func() { m.Combo("/new").Get(repo.NewMilestone). Post(web.Bind(forms.CreateMilestoneForm{}), repo.NewMilestonePost) @@ -1280,11 +1278,15 @@ func registerRoutes(m *web.Router) { m.Post("/{id}/edit", web.Bind(forms.CreateMilestoneForm{}), repo.EditMilestonePost) m.Post("/{id}/{action}", repo.ChangeMilestoneStatus) m.Post("/delete", repo.DeleteMilestone) - }, context.RepoMustNotBeArchived(), reqRepoIssuesOrPullsWriter, context.RepoRef()) - m.Group("/pull", func() { - m.Post("/{index}/target_branch", repo.UpdatePullRequestTarget) - }, context.RepoMustNotBeArchived()) - }, reqSignIn, context.RepoAssignment, reqRepoIssuesOrPullsReader) + }, reqRepoIssuesOrPullsWriter, context.RepoRef()) + + m.Group("", func() { + m.Post("/request_review", repo.UpdatePullReviewRequest) + m.Post("/dismiss_review", reqRepoAdmin, web.Bind(forms.DismissReviewForm{}), repo.DismissReview) + m.Post("/resolve_conversation", repo.SetShowOutdatedComments, repo.UpdateResolveConversation) + m.Post("/pull/{index}/target_branch", repo.UpdatePullRequestTarget) + }, reqUnitPullsReader) + }, reqSignIn, context.RepoAssignment, context.RepoMustNotBeArchived()) // end "/{username}/{reponame}": create or edit issues, pulls, labels, milestones m.Group("/{username}/{reponame}", func() { // repo code @@ -1324,7 +1326,7 @@ func registerRoutes(m *web.Router) { }, context.RepoMustNotBeArchived(), reqRepoCodeWriter, repo.MustBeNotEmpty) m.Combo("/fork").Get(repo.Fork).Post(web.Bind(forms.CreateRepoForm{}), repo.ForkPost) - }, reqSignIn, context.RepoAssignment, reqRepoCodeReader) + }, reqSignIn, context.RepoAssignment, reqUnitCodeReader) // end "/{username}/{reponame}": repo code m.Group("/{username}/{reponame}", func() { // repo tags @@ -1337,7 +1339,7 @@ func registerRoutes(m *web.Router) { repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true})) m.Post("/tags/delete", repo.DeleteTag, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoCodeWriter, context.RepoRef()) - }, optSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqUnitCodeReader) // end "/{username}/{reponame}": repo tags m.Group("/{username}/{reponame}", func() { // repo releases @@ -1440,43 +1442,48 @@ func registerRoutes(m *web.Router) { m.Group("/{username}/{reponame}/wiki", func() { m.Combo(""). Get(repo.Wiki). - Post(context.RepoMustNotBeArchived(), reqSignIn, reqRepoWikiWriter, web.Bind(forms.NewWikiForm{}), repo.WikiPost) + Post(context.RepoMustNotBeArchived(), reqSignIn, reqUnitWikiWriter, web.Bind(forms.NewWikiForm{}), repo.WikiPost) m.Combo("/*"). Get(repo.Wiki). - Post(context.RepoMustNotBeArchived(), reqSignIn, reqRepoWikiWriter, web.Bind(forms.NewWikiForm{}), repo.WikiPost) + Post(context.RepoMustNotBeArchived(), reqSignIn, reqUnitWikiWriter, web.Bind(forms.NewWikiForm{}), repo.WikiPost) m.Get("/blob_excerpt/{sha}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ExcerptBlob) m.Get("/commit/{sha:[a-f0-9]{7,64}}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.Diff) m.Get("/commit/{sha:[a-f0-9]{7,64}}.{ext:patch|diff}", repo.RawDiff) m.Get("/raw/*", repo.WikiRaw) - }, optSignIn, context.RepoAssignment, repo.MustEnableWiki, reqRepoWikiReader, func(ctx *context.Context) { + }, optSignIn, context.RepoAssignment, repo.MustEnableWiki, reqUnitWikiReader, func(ctx *context.Context) { ctx.Data["PageIsWiki"] = true ctx.Data["CloneButtonOriginLink"] = ctx.Repo.Repository.WikiCloneLink(ctx, ctx.Doer) }) // end "/{username}/{reponame}/wiki" m.Group("/{username}/{reponame}/activity", func() { + // activity has its own permission checks m.Get("", repo.Activity) m.Get("/{period}", repo.Activity) - m.Group("/contributors", func() { - m.Get("", repo.Contributors) - m.Get("/data", repo.ContributorsData) - }) - m.Group("/code-frequency", func() { - m.Get("", repo.CodeFrequency) - m.Get("/data", repo.CodeFrequencyData) - }) - m.Group("/recent-commits", func() { - m.Get("", repo.RecentCommits) - m.Get("/data", repo.RecentCommitsData) - }) + + m.Group("", func() { + m.Group("/contributors", func() { + m.Get("", repo.Contributors) + m.Get("/data", repo.ContributorsData) + }) + m.Group("/code-frequency", func() { + m.Get("", repo.CodeFrequency) + m.Get("/data", repo.CodeFrequencyData) + }) + m.Group("/recent-commits", func() { + m.Get("", repo.RecentCommits) + m.Get("/data", repo.RecentCommitsData) + }) + }, reqUnitCodeReader) }, - optSignIn, context.RepoAssignment, context.RequireRepoReaderOr(unit.TypePullRequests, unit.TypeIssues, unit.TypeReleases), - context.RepoRef(), repo.MustBeNotEmpty, + optSignIn, context.RepoAssignment, repo.MustBeNotEmpty, + context.RequireUnitReader(unit.TypeCode, unit.TypeIssues, unit.TypePullRequests, unit.TypeReleases), ) // end "/{username}/{reponame}/activity" m.Group("/{username}/{reponame}", func() { - m.Group("/pulls/{index}", func() { + m.Get("/{type:pulls}", repo.Issues) + m.Group("/{type:pulls}/{index}", func() { m.Get("", repo.SetWhitespaceBehavior, repo.GetPullDiffStats, repo.ViewIssue) m.Get(".diff", repo.DownloadPullDiff) m.Get(".patch", repo.DownloadPullPatch) @@ -1501,7 +1508,7 @@ func registerRoutes(m *web.Router) { }, context.RepoMustNotBeArchived()) }) }) - }, optSignIn, context.RepoAssignment, repo.MustAllowPulls, reqRepoPullsReader) + }, optSignIn, context.RepoAssignment, repo.MustAllowPulls, reqUnitPullsReader) // end "/{username}/{reponame}/pulls/{index}": repo pull request m.Group("/{username}/{reponame}", func() { @@ -1582,13 +1589,13 @@ func registerRoutes(m *web.Router) { m.Get("/forks", context.RepoRef(), repo.Forks) m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff) m.Post("/lastcommit/*", context.RepoRefByType(context.RepoRefCommit), repo.LastCommit) - }, optSignIn, context.RepoAssignment, reqRepoCodeReader) + }, optSignIn, context.RepoAssignment, reqUnitCodeReader) // end "/{username}/{reponame}": repo code m.Group("/{username}/{reponame}", func() { m.Get("/stars", repo.Stars) m.Get("/watchers", repo.Watchers) - m.Get("/search", reqRepoCodeReader, repo.Search) + m.Get("/search", reqUnitCodeReader, repo.Search) m.Post("/action/{action}", reqSignIn, repo.Action) }, optSignIn, context.RepoAssignment, context.RepoRef()) diff --git a/services/context/permission.go b/services/context/permission.go index 9338587257..359d51c272 100644 --- a/services/context/permission.go +++ b/services/context/permission.go @@ -9,24 +9,13 @@ import ( auth_model "code.gitea.io/gitea/models/auth" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" - "code.gitea.io/gitea/modules/log" ) // RequireRepoAdmin returns a middleware for requiring repository admin permission func RequireRepoAdmin() func(ctx *Context) { return func(ctx *Context) { if !ctx.IsSigned || !ctx.Repo.IsAdmin() { - ctx.NotFound(ctx.Req.URL.RequestURI(), nil) - return - } - } -} - -// RequireRepoWriter returns a middleware for requiring repository write to the specify unitType -func RequireRepoWriter(unitType unit.Type) func(ctx *Context) { - return func(ctx *Context) { - if !ctx.Repo.CanWrite(unitType) { - ctx.NotFound(ctx.Req.URL.RequestURI(), nil) + ctx.NotFound("RequireRepoAdmin denies the request", nil) return } } @@ -42,75 +31,30 @@ func CanEnableEditor() func(ctx *Context) { } } -// RequireRepoWriterOr returns a middleware for requiring repository write to one of the unit permission -func RequireRepoWriterOr(unitTypes ...unit.Type) func(ctx *Context) { +// RequireUnitWriter returns a middleware for requiring repository write to one of the unit permission +func RequireUnitWriter(unitTypes ...unit.Type) func(ctx *Context) { return func(ctx *Context) { for _, unitType := range unitTypes { if ctx.Repo.CanWrite(unitType) { return } } - ctx.NotFound(ctx.Req.URL.RequestURI(), nil) + ctx.NotFound("RequireUnitWriter denies the request", nil) } } -// RequireRepoReader returns a middleware for requiring repository read to the specify unitType -func RequireRepoReader(unitType unit.Type) func(ctx *Context) { - return func(ctx *Context) { - if !ctx.Repo.CanRead(unitType) { - if unitType == unit.TypeCode && canWriteAsMaintainer(ctx) { - return - } - if log.IsTrace() { - if ctx.IsSigned { - log.Trace("Permission Denied: User %-v cannot read %-v in Repo %-v\n"+ - "User in Repo has Permissions: %-+v", - ctx.Doer, - unitType, - ctx.Repo.Repository, - ctx.Repo.Permission) - } else { - log.Trace("Permission Denied: Anonymous user cannot read %-v in Repo %-v\n"+ - "Anonymous user in Repo has Permissions: %-+v", - unitType, - ctx.Repo.Repository, - ctx.Repo.Permission) - } - } - ctx.NotFound(ctx.Req.URL.RequestURI(), nil) - return - } - } -} - -// RequireRepoReaderOr returns a middleware for requiring repository write to one of the unit permission -func RequireRepoReaderOr(unitTypes ...unit.Type) func(ctx *Context) { +// RequireUnitReader returns a middleware for requiring repository write to one of the unit permission +func RequireUnitReader(unitTypes ...unit.Type) func(ctx *Context) { return func(ctx *Context) { for _, unitType := range unitTypes { if ctx.Repo.CanRead(unitType) { return } - } - if log.IsTrace() { - var format string - var args []any - if ctx.IsSigned { - format = "Permission Denied: User %-v cannot read [" - args = append(args, ctx.Doer) - } else { - format = "Permission Denied: Anonymous user cannot read [" + if unitType == unit.TypeCode && canWriteAsMaintainer(ctx) { + return } - for _, unit := range unitTypes { - format += "%-v, " - args = append(args, unit) - } - - format = format[:len(format)-2] + "] in Repo %-v\n" + - "User in Repo has Permissions: %-+v" - args = append(args, ctx.Repo.Repository, ctx.Repo.Permission) - log.Trace(format, args...) } - ctx.NotFound(ctx.Req.URL.RequestURI(), nil) + ctx.NotFound("RequireUnitReader denies the request", nil) } } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index 35ea5378d3..4f9806dc93 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -110,41 +110,51 @@ type RepoSettingForm struct { EnablePrune bool // Advanced settings - EnableCode bool - EnableWiki bool - EnableExternalWiki bool - DefaultWikiBranch string - DefaultWikiEveryoneAccess string - ExternalWikiURL string + EnableCode bool + DefaultCodeEveryoneAccess string + + EnableWiki bool + EnableExternalWiki bool + DefaultWikiBranch string + DefaultWikiEveryoneAccess string + ExternalWikiURL string + EnableIssues bool + DefaultIssuesEveryoneAccess string EnableExternalTracker bool ExternalTrackerURL string TrackerURLFormat string TrackerIssueStyle string ExternalTrackerRegexpPattern string EnableCloseIssuesViaCommitInAnyBranch bool - EnableProjects bool - ProjectsMode string - EnableReleases bool - EnablePackages bool - EnablePulls bool - EnableActions bool - PullsIgnoreWhitespace bool - PullsAllowMerge bool - PullsAllowRebase bool - PullsAllowRebaseMerge bool - PullsAllowSquash bool - PullsAllowFastForwardOnly bool - PullsAllowManualMerge bool - PullsDefaultMergeStyle string - EnableAutodetectManualMerge bool - PullsAllowRebaseUpdate bool - DefaultDeleteBranchAfterMerge bool - DefaultAllowMaintainerEdit bool - EnableTimetracker bool - AllowOnlyContributorsToTrackTime bool - EnableIssueDependencies bool - IsArchived bool + + EnableProjects bool + ProjectsMode string + + EnableReleases bool + + EnablePackages bool + + EnablePulls bool + PullsIgnoreWhitespace bool + PullsAllowMerge bool + PullsAllowRebase bool + PullsAllowRebaseMerge bool + PullsAllowSquash bool + PullsAllowFastForwardOnly bool + PullsAllowManualMerge bool + PullsDefaultMergeStyle string + EnableAutodetectManualMerge bool + PullsAllowRebaseUpdate bool + DefaultDeleteBranchAfterMerge bool + DefaultAllowMaintainerEdit bool + EnableTimetracker bool + AllowOnlyContributorsToTrackTime bool + EnableIssueDependencies bool + + EnableActions bool + + IsArchived bool // Signing Settings TrustModel string diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 21eaf6a651..cb596f013b 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -302,6 +302,15 @@ +
+ {{$unitCode := .Repository.MustGetUnit ctx ctx.Consts.RepoUnitTypeCode}} + + +
{{$isInternalWikiEnabled := .Repository.UnitEnabled ctx ctx.Consts.RepoUnitTypeWiki}} @@ -331,7 +340,7 @@
{{$unitInternalWiki := .Repository.MustGetUnit ctx ctx.Consts.RepoUnitTypeWiki}} - + + {{/* everyone access mode is different from others, none means it is unset and won't be applied */}} + + + +
{{if .Repository.CanEnableTimetracker}}
From 4672ddcdd7a9a1d27ebd4f22be0a911ccd3cb2c5 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 14 Jan 2025 14:44:12 +0900 Subject: [PATCH 3/8] Fix missing license when sync mirror (#33255) Fix #33222 --- models/repo/license.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/repo/license.go b/models/repo/license.go index 366b4598cc..9bcf0f7bc9 100644 --- a/models/repo/license.go +++ b/models/repo/license.go @@ -54,6 +54,7 @@ func UpdateRepoLicenses(ctx context.Context, repo *Repository, commitID string, for _, o := range oldLicenses { // Update already existing license if o.License == license { + o.CommitID = commitID if _, err := db.GetEngine(ctx).ID(o.ID).Cols("`commit_id`").Update(o); err != nil { return err } From 3a749fc816ed2957527bd9529e688adf84dadecf Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 14 Jan 2025 15:29:44 +0900 Subject: [PATCH 4/8] Fix 500 error when error occurred in migration page (#33256) The template should be `repo/migrate/{service type}` But input element `service` is not in the form. Related: #33081 --------- Co-authored-by: wxiaoguang --- templates/repo/migrate/codebase.tmpl | 4 +++- templates/repo/migrate/codecommit.tmpl | 4 +++- templates/repo/migrate/git.tmpl | 4 +++- templates/repo/migrate/gitbucket.tmpl | 4 +++- templates/repo/migrate/gitea.tmpl | 4 +++- templates/repo/migrate/github.tmpl | 4 +++- templates/repo/migrate/gitlab.tmpl | 4 +++- templates/repo/migrate/gogs.tmpl | 4 +++- templates/repo/migrate/onedev.tmpl | 4 +++- tests/integration/migrate_test.go | 6 +++++- 10 files changed, 32 insertions(+), 10 deletions(-) diff --git a/templates/repo/migrate/codebase.tmpl b/templates/repo/migrate/codebase.tmpl index 35f3614ec5..d4ca269f02 100644 --- a/templates/repo/migrate/codebase.tmpl +++ b/templates/repo/migrate/codebase.tmpl @@ -3,13 +3,15 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}}
{{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/codecommit.tmpl b/templates/repo/migrate/codecommit.tmpl index f75112f896..53ca7dda3d 100644 --- a/templates/repo/migrate/codecommit.tmpl +++ b/templates/repo/migrate/codecommit.tmpl @@ -3,13 +3,15 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/git.tmpl b/templates/repo/migrate/git.tmpl index b10c49c10e..3f447f76eb 100644 --- a/templates/repo/migrate/git.tmpl +++ b/templates/repo/migrate/git.tmpl @@ -3,13 +3,15 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/gitbucket.tmpl b/templates/repo/migrate/gitbucket.tmpl index 80d2491e91..559988951b 100644 --- a/templates/repo/migrate/gitbucket.tmpl +++ b/templates/repo/migrate/gitbucket.tmpl @@ -3,13 +3,15 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/gitea.tmpl b/templates/repo/migrate/gitea.tmpl index 220295662e..3d692129d5 100644 --- a/templates/repo/migrate/gitea.tmpl +++ b/templates/repo/migrate/gitea.tmpl @@ -3,12 +3,14 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/github.tmpl b/templates/repo/migrate/github.tmpl index d1aa4c1f29..850a2b3c71 100644 --- a/templates/repo/migrate/github.tmpl +++ b/templates/repo/migrate/github.tmpl @@ -3,12 +3,14 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/gitlab.tmpl b/templates/repo/migrate/gitlab.tmpl index 87a04d7849..9bafa122f1 100644 --- a/templates/repo/migrate/gitlab.tmpl +++ b/templates/repo/migrate/gitlab.tmpl @@ -3,12 +3,14 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/gogs.tmpl b/templates/repo/migrate/gogs.tmpl index a4d05e8acd..0495ce67fb 100644 --- a/templates/repo/migrate/gogs.tmpl +++ b/templates/repo/migrate/gogs.tmpl @@ -3,12 +3,14 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{.CsrfTokenHtml}} + + +
diff --git a/templates/repo/migrate/onedev.tmpl b/templates/repo/migrate/onedev.tmpl index a27188ed24..55945154ef 100644 --- a/templates/repo/migrate/onedev.tmpl +++ b/templates/repo/migrate/onedev.tmpl @@ -3,13 +3,15 @@

{{ctx.Locale.Tr "repo.migrate.migrate" .service.Title}} -

{{template "base/alert" .}} {{template "base/disable_form_autofill"}} {{.CsrfTokenHtml}} + + +
diff --git a/tests/integration/migrate_test.go b/tests/integration/migrate_test.go index 4c784dd22b..59dd6907db 100644 --- a/tests/integration/migrate_test.go +++ b/tests/integration/migrate_test.go @@ -79,8 +79,12 @@ func TestMigrateGiteaForm(t *testing.T) { resp := session.MakeRequest(t, req, http.StatusOK) // Step 2: load the form htmlDoc := NewHTMLParser(t, resp.Body) - link, exists := htmlDoc.doc.Find(`form.ui.form[action^="/repo/migrate"]`).Attr("action") + form := htmlDoc.doc.Find(`form.ui.form[action^="/repo/migrate"]`) + link, exists := form.Attr("action") assert.True(t, exists, "The template has changed") + serviceInput, exists := form.Find(`input[name="service"]`).Attr("value") + assert.True(t, exists) + assert.EqualValues(t, fmt.Sprintf("%d", structs.GiteaService), serviceInput) // Step 4: submit the migration to only migrate issues migratedRepoName := "otherrepo" req = NewRequestWithValues(t, "POST", link, map[string]string{ From 6410c34b7f6b7ee5daf27b057b197e63fdeb9be6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 13 Jan 2025 23:35:34 -0800 Subject: [PATCH 5/8] Refactor ref type (#33242) Major changes: 1. do not sync ".keep" file during tests 2. fix incorrect route handler and empty repo handling (backported as #33253 with tests) 3. do not use `RepoRef`: most of the calls are abuses. 4. Use `git.RefType` instead of a new type definition `RepoRefType` on `context`. --------- Co-authored-by: wxiaoguang --- models/unittest/fscopy.go | 50 +++++-------- routers/web/repo/repo.go | 2 +- routers/web/repo/view_home.go | 2 +- routers/web/web.go | 102 +++++++++++++-------------- services/context/permission.go | 4 +- services/context/repo.go | 78 +++++++------------- tests/integration/empty_repo_test.go | 25 +++++++ 7 files changed, 120 insertions(+), 143 deletions(-) diff --git a/models/unittest/fscopy.go b/models/unittest/fscopy.go index 690089bbc5..b7ba6b7ef5 100644 --- a/models/unittest/fscopy.go +++ b/models/unittest/fscopy.go @@ -11,35 +11,13 @@ import ( "code.gitea.io/gitea/modules/util" ) -// Copy copies file from source to target path. -func Copy(src, dest string) error { - // Gather file information to set back later. - si, err := os.Lstat(src) - if err != nil { - return err - } - - // Handle symbolic link. - if si.Mode()&os.ModeSymlink != 0 { - target, err := os.Readlink(src) - if err != nil { - return err - } - // NOTE: os.Chmod and os.Chtimes don't recognize symbolic link, - // which will lead "no such file or directory" error. - return os.Symlink(target, dest) - } - - return util.CopyFile(src, dest) -} - -// Sync synchronizes the two files. This is skipped if both files +// SyncFile synchronizes the two files. This is skipped if both files // exist and the size, modtime, and mode match. -func Sync(srcPath, destPath string) error { +func SyncFile(srcPath, destPath string) error { dest, err := os.Stat(destPath) if err != nil { if os.IsNotExist(err) { - return Copy(srcPath, destPath) + return util.CopyFile(srcPath, destPath) } return err } @@ -55,7 +33,7 @@ func Sync(srcPath, destPath string) error { return nil } - return Copy(srcPath, destPath) + return util.CopyFile(srcPath, destPath) } // SyncDirs synchronizes files recursively from source to target directory. @@ -66,6 +44,10 @@ func SyncDirs(srcPath, destPath string) error { return err } + // the keep file is used to keep the directory in a git repository, it doesn't need to be synced + // and go-git doesn't work with the ".keep" file (it would report errors like "ref is empty") + const keepFile = ".keep" + // find and delete all untracked files destFiles, err := util.ListDirRecursively(destPath, &util.ListDirOptions{IncludeDir: true}) if err != nil { @@ -73,16 +55,20 @@ func SyncDirs(srcPath, destPath string) error { } for _, destFile := range destFiles { destFilePath := filepath.Join(destPath, destFile) + shouldRemove := filepath.Base(destFilePath) == keepFile if _, err = os.Stat(filepath.Join(srcPath, destFile)); err != nil { if os.IsNotExist(err) { - // if src file does not exist, remove dest file - if err = os.RemoveAll(destFilePath); err != nil { - return err - } + shouldRemove = true } else { return err } } + // if src file does not exist, remove dest file + if shouldRemove { + if err = os.RemoveAll(destFilePath); err != nil { + return err + } + } } // sync src files to dest @@ -95,8 +81,8 @@ func SyncDirs(srcPath, destPath string) error { // util.ListDirRecursively appends a slash to the directory name if strings.HasSuffix(srcFile, "/") { err = os.MkdirAll(destFilePath, os.ModePerm) - } else { - err = Sync(filepath.Join(srcPath, srcFile), destFilePath) + } else if filepath.Base(destFilePath) != keepFile { + err = SyncFile(filepath.Join(srcPath, srcFile), destFilePath) } if err != nil { return err diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index e5c397ec54..6c2bc74e69 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -51,7 +51,7 @@ func MustBeNotEmpty(ctx *context.Context) { // MustBeEditable check that repo can be edited func MustBeEditable(ctx *context.Context) { - if !ctx.Repo.Repository.CanEnableEditor() || ctx.Repo.IsViewCommit { + if !ctx.Repo.Repository.CanEnableEditor() { ctx.NotFound("", nil) return } diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index 622072adeb..54a58cd8be 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -249,7 +249,7 @@ func handleRepoEmptyOrBroken(ctx *context.Context) { } else if reallyEmpty { showEmpty = true // the repo is really empty updateContextRepoEmptyAndStatus(ctx, true, repo_model.RepositoryReady) - } else if ctx.Repo.Commit == nil { + } else if branches, _, _ := ctx.Repo.GitRepo.GetBranches(0, 1); len(branches) == 0 { showEmpty = true // it is not really empty, but there is no branch // at the moment, other repo units like "actions" are not able to handle such case, // so we just mark the repo as empty to prevent from displaying these units. diff --git a/routers/web/web.go b/routers/web/web.go index d3d0360396..c2dac70546 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/metrics" "code.gitea.io/gitea/modules/public" @@ -817,7 +818,6 @@ func registerRoutes(m *web.Router) { reqRepoAdmin := context.RequireRepoAdmin() reqRepoCodeWriter := context.RequireUnitWriter(unit.TypeCode) - canEnableEditor := context.CanEnableEditor() reqRepoReleaseWriter := context.RequireUnitWriter(unit.TypeReleases) reqRepoReleaseReader := context.RequireUnitReader(unit.TypeReleases) reqRepoIssuesOrPullsWriter := context.RequireUnitWriter(unit.TypeIssues, unit.TypePullRequests) @@ -1152,16 +1152,16 @@ func registerRoutes(m *web.Router) { // end "/{username}/{reponame}/settings" // user/org home, including rss feeds - m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRef(), repo.SetEditorconfigIfExists, repo.Home) + m.Get("/{username}/{reponame}", optSignIn, context.RepoAssignment, context.RepoRefByType(git.RefTypeBranch), repo.SetEditorconfigIfExists, repo.Home) m.Post("/{username}/{reponame}/markup", optSignIn, context.RepoAssignment, reqUnitsWithMarkdown, web.Bind(structs.MarkupOption{}), misc.Markup) m.Group("/{username}/{reponame}", func() { m.Get("/find/*", repo.FindFiles) m.Group("/tree-list", func() { - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.TreeList) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.TreeList) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.TreeList) + m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.TreeList) + m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.TreeList) + m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.TreeList) }) m.Get("/compare", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.CompareDiff) m.Combo("/compare/*", repo.MustBeNotEmpty, repo.SetEditorconfigIfExists). @@ -1306,18 +1306,18 @@ func registerRoutes(m *web.Router) { Post(web.Bind(forms.EditRepoFileForm{}), repo.NewDiffPatchPost) m.Combo("/_cherrypick/{sha:([a-f0-9]{7,64})}/*").Get(repo.CherryPick). Post(web.Bind(forms.CherryPickForm{}), repo.CherryPickPost) - }, repo.MustBeEditable) + }, context.RepoRefByType(git.RefTypeBranch), context.CanWriteToBranch()) m.Group("", func() { m.Post("/upload-file", repo.UploadFileToServer) m.Post("/upload-remove", web.Bind(forms.RemoveUploadFileForm{}), repo.RemoveUploadFileFromServer) - }, repo.MustBeEditable, repo.MustBeAbleToUpload) - }, context.RepoRef(), canEnableEditor, context.RepoMustNotBeArchived()) + }, repo.MustBeAbleToUpload, reqRepoCodeWriter) + }, repo.MustBeEditable, context.RepoMustNotBeArchived()) m.Group("/branches", func() { m.Group("/_new", func() { - m.Post("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.CreateBranch) - m.Post("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.CreateBranch) - m.Post("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.CreateBranch) + m.Post("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.CreateBranch) + m.Post("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.CreateBranch) + m.Post("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.CreateBranch) }, web.Bind(forms.NewBranchForm{})) m.Post("/delete", repo.DeleteBranchPost) m.Post("/restore", repo.RestoreBranchPost) @@ -1332,39 +1332,36 @@ func registerRoutes(m *web.Router) { m.Group("/{username}/{reponame}", func() { // repo tags m.Group("/tags", func() { m.Get("", repo.TagsList) - m.Get("/list", repo.GetTagList) m.Get(".rss", feedEnabled, repo.TagsListFeedRSS) m.Get(".atom", feedEnabled, repo.TagsListFeedAtom) - }, ctxDataSet("EnableFeed", setting.Other.EnableFeed), - repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true})) - m.Post("/tags/delete", repo.DeleteTag, reqSignIn, - repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoCodeWriter, context.RepoRef()) - }, optSignIn, context.RepoAssignment, reqUnitCodeReader) + m.Get("/list", repo.GetTagList) + }, ctxDataSet("EnableFeed", setting.Other.EnableFeed)) + m.Post("/tags/delete", reqSignIn, reqRepoCodeWriter, context.RepoMustNotBeArchived(), repo.DeleteTag) + }, optSignIn, context.RepoAssignment, repo.MustBeNotEmpty, reqUnitCodeReader) // end "/{username}/{reponame}": repo tags m.Group("/{username}/{reponame}", func() { // repo releases m.Group("/releases", func() { m.Get("", repo.Releases) - m.Get("/tag/*", repo.SingleRelease) - m.Get("/latest", repo.LatestRelease) m.Get(".rss", feedEnabled, repo.ReleasesFeedRSS) m.Get(".atom", feedEnabled, repo.ReleasesFeedAtom) - }, ctxDataSet("EnableFeed", setting.Other.EnableFeed), - repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true})) - m.Get("/releases/attachments/{uuid}", repo.MustBeNotEmpty, repo.GetAttachment) - m.Get("/releases/download/{vTag}/{fileName}", repo.MustBeNotEmpty, repo.RedirectDownload) + m.Get("/tag/*", repo.SingleRelease) + m.Get("/latest", repo.LatestRelease) + }, ctxDataSet("EnableFeed", setting.Other.EnableFeed)) + m.Get("/releases/attachments/{uuid}", repo.GetAttachment) + m.Get("/releases/download/{vTag}/{fileName}", repo.RedirectDownload) m.Group("/releases", func() { m.Get("/new", repo.NewRelease) m.Post("/new", web.Bind(forms.NewReleaseForm{}), repo.NewReleasePost) m.Post("/delete", repo.DeleteRelease) m.Post("/attachments", repo.UploadReleaseAttachment) m.Post("/attachments/remove", repo.DeleteAttachment) - }, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, context.RepoRef()) + }, reqSignIn, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, context.RepoRef()) m.Group("/releases", func() { m.Get("/edit/*", repo.EditRelease) m.Post("/edit/*", web.Bind(forms.EditReleaseForm{}), repo.EditReleasePost) - }, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache) - }, optSignIn, context.RepoAssignment, reqRepoReleaseReader) + }, reqSignIn, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache) + }, optSignIn, context.RepoAssignment, repo.MustBeNotEmpty, reqRepoReleaseReader) // end "/{username}/{reponame}": repo releases m.Group("/{username}/{reponame}", func() { // to maintain compatibility with old attachments @@ -1528,42 +1525,39 @@ func registerRoutes(m *web.Router) { }, repo.MustBeNotEmpty, context.RepoRef()) m.Group("/media", func() { - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownloadOrLFS) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownloadOrLFS) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownloadOrLFS) m.Get("/blob/{sha}", repo.DownloadByIDOrLFS) - // "/*" route is deprecated, and kept for backward compatibility - m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownloadOrLFS) + m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.SingleDownloadOrLFS) + m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.SingleDownloadOrLFS) + m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.SingleDownloadOrLFS) + m.Get("/*", context.RepoRefByType(""), repo.SingleDownloadOrLFS) // "/*" route is deprecated, and kept for backward compatibility }, repo.MustBeNotEmpty) m.Group("/raw", func() { - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownload) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownload) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload) m.Get("/blob/{sha}", repo.DownloadByID) - // "/*" route is deprecated, and kept for backward compatibility - m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownload) + m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.SingleDownload) + m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.SingleDownload) + m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.SingleDownload) + m.Get("/*", context.RepoRefByType(""), repo.SingleDownload) // "/*" route is deprecated, and kept for backward compatibility }, repo.MustBeNotEmpty) m.Group("/render", func() { - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RenderFile) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RenderFile) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RenderFile) + m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.RenderFile) + m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.RenderFile) + m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.RenderFile) m.Get("/blob/{sha}", repo.RenderFile) }, repo.MustBeNotEmpty) m.Group("/commits", func() { - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RefCommits) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RefCommits) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RefCommits) - // "/*" route is deprecated, and kept for backward compatibility - m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.RefCommits) + m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.RefCommits) + m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.RefCommits) + m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.RefCommits) + m.Get("/*", context.RepoRefByType(""), repo.RefCommits) // "/*" route is deprecated, and kept for backward compatibility }, repo.MustBeNotEmpty) m.Group("/blame", func() { - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RefBlame) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RefBlame) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RefBlame) + m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.RefBlame) + m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.RefBlame) + m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.RefBlame) }, repo.MustBeNotEmpty) m.Get("/blob_excerpt/{sha}", repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.ExcerptBlob) @@ -1575,20 +1569,20 @@ func registerRoutes(m *web.Router) { m.Get("/cherry-pick/{sha:([a-f0-9]{7,64})$}", repo.SetEditorconfigIfExists, repo.CherryPick) }, repo.MustBeNotEmpty, context.RepoRef()) - m.Get("/rss/branch/*", context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed) - m.Get("/atom/branch/*", context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed) + m.Get("/rss/branch/*", context.RepoRefByType(git.RefTypeBranch), feedEnabled, feed.RenderBranchFeed) + m.Get("/atom/branch/*", context.RepoRefByType(git.RefTypeBranch), feedEnabled, feed.RenderBranchFeed) m.Group("/src", func() { m.Get("", func(ctx *context.Context) { ctx.Redirect(ctx.Repo.RepoLink) }) // there is no "{owner}/{repo}/src" page, so redirect to "{owner}/{repo}" to avoid 404 - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.Home) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.Home) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.Home) - m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.Home) // "/*" route is deprecated, and kept for backward compatibility + m.Get("/branch/*", context.RepoRefByType(git.RefTypeBranch), repo.Home) + m.Get("/tag/*", context.RepoRefByType(git.RefTypeTag), repo.Home) + m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.Home) + m.Get("/*", context.RepoRefByType(""), repo.Home) // "/*" route is deprecated, and kept for backward compatibility }, repo.SetEditorconfigIfExists) m.Get("/forks", context.RepoRef(), repo.Forks) m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff) - m.Post("/lastcommit/*", context.RepoRefByType(context.RepoRefCommit), repo.LastCommit) + m.Post("/lastcommit/*", context.RepoRefByType(git.RefTypeCommit), repo.LastCommit) }, optSignIn, context.RepoAssignment, reqUnitCodeReader) // end "/{username}/{reponame}": repo code diff --git a/services/context/permission.go b/services/context/permission.go index 359d51c272..0d69ccc4a4 100644 --- a/services/context/permission.go +++ b/services/context/permission.go @@ -21,8 +21,8 @@ func RequireRepoAdmin() func(ctx *Context) { } } -// CanEnableEditor checks if the user is allowed to write to the branch of the repo -func CanEnableEditor() func(ctx *Context) { +// CanWriteToBranch checks if the user is allowed to write to the branch of the repo +func CanWriteToBranch() func(ctx *Context) { return func(ctx *Context) { if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) { ctx.NotFound("CanWriteToBranch denies permission", nil) diff --git a/services/context/repo.go b/services/context/repo.go index cd5ac7c6c9..33a39dced1 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -677,24 +677,12 @@ func RepoAssignment(ctx *Context) { } } -// RepoRefType type of repo reference -type RepoRefType int - -const ( - // RepoRefUnknown is for legacy support, makes the code to "guess" the ref type - RepoRefUnknown RepoRefType = iota - RepoRefBranch - RepoRefTag - RepoRefCommit -) - const headRefName = "HEAD" -// RepoRef handles repository reference names when the ref name is not -// explicitly given func RepoRef() func(*Context) { - // since no ref name is explicitly specified, ok to just use branch - return RepoRefByType(RepoRefBranch) + // old code does: return RepoRefByType(git.RefTypeBranch) + // in most cases, it is an abuse, so we just disable it completely and fix the abuses one by one (if there is anything wrong) + return nil } func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool) string { @@ -710,29 +698,29 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool return "" } -func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (string, RepoRefType) { +func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (string, git.RefType) { reqRefPath := path.Join(extraRef, reqPath) reqRefPathParts := strings.Split(reqRefPath, "/") - if refName := getRefName(ctx, repo, reqRefPath, RepoRefBranch); refName != "" { - return refName, RepoRefBranch + if refName := getRefName(ctx, repo, reqRefPath, git.RefTypeBranch); refName != "" { + return refName, git.RefTypeBranch } - if refName := getRefName(ctx, repo, reqRefPath, RepoRefTag); refName != "" { - return refName, RepoRefTag + if refName := getRefName(ctx, repo, reqRefPath, git.RefTypeTag); refName != "" { + return refName, git.RefTypeTag } if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), reqRefPathParts[0]) { // FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists repo.TreePath = strings.Join(reqRefPathParts[1:], "/") - return reqRefPathParts[0], RepoRefCommit + return reqRefPathParts[0], git.RefTypeCommit } // FIXME: the old code falls back to default branch if "ref" doesn't exist, there could be an edge case: // "README?ref=no-such" would read the README file from the default branch, but the user might expect a 404 repo.TreePath = reqPath - return repo.Repository.DefaultBranch, RepoRefBranch + return repo.Repository.DefaultBranch, git.RefTypeBranch } -func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType) string { - switch pathType { - case RepoRefBranch: +func getRefName(ctx *Base, repo *Repository, path string, refType git.RefType) string { + switch refType { + case git.RefTypeBranch: ref := getRefNameFromPath(repo, path, repo.GitRepo.IsBranchExist) if len(ref) == 0 { // check if ref is HEAD @@ -762,9 +750,9 @@ func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType) } return ref - case RepoRefTag: + case git.RefTypeTag: return getRefNameFromPath(repo, path, repo.GitRepo.IsTagExist) - case RepoRefCommit: + case git.RefTypeCommit: parts := strings.Split(path, "/") if git.IsStringLikelyCommitID(repo.GetObjectFormat(), parts[0], 7) { // FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists @@ -782,22 +770,18 @@ func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType) return commit.ID.String() } default: - panic(fmt.Sprintf("Unrecognized path type: %v", pathType)) + panic(fmt.Sprintf("Unrecognized ref type: %v", refType)) } return "" } -type RepoRefByTypeOptions struct { - IgnoreNotExistErr bool -} - -func repoRefFullName(shortName string, typ RepoRefType) git.RefName { +func repoRefFullName(typ git.RefType, shortName string) git.RefName { switch typ { - case RepoRefBranch: + case git.RefTypeBranch: return git.RefNameFromBranch(shortName) - case RepoRefTag: + case git.RefTypeTag: return git.RefNameFromTag(shortName) - case RepoRefCommit: + case git.RefTypeCommit: return git.RefNameFromCommit(shortName) default: setting.PanicInDevOrTesting("Unknown RepoRefType: %v", typ) @@ -807,8 +791,7 @@ func repoRefFullName(shortName string, typ RepoRefType) git.RefName { // RepoRefByType handles repository reference name for a specific type // of repository reference -func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) { - opt := util.OptionalArg(opts) +func RepoRefByType(detectRefType git.RefType) func(*Context) { return func(ctx *Context) { var err error refType := detectRefType @@ -824,14 +807,6 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func return } - if ctx.Repo.GitRepo == nil { - ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository) - if err != nil { - ctx.ServerError(fmt.Sprintf("Open Repository %v failed", ctx.Repo.Repository.FullName()), err) - return - } - } - // Get default branch. var refShortName string reqPath := ctx.PathParam("*") @@ -861,13 +836,13 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func } ctx.Repo.IsViewBranch = true } else { // there is a path in request - guessLegacyPath := refType == RepoRefUnknown + guessLegacyPath := refType == "" if guessLegacyPath { refShortName, refType = getRefNameLegacy(ctx.Base, ctx.Repo, reqPath, "") } else { refShortName = getRefName(ctx.Base, ctx.Repo, reqPath, refType) } - ctx.Repo.RefFullName = repoRefFullName(refShortName, refType) + ctx.Repo.RefFullName = repoRefFullName(refType, refShortName) isRenamedBranch, has := ctx.Data["IsRenamedBranch"].(bool) if isRenamedBranch && has { renamedBranchName := ctx.Data["RenamedBranchName"].(string) @@ -877,7 +852,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func return } - if refType == RepoRefBranch && ctx.Repo.GitRepo.IsBranchExist(refShortName) { + if refType == git.RefTypeBranch && ctx.Repo.GitRepo.IsBranchExist(refShortName) { ctx.Repo.IsViewBranch = true ctx.Repo.BranchName = refShortName ctx.Repo.RefFullName = git.RefNameFromBranch(refShortName) @@ -888,7 +863,7 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func return } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if refType == RepoRefTag && ctx.Repo.GitRepo.IsTagExist(refShortName) { + } else if refType == git.RefTypeTag && ctx.Repo.GitRepo.IsTagExist(refShortName) { ctx.Repo.IsViewTag = true ctx.Repo.RefFullName = git.RefNameFromTag(refShortName) ctx.Repo.TagName = refShortName @@ -919,9 +894,6 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func ctx.RespHeader().Set("Link", fmt.Sprintf(`<%s>; rel="canonical"`, canonicalURL)) } } else { - if opt.IgnoreNotExistErr { - return - } ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refShortName)) return } diff --git a/tests/integration/empty_repo_test.go b/tests/integration/empty_repo_test.go index 8cab04a5a5..b19774a826 100644 --- a/tests/integration/empty_repo_test.go +++ b/tests/integration/empty_repo_test.go @@ -14,6 +14,7 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -24,6 +25,7 @@ import ( "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func testAPINewFile(t *testing.T, session *TestSession, user, repo, branch, treePath, content string) *httptest.ResponseRecorder { @@ -82,6 +84,29 @@ func TestEmptyRepoAddFile(t *testing.T) { req = NewRequest(t, "GET", redirect) resp = session.MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), "newly-added-test-file") + + // the repo is not empty anymore + req = NewRequest(t, "GET", "/user30/empty") + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "test-file.md") + + // if the repo is in incorrect state, it should be able to self-heal (recover to correct state) + user30EmptyRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 30, Name: "empty"}) + user30EmptyRepo.IsEmpty = true + user30EmptyRepo.DefaultBranch = "no-such" + _, err := db.GetEngine(db.DefaultContext).ID(user30EmptyRepo.ID).Cols("is_empty", "default_branch").Update(user30EmptyRepo) + require.NoError(t, err) + user30EmptyRepo = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerID: 30, Name: "empty"}) + assert.True(t, user30EmptyRepo.IsEmpty) + + req = NewRequest(t, "GET", "/user30/empty") + resp = session.MakeRequest(t, req, http.StatusSeeOther) + redirect = test.RedirectURL(resp) + assert.Equal(t, "/user30/empty", redirect) + + req = NewRequest(t, "GET", "/user30/empty") + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Contains(t, resp.Body.String(), "test-file.md") } func TestEmptyRepoUploadFile(t *testing.T) { From 5eff19a77a0e047d9712717ab4b4707eaceca642 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 15 Jan 2025 02:01:53 +0800 Subject: [PATCH 6/8] Fix sidebar milestone link (#33269) Fix #33266 --- templates/repo/issue/sidebar/milestone_list.tmpl | 8 ++++---- templates/repo/issue/sidebar/project_list.tmpl | 4 ++-- templates/repo/issue/sidebar/reviewer_list.tmpl | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/templates/repo/issue/sidebar/milestone_list.tmpl b/templates/repo/issue/sidebar/milestone_list.tmpl index 8b3e5b0eee..18a16dd9a6 100644 --- a/templates/repo/issue/sidebar/milestone_list.tmpl +++ b/templates/repo/issue/sidebar/milestone_list.tmpl @@ -24,7 +24,7 @@ {{if $data.OpenMilestones}}
{{ctx.Locale.Tr "repo.issues.filter_milestone_open"}}
{{range $data.OpenMilestones}} - + {{svg "octicon-milestone" 18}} {{.Name}} {{end}} @@ -33,7 +33,7 @@ {{if $data.ClosedMilestones}}
{{ctx.Locale.Tr "repo.issues.filter_milestone_closed"}}
{{range $data.ClosedMilestones}} - + {{svg "octicon-milestone" 18}} {{.Name}} {{end}} @@ -43,10 +43,10 @@
-
+
{{ctx.Locale.Tr "repo.issues.new.no_milestone"}} {{if $issueMilestone}} - + {{svg "octicon-milestone" 18}} {{$issueMilestone.Name}} {{end}} diff --git a/templates/repo/issue/sidebar/project_list.tmpl b/templates/repo/issue/sidebar/project_list.tmpl index 0cbbdce760..39feb089ef 100644 --- a/templates/repo/issue/sidebar/project_list.tmpl +++ b/templates/repo/issue/sidebar/project_list.tmpl @@ -39,10 +39,10 @@
-
+
{{ctx.Locale.Tr "repo.issues.new.no_projects"}} {{if $issueProject}} - + {{svg $issueProject.IconName 18}} {{$issueProject.Title}} {{end}} diff --git a/templates/repo/issue/sidebar/reviewer_list.tmpl b/templates/repo/issue/sidebar/reviewer_list.tmpl index b3d9590d58..b08af03a01 100644 --- a/templates/repo/issue/sidebar/reviewer_list.tmpl +++ b/templates/repo/issue/sidebar/reviewer_list.tmpl @@ -51,9 +51,9 @@
{{if .User}} - {{ctx.AvatarUtils.Avatar .User 20}} {{.User.GetDisplayName}} + {{ctx.AvatarUtils.Avatar .User 20}} {{.User.GetDisplayName}} {{else if .Team}} - {{svg "octicon-people" 20}} {{$repoOwnerName}}/{{.Team.Name}} + {{svg "octicon-people" 20}} {{$repoOwnerName}}/{{.Team.Name}} {{end}}
@@ -92,7 +92,7 @@
{{$originalURLHostname := $pageMeta.Repository.GetOriginalURLHostname}} {{$originalURL := $pageMeta.Repository.OriginalURL}} - + {{svg (MigrationIcon $originalURLHostname) 20}} {{.OriginalAuthor}}
From 1299fdb0849cac862b00717070c27846075c2e85 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 15 Jan 2025 02:30:43 +0800 Subject: [PATCH 7/8] Add a confirm dialog for "sync fork" (#33270) Try to quickly fix #33264 --- options/locale/locale_en-US.ini | 1 + templates/repo/code/upstream_diverging_info.tmpl | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index d336b6a700..e5fab5911a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1953,6 +1953,7 @@ pulls.upstream_diverging_prompt_behind_1 = This branch is %[1]d commit behind %[ pulls.upstream_diverging_prompt_behind_n = This branch is %[1]d commits behind %[2]s pulls.upstream_diverging_prompt_base_newer = The base branch %s has new changes pulls.upstream_diverging_merge = Sync fork +pulls.upstream_diverging_merge_confirm = Would you like to merge base repository's default branch onto this repository's branch %s? pull.deleted_branch = (deleted):%s pull.agit_documentation = Review documentation about AGit diff --git a/templates/repo/code/upstream_diverging_info.tmpl b/templates/repo/code/upstream_diverging_info.tmpl index bdcd99a7f7..a1f37b8c05 100644 --- a/templates/repo/code/upstream_diverging_info.tmpl +++ b/templates/repo/code/upstream_diverging_info.tmpl @@ -10,7 +10,10 @@ {{end}}
{{if .CanWriteCode}} - {{end}} From 4d399e717dd982fddd2e6e932a98cf757bab9207 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 15 Jan 2025 03:03:02 +0800 Subject: [PATCH 8/8] Fix some broken route handlers (#33268) Some mistakes introduced by recent refactoring PRs (some sidebar dropdowns doesn't work) --- routers/web/web.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index c2dac70546..1fc5bc6545 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1246,6 +1246,8 @@ func registerRoutes(m *web.Router) { m.Post("/attachments", repo.UploadIssueAttachment) m.Post("/attachments/remove", repo.DeleteAttachment) + m.Post("/labels", reqRepoIssuesOrPullsWriter, repo.UpdateIssueLabel) + m.Post("/milestone", reqRepoIssuesOrPullsWriter, repo.UpdateIssueMilestone) m.Post("/projects", reqRepoIssuesOrPullsWriter, reqRepoProjectsReader, repo.UpdateIssueProject) m.Post("/assignee", reqRepoIssuesOrPullsWriter, repo.UpdateIssueAssignee) m.Post("/status", reqRepoIssuesOrPullsWriter, repo.UpdateIssueStatus) @@ -1262,7 +1264,6 @@ func registerRoutes(m *web.Router) { m.Post("/reactions/{action}", web.Bind(forms.ReactionForm{}), repo.ChangeCommentReaction) }, reqRepoIssuesOrPullsReader) // edit issue/pull comment - m.Post("/labels", reqRepoIssuesOrPullsWriter, repo.UpdateIssueLabel) m.Group("/labels", func() { m.Post("/new", web.Bind(forms.CreateLabelForm{}), repo.NewLabel) m.Post("/edit", web.Bind(forms.CreateLabelForm{}), repo.UpdateLabel) @@ -1270,7 +1271,6 @@ func registerRoutes(m *web.Router) { m.Post("/initialize", web.Bind(forms.InitializeLabelsForm{}), repo.InitializeLabels) }, reqRepoIssuesOrPullsWriter, context.RepoRef()) - m.Post("/milestone", reqRepoIssuesOrPullsWriter, repo.UpdateIssueMilestone) m.Group("/milestones", func() { m.Combo("/new").Get(repo.NewMilestone). Post(web.Bind(forms.CreateMilestoneForm{}), repo.NewMilestonePost) @@ -1280,12 +1280,13 @@ func registerRoutes(m *web.Router) { m.Post("/delete", repo.DeleteMilestone) }, reqRepoIssuesOrPullsWriter, context.RepoRef()) - m.Group("", func() { + // FIXME: need to move these routes to the proper place + m.Group("/issues", func() { m.Post("/request_review", repo.UpdatePullReviewRequest) m.Post("/dismiss_review", reqRepoAdmin, web.Bind(forms.DismissReviewForm{}), repo.DismissReview) m.Post("/resolve_conversation", repo.SetShowOutdatedComments, repo.UpdateResolveConversation) - m.Post("/pull/{index}/target_branch", repo.UpdatePullRequestTarget) }, reqUnitPullsReader) + m.Post("/pull/{index}/target_branch", reqUnitPullsReader, repo.UpdatePullRequestTarget) }, reqSignIn, context.RepoAssignment, context.RepoMustNotBeArchived()) // end "/{username}/{reponame}": create or edit issues, pulls, labels, milestones