From f1bea3c3b878fba066bd25383f690a31fa9e5489 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 28 Nov 2024 02:50:54 +0100 Subject: [PATCH 1/5] Add webpack EnvironmentPlugin (#32661) Fixes: https://github.com/go-gitea/gitea/issues/32660 Environment vars in Webpack need to be declared in the config, otherwise they will not be elimininated during compilation. --- webpack.config.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/webpack.config.js b/webpack.config.js index 838fb51f43..276c758e2c 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -17,7 +17,7 @@ import tailwindcssNesting from 'tailwindcss/nesting/index.js'; import postcssNesting from 'postcss-nesting'; const {EsbuildPlugin} = EsBuildLoader; -const {SourceMapDevToolPlugin, DefinePlugin} = webpack; +const {SourceMapDevToolPlugin, DefinePlugin, EnvironmentPlugin} = webpack; const formatLicenseText = (licenseText) => wrapAnsi(licenseText || '', 80).trim(); const glob = (pattern) => fastGlob.sync(pattern, { @@ -213,6 +213,10 @@ export default { __VUE_PROD_DEVTOOLS__: false, // do not enable devtools support in production __VUE_PROD_HYDRATION_MISMATCH_DETAILS__: false, // https://github.com/vuejs/vue-cli/pull/7443 }), + // all environment variables used in bundled js via process.env must be declared here + new EnvironmentPlugin({ + TEST: 'false', + }), new VueLoaderPlugin(), new MiniCssExtractPlugin({ filename: 'css/[name].css', From 68d9f365437967e30c49550539f0e24de815408c Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Thu, 28 Nov 2024 10:15:59 +0800 Subject: [PATCH 2/5] Allow cropping an avatar before setting it (#32565) Provide a cropping tool on the avatar editing page, allowing users to select the cropping area themselves. This way, users can decide the displayed area of the image, rather than us deciding for them. --------- Co-authored-by: silverwind Co-authored-by: wxiaoguang Co-authored-by: delvh Co-authored-by: Giteabot --- options/locale/locale_en-US.ini | 1 + package-lock.json | 7 ++++ package.json | 1 + templates/user/settings/profile.tmpl | 5 +++ web_src/css/features/cropper.css | 6 +++ web_src/css/index.css | 1 + web_src/js/features/comp/Cropper.ts | 40 +++++++++++++++++++ .../features/repo-settings-branches.test.ts | 8 ++-- web_src/js/features/repo-settings-branches.ts | 2 +- web_src/js/features/repo-settings.ts | 4 +- web_src/js/features/user-settings.ts | 12 +++++- web_src/js/modules/sortable.ts | 2 +- 12 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 web_src/css/features/cropper.css create mode 100644 web_src/js/features/comp/Cropper.ts diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 9945eb4949..ffce4b7e2f 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -765,6 +765,7 @@ uploaded_avatar_not_a_image = The uploaded file is not an image. uploaded_avatar_is_too_big = The uploaded file size (%d KiB) exceeds the maximum size (%d KiB). update_avatar_success = Your avatar has been updated. update_user_avatar_success = The user's avatar has been updated. +cropper_prompt = You can edit the image before saving. The edited image will be saved as PNG. change_password = Update Password old_password = Current Password diff --git a/package-lock.json b/package-lock.json index 989c2bd77f..54e387a107 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22,6 +22,7 @@ "chartjs-adapter-dayjs-4": "1.0.4", "chartjs-plugin-zoom": "2.0.1", "clippie": "4.1.3", + "cropperjs": "1.6.2", "css-loader": "7.1.2", "dayjs": "1.11.13", "dropzone": "6.0.0-beta.2", @@ -6876,6 +6877,12 @@ } } }, + "node_modules/cropperjs": { + "version": "1.6.2", + "resolved": "https://registry.npmjs.org/cropperjs/-/cropperjs-1.6.2.tgz", + "integrity": "sha512-nhymn9GdnV3CqiEHJVai54TULFAE3VshJTXSqSJKa8yXAKyBKDWdhHarnlIPrshJ0WMFTGuFvG02YjLXfPiuOA==", + "license": "MIT" + }, "node_modules/cross-spawn": { "version": "7.0.5", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.5.tgz", diff --git a/package.json b/package.json index 03c3b79990..e596b444b6 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "chartjs-adapter-dayjs-4": "1.0.4", "chartjs-plugin-zoom": "2.0.1", "clippie": "4.1.3", + "cropperjs": "1.6.2", "css-loader": "7.1.2", "dayjs": "1.11.13", "dropzone": "6.0.0-beta.2", diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl index 9c7e2de218..f879587c71 100644 --- a/templates/user/settings/profile.tmpl +++ b/templates/user/settings/profile.tmpl @@ -127,6 +127,11 @@ +
+
{{ctx.Locale.Tr "settings.cropper_prompt"}}
+
+
+
diff --git a/web_src/css/features/cropper.css b/web_src/css/features/cropper.css new file mode 100644 index 0000000000..ed7171e770 --- /dev/null +++ b/web_src/css/features/cropper.css @@ -0,0 +1,6 @@ +@import "cropperjs/dist/cropper.css"; + +.page-content.user.profile .cropper-panel .cropper-wrapper { + max-width: 400px; + max-height: 400px; +} diff --git a/web_src/css/index.css b/web_src/css/index.css index 817f6997da..174a4a9cbc 100644 --- a/web_src/css/index.css +++ b/web_src/css/index.css @@ -40,6 +40,7 @@ @import "./features/codeeditor.css"; @import "./features/projects.css"; @import "./features/tribute.css"; +@import "./features/cropper.css"; @import "./features/console.css"; @import "./markup/content.css"; diff --git a/web_src/js/features/comp/Cropper.ts b/web_src/js/features/comp/Cropper.ts new file mode 100644 index 0000000000..3961b79b49 --- /dev/null +++ b/web_src/js/features/comp/Cropper.ts @@ -0,0 +1,40 @@ +import {showElem} from '../../utils/dom.ts'; + +type CropperOpts = { + container: HTMLElement, + imageSource: HTMLImageElement, + fileInput: HTMLInputElement, +} + +export async function initCompCropper({container, fileInput, imageSource}: CropperOpts) { + const {default: Cropper} = await import(/* webpackChunkName: "cropperjs" */'cropperjs'); + let currentFileName = ''; + let currentFileLastModified = 0; + const cropper = new Cropper(imageSource, { + aspectRatio: 1, + viewMode: 2, + autoCrop: false, + crop() { + const canvas = cropper.getCroppedCanvas(); + canvas.toBlob((blob) => { + const croppedFileName = currentFileName.replace(/\.[^.]{3,4}$/, '.png'); + const croppedFile = new File([blob], croppedFileName, {type: 'image/png', lastModified: currentFileLastModified}); + const dataTransfer = new DataTransfer(); + dataTransfer.items.add(croppedFile); + fileInput.files = dataTransfer.files; + }); + }, + }); + + fileInput.addEventListener('input', (e: Event & {target: HTMLInputElement}) => { + const files = e.target.files; + if (files?.length > 0) { + currentFileName = files[0].name; + currentFileLastModified = files[0].lastModified; + const fileURL = URL.createObjectURL(files[0]); + imageSource.src = fileURL; + cropper.replace(fileURL); + showElem(container); + } + }); +} diff --git a/web_src/js/features/repo-settings-branches.test.ts b/web_src/js/features/repo-settings-branches.test.ts index 023039334f..c4609999be 100644 --- a/web_src/js/features/repo-settings-branches.test.ts +++ b/web_src/js/features/repo-settings-branches.test.ts @@ -1,5 +1,5 @@ import {beforeEach, describe, expect, test, vi} from 'vitest'; -import {initRepoBranchesSettings} from './repo-settings-branches.ts'; +import {initRepoSettingsBranchesDrag} from './repo-settings-branches.ts'; import {POST} from '../modules/fetch.ts'; import {createSortable} from '../modules/sortable.ts'; @@ -31,7 +31,7 @@ describe('Repository Branch Settings', () => { }); test('should initialize sortable for protected branches list', () => { - initRepoBranchesSettings(); + initRepoSettingsBranchesDrag(); expect(createSortable).toHaveBeenCalledWith( document.querySelector('#protected-branches-list'), @@ -45,7 +45,7 @@ describe('Repository Branch Settings', () => { test('should not initialize if protected branches list is not present', () => { document.body.innerHTML = ''; - initRepoBranchesSettings(); + initRepoSettingsBranchesDrag(); expect(createSortable).not.toHaveBeenCalled(); }); @@ -59,7 +59,7 @@ describe('Repository Branch Settings', () => { return {destroy: vi.fn()}; }); - initRepoBranchesSettings(); + initRepoSettingsBranchesDrag(); expect(POST).toHaveBeenCalledWith( 'some/repo/branches/priority', diff --git a/web_src/js/features/repo-settings-branches.ts b/web_src/js/features/repo-settings-branches.ts index 43b98f79b3..40cdf9f981 100644 --- a/web_src/js/features/repo-settings-branches.ts +++ b/web_src/js/features/repo-settings-branches.ts @@ -3,7 +3,7 @@ import {POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; import {queryElemChildren} from '../utils/dom.ts'; -export function initRepoBranchesSettings() { +export function initRepoSettingsBranchesDrag() { const protectedBranchesList = document.querySelector('#protected-branches-list'); if (!protectedBranchesList) return; diff --git a/web_src/js/features/repo-settings.ts b/web_src/js/features/repo-settings.ts index 5a009cfea4..9ea546f76d 100644 --- a/web_src/js/features/repo-settings.ts +++ b/web_src/js/features/repo-settings.ts @@ -3,7 +3,7 @@ import {minimatch} from 'minimatch'; import {createMonaco} from './codeeditor.ts'; import {onInputDebounce, queryElems, toggleElem} from '../utils/dom.ts'; import {POST} from '../modules/fetch.ts'; -import {initRepoBranchesSettings} from './repo-settings-branches.ts'; +import {initRepoSettingsBranchesDrag} from './repo-settings-branches.ts'; const {appSubUrl, csrfToken} = window.config; @@ -155,5 +155,5 @@ export function initRepoSettings() { initRepoSettingsCollaboration(); initRepoSettingsSearchTeamBox(); initRepoSettingsGitHook(); - initRepoBranchesSettings(); + initRepoSettingsBranchesDrag(); } diff --git a/web_src/js/features/user-settings.ts b/web_src/js/features/user-settings.ts index 41939c0f52..c097df7b6c 100644 --- a/web_src/js/features/user-settings.ts +++ b/web_src/js/features/user-settings.ts @@ -1,7 +1,17 @@ import {hideElem, showElem} from '../utils/dom.ts'; +import {initCompCropper} from './comp/Cropper.ts'; + +function initUserSettingsAvatarCropper() { + const fileInput = document.querySelector('#new-avatar'); + const container = document.querySelector('.user.settings.profile .cropper-panel'); + const imageSource = container.querySelector('.cropper-source'); + initCompCropper({container, fileInput, imageSource}); +} export function initUserSettings() { - if (!document.querySelectorAll('.user.settings.profile').length) return; + if (!document.querySelector('.user.settings.profile')) return; + + initUserSettingsAvatarCropper(); const usernameInput = document.querySelector('#username'); if (!usernameInput) return; diff --git a/web_src/js/modules/sortable.ts b/web_src/js/modules/sortable.ts index c31135357c..b318386d08 100644 --- a/web_src/js/modules/sortable.ts +++ b/web_src/js/modules/sortable.ts @@ -1,6 +1,6 @@ import type {SortableOptions, SortableEvent} from 'sortablejs'; -export async function createSortable(el: HTMLElement, opts: {handle?: string} & SortableOptions = {}) { +export async function createSortable(el: Element, opts: {handle?: string} & SortableOptions = {}) { // @ts-expect-error: wrong type derived by typescript const {Sortable} = await import(/* webpackChunkName: "sortablejs" */'sortablejs'); From 16a7d343d78807e39df124756e5d43a69a2203a3 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Wed, 27 Nov 2024 20:50:27 -0600 Subject: [PATCH 3/5] Validate OAuth Redirect URIs (#32643) This fixes a TODO in the code to validate the RedirectURIs when adding or editing an OAuth application in user settings. This also includes a refactor of the user settings tests to only create the DB once per top-level test to avoid reloading fixtures. --- modules/util/truncate.go | 2 + modules/validation/binding.go | 37 ++++- modules/validation/binding_test.go | 1 + modules/validation/validurllist_test.go | 157 ++++++++++++++++++++++ routers/web/user/setting/oauth2_common.go | 17 ++- services/forms/user_form.go | 2 +- tests/integration/user_settings_test.go | 117 ++++++++++++---- 7 files changed, 302 insertions(+), 31 deletions(-) create mode 100644 modules/validation/validurllist_test.go diff --git a/modules/util/truncate.go b/modules/util/truncate.go index 77b116eeff..f2edbdc673 100644 --- a/modules/util/truncate.go +++ b/modules/util/truncate.go @@ -41,6 +41,8 @@ func SplitStringAtByteN(input string, n int) (left, right string) { // SplitTrimSpace splits the string at given separator and trims leading and trailing space func SplitTrimSpace(input, sep string) []string { + // Trim initial leading & trailing space + input = strings.TrimSpace(input) // replace CRLF with LF input = strings.ReplaceAll(input, "\r\n", "\n") diff --git a/modules/validation/binding.go b/modules/validation/binding.go index cb0a5063e5..75190e31e1 100644 --- a/modules/validation/binding.go +++ b/modules/validation/binding.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/modules/auth" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/util" "gitea.com/go-chi/binding" "github.com/gobwas/glob" @@ -31,6 +32,7 @@ const ( // AddBindingRules adds additional binding rules func AddBindingRules() { addGitRefNameBindingRule() + addValidURLListBindingRule() addValidURLBindingRule() addValidSiteURLBindingRule() addGlobPatternRule() @@ -44,7 +46,7 @@ func addGitRefNameBindingRule() { // Git refname validation rule binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "GitRefName") + return rule == "GitRefName" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { str := fmt.Sprintf("%v", val) @@ -58,11 +60,38 @@ func addGitRefNameBindingRule() { }) } +func addValidURLListBindingRule() { + // URL validation rule + binding.AddRule(&binding.Rule{ + IsMatch: func(rule string) bool { + return rule == "ValidUrlList" + }, + IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { + str := fmt.Sprintf("%v", val) + if len(str) == 0 { + errs.Add([]string{name}, binding.ERR_URL, "Url") + return false, errs + } + + ok := true + urls := util.SplitTrimSpace(str, "\n") + for _, u := range urls { + if !IsValidURL(u) { + ok = false + errs.Add([]string{name}, binding.ERR_URL, u) + } + } + + return ok, errs + }, + }) +} + func addValidURLBindingRule() { // URL validation rule binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "ValidUrl") + return rule == "ValidUrl" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { str := fmt.Sprintf("%v", val) @@ -80,7 +109,7 @@ func addValidSiteURLBindingRule() { // URL validation rule binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "ValidSiteUrl") + return rule == "ValidSiteUrl" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { str := fmt.Sprintf("%v", val) @@ -171,7 +200,7 @@ func addUsernamePatternRule() { func addValidGroupTeamMapRule() { binding.AddRule(&binding.Rule{ IsMatch: func(rule string) bool { - return strings.HasPrefix(rule, "ValidGroupTeamMap") + return rule == "ValidGroupTeamMap" }, IsValid: func(errs binding.Errors, name string, val any) (bool, binding.Errors) { _, err := auth.UnmarshalGroupTeamMapping(fmt.Sprintf("%v", val)) diff --git a/modules/validation/binding_test.go b/modules/validation/binding_test.go index 01ff4e3435..28d0f57b5c 100644 --- a/modules/validation/binding_test.go +++ b/modules/validation/binding_test.go @@ -27,6 +27,7 @@ type ( TestForm struct { BranchName string `form:"BranchName" binding:"GitRefName"` URL string `form:"ValidUrl" binding:"ValidUrl"` + URLs string `form:"ValidUrls" binding:"ValidUrlList"` GlobPattern string `form:"GlobPattern" binding:"GlobPattern"` RegexPattern string `form:"RegexPattern" binding:"RegexPattern"` } diff --git a/modules/validation/validurllist_test.go b/modules/validation/validurllist_test.go new file mode 100644 index 0000000000..c6f862a962 --- /dev/null +++ b/modules/validation/validurllist_test.go @@ -0,0 +1,157 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package validation + +import ( + "testing" + + "gitea.com/go-chi/binding" +) + +// This is a copy of all the URL tests cases, plus additional ones to +// account for multiple URLs +var urlListValidationTestCases = []validationTestCase{ + { + description: "Empty URL", + data: TestForm{ + URLs: "", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL without port", + data: TestForm{ + URLs: "http://test.lan/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with port", + data: TestForm{ + URLs: "http://test.lan:3000/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with IPv6 address without port", + data: TestForm{ + URLs: "http://[::1]/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "URL with IPv6 address with port", + data: TestForm{ + URLs: "http://[::1]:3000/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Invalid URL", + data: TestForm{ + URLs: "http//test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http//test.lan/", + }, + }, + }, + { + description: "Invalid schema", + data: TestForm{ + URLs: "ftp://test.lan/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "ftp://test.lan/", + }, + }, + }, + { + description: "Invalid port", + data: TestForm{ + URLs: "http://test.lan:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://test.lan:3x4/", + }, + }, + }, + { + description: "Invalid port with IPv6 address", + data: TestForm{ + URLs: "http://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, + }, + { + description: "Multi URLs", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://test.local/", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "Multi URLs with newline", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://test.local/\n", + }, + expectedErrors: binding.Errors{}, + }, + { + description: "List with invalid entry", + data: TestForm{ + URLs: "http://test.lan:3000/\nhttp://[::1]:3x4/", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, + }, + { + description: "List with two invalid entries", + data: TestForm{ + URLs: "ftp://test.lan:3000/\nhttp://[::1]:3x4/\n", + }, + expectedErrors: binding.Errors{ + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "ftp://test.lan:3000/", + }, + binding.Error{ + FieldNames: []string{"URLs"}, + Classification: binding.ERR_URL, + Message: "http://[::1]:3x4/", + }, + }, + }, +} + +func Test_ValidURLListValidation(t *testing.T) { + AddBindingRules() + + for _, testCase := range urlListValidationTestCases { + t.Run(testCase.description, func(t *testing.T) { + performValidationTest(t, testCase) + }) + } +} diff --git a/routers/web/user/setting/oauth2_common.go b/routers/web/user/setting/oauth2_common.go index 6029d7bedb..e93e9e1954 100644 --- a/routers/web/user/setting/oauth2_common.go +++ b/routers/web/user/setting/oauth2_common.go @@ -47,7 +47,6 @@ func (oa *OAuth2CommonHandlers) AddApp(ctx *context.Context) { return } - // TODO validate redirect URI app, err := auth.CreateOAuth2Application(ctx, auth.CreateOAuth2ApplicationOptions{ Name: form.Name, RedirectURIs: util.SplitTrimSpace(form.RedirectURIs, "\n"), @@ -96,11 +95,25 @@ func (oa *OAuth2CommonHandlers) EditSave(ctx *context.Context) { form := web.GetForm(ctx).(*forms.EditOAuth2ApplicationForm) if ctx.HasError() { + app, err := auth.GetOAuth2ApplicationByID(ctx, ctx.PathParamInt64("id")) + if err != nil { + if auth.IsErrOAuthApplicationNotFound(err) { + ctx.NotFound("Application not found", err) + return + } + ctx.ServerError("GetOAuth2ApplicationByID", err) + return + } + if app.UID != oa.OwnerID { + ctx.NotFound("Application not found", nil) + return + } + ctx.Data["App"] = app + oa.renderEditPage(ctx) return } - // TODO validate redirect URI var err error if ctx.Data["App"], err = auth.UpdateOAuth2Application(ctx, auth.UpdateOAuth2ApplicationOptions{ ID: ctx.PathParamInt64("id"), diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 5b7a43642a..ed79936add 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -366,7 +366,7 @@ func (f *NewAccessTokenForm) GetScope() (auth_model.AccessTokenScope, error) { // EditOAuth2ApplicationForm form for editing oauth2 applications type EditOAuth2ApplicationForm struct { Name string `binding:"Required;MaxSize(255)" form:"application_name"` - RedirectURIs string `binding:"Required" form:"redirect_uris"` + RedirectURIs string `binding:"Required;ValidUrlList" form:"redirect_uris"` ConfidentialClient bool `form:"confidential_client"` SkipSecondaryAuthorization bool `form:"skip_secondary_authorization"` } diff --git a/tests/integration/user_settings_test.go b/tests/integration/user_settings_test.go index 2103c92d58..d8402eb25f 100644 --- a/tests/integration/user_settings_test.go +++ b/tests/integration/user_settings_test.go @@ -10,6 +10,8 @@ import ( "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" ) // Validate that each navbar setting is correct. This checks that the @@ -51,8 +53,10 @@ func WithDisabledFeatures(t *testing.T, features ...string) { } func TestUserSettingsAccount(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("all features enabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() session := loginUser(t, "user2") req := NewRequest(t, "GET", "/user/settings/account") @@ -68,7 +72,7 @@ func TestUserSettingsAccount(t *testing.T) { }) t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -85,7 +89,7 @@ func TestUserSettingsAccount(t *testing.T) { }) t.Run("deletion disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureDeletion) @@ -102,7 +106,7 @@ func TestUserSettingsAccount(t *testing.T) { }) t.Run("deletion, credentials and email notifications are disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() mail := setting.Service.EnableNotifyMail setting.Service.EnableNotifyMail = false @@ -119,8 +123,10 @@ func TestUserSettingsAccount(t *testing.T) { } func TestUserSettingsUpdatePassword(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("enabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() session := loginUser(t, "user2") @@ -138,7 +144,7 @@ func TestUserSettingsUpdatePassword(t *testing.T) { }) t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -156,8 +162,10 @@ func TestUserSettingsUpdatePassword(t *testing.T) { } func TestUserSettingsUpdateEmail(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -175,8 +183,10 @@ func TestUserSettingsUpdateEmail(t *testing.T) { } func TestUserSettingsDeleteEmail(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) @@ -194,8 +204,10 @@ func TestUserSettingsDeleteEmail(t *testing.T) { } func TestUserSettingsDelete(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("deletion disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureDeletion) @@ -224,9 +236,10 @@ func TestUserSettingsAppearance(t *testing.T) { } func TestUserSettingsSecurity(t *testing.T) { - t.Run("credentials disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrepareTestEnv(t)() + t.Run("credentials disabled", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials) session := loginUser(t, "user2") @@ -240,8 +253,7 @@ func TestUserSettingsSecurity(t *testing.T) { }) t.Run("mfa disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() - + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageMFA) session := loginUser(t, "user2") @@ -255,8 +267,7 @@ func TestUserSettingsSecurity(t *testing.T) { }) t.Run("credentials and mfa disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() - + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageCredentials, setting.UserFeatureManageMFA) session := loginUser(t, "user2") @@ -268,17 +279,75 @@ func TestUserSettingsSecurity(t *testing.T) { func TestUserSettingsApplications(t *testing.T) { defer tests.PrepareTestEnv(t)() - session := loginUser(t, "user2") - req := NewRequest(t, "GET", "/user/settings/applications") - resp := session.MakeRequest(t, req, http.StatusOK) - doc := NewHTMLParser(t, resp.Body) + t.Run("Applications", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - assertNavbar(t, doc) + session := loginUser(t, "user2") + req := NewRequest(t, "GET", "/user/settings/applications") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + assertNavbar(t, doc) + }) + + t.Run("OAuth2", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, "user2") + + t.Run("OAuth2ApplicationShow", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user/settings/applications/oauth2/2") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + assertNavbar(t, doc) + }) + + t.Run("OAuthApplicationsEdit", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequest(t, "GET", "/user/settings/applications/oauth2/2") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + t.Run("Invalid URL", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user/settings/applications/oauth2/2", map[string]string{ + "_csrf": doc.GetCSRF(), + "application_name": "Test native app", + "redirect_uris": "ftp://127.0.0.1", + "confidential_client": "false", + }) + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + + msg := doc.Find(".flash-error p").Text() + assert.Equal(t, `form.RedirectURIs"ftp://127.0.0.1" is not a valid URL.`, msg) + }) + + t.Run("OK", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user/settings/applications/oauth2/2", map[string]string{ + "_csrf": doc.GetCSRF(), + "application_name": "Test native app", + "redirect_uris": "http://127.0.0.1", + "confidential_client": "false", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + }) + }) + }) } func TestUserSettingsKeys(t *testing.T) { + defer tests.PrepareTestEnv(t)() + t.Run("all enabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() session := loginUser(t, "user2") req := NewRequest(t, "GET", "/user/settings/keys") @@ -292,7 +361,7 @@ func TestUserSettingsKeys(t *testing.T) { }) t.Run("ssh keys disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageSSHKeys) @@ -308,7 +377,7 @@ func TestUserSettingsKeys(t *testing.T) { }) t.Run("gpg keys disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageGPGKeys) @@ -324,7 +393,7 @@ func TestUserSettingsKeys(t *testing.T) { }) t.Run("ssh & gpg keys disabled", func(t *testing.T) { - defer tests.PrepareTestEnv(t)() + defer tests.PrintCurrentTest(t)() WithDisabledFeatures(t, setting.UserFeatureManageSSHKeys, setting.UserFeatureManageGPGKeys) From 1b296ed1a422b8f29ea0c0e887f6470b8895116c Mon Sep 17 00:00:00 2001 From: Pedro Nishiyama Date: Thu, 28 Nov 2024 04:18:23 -0300 Subject: [PATCH 4/5] Allow users with write permission to run actions (#32644) --- I have a use case where I need a team to be able to run actions without admin access. --- routers/web/repo/actions/actions.go | 4 ++-- routers/web/web.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index f5fb056494..ad16b9fb4e 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -168,8 +168,8 @@ func List(ctx *context.Context) { actionsConfig := ctx.Repo.Repository.MustGetUnit(ctx, unit.TypeActions).ActionsConfig() ctx.Data["ActionsConfig"] = actionsConfig - if len(workflowID) > 0 && ctx.Repo.IsAdmin() { - ctx.Data["AllowDisableOrEnableWorkflow"] = true + if len(workflowID) > 0 && ctx.Repo.CanWrite(unit.TypeActions) { + ctx.Data["AllowDisableOrEnableWorkflow"] = ctx.Repo.IsAdmin() isWorkflowDisabled := actionsConfig.IsWorkflowDisabled(workflowID) ctx.Data["CurWorkflowDisabled"] = isWorkflowDisabled diff --git a/routers/web/web.go b/routers/web/web.go index a2c14993ac..5ed046a983 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1406,7 +1406,7 @@ func registerRoutes(m *web.Router) { m.Get("", actions.List) m.Post("/disable", reqRepoAdmin, actions.DisableWorkflowFile) m.Post("/enable", reqRepoAdmin, actions.EnableWorkflowFile) - m.Post("/run", reqRepoAdmin, actions.Run) + m.Post("/run", reqRepoActionsWriter, actions.Run) m.Group("/runs/{run}", func() { m.Combo(""). From 00f8090de4b42305f6e717dc2cdb32c14b4a8fe2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 27 Nov 2024 23:43:38 -0800 Subject: [PATCH 5/5] Don't create action when syncing mirror pull refs (#32659) Fix #27961 --- services/feed/action.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/feed/action.go b/services/feed/action.go index 83daaa1438..a8820aeb77 100644 --- a/services/feed/action.go +++ b/services/feed/action.go @@ -390,6 +390,12 @@ func (a *actionNotifier) DeleteRef(ctx context.Context, doer *user_model.User, r } func (a *actionNotifier) SyncPushCommits(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) { + // ignore pull sync message for pull requests refs + // TODO: it's better to have a UI to let users chose + if opts.RefFullName.IsPull() { + return + } + data, err := json.Marshal(commits) if err != nil { log.Error("json.Marshal: %v", err)