From a90af22003308e54dd4b0604b26f99c9325e37ab Mon Sep 17 00:00:00 2001 From: "Michael B." <153499594+mbollmann-v@users.noreply.github.com> Date: Mon, 13 Jan 2025 18:17:39 +0100 Subject: [PATCH 1/8] Let API create and edit system webhooks, attempt 2 (#33180) This PR fixes inconsistencies between system and default webhooks in the Gitea API. (See also #26418) - A system webhook is a webhook that captures events for all repositories. - A default webhook is copied to a new repository when it is created. Before this PR `POST /api/v1/admin/hooks/` creates default webhooks (if not configured otherwise) and `GET /api/v1/admin/hooks/` returns system webhooks. The PR introduces an optional query parameter to `GET /api/v1/admin/hooks/` to enable selecting if either default, system or both kind of webhooks should be retrieved. By default the flag is set to return system webhooks keep current behaviour. ## Examples ### System Webhooks #### Create ``` POST /api/v1/admin/hooks/ { "type": "gitea", "active": false, "branch_filter": "*", "events": [ "create", "..." ], "config": { "url": "http://...", "content_type": "json", "secret": "secret", "is_system_webhook": true // <-- controls hook type } } ``` #### List ``` GET/api/v1/admin/hooks?type=system //type argument is optional here since it's the default ``` #### Others The other relevant endpoints work as expected by referencing the hook by id ``` GET /api/v1/admin/hooks/:id PATCH /api/v1/admin/hooks/:id DELETE /api/v1/admin/hooks/:id ``` ### Default Webhooks #### Create ``` POST /api/v1/admin/hooks/ { "type": "gitea", "active": false, "branch_filter": "*", "events": [ "create", "..." ], "config": { "url": "http://...", "content_type": "json", "secret": "secret", "is_system_webhook": false // optional, as false is the default value } } ``` #### List ``` GET/api/v1/admin/hooks?type=default ``` #### Others The other relevant endpoints work as expected by referencing the hook by id ``` GET /api/v1/admin/hooks/:id PATCH /api/v1/admin/hooks/:id DELETE /api/v1/admin/hooks/:id ``` --- models/fixtures/webhook.yml | 21 +++++++++++++++ models/webhook/webhook_system.go | 13 ++++++++++ models/webhook/webhook_system_test.go | 37 +++++++++++++++++++++++++++ routers/api/v1/admin/hooks.go | 21 ++++++++++++++- templates/swagger/v1_json.tmpl | 12 +++++++++ 5 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 models/webhook/webhook_system_test.go diff --git a/models/fixtures/webhook.yml b/models/fixtures/webhook.yml index f62bae1f311ce..ebc4062b60ba5 100644 --- a/models/fixtures/webhook.yml +++ b/models/fixtures/webhook.yml @@ -22,6 +22,7 @@ content_type: 1 # json events: '{"push_only":false,"send_everything":false,"choose_events":false,"events":{"create":false,"push":true,"pull_request":true}}' is_active: true + - id: 4 repo_id: 2 @@ -29,3 +30,23 @@ content_type: 1 # json events: '{"push_only":true,"branch_filter":"{master,feature*}"}' is_active: true + +- + id: 5 + repo_id: 0 + owner_id: 0 + url: www.example.com/url5 + content_type: 1 # json + events: '{"push_only":true,"branch_filter":"{master,feature*}"}' + is_active: true + is_system_webhook: true + +- + id: 6 + repo_id: 0 + owner_id: 0 + url: www.example.com/url6 + content_type: 1 # json + events: '{"push_only":true,"branch_filter":"{master,feature*}"}' + is_active: true + is_system_webhook: false diff --git a/models/webhook/webhook_system.go b/models/webhook/webhook_system.go index a2a9ee321ae92..58d9d4a5c1b9d 100644 --- a/models/webhook/webhook_system.go +++ b/models/webhook/webhook_system.go @@ -11,6 +11,19 @@ import ( "code.gitea.io/gitea/modules/optional" ) +// GetSystemOrDefaultWebhooks returns webhooks by given argument or all if argument is missing. +func GetSystemOrDefaultWebhooks(ctx context.Context, isSystemWebhook optional.Option[bool]) ([]*Webhook, error) { + webhooks := make([]*Webhook, 0, 5) + if !isSystemWebhook.Has() { + return webhooks, db.GetEngine(ctx).Where("repo_id=? AND owner_id=?", 0, 0). + Find(&webhooks) + } + + return webhooks, db.GetEngine(ctx). + Where("repo_id=? AND owner_id=? AND is_system_webhook=?", 0, 0, isSystemWebhook.Value()). + Find(&webhooks) +} + // GetDefaultWebhooks returns all admin-default webhooks. func GetDefaultWebhooks(ctx context.Context) ([]*Webhook, error) { webhooks := make([]*Webhook, 0, 5) diff --git a/models/webhook/webhook_system_test.go b/models/webhook/webhook_system_test.go new file mode 100644 index 0000000000000..96157ed9c9d37 --- /dev/null +++ b/models/webhook/webhook_system_test.go @@ -0,0 +1,37 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package webhook + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/optional" + + "github.com/stretchr/testify/assert" +) + +func TestGetSystemOrDefaultWebhooks(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + hooks, err := GetSystemOrDefaultWebhooks(db.DefaultContext, optional.None[bool]()) + assert.NoError(t, err) + if assert.Len(t, hooks, 2) { + assert.Equal(t, int64(5), hooks[0].ID) + assert.Equal(t, int64(6), hooks[1].ID) + } + + hooks, err = GetSystemOrDefaultWebhooks(db.DefaultContext, optional.Some(true)) + assert.NoError(t, err) + if assert.Len(t, hooks, 1) { + assert.Equal(t, int64(5), hooks[0].ID) + } + + hooks, err = GetSystemOrDefaultWebhooks(db.DefaultContext, optional.Some(false)) + assert.NoError(t, err) + if assert.Len(t, hooks, 1) { + assert.Equal(t, int64(6), hooks[0].ID) + } +} diff --git a/routers/api/v1/admin/hooks.go b/routers/api/v1/admin/hooks.go index 6b4689047b5fe..c812ca182d502 100644 --- a/routers/api/v1/admin/hooks.go +++ b/routers/api/v1/admin/hooks.go @@ -34,11 +34,30 @@ func ListHooks(ctx *context.APIContext) { // in: query // description: page size of results // type: integer + // - type: string + // enum: + // - system + // - default + // - all + // description: system, default or both kinds of webhooks + // name: type + // default: system + // in: query + // // responses: // "200": // "$ref": "#/responses/HookList" - sysHooks, err := webhook.GetSystemWebhooks(ctx, optional.None[bool]()) + // for compatibility the default value is true + isSystemWebhook := optional.Some(true) + typeValue := ctx.FormString("type") + if typeValue == "default" { + isSystemWebhook = optional.Some(false) + } else if typeValue == "all" { + isSystemWebhook = optional.None[bool]() + } + + sysHooks, err := webhook.GetSystemOrDefaultWebhooks(ctx, isSystemWebhook) if err != nil { ctx.Error(http.StatusInternalServerError, "GetSystemWebhooks", err) return diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index fb37d45ce8de2..8082fc594ac02 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -234,6 +234,18 @@ "description": "page size of results", "name": "limit", "in": "query" + }, + { + "enum": [ + "system", + "default", + "all" + ], + "type": "string", + "default": "system", + "description": "system, default or both kinds of webhooks", + "name": "type", + "in": "query" } ], "responses": { From 98d7e04767fe2835da9d90fdb71bc4a8f8bdc57f Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 13 Jan 2025 21:57:52 +0100 Subject: [PATCH 2/8] Switch back to `vue-tsc` (#33248) It supports Typescript 5.7 now, so we can switch back to the official version. --- package-lock.json | 55 +++++++++++++++++++++++------------------------ package.json | 4 ++-- 2 files changed, 29 insertions(+), 30 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1e3c5ab155d47..89bffd0ff345f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -67,7 +67,6 @@ "devDependencies": { "@eslint-community/eslint-plugin-eslint-comments": "4.4.1", "@playwright/test": "1.49.1", - "@silverwind/vue-tsc": "2.1.13", "@stoplight/spectral-cli": "6.14.2", "@stylistic/eslint-plugin-js": "2.12.1", "@stylistic/stylelint-plugin": "3.1.1", @@ -112,7 +111,8 @@ "type-fest": "4.30.2", "updates": "16.4.1", "vite-string-plugin": "1.3.4", - "vitest": "2.1.8" + "vitest": "2.1.8", + "vue-tsc": "2.2.0" }, "engines": { "node": ">= 18.0.0" @@ -3643,24 +3643,6 @@ "hasInstallScript": true, "license": "Apache-2.0" }, - "node_modules/@silverwind/vue-tsc": { - "version": "2.1.13", - "resolved": "https://registry.npmjs.org/@silverwind/vue-tsc/-/vue-tsc-2.1.13.tgz", - "integrity": "sha512-ejFxz1KZiUGAESbC+eURnjqt0N95qkU9eZU7W15wgF9zV+v2FEu3ZLduuXTC7D/Sg6lL1R/QjPfUbxbAbBQOsw==", - "dev": true, - "license": "MIT", - "dependencies": { - "@volar/typescript": "~2.4.11", - "@vue/language-core": "2.1.10", - "semver": "^7.5.4" - }, - "bin": { - "vue-tsc": "bin/vue-tsc.js" - }, - "peerDependencies": { - "typescript": ">=5.0.0" - } - }, "node_modules/@silverwind/vue3-calendar-heatmap": { "version": "2.0.6", "resolved": "https://registry.npmjs.org/@silverwind/vue3-calendar-heatmap/-/vue3-calendar-heatmap-2.0.6.tgz", @@ -5251,17 +5233,17 @@ } }, "node_modules/@vue/language-core": { - "version": "2.1.10", - "resolved": "https://registry.npmjs.org/@vue/language-core/-/language-core-2.1.10.tgz", - "integrity": "sha512-DAI289d0K3AB5TUG3xDp9OuQ71CnrujQwJrQnfuZDwo6eGNf0UoRlPuaVNO+Zrn65PC3j0oB2i7mNmVPggeGeQ==", + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/@vue/language-core/-/language-core-2.2.0.tgz", + "integrity": "sha512-O1ZZFaaBGkKbsRfnVH1ifOK1/1BUkyK+3SQsfnh6PmMmD4qJcTU8godCeA96jjDRTL6zgnK7YzCHfaUlH2r0Mw==", "dev": true, "license": "MIT", "dependencies": { - "@volar/language-core": "~2.4.8", + "@volar/language-core": "~2.4.11", "@vue/compiler-dom": "^3.5.0", "@vue/compiler-vue2": "^2.7.16", "@vue/shared": "^3.5.0", - "alien-signals": "^0.2.0", + "alien-signals": "^0.4.9", "minimatch": "^9.0.3", "muggle-string": "^0.4.1", "path-browserify": "^1.0.1" @@ -5664,9 +5646,9 @@ } }, "node_modules/alien-signals": { - "version": "0.2.2", - "resolved": "https://registry.npmjs.org/alien-signals/-/alien-signals-0.2.2.tgz", - "integrity": "sha512-cZIRkbERILsBOXTQmMrxc9hgpxglstn69zm+F1ARf4aPAzdAFYd6sBq87ErO0Fj3DV94tglcyHG5kQz9nDC/8A==", + "version": "0.4.14", + "resolved": "https://registry.npmjs.org/alien-signals/-/alien-signals-0.4.14.tgz", + "integrity": "sha512-itUAVzhczTmP2U5yX67xVpsbbOiquusbWVyA9N+sy6+r6YVbFkahXvNCeEPWEOMhwDYwbVbGHFkVL03N9I5g+Q==", "dev": true, "license": "MIT" }, @@ -14526,6 +14508,23 @@ } } }, + "node_modules/vue-tsc": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/vue-tsc/-/vue-tsc-2.2.0.tgz", + "integrity": "sha512-gtmM1sUuJ8aSb0KoAFmK9yMxb8TxjewmxqTJ1aKphD5Cbu0rULFY6+UQT51zW7SpUcenfPUuflKyVwyx9Qdnxg==", + "dev": true, + "license": "MIT", + "dependencies": { + "@volar/typescript": "~2.4.11", + "@vue/language-core": "2.2.0" + }, + "bin": { + "vue-tsc": "bin/vue-tsc.js" + }, + "peerDependencies": { + "typescript": ">=5.0.0" + } + }, "node_modules/watchpack": { "version": "2.4.2", "resolved": "https://registry.npmjs.org/watchpack/-/watchpack-2.4.2.tgz", diff --git a/package.json b/package.json index 6881ddb306ce6..31a65c647c103 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,6 @@ "devDependencies": { "@eslint-community/eslint-plugin-eslint-comments": "4.4.1", "@playwright/test": "1.49.1", - "@silverwind/vue-tsc": "2.1.13", "@stoplight/spectral-cli": "6.14.2", "@stylistic/eslint-plugin-js": "2.12.1", "@stylistic/stylelint-plugin": "3.1.1", @@ -111,7 +110,8 @@ "type-fest": "4.30.2", "updates": "16.4.1", "vite-string-plugin": "1.3.4", - "vitest": "2.1.8" + "vitest": "2.1.8", + "vue-tsc": "2.2.0" }, "browserslist": [ "defaults" From 58ac17c005e9b554f419d179452d57b9e9062af5 Mon Sep 17 00:00:00 2001 From: GiteaBot Date: Tue, 14 Jan 2025 00:31:05 +0000 Subject: [PATCH 3/8] [skip ci] Updated translations via Crowdin --- options/locale/locale_cs-CZ.ini | 3 --- options/locale/locale_de-DE.ini | 3 --- options/locale/locale_el-GR.ini | 3 --- options/locale/locale_es-ES.ini | 3 --- options/locale/locale_fr-FR.ini | 3 --- options/locale/locale_ga-IE.ini | 3 --- options/locale/locale_ja-JP.ini | 3 --- options/locale/locale_lv-LV.ini | 3 --- options/locale/locale_pt-PT.ini | 3 --- options/locale/locale_ru-RU.ini | 2 -- options/locale/locale_tr-TR.ini | 3 --- options/locale/locale_zh-CN.ini | 3 --- options/locale/locale_zh-TW.ini | 3 --- 13 files changed, 38 deletions(-) diff --git a/options/locale/locale_cs-CZ.ini b/options/locale/locale_cs-CZ.ini index ad15d22dd2400..eb0e0dff97256 100644 --- a/options/locale/locale_cs-CZ.ini +++ b/options/locale/locale_cs-CZ.ini @@ -1115,9 +1115,6 @@ blame.ignore_revs=Ignorování revizí v .git-blame-ignorerevs. blame.ignore_revs.failed=Nepodařilo se ignorovat revize v .git-blame-ignore-revs. user_search_tooltip=Zobrazí maximálně 30 uživatelů -tree_path_not_found_commit=Cesta %[1]s v commitu %[2]s neexistuje -tree_path_not_found_branch=Cesta %[1]s ve větvi %[2]s neexistuje -tree_path_not_found_tag=Cesta %[1]s ve značce %[2]s neexistuje transfer.accept=Přijmout převod transfer.accept_desc=Převést do „%s“ diff --git a/options/locale/locale_de-DE.ini b/options/locale/locale_de-DE.ini index e805f7fe0ac39..120b3ea7db5f0 100644 --- a/options/locale/locale_de-DE.ini +++ b/options/locale/locale_de-DE.ini @@ -1111,9 +1111,6 @@ blame.ignore_revs=Revisionen in .git-blame-ignore-revs werden i blame.ignore_revs.failed=Fehler beim Ignorieren der Revisionen in .git-blame-ignore-revs. user_search_tooltip=Zeigt maximal 30 Benutzer -tree_path_not_found_commit=Pfad %[1]s existiert nicht in Commit%[2]s -tree_path_not_found_branch=Pfad %[1]s existiert nicht in Branch %[2]s -tree_path_not_found_tag=Pfad %[1]s existiert nicht in Tag %[2]s transfer.accept=Übertragung Akzeptieren transfer.accept_desc=`Übertragung nach "%s"` diff --git a/options/locale/locale_el-GR.ini b/options/locale/locale_el-GR.ini index 454e57eb128ab..af26256314ba1 100644 --- a/options/locale/locale_el-GR.ini +++ b/options/locale/locale_el-GR.ini @@ -992,9 +992,6 @@ blame_prior=Προβολή ευθύνης πριν από αυτή την αλλ blame.ignore_revs=Αγνόηση των αναθεωρήσεων στο .git-blame-ignore-revs. Πατήστε εδώ για να το παρακάμψετε και να δείτε την κανονική προβολή ευθυνών. blame.ignore_revs.failed=Αποτυχία αγνόησης των αναθεωρήσεων στο .git-blame-ignore-revs. -tree_path_not_found_commit=Η διαδρομή %[1]s δεν υπάρχει στην υποβολή %[2]s -tree_path_not_found_branch=Η διαδρομή %[1]s δεν υπάρχει στον κλάδο %[2]s -tree_path_not_found_tag=Η διαδρομή %[1]s δεν υπάρχει στην ετικέτα %[2]s transfer.accept=Αποδοχή Μεταφοράς transfer.reject=Απόρριψη Μεταφοράς diff --git a/options/locale/locale_es-ES.ini b/options/locale/locale_es-ES.ini index 7dd8030d2b0c6..e85e6b3399c1b 100644 --- a/options/locale/locale_es-ES.ini +++ b/options/locale/locale_es-ES.ini @@ -982,9 +982,6 @@ blame_prior=Ver la culpa antes de este cambio blame.ignore_revs=Ignorando revisiones en .git-blame-ignore-revs. Haga clic aquí para saltar y para a la vista normal. blame.ignore_revs.failed=No se pudieron ignorar las revisiones en .git-blame-ignore-revs. -tree_path_not_found_commit=La ruta %[1]s no existe en el commit %[2]s -tree_path_not_found_branch=La ruta %[1]s no existe en la rama %[2]s -tree_path_not_found_tag=La ruta %[1]s no existe en la etiqueta %[2]s transfer.accept=Aceptar transferencia transfer.reject=Rechazar transferencia diff --git a/options/locale/locale_fr-FR.ini b/options/locale/locale_fr-FR.ini index a63613cf886aa..e8c621a1fa184 100644 --- a/options/locale/locale_fr-FR.ini +++ b/options/locale/locale_fr-FR.ini @@ -1115,9 +1115,6 @@ blame.ignore_revs=Les révisions dans .git-blame-ignore-revs so blame.ignore_revs.failed=Impossible d'ignorer les révisions dans .git-blame-ignore-revs. user_search_tooltip=Affiche un maximum de 30 utilisateurs -tree_path_not_found_commit=Le chemin %[1]s n’existe pas dans la révision %[2]s. -tree_path_not_found_branch=Le chemin %[1]s n’existe pas dans la branche %[2]s. -tree_path_not_found_tag=Le chemin %[1]s n’existe pas dans l’étiquette %[2]s. transfer.accept=Accepter le transfert transfer.accept_desc=Transférer à « %s » diff --git a/options/locale/locale_ga-IE.ini b/options/locale/locale_ga-IE.ini index ef486b9e7a703..51c3ab20cc804 100644 --- a/options/locale/locale_ga-IE.ini +++ b/options/locale/locale_ga-IE.ini @@ -1115,9 +1115,6 @@ blame.ignore_revs=Ag déanamh neamhairde de leasuithe i .git-blame- blame.ignore_revs.failed=Theip ar neamhaird a dhéanamh ar leasuithe i .git-blame-ignore-revs. user_search_tooltip=Taispeáint uasmhéid de 30 úsáideoir -tree_path_not_found_commit=Níl cosán %[1]s ann i dtiomantas %[2]s -tree_path_not_found_branch=Níl cosán %[1]s ann i mbrainse %[2]s -tree_path_not_found_tag=Níl cosán %[1]s ann i gclib %[2]s transfer.accept=Glac le hAistriú transfer.accept_desc=Aistriú chuig “%s” diff --git a/options/locale/locale_ja-JP.ini b/options/locale/locale_ja-JP.ini index e5f3d14115177..7e39db8353842 100644 --- a/options/locale/locale_ja-JP.ini +++ b/options/locale/locale_ja-JP.ini @@ -1109,9 +1109,6 @@ blame_prior=この変更より前のBlameを表示 blame.ignore_revs=.git-blame-ignore-revs で指定されたリビジョンは除外しています。 これを迂回して通常のBlame表示を見るには ここをクリック。 blame.ignore_revs.failed=.git-blame-ignore-revs によるリビジョンの無視は失敗しました。 -tree_path_not_found_commit=パス %[1]s はコミット %[2]s に存在しません -tree_path_not_found_branch=パス %[1]s はブランチ %[2]s に存在しません -tree_path_not_found_tag=パス %[1]s はタグ %[2]s に存在しません transfer.accept=移転を承認 transfer.accept_desc=`"%s" に移転` diff --git a/options/locale/locale_lv-LV.ini b/options/locale/locale_lv-LV.ini index c90de9578b6e2..8e1c4a651cacf 100644 --- a/options/locale/locale_lv-LV.ini +++ b/options/locale/locale_lv-LV.ini @@ -997,9 +997,6 @@ blame_prior=Aplūkot vainīgo par izmaiņām pirms šīs revīzijas blame.ignore_revs=Neņem vērā izmaiņas no .git-blame-ignore-revs. Nospiediet šeit, lai to apietu un redzētu visu izmaiņu skatu. blame.ignore_revs.failed=Neizdevās neņemt vērā izmaiņas no .git-blam-ignore-revs. -tree_path_not_found_commit=Revīzijā %[2]s neeksistē ceļš %[1]s -tree_path_not_found_branch=Atzarā %[2]s nepastāv ceļš %[1]s -tree_path_not_found_tag=Tagā %[2]s nepastāv ceļš %[1]s transfer.accept=Apstiprināt īpašnieka maiņu transfer.reject=Noraidīt īpašnieka maiņu diff --git a/options/locale/locale_pt-PT.ini b/options/locale/locale_pt-PT.ini index 7b57e776d12d2..a0d0201a37cee 100644 --- a/options/locale/locale_pt-PT.ini +++ b/options/locale/locale_pt-PT.ini @@ -1115,9 +1115,6 @@ blame.ignore_revs=Ignorando as revisões em .git-blame-ignore-revs< blame.ignore_revs.failed=Falhou ao ignorar as revisões em .git-blame-ignore-revs. user_search_tooltip=Mostra um máximo de 30 utilizadores -tree_path_not_found_commit=A localização %[1]s não existe no cometimento %[2]s -tree_path_not_found_branch=A localização %[1]s não existe no ramo %[2]s -tree_path_not_found_tag=A localização %[1]s não existe na etiqueta %[2]s transfer.accept=Aceitar transferência transfer.accept_desc=`Transferir para "%s"` diff --git a/options/locale/locale_ru-RU.ini b/options/locale/locale_ru-RU.ini index 9c6c706288b86..17ebb275a3c7a 100644 --- a/options/locale/locale_ru-RU.ini +++ b/options/locale/locale_ru-RU.ini @@ -978,8 +978,6 @@ delete_preexisting_content=Удалить файлы из %s delete_preexisting_success=Удалены непринятые файлы в %s blame_prior=Показать авторство предшествующих изменений -tree_path_not_found_commit=Путь %[1]s не существует в коммите %[2]s -tree_path_not_found_branch=Путь %[1]s не существует в ветке %[2]s transfer.accept=Принять трансфер transfer.reject=Отказаться от перемещения diff --git a/options/locale/locale_tr-TR.ini b/options/locale/locale_tr-TR.ini index e939d9f04ea0a..8840b45a066cd 100644 --- a/options/locale/locale_tr-TR.ini +++ b/options/locale/locale_tr-TR.ini @@ -1083,9 +1083,6 @@ blame_prior=Bu değişiklikten önceki suçu görüntüle blame.ignore_revs=.git-blame-ignore-revs dosyasındaki sürümler yok sayılıyor. Bunun yerine normal sorumlu görüntüsü için buraya tıklayın. blame.ignore_revs.failed=.git-blame-ignore-revs dosyasındaki sürümler yok sayılamadı. -tree_path_not_found_commit=%[1] yolu, %[2]s işlemesinde mevcut değil -tree_path_not_found_branch=%[1] yolu, %[2]s dalında mevcut değil -tree_path_not_found_tag=%[1] yolu, %[2]s etiketinde mevcut değil transfer.accept=Aktarımı Kabul Et transfer.reject=Aktarımı Reddet diff --git a/options/locale/locale_zh-CN.ini b/options/locale/locale_zh-CN.ini index 564e9f9d8df1a..3215ea9045923 100644 --- a/options/locale/locale_zh-CN.ini +++ b/options/locale/locale_zh-CN.ini @@ -1111,9 +1111,6 @@ blame.ignore_revs=忽略 .git-blame-ignore-revs 的修订。点 blame.ignore_revs.failed=忽略 .git-blame-ignore-revs 版本失败。 user_search_tooltip=最多显示30名用户 -tree_path_not_found_commit=路径%[1]s 在提交 %[2]s 中不存在 -tree_path_not_found_branch=路径 %[1]s 不存在于分支 %[2]s 中。 -tree_path_not_found_tag=路径 %[1]s 不存在于标签 %[2]s 中 transfer.accept=接受转移 transfer.accept_desc=`转移到 "%s"` diff --git a/options/locale/locale_zh-TW.ini b/options/locale/locale_zh-TW.ini index b4183aa70a07f..eafe060ff4433 100644 --- a/options/locale/locale_zh-TW.ini +++ b/options/locale/locale_zh-TW.ini @@ -1108,9 +1108,6 @@ blame.ignore_revs=忽略 .git-blame-ignore-revs 中的修訂。 blame.ignore_revs.failed=忽略 .git-blame-ignore-revs 中的修訂失敗。 user_search_tooltip=顯示最多 30 個使用者 -tree_path_not_found_commit=路徑 %[1]s 在提交 %[2]s 中不存在 -tree_path_not_found_branch=路徑 %[1]s 在分支 %[2]s 中不存在 -tree_path_not_found_tag=路徑 %[1]s 在標籤 %[2]s 中不存在 transfer.accept=同意轉移 transfer.accept_desc=轉移到「%s」 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 4/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 | 228 +++++++++++++++--------------- 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, 238 insertions(+), 167 deletions(-) diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 53655703fcb9f..8909dedbb16a9 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,12 +329,42 @@ 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 + + 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 { + ctx.ServerError("GetRepoAssignees", err) + return + } + ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers) + + upload.AddUploadContext(ctx, "release") + + PrepareBranchList(ctx) // for New Release page +} + +// NewRelease render creating or edit release page +func NewRelease(ctx *context.Context) { + newReleaseCommon(ctx) + if ctx.Written() { + 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"); len(tagName) > 0 { + 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) @@ -344,59 +373,51 @@ func NewRelease(ctx *context.Context) { if rel != nil { rel.Repo = ctx.Repo.Repository - if err := rel.LoadAttributes(ctx); err != nil { + if err = rel.LoadAttributes(ctx); err != nil { ctx.ServerError("LoadAttributes", err) return } + ctx.Data["ShowCreateTagOnlyButton"] = false ctx.Data["tag_name"] = rel.TagName - if rel.Target != "" { - ctx.Data["tag_target"] = rel.Target - } + ctx.Data["tag_target"] = rel.Target ctx.Data["title"] = rel.Title ctx.Data["content"] = rel.Note ctx.Data["attachments"] = rel.Attachments } } - ctx.Data["IsAttachmentEnabled"] = setting.Attachment.Enabled - assigneeUsers, err := repo_model.GetRepoAssignees(ctx, ctx.Repo.Repository) - if err != nil { - ctx.ServerError("GetRepoAssignees", err) - return - } - ctx.Data["Assignees"] = shared_user.MakeSelfOnTop(ctx.Doer, assigneeUsers) - - upload.AddUploadContext(ctx, "release") - - // For New Release page - PrepareBranchList(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["Tags"] = tags ctx.HTML(http.StatusOK, tplReleaseNew) } // NewReleasePost response for creating a release func NewReleasePost(ctx *context.Context) { + newReleaseCommon(ctx) + if ctx.Written() { + return + } + 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) + // 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 } - ctx.Data["Tags"] = tags + // 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 - } - - rel, err := repo_model.GetRelease(ctx, ctx.Repo.Repository.ID, form.TagName) - if err != nil { - if !repo_model.IsErrReleaseNotExist(err) { - ctx.ServerError("GetRelease", err) - return - } - - msg := "" - if len(form.Title) > 0 && form.AddTagMsg { - msg = form.Title + "\n\n" + form.Content + 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) } + } - 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 - } + // prepare the git message for creating a new tag + newTagMsg := "" + if form.Title != "" && form.AddTagMsg { + newTagMsg = form.Title + "\n\n" + form.Content + } - ctx.Flash.Success(ctx.Tr("repo.tag.create_success", form.TagName)) - ctx.Redirect(ctx.Repo.RepoLink + "/src/tag/" + util.PathEscapeSegments(form.TagName)) + // 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 + } + + 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) + if err = release_service.CreateRelease(ctx.Repo.GitRepo, rel, attachmentUUIDs, newTagMsg); err != nil { + handleTagReleaseError(err) return } + ctx.Redirect(ctx.Repo.RepoLink + "/releases") + 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) - return - } + // 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 } - log.Trace("Release created: %s/%s:%s", ctx.Doer.LowerName, ctx.Repo.Repository.Name, form.TagName) + // 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 7ebea4c3fbe30..9f49fc750070c 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 b14171787e466..35ea5378d3a67 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 574b0d0311e05..8b6aa252affda 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 40bd798d16159..d3c4ed6a83eda 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 5/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 | 183 +++++++++++---------- services/context/permission.go | 74 +-------- services/forms/repo_form.go | 66 ++++---- templates/repo/settings/options.tmpl | 20 ++- 9 files changed, 218 insertions(+), 213 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 0ed116a132465..e00b7c5320fb7 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 50070c436845b..9862da06731f7 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 93155caa109f4..d336b6a700daa 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 91ef947cb46db..07059b9b7bfc3 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 7399c681e2e9d..5b34fc60da542 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 2f7ee8ade76fa..d3d0360396de4 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("/attachments", repo.UploadIssueAttachment) + m.Post("/attachments/remove", repo.DeleteAttachment) + 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.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 9338587257cdc..359d51c2726b7 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 [" - } - for _, unit := range unitTypes { - format += "%-v, " - args = append(args, unit) + if unitType == unit.TypeCode && canWriteAsMaintainer(ctx) { + return } - - 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 35ea5378d3a67..4f9806dc9374d 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 21eaf6a6518f5..cb596f013b724 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 6/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 366b4598cc4e0..9bcf0f7bc97f5 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 7/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 35f3614ec5632..d4ca269f020f1 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 f75112f896bbc..53ca7dda3d617 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 b10c49c10eec1..3f447f76eb333 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 80d2491e915e1..559988951b4c2 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 220295662e85a..3d692129d520b 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 d1aa4c1f29858..850a2b3c7181d 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 87a04d7849364..9bafa122f18ad 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 a4d05e8acdcf2..0495ce67fb2bd 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 a27188ed24296..55945154efd19 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 4c784dd22b931..59dd6907db471 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 8/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 690089bbc50b1..b7ba6b7ef5430 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 e5c397ec5415e..6c2bc74e69074 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 622072adeb719..54a58cd8be923 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 d3d0360396de4..c2dac70546d2d 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 359d51c2726b7..0d69ccc4a405c 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 cd5ac7c6c9ba1..33a39dced1b12 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 8cab04a5a5e49..b19774a826814 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) {