From 2e3a1910979bcf746079d595dd6892bddae8d515 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 5 Oct 2024 01:58:04 +0800 Subject: [PATCH] Fix javascript error when an anonymous user visiting migration page (#32144) (#32179) backport #32144 This PR fixes javascript errors when an anonymous user visits the migration page. It also makes task view checking more restrictive. The router moved from `/user/task/{id}/status` to `/username/reponame/-/migrate/status` because it's a migrate status. Co-authored-by: wxiaoguang --- models/admin/task.go | 21 ----------- routers/web/repo/migrate.go | 38 +++++++++++++++++++ routers/web/user/task.go | 53 --------------------------- routers/web/web.go | 8 +++- services/context/repo.go | 5 ++- templates/repo/migrate/migrating.tmpl | 2 +- web_src/js/features/repo-migrate.js | 8 ++-- 7 files changed, 53 insertions(+), 82 deletions(-) delete mode 100644 routers/web/user/task.go diff --git a/models/admin/task.go b/models/admin/task.go index c8bc95f981..10f8e6d570 100644 --- a/models/admin/task.go +++ b/models/admin/task.go @@ -179,27 +179,6 @@ func GetMigratingTask(ctx context.Context, repoID int64) (*Task, error) { return &task, nil } -// GetMigratingTaskByID returns the migrating task by repo's id -func GetMigratingTaskByID(ctx context.Context, id, doerID int64) (*Task, *migration.MigrateOptions, error) { - task := Task{ - ID: id, - DoerID: doerID, - Type: structs.TaskTypeMigrateRepo, - } - has, err := db.GetEngine(ctx).Get(&task) - if err != nil { - return nil, nil, err - } else if !has { - return nil, nil, ErrTaskDoesNotExist{id, 0, task.Type} - } - - var opts migration.MigrateOptions - if err := json.Unmarshal([]byte(task.PayloadContent), &opts); err != nil { - return nil, nil, err - } - return &task, &opts, nil -} - // CreateTask creates a task on database func CreateTask(ctx context.Context, task *Task) error { return db.Insert(ctx, task) diff --git a/routers/web/repo/migrate.go b/routers/web/repo/migrate.go index 97b0c425ea..97a5da15ee 100644 --- a/routers/web/repo/migrate.go +++ b/routers/web/repo/migrate.go @@ -15,6 +15,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -284,3 +285,40 @@ func MigrateCancelPost(ctx *context.Context) { } ctx.Redirect(ctx.Repo.Repository.Link()) } + +// MigrateStatus returns migrate task's status +func MigrateStatus(ctx *context.Context) { + task, err := admin_model.GetMigratingTask(ctx, ctx.Repo.Repository.ID) + if err != nil { + if admin_model.IsErrTaskDoesNotExist(err) { + ctx.JSON(http.StatusNotFound, map[string]any{ + "err": "task does not exist or you do not have access to this task", + }) + return + } + log.Error("GetMigratingTask: %v", err) + ctx.JSON(http.StatusInternalServerError, map[string]any{ + "err": http.StatusText(http.StatusInternalServerError), + }) + return + } + + message := task.Message + + if task.Message != "" && task.Message[0] == '{' { + // assume message is actually a translatable string + var translatableMessage admin_model.TranslatableMessage + if err := json.Unmarshal([]byte(message), &translatableMessage); err != nil { + translatableMessage = admin_model.TranslatableMessage{ + Format: "migrate.migrating_failed.error", + Args: []any{task.Message}, + } + } + message = ctx.Locale.TrString(translatableMessage.Format, translatableMessage.Args...) + } + + ctx.JSON(http.StatusOK, map[string]any{ + "status": task.Status, + "message": message, + }) +} diff --git a/routers/web/user/task.go b/routers/web/user/task.go deleted file mode 100644 index 8476767e9e..0000000000 --- a/routers/web/user/task.go +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package user - -import ( - "net/http" - "strconv" - - admin_model "code.gitea.io/gitea/models/admin" - "code.gitea.io/gitea/modules/json" - "code.gitea.io/gitea/services/context" -) - -// TaskStatus returns task's status -func TaskStatus(ctx *context.Context) { - task, opts, err := admin_model.GetMigratingTaskByID(ctx, ctx.ParamsInt64("task"), ctx.Doer.ID) - if err != nil { - if admin_model.IsErrTaskDoesNotExist(err) { - ctx.JSON(http.StatusNotFound, map[string]any{ - "error": "task `" + strconv.FormatInt(ctx.ParamsInt64("task"), 10) + "` does not exist", - }) - return - } - ctx.JSON(http.StatusInternalServerError, map[string]any{ - "err": err, - }) - return - } - - message := task.Message - - if task.Message != "" && task.Message[0] == '{' { - // assume message is actually a translatable string - var translatableMessage admin_model.TranslatableMessage - if err := json.Unmarshal([]byte(message), &translatableMessage); err != nil { - translatableMessage = admin_model.TranslatableMessage{ - Format: "migrate.migrating_failed.error", - Args: []any{task.Message}, - } - } - message = ctx.Locale.TrString(translatableMessage.Format, translatableMessage.Args...) - } - - ctx.JSON(http.StatusOK, map[string]any{ - "status": task.Status, - "message": message, - "repo-id": task.RepoID, - "repo-name": opts.RepoName, - "start": task.StartTime, - "end": task.EndTime, - }) -} diff --git a/routers/web/web.go b/routers/web/web.go index 328be992d4..5e19d1fd13 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -667,7 +667,6 @@ func registerRoutes(m *web.Route) { m.Get("/forgot_password", auth.ForgotPasswd) m.Post("/forgot_password", auth.ForgotPasswdPost) m.Post("/logout", auth.SignOut) - m.Get("/task/{task}", reqSignIn, user.TaskStatus) m.Get("/stopwatches", reqSignIn, user.GetStopwatches) m.Get("/search", ignExploreSignIn, user.Search) m.Group("/oauth2", func() { @@ -1036,6 +1035,13 @@ func registerRoutes(m *web.Route) { }, ignSignIn, context.UserAssignmentWeb(), context.OrgAssignment()) // end "/{username}/-": packages, projects, code + m.Group("/{username}/{reponame}/-", func() { + m.Group("/migrate", func() { + m.Get("/status", repo.MigrateStatus) + }) + }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) + // end "/{username}/{reponame}/-": migrate + m.Group("/{username}/{reponame}/settings", func() { m.Group("", func() { m.Combo("").Get(repo_setting.Settings). diff --git a/services/context/repo.go b/services/context/repo.go index 4836c1456c..c8005cfc72 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -607,7 +607,10 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } } - isHomeOrSettings := ctx.Link == ctx.Repo.RepoLink || ctx.Link == ctx.Repo.RepoLink+"/settings" || strings.HasPrefix(ctx.Link, ctx.Repo.RepoLink+"/settings/") + isHomeOrSettings := ctx.Link == ctx.Repo.RepoLink || + ctx.Link == ctx.Repo.RepoLink+"/settings" || + strings.HasPrefix(ctx.Link, ctx.Repo.RepoLink+"/settings/") || + ctx.Link == ctx.Repo.RepoLink+"/-/migrate/status" // Disable everything when the repo is being created if ctx.Repo.Repository.IsBeingCreated() || ctx.Repo.Repository.IsBroken() { diff --git a/templates/repo/migrate/migrating.tmpl b/templates/repo/migrate/migrating.tmpl index ed03db2ebd..bc07b488f2 100644 --- a/templates/repo/migrate/migrating.tmpl +++ b/templates/repo/migrate/migrating.tmpl @@ -7,7 +7,7 @@ {{template "base/alert" .}}
-
+
diff --git a/web_src/js/features/repo-migrate.js b/web_src/js/features/repo-migrate.js index 490e7df0e4..d6a7eeb125 100644 --- a/web_src/js/features/repo-migrate.js +++ b/web_src/js/features/repo-migrate.js @@ -1,19 +1,17 @@ import {hideElem, showElem} from '../utils/dom.js'; import {GET, POST} from '../modules/fetch.js'; -const {appSubUrl} = window.config; - export function initRepoMigrationStatusChecker() { const repoMigrating = document.getElementById('repo_migrating'); if (!repoMigrating) return; - document.getElementById('repo_migrating_retry').addEventListener('click', doMigrationRetry); + document.getElementById('repo_migrating_retry')?.addEventListener('click', doMigrationRetry); - const task = repoMigrating.getAttribute('data-migrating-task-id'); + const repoLink = repoMigrating.getAttribute('data-migrating-repo-link'); // returns true if the refresh still needs to be called after a while const refresh = async () => { - const res = await GET(`${appSubUrl}/user/task/${task}`); + const res = await GET(`${repoLink}/-/migrate/status`); if (res.status !== 200) return true; // continue to refresh if network error occurs const data = await res.json();