+ {{$statusUnread := 1}}{{$statusRead := 2}}{{$statusPinned := 3}} {{$notificationUnreadCount := call .PageGlobalData.GetNotificationUnreadCount}} -
+ {{$pageTypeIsRead := eq $.PageType "read"}} +
- + {{ctx.Locale.Tr "notification.unread"}}
{{$notificationUnreadCount}}
 - + {{ctx.Locale.Tr "notification.read"}}
- {{if and (eq .Status 1)}} + {{if and (not $pageTypeIsRead) $notificationUnreadCount}}
{{$.CsrfTokenHtml}} -
- -
+
{{end}}
-
-
- {{if not .Notifications}} -
- {{svg "octicon-inbox" 56 "tw-mb-4"}} - {{if eq .Status 1}} - {{ctx.Locale.Tr "notification.no_unread"}} +
+ {{range $one := .Notifications}} +
+
+ {{if $one.Issue}} + {{template "shared/issueicon" $one.Issue}} {{else}} - {{ctx.Locale.Tr "notification.no_read"}} + {{svg "octicon-repo" 16 "text grey"}} {{end}}
- {{else}} - {{range $notification := .Notifications}} -
-
- {{if .Issue}} - {{template "shared/issueicon" .Issue}} - {{else}} - {{svg "octicon-repo" 16 "text grey"}} - {{end}} -
- -
- {{.Repository.FullName}} {{if .Issue}}#{{.Issue.Index}}{{end}} - {{if eq .Status 3}} - {{svg "octicon-pin" 13 "text blue tw-mt-0.5 tw-ml-1"}} - {{end}} -
-
- - {{if .Issue}} - {{.Issue.Title | ctx.RenderUtils.RenderIssueSimpleTitle}} - {{else}} - {{.Repository.FullName}} - {{end}} - -
- -
- {{if .Issue}} - {{DateUtils.TimeSince .Issue.UpdatedUnix}} - {{else}} - {{DateUtils.TimeSince .UpdatedUnix}} - {{end}} -
-
- {{if ne .Status 3}} -
- {{$.CsrfTokenHtml}} - - - -
- {{end}} - {{if or (eq .Status 1) (eq .Status 3)}} -
- {{$.CsrfTokenHtml}} - - - - -
- {{else if eq .Status 2}} -
- {{$.CsrfTokenHtml}} - - - - -
- {{end}} -
+ +
+ {{$one.Repository.FullName}} {{if $one.Issue}}#{{$one.Issue.Index}}{{end}} + {{if eq $one.Status $statusPinned}} + {{svg "octicon-pin" 13 "text blue"}} + {{end}}
+
+ {{if $one.Issue}} + {{$one.Issue.Title | ctx.RenderUtils.RenderIssueSimpleTitle}} + {{else}} + {{$one.Repository.FullName}} + {{end}} +
+ +
+ {{if $one.Issue}} + {{DateUtils.TimeSince $one.Issue.UpdatedUnix}} + {{else}} + {{DateUtils.TimeSince $one.UpdatedUnix}} + {{end}} +
+
+ {{$.CsrfTokenHtml}} + + {{if ne $one.Status $statusPinned}} + + {{end}} + {{if or (eq $one.Status $statusUnread) (eq $one.Status $statusPinned)}} + + {{else if eq $one.Status $statusRead}} + + {{end}} +
+
+ {{else}} +
+ {{svg "octicon-inbox" 56 "tw-mb-4"}} + {{if $pageTypeIsRead}} + {{ctx.Locale.Tr "notification.no_read"}} + {{else}} + {{ctx.Locale.Tr "notification.no_unread"}} {{end}} - {{end}} -
+
+ {{end}}
{{template "base/paginate" .}}
diff --git a/web_src/css/user.css b/web_src/css/user.css index caabf1834c..d42e8688fb 100644 --- a/web_src/css/user.css +++ b/web_src/css/user.css @@ -114,6 +114,14 @@ border-radius: var(--border-radius); } +.notifications-item { + display: flex; + align-items: center; + flex-wrap: wrap; + gap: 0.5em; + padding: 0.5em 1em; +} + .notifications-item:hover { background: var(--color-hover); } @@ -129,6 +137,9 @@ .notifications-item:hover .notifications-buttons { display: flex; + align-items: center; + justify-content: end; + gap: 0.25em; } .notifications-item:hover .notifications-updated { diff --git a/web_src/js/features/notification.ts b/web_src/js/features/notification.ts index dc0acb0244..4a1aa3ede9 100644 --- a/web_src/js/features/notification.ts +++ b/web_src/js/features/notification.ts @@ -1,40 +1,13 @@ import {GET} from '../modules/fetch.ts'; -import {toggleElem, type DOMEvent, createElementFromHTML} from '../utils/dom.ts'; +import {toggleElem, createElementFromHTML} from '../utils/dom.ts'; import {logoutFromWorker} from '../modules/worker.ts'; const {appSubUrl, notificationSettings, assetVersionEncoded} = window.config; let notificationSequenceNumber = 0; -export function initNotificationsTable() { - const table = document.querySelector('#notification_table'); - if (!table) return; - - // when page restores from bfcache, delete previously clicked items - window.addEventListener('pageshow', (e) => { - if (e.persisted) { // page was restored from bfcache - const table = document.querySelector('#notification_table'); - const unreadCountEl = document.querySelector('.notifications-unread-count'); - let unreadCount = parseInt(unreadCountEl.textContent); - for (const item of table.querySelectorAll('.notifications-item[data-remove="true"]')) { - item.remove(); - unreadCount -= 1; - } - unreadCountEl.textContent = String(unreadCount); - } - }); - - // mark clicked unread links for deletion on bfcache restore - for (const link of table.querySelectorAll('.notifications-item[data-status="1"] .notifications-link')) { - link.addEventListener('click', (e: DOMEvent) => { - e.target.closest('.notifications-item').setAttribute('data-remove', 'true'); - }); - } -} - -async function receiveUpdateCount(event: MessageEvent) { +async function receiveUpdateCount(event: MessageEvent<{type: string, data: string}>) { try { - const data = JSON.parse(event.data); - + const data = JSON.parse(event.data.data); for (const count of document.querySelectorAll('.notification_count')) { count.classList.toggle('tw-hidden', data.Count === 0); count.textContent = `${data.Count}`; @@ -71,7 +44,7 @@ export function initNotificationCount() { type: 'start', url: `${window.location.origin}${appSubUrl}/user/events`, }); - worker.port.addEventListener('message', (event: MessageEvent) => { + worker.port.addEventListener('message', (event: MessageEvent<{type: string, data: string}>) => { if (!event.data || !event.data.type) { console.error('unknown worker message event', event); return; @@ -144,11 +117,11 @@ async function updateNotificationCountWithCallback(callback: (timeout: number, n } async function updateNotificationTable() { - const notificationDiv = document.querySelector('#notification_div'); + let notificationDiv = document.querySelector('#notification_div'); if (notificationDiv) { try { const params = new URLSearchParams(window.location.search); - params.set('div-only', String(true)); + params.set('div-only', 'true'); params.set('sequence-number', String(++notificationSequenceNumber)); const response = await GET(`${appSubUrl}/notifications?${params.toString()}`); @@ -160,7 +133,8 @@ async function updateNotificationTable() { const el = createElementFromHTML(data); if (parseInt(el.getAttribute('data-sequence-number')) === notificationSequenceNumber) { notificationDiv.outerHTML = data; - initNotificationsTable(); + notificationDiv = document.querySelector('#notification_div'); + window.htmx.process(notificationDiv); // when using htmx, we must always remember to process the new content changed by us } } catch (error) { console.error(error); diff --git a/web_src/js/index-domready.ts b/web_src/js/index-domready.ts index ca18d1e828..770c7fc00c 100644 --- a/web_src/js/index-domready.ts +++ b/web_src/js/index-domready.ts @@ -15,7 +15,7 @@ import {initTableSort} from './features/tablesort.ts'; import {initAdminUserListSearchForm} from './features/admin/users.ts'; import {initAdminConfigs} from './features/admin/config.ts'; import {initMarkupAnchors} from './markup/anchors.ts'; -import {initNotificationCount, initNotificationsTable} from './features/notification.ts'; +import {initNotificationCount} from './features/notification.ts'; import {initRepoIssueContentHistory} from './features/repo-issue-content.ts'; import {initStopwatch} from './features/stopwatch.ts'; import {initFindFileInRepo} from './features/repo-findfile.ts'; @@ -117,7 +117,6 @@ const initPerformanceTracer = callInitFunctions([ initDashboardRepoList, initNotificationCount, - initNotificationsTable, initOrgTeam, From af0196c1452404e1055693a0683d61e69d6b7ad4 Mon Sep 17 00:00:00 2001 From: Scion Date: Wed, 9 Jul 2025 22:58:07 -0700 Subject: [PATCH 05/15] Fix ListWorkflowRuns OpenAPI response model. (#35026) Change the OpenAPI response of `ListWorkflowRuns` to `WorkflowRunsList` like it is supposed to be. --------- Signed-off-by: wxiaoguang Co-authored-by: wxiaoguang --- modules/structs/repo.go | 4 ++-- routers/api/v1/repo/action.go | 2 +- services/convert/repository.go | 2 +- templates/swagger/v1_json.tmpl | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/structs/repo.go b/modules/structs/repo.go index abc8076387..aca5d9c3f4 100644 --- a/modules/structs/repo.go +++ b/modules/structs/repo.go @@ -57,7 +57,7 @@ type Repository struct { Private bool `json:"private"` Fork bool `json:"fork"` Template bool `json:"template"` - Parent *Repository `json:"parent"` + Parent *Repository `json:"parent,omitempty"` Mirror bool `json:"mirror"` Size int `json:"size"` Language string `json:"language"` @@ -114,7 +114,7 @@ type Repository struct { ObjectFormatName string `json:"object_format_name"` // swagger:strfmt date-time MirrorUpdated time.Time `json:"mirror_updated"` - RepoTransfer *RepoTransfer `json:"repo_transfer"` + RepoTransfer *RepoTransfer `json:"repo_transfer,omitempty"` Topics []string `json:"topics"` Licenses []string `json:"licenses"` } diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index a57db015f0..ef0c5cc199 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -747,7 +747,7 @@ func (Action) ListWorkflowRuns(ctx *context.APIContext) { // type: integer // responses: // "200": - // "$ref": "#/responses/ArtifactsList" + // "$ref": "#/responses/WorkflowRunsList" // "400": // "$ref": "#/responses/error" // "404": diff --git a/services/convert/repository.go b/services/convert/repository.go index 614eb58a88..a364591bb8 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -251,7 +251,7 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR RepoTransfer: transfer, Topics: util.SliceNilAsEmpty(repo.Topics), ObjectFormatName: repo.ObjectFormatName, - Licenses: repoLicenses.StringList(), + Licenses: util.SliceNilAsEmpty(repoLicenses.StringList()), } } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 6cf2810baf..323e0d64ac 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -5165,7 +5165,7 @@ ], "responses": { "200": { - "$ref": "#/responses/ArtifactsList" + "$ref": "#/responses/WorkflowRunsList" }, "400": { "$ref": "#/responses/error" From 091b3e696daedc9e8a7531f71329ba57b41cb8f0 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 10 Jul 2025 10:28:25 +0200 Subject: [PATCH 06/15] Tweak eslint config, fix new issues (#35019) 1. Enable [`@typescript-eslint/no-unnecessary-type-conversion`](https://typescript-eslint.io/rules/no-unnecessary-type-conversion/), I think the two cases that were hit are safe cases. 2. Disable `no-new-func`, `@typescript-eslint/no-implied-eval` does the same but better. --- .eslintrc.cjs | 3 ++- web_src/js/modules/toast.ts | 2 +- web_src/js/utils/dom.ts | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 57c6b19600..cf3ff53b30 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -326,6 +326,7 @@ module.exports = { '@typescript-eslint/no-unnecessary-type-arguments': [0], '@typescript-eslint/no-unnecessary-type-assertion': [2], '@typescript-eslint/no-unnecessary-type-constraint': [2], + '@typescript-eslint/no-unnecessary-type-conversion': [2], '@typescript-eslint/no-unsafe-argument': [0], '@typescript-eslint/no-unsafe-assignment': [0], '@typescript-eslint/no-unsafe-call': [0], @@ -645,7 +646,7 @@ module.exports = { 'no-multi-str': [2], 'no-negated-condition': [0], 'no-nested-ternary': [0], - 'no-new-func': [2], + 'no-new-func': [0], // handled by @typescript-eslint/no-implied-eval 'no-new-native-nonconstructor': [2], 'no-new-object': [2], 'no-new-symbol': [2], diff --git a/web_src/js/modules/toast.ts b/web_src/js/modules/toast.ts index ed807a4977..087103cbd8 100644 --- a/web_src/js/modules/toast.ts +++ b/web_src/js/modules/toast.ts @@ -44,7 +44,7 @@ type ToastifyElement = HTMLElement & {_giteaToastifyInstance?: Toast }; // See https://github.com/apvarun/toastify-js#api for options function showToast(message: string, level: Intent, {gravity, position, duration, useHtmlBody, preventDuplicates = true, ...other}: ToastOpts = {}): Toast { - const body = useHtmlBody ? String(message) : htmlEscape(message); + const body = useHtmlBody ? message : htmlEscape(message); const parent = document.querySelector('.ui.dimmer.active') ?? document.body; const duplicateKey = preventDuplicates ? (preventDuplicates === true ? `${level}-${body}` : preventDuplicates) : ''; diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 9cdf6f5005..6d6a3735da 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -283,7 +283,7 @@ export function isElemVisible(el: HTMLElement): boolean { // This function DOESN'T account for all possible visibility scenarios, its behavior is covered by the tests of "querySingleVisibleElem" if (!el) return false; // checking el.style.display is not necessary for browsers, but it is required by some tests with happy-dom because happy-dom doesn't really do layout - return !el.classList.contains('tw-hidden') && Boolean((el.offsetWidth || el.offsetHeight || el.getClientRects().length) && el.style.display !== 'none'); + return !el.classList.contains('tw-hidden') && (el.offsetWidth || el.offsetHeight || el.getClientRects().length) && el.style.display !== 'none'; } // replace selected text in a textarea while preserving editor history, e.g. CTRL-Z works after this From 4b174e44a817cf6b11fdf439e3c60dfc5c55dfe2 Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Thu, 10 Jul 2025 13:36:55 +0200 Subject: [PATCH 07/15] Improve CLI commands (#34973) Improve help related commands and flags and add tests Co-authored-by: wxiaoguang --- cmd/hook.go | 1 + cmd/keys.go | 1 + cmd/main.go | 158 ++++++++++------------- cmd/main_test.go | 46 ++++++- cmd/serv.go | 1 + contrib/autocompletion/README | 17 --- contrib/autocompletion/bash_autocomplete | 30 ----- contrib/autocompletion/zsh_autocomplete | 30 ----- 8 files changed, 116 insertions(+), 168 deletions(-) delete mode 100644 contrib/autocompletion/README delete mode 100755 contrib/autocompletion/bash_autocomplete delete mode 100644 contrib/autocompletion/zsh_autocomplete diff --git a/cmd/hook.go b/cmd/hook.go index 2ce272b411..b741127ca3 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -32,6 +32,7 @@ var ( CmdHook = &cli.Command{ Name: "hook", Usage: "(internal) Should only be called by Git", + Hidden: true, // internal commands shouldn't be visible Description: "Delegate commands to corresponding Git hooks", Before: PrepareConsoleLoggerLevel(log.FATAL), Commands: []*cli.Command{ diff --git a/cmd/keys.go b/cmd/keys.go index 8710756a81..5ca3b91e15 100644 --- a/cmd/keys.go +++ b/cmd/keys.go @@ -19,6 +19,7 @@ import ( var CmdKeys = &cli.Command{ Name: "keys", Usage: "(internal) Should only be called by SSH server", + Hidden: true, // internal commands shouldn't not be visible Description: "Queries the Gitea database to get the authorized command for a given ssh key fingerprint", Before: PrepareConsoleLoggerLevel(log.FATAL), Action: runKeys, diff --git a/cmd/main.go b/cmd/main.go index 3b8a8a9311..3fdaf48ed9 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -6,6 +6,7 @@ package cmd import ( "context" "fmt" + "io" "os" "strings" @@ -15,26 +16,28 @@ import ( "github.com/urfave/cli/v3" ) -// cmdHelp is our own help subcommand with more information -// Keep in mind that the "./gitea help"(subcommand) is different from "./gitea --help"(flag), the flag doesn't parse the config or output "DEFAULT CONFIGURATION:" information -func cmdHelp() *cli.Command { - c := &cli.Command{ - Name: "help", - Aliases: []string{"h"}, - Usage: "Shows a list of commands or help for one command", - ArgsUsage: "[command]", - Action: func(ctx context.Context, c *cli.Command) (err error) { - lineage := c.Lineage() // The order is from child to parent: help, doctor, Gitea - targetCmdIdx := 0 - if c.Name == "help" { - targetCmdIdx = 1 - } - if lineage[targetCmdIdx] != lineage[targetCmdIdx].Root() { - err = cli.ShowCommandHelp(ctx, lineage[targetCmdIdx+1] /* parent cmd */, lineage[targetCmdIdx].Name /* sub cmd */) - } else { - err = cli.ShowAppHelp(c) - } - _, _ = fmt.Fprintf(c.Root().Writer, ` +var cliHelpPrinterOld = cli.HelpPrinter + +func init() { + cli.HelpPrinter = cliHelpPrinterNew +} + +// cliHelpPrinterNew helps to print "DEFAULT CONFIGURATION" for the following cases ( "-c" can apper in any position): +// * ./gitea -c /dev/null -h +// * ./gitea -c help /dev/null help +// * ./gitea help -c /dev/null +// * ./gitea help -c /dev/null web +// * ./gitea help web -c /dev/null +// * ./gitea web help -c /dev/null +// * ./gitea web -h -c /dev/null +func cliHelpPrinterNew(out io.Writer, templ string, data any) { + cmd, _ := data.(*cli.Command) + if cmd != nil { + prepareWorkPathAndCustomConf(cmd) + } + cliHelpPrinterOld(out, templ, data) + if setting.CustomConf != "" { + _, _ = fmt.Fprintf(out, ` DEFAULT CONFIGURATION: AppPath: %s WorkPath: %s @@ -42,77 +45,36 @@ DEFAULT CONFIGURATION: ConfigFile: %s `, setting.AppPath, setting.AppWorkPath, setting.CustomPath, setting.CustomConf) - return err - }, - } - return c -} - -func appGlobalFlags() []cli.Flag { - return []cli.Flag{ - // make the builtin flags at the top - cli.HelpFlag, - - // shared configuration flags, they are for global and for each sub-command at the same time - // eg: such command is valid: "./gitea --config /tmp/app.ini web --config /tmp/app.ini", while it's discouraged indeed - // keep in mind that the short flags like "-C", "-c" and "-w" are globally polluted, they can't be used for sub-commands anymore. - &cli.StringFlag{ - Name: "custom-path", - Aliases: []string{"C"}, - Usage: "Set custom path (defaults to '{WorkPath}/custom')", - }, - &cli.StringFlag{ - Name: "config", - Aliases: []string{"c"}, - Value: setting.CustomConf, - Usage: "Set custom config file (defaults to '{WorkPath}/custom/conf/app.ini')", - }, - &cli.StringFlag{ - Name: "work-path", - Aliases: []string{"w"}, - Usage: "Set Gitea's working path (defaults to the Gitea's binary directory)", - }, } } -func prepareSubcommandWithGlobalFlags(command *cli.Command) { - command.Flags = append(append([]cli.Flag{}, appGlobalFlags()...), command.Flags...) - command.Action = prepareWorkPathAndCustomConf(command.Action) - command.HideHelp = true - if command.Name != "help" { - command.Commands = append(command.Commands, cmdHelp()) - } - for i := range command.Commands { - prepareSubcommandWithGlobalFlags(command.Commands[i]) - } -} - -// prepareWorkPathAndCustomConf wraps the Action to prepare the work path and custom config -// It can't use "Before", because each level's sub-command's Before will be called one by one, so the "init" would be done multiple times -func prepareWorkPathAndCustomConf(action cli.ActionFunc) func(context.Context, *cli.Command) error { - return func(ctx context.Context, cmd *cli.Command) error { - var args setting.ArgWorkPathAndCustomConf - // from children to parent, check the global flags - for _, curCtx := range cmd.Lineage() { - if curCtx.IsSet("work-path") && args.WorkPath == "" { - args.WorkPath = curCtx.String("work-path") - } - if curCtx.IsSet("custom-path") && args.CustomPath == "" { - args.CustomPath = curCtx.String("custom-path") - } - if curCtx.IsSet("config") && args.CustomConf == "" { - args.CustomConf = curCtx.String("config") - } +func prepareSubcommandWithGlobalFlags(originCmd *cli.Command) { + originBefore := originCmd.Before + originCmd.Before = func(ctx context.Context, cmd *cli.Command) (context.Context, error) { + prepareWorkPathAndCustomConf(cmd) + if originBefore != nil { + return originBefore(ctx, cmd) } - setting.InitWorkPathAndCommonConfig(os.Getenv, args) - if cmd.Bool("help") || action == nil { - // the default behavior of "urfave/cli": "nil action" means "show help" - return cmdHelp().Action(ctx, cmd) - } - return action(ctx, cmd) + return ctx, nil } } +// prepareWorkPathAndCustomConf tries to prepare the work path, custom path and custom config from various inputs: +// command line flags, environment variables, config file +func prepareWorkPathAndCustomConf(cmd *cli.Command) { + var args setting.ArgWorkPathAndCustomConf + if cmd.IsSet("work-path") { + args.WorkPath = cmd.String("work-path") + } + if cmd.IsSet("custom-path") { + args.CustomPath = cmd.String("custom-path") + } + if cmd.IsSet("config") { + args.CustomConf = cmd.String("config") + } + setting.InitWorkPathAndCommonConfig(os.Getenv, args) +} + type AppVersion struct { Version string Extra string @@ -125,10 +87,29 @@ func NewMainApp(appVer AppVersion) *cli.Command { app.Description = `Gitea program contains "web" and other subcommands. If no subcommand is given, it starts the web server by default. Use "web" subcommand for more web server arguments, use other subcommands for other purposes.` app.Version = appVer.Version + appVer.Extra app.EnableShellCompletion = true - - // these sub-commands need to use config file + app.Flags = []cli.Flag{ + &cli.StringFlag{ + Name: "work-path", + Aliases: []string{"w"}, + TakesFile: true, + Usage: "Set Gitea's working path (defaults to the Gitea's binary directory)", + }, + &cli.StringFlag{ + Name: "config", + Aliases: []string{"c"}, + TakesFile: true, + Value: setting.CustomConf, + Usage: "Set custom config file (defaults to '{WorkPath}/custom/conf/app.ini')", + }, + &cli.StringFlag{ + Name: "custom-path", + Aliases: []string{"C"}, + TakesFile: true, + Usage: "Set custom path (defaults to '{WorkPath}/custom')", + }, + } + // these sub-commands need to use a config file subCmdWithConfig := []*cli.Command{ - cmdHelp(), // the "help" sub-command was used to show the more information for "work path" and "custom config" CmdWeb, CmdServ, CmdHook, @@ -156,9 +137,6 @@ func NewMainApp(appVer AppVersion) *cli.Command { // but not sure whether it would break Windows users who used to double-click the EXE to run. app.DefaultCommand = CmdWeb.Name - app.Flags = append(app.Flags, cli.VersionFlag) - app.Flags = append(app.Flags, appGlobalFlags()...) - app.HideHelp = true // use our own help action to show helps (with more information like default config) app.Before = PrepareConsoleLoggerLevel(log.INFO) for i := range subCmdWithConfig { prepareSubcommandWithGlobalFlags(subCmdWithConfig[i]) diff --git a/cmd/main_test.go b/cmd/main_test.go index 7dfa87a0ef..d49ebfd4df 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -74,12 +74,56 @@ func TestCliCmd(t *testing.T) { cmd string exp string }{ - // main command help + // help commands + { + cmd: "./gitea -h", + exp: "DEFAULT CONFIGURATION:", + }, { cmd: "./gitea help", exp: "DEFAULT CONFIGURATION:", }, + { + cmd: "./gitea -c /dev/null -h", + exp: "ConfigFile: /dev/null", + }, + + { + cmd: "./gitea -c /dev/null help", + exp: "ConfigFile: /dev/null", + }, + { + cmd: "./gitea help -c /dev/null", + exp: "ConfigFile: /dev/null", + }, + + { + cmd: "./gitea -c /dev/null test-cmd -h", + exp: "ConfigFile: /dev/null", + }, + { + cmd: "./gitea test-cmd -c /dev/null -h", + exp: "ConfigFile: /dev/null", + }, + { + cmd: "./gitea test-cmd -h -c /dev/null", + exp: "ConfigFile: /dev/null", + }, + + { + cmd: "./gitea -c /dev/null test-cmd help", + exp: "ConfigFile: /dev/null", + }, + { + cmd: "./gitea test-cmd -c /dev/null help", + exp: "ConfigFile: /dev/null", + }, + { + cmd: "./gitea test-cmd help -c /dev/null", + exp: "ConfigFile: /dev/null", + }, + // parse paths { cmd: "./gitea test-cmd", diff --git a/cmd/serv.go b/cmd/serv.go index 8c6001e727..38c79f68cd 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -41,6 +41,7 @@ var CmdServ = &cli.Command{ Name: "serv", Usage: "(internal) Should only be called by SSH shell", Description: "Serv provides access auth for repositories", + Hidden: true, // Internal commands shouldn't be visible in help Before: PrepareConsoleLoggerLevel(log.FATAL), Action: runServ, Flags: []cli.Flag{ diff --git a/contrib/autocompletion/README b/contrib/autocompletion/README deleted file mode 100644 index 1defd219d8..0000000000 --- a/contrib/autocompletion/README +++ /dev/null @@ -1,17 +0,0 @@ -Bash and Zsh completion -======================= - -From within the gitea root run: - -```bash -source contrib/autocompletion/bash_autocomplete -``` - -or for zsh run: - -```bash -source contrib/autocompletion/zsh_autocomplete -``` - -These scripts will check if gitea is on the path and if so add autocompletion for `gitea`. Or if not autocompletion will work for `./gitea`. -If gitea has been installed as a different program pass in the `PROG` environment variable to set the correct program name. diff --git a/contrib/autocompletion/bash_autocomplete b/contrib/autocompletion/bash_autocomplete deleted file mode 100755 index 5cb62f26a7..0000000000 --- a/contrib/autocompletion/bash_autocomplete +++ /dev/null @@ -1,30 +0,0 @@ -#! /bin/bash -# Heavily inspired by https://github.com/urfave/cli - -_cli_bash_autocomplete() { - if [[ "${COMP_WORDS[0]}" != "source" ]]; then - local cur opts base - COMPREPLY=() - cur="${COMP_WORDS[COMP_CWORD]}" - if [[ "$cur" == "-"* ]]; then - opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} ${cur} --generate-bash-completion ) - else - opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} --generate-bash-completion ) - fi - COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) - return 0 - fi -} - -if [ -z "$PROG" ] && [ ! "$(command -v gitea &> /dev/null)" ] ; then - complete -o bashdefault -o default -o nospace -F _cli_bash_autocomplete gitea -elif [ -z "$PROG" ]; then - complete -o bashdefault -o default -o nospace -F _cli_bash_autocomplete ./gitea - complete -o bashdefault -o default -o nospace -F _cli_bash_autocomplete "$PWD/gitea" -else - complete -o bashdefault -o default -o nospace -F _cli_bash_autocomplete "$PROG" - unset PROG -fi - - - diff --git a/contrib/autocompletion/zsh_autocomplete b/contrib/autocompletion/zsh_autocomplete deleted file mode 100644 index b3b40df503..0000000000 --- a/contrib/autocompletion/zsh_autocomplete +++ /dev/null @@ -1,30 +0,0 @@ -#compdef ${PROG:=gitea} - - -# Heavily inspired by https://github.com/urfave/cli - -_cli_zsh_autocomplete() { - - local -a opts - local cur - cur=${words[-1]} - if [[ "$cur" == "-"* ]]; then - opts=("${(@f)$(_CLI_ZSH_AUTOCOMPLETE_HACK=1 ${words[@]:0:#words[@]-1} ${cur} --generate-bash-completion)}") - else - opts=("${(@f)$(_CLI_ZSH_AUTOCOMPLETE_HACK=1 ${words[@]:0:#words[@]-1} --generate-bash-completion)}") - fi - - if [[ "${opts[1]}" != "" ]]; then - _describe 'values' opts - else - _files - fi - - return -} - -if [ -z $PROG ] ; then - compdef _cli_zsh_autocomplete gitea -else - compdef _cli_zsh_autocomplete $(basename $PROG) -fi From 36a19f2569493163d76ea95a5a3060fd1daae6de Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 10 Jul 2025 17:48:36 +0200 Subject: [PATCH 08/15] Update to go 1.24.5 (#35031) https://go.dev/doc/devel/release#go1.24.5 --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index afe7c990e4..8b8d61dc65 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module code.gitea.io/gitea -go 1.24.4 +go 1.24.5 // rfc5280 said: "The serial number is an integer assigned by the CA to each certificate." // But some CAs use negative serial number, just relax the check. related: From f35dcfd489490ec9ad552bb38afc237ad94ed5a2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 11 Jul 2025 00:38:42 +0800 Subject: [PATCH 09/15] Make submodule link work with relative path (#35034) Fix #35033 --- modules/git/commit_submodule_file.go | 36 +++++++++++++-------- modules/git/commit_submodule_file_test.go | 39 ++++++++++++++--------- web_src/css/repo/home-file-list.css | 2 +- 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/modules/git/commit_submodule_file.go b/modules/git/commit_submodule_file.go index 729401f752..5def80f3bd 100644 --- a/modules/git/commit_submodule_file.go +++ b/modules/git/commit_submodule_file.go @@ -6,17 +6,18 @@ package git import ( "context" + "strings" giturl "code.gitea.io/gitea/modules/git/url" ) // CommitSubmoduleFile represents a file with submodule type. type CommitSubmoduleFile struct { - refURL string - parsedURL *giturl.RepositoryURL - parsed bool - refID string - repoLink string + refURL string + refID string + + parsed bool + targetRepoLink string } // NewCommitSubmoduleFile create a new submodule file @@ -35,20 +36,27 @@ func (sf *CommitSubmoduleFile) SubmoduleWebLink(ctx context.Context, optCommitID } if !sf.parsed { sf.parsed = true - parsedURL, err := giturl.ParseRepositoryURL(ctx, sf.refURL) - if err != nil { - return nil + if strings.HasPrefix(sf.refURL, "../") { + // FIXME: when handling relative path, this logic is not right. It needs to: + // 1. Remember the submodule's full path and its commit's repo home link + // 2. Resolve the relative path: targetRepoLink = path.Join(repoHomeLink, path.Dir(submoduleFullPath), refURL) + // Not an easy task and need to refactor related code a lot. + sf.targetRepoLink = sf.refURL + } else { + parsedURL, err := giturl.ParseRepositoryURL(ctx, sf.refURL) + if err != nil { + return nil + } + sf.targetRepoLink = giturl.MakeRepositoryWebLink(parsedURL) } - sf.parsedURL = parsedURL - sf.repoLink = giturl.MakeRepositoryWebLink(sf.parsedURL) } var commitLink string if len(optCommitID) == 2 { - commitLink = sf.repoLink + "/compare/" + optCommitID[0] + "..." + optCommitID[1] + commitLink = sf.targetRepoLink + "/compare/" + optCommitID[0] + "..." + optCommitID[1] } else if len(optCommitID) == 1 { - commitLink = sf.repoLink + "/tree/" + optCommitID[0] + commitLink = sf.targetRepoLink + "/tree/" + optCommitID[0] } else { - commitLink = sf.repoLink + "/tree/" + sf.refID + commitLink = sf.targetRepoLink + "/tree/" + sf.refID } - return &SubmoduleWebLink{RepoWebLink: sf.repoLink, CommitWebLink: commitLink} + return &SubmoduleWebLink{RepoWebLink: sf.targetRepoLink, CommitWebLink: commitLink} } diff --git a/modules/git/commit_submodule_file_test.go b/modules/git/commit_submodule_file_test.go index 6581fa8712..103e55e920 100644 --- a/modules/git/commit_submodule_file_test.go +++ b/modules/git/commit_submodule_file_test.go @@ -10,20 +10,29 @@ import ( ) func TestCommitSubmoduleLink(t *testing.T) { - sf := NewCommitSubmoduleFile("git@github.com:user/repo.git", "aaaa") - - wl := sf.SubmoduleWebLink(t.Context()) - assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) - assert.Equal(t, "https://github.com/user/repo/tree/aaaa", wl.CommitWebLink) - - wl = sf.SubmoduleWebLink(t.Context(), "1111") - assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) - assert.Equal(t, "https://github.com/user/repo/tree/1111", wl.CommitWebLink) - - wl = sf.SubmoduleWebLink(t.Context(), "1111", "2222") - assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) - assert.Equal(t, "https://github.com/user/repo/compare/1111...2222", wl.CommitWebLink) - - wl = (*CommitSubmoduleFile)(nil).SubmoduleWebLink(t.Context()) + wl := (*CommitSubmoduleFile)(nil).SubmoduleWebLink(t.Context()) assert.Nil(t, wl) + + t.Run("GitHubRepo", func(t *testing.T) { + sf := NewCommitSubmoduleFile("git@github.com:user/repo.git", "aaaa") + + wl := sf.SubmoduleWebLink(t.Context()) + assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) + assert.Equal(t, "https://github.com/user/repo/tree/aaaa", wl.CommitWebLink) + + wl = sf.SubmoduleWebLink(t.Context(), "1111") + assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) + assert.Equal(t, "https://github.com/user/repo/tree/1111", wl.CommitWebLink) + + wl = sf.SubmoduleWebLink(t.Context(), "1111", "2222") + assert.Equal(t, "https://github.com/user/repo", wl.RepoWebLink) + assert.Equal(t, "https://github.com/user/repo/compare/1111...2222", wl.CommitWebLink) + }) + + t.Run("RelativePath", func(t *testing.T) { + sf := NewCommitSubmoduleFile("../../user/repo", "aaaa") + wl := sf.SubmoduleWebLink(t.Context()) + assert.Equal(t, "../../user/repo", wl.RepoWebLink) + assert.Equal(t, "../../user/repo/tree/aaaa", wl.CommitWebLink) + }) } diff --git a/web_src/css/repo/home-file-list.css b/web_src/css/repo/home-file-list.css index f2ab052a54..6aa9e4bca3 100644 --- a/web_src/css/repo/home-file-list.css +++ b/web_src/css/repo/home-file-list.css @@ -71,7 +71,7 @@ #repo-files-table .repo-file-cell.name .entry-name { flex-shrink: 1; - min-width: 3em; + min-width: 1ch; /* leave about one letter space when shrinking, need to fine tune the "shrinks" in this grid in the future */ } @media (max-width: 767.98px) { From 32152a0ac03e912567a9d9a243729a9ed6288255 Mon Sep 17 00:00:00 2001 From: Naxdy Date: Thu, 10 Jul 2025 19:17:56 +0200 Subject: [PATCH 10/15] Also display "recently pushed branch" alert on PR view (#35001) This commit adds the "You recently pushed to branch X" alert also to PR overview, as opposed to only the repository's home page. GitHub also shows this alert on the PR list, as well as the home page. --------- Co-authored-by: wxiaoguang Co-authored-by: Giteabot --- models/git/branch.go | 2 +- models/repo/repo.go | 6 ++ routers/web/repo/common_recentbranches.go | 73 +++++++++++++++++++ routers/web/repo/issue_list.go | 4 + routers/web/repo/view_home.go | 51 ------------- .../code/recently_pushed_new_branches.tmpl | 18 +++-- templates/repo/home.tmpl | 2 +- templates/repo/issue/list.tmpl | 2 + templates/repo/view.tmpl | 2 +- 9 files changed, 100 insertions(+), 60 deletions(-) create mode 100644 routers/web/repo/common_recentbranches.go diff --git a/models/git/branch.go b/models/git/branch.go index 07c94a8ba5..6021e1101f 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -472,7 +472,7 @@ type RecentlyPushedNewBranch struct { // if opts.CommitAfterUnix is 0, we will find the branches that were committed to in the last 2 hours // if opts.ListOptions is not set, we will only display top 2 latest branches. // Protected branches will be skipped since they are unlikely to be used to create new PRs. -func FindRecentlyPushedNewBranches(ctx context.Context, doer *user_model.User, opts *FindRecentlyPushedNewBranchesOptions) ([]*RecentlyPushedNewBranch, error) { +func FindRecentlyPushedNewBranches(ctx context.Context, doer *user_model.User, opts FindRecentlyPushedNewBranchesOptions) ([]*RecentlyPushedNewBranch, error) { if doer == nil { return []*RecentlyPushedNewBranch{}, nil } diff --git a/models/repo/repo.go b/models/repo/repo.go index 34d1bf55f6..2403b3b40b 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -652,7 +652,13 @@ func (repo *Repository) AllowsPulls(ctx context.Context) bool { } // CanEnableEditor returns true if repository meets the requirements of web editor. +// FIXME: most CanEnableEditor calls should be replaced with CanContentChange +// And all other like CanCreateBranch / CanEnablePulls should also be updated func (repo *Repository) CanEnableEditor() bool { + return repo.CanContentChange() +} + +func (repo *Repository) CanContentChange() bool { return !repo.IsMirror && !repo.IsArchived } diff --git a/routers/web/repo/common_recentbranches.go b/routers/web/repo/common_recentbranches.go new file mode 100644 index 0000000000..c2083dec73 --- /dev/null +++ b/routers/web/repo/common_recentbranches.go @@ -0,0 +1,73 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + git_model "code.gitea.io/gitea/models/git" + access_model "code.gitea.io/gitea/models/perm/access" + unit_model "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/services/context" + repo_service "code.gitea.io/gitea/services/repository" +) + +type RecentBranchesPromptDataStruct struct { + RecentlyPushedNewBranches []*git_model.RecentlyPushedNewBranch +} + +func prepareRecentlyPushedNewBranches(ctx *context.Context) { + if ctx.Doer == nil { + return + } + if err := ctx.Repo.Repository.GetBaseRepo(ctx); err != nil { + log.Error("GetBaseRepo: %v", err) + return + } + + opts := git_model.FindRecentlyPushedNewBranchesOptions{ + Repo: ctx.Repo.Repository, + BaseRepo: ctx.Repo.Repository, + } + if ctx.Repo.Repository.IsFork { + opts.BaseRepo = ctx.Repo.Repository.BaseRepo + } + + baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, opts.BaseRepo, ctx.Doer) + if err != nil { + log.Error("GetUserRepoPermission: %v", err) + return + } + if !opts.Repo.CanContentChange() || !opts.BaseRepo.CanContentChange() { + return + } + if !opts.BaseRepo.UnitEnabled(ctx, unit_model.TypePullRequests) || !baseRepoPerm.CanRead(unit_model.TypePullRequests) { + return + } + + var finalBranches []*git_model.RecentlyPushedNewBranch + branches, err := git_model.FindRecentlyPushedNewBranches(ctx, ctx.Doer, opts) + if err != nil { + log.Error("FindRecentlyPushedNewBranches failed: %v", err) + return + } + + for _, branch := range branches { + divergingInfo, err := repo_service.GetBranchDivergingInfo(ctx, + branch.BranchRepo, branch.BranchName, // "base" repo for diverging info + opts.BaseRepo, opts.BaseRepo.DefaultBranch, // "head" repo for diverging info + ) + if err != nil { + log.Error("GetBranchDivergingInfo failed: %v", err) + continue + } + branchRepoHasNewCommits := divergingInfo.BaseHasNewCommits + baseRepoCommitsBehind := divergingInfo.HeadCommitsBehind + if branchRepoHasNewCommits || baseRepoCommitsBehind > 0 { + finalBranches = append(finalBranches, branch) + } + } + if len(finalBranches) > 0 { + ctx.Data["RecentBranchesPromptData"] = RecentBranchesPromptDataStruct{finalBranches} + } +} diff --git a/routers/web/repo/issue_list.go b/routers/web/repo/issue_list.go index b55f4bcc90..fd34422cfc 100644 --- a/routers/web/repo/issue_list.go +++ b/routers/web/repo/issue_list.go @@ -767,6 +767,10 @@ func Issues(ctx *context.Context) { } ctx.Data["Title"] = ctx.Tr("repo.pulls") ctx.Data["PageIsPullList"] = true + prepareRecentlyPushedNewBranches(ctx) + if ctx.Written() { + return + } } else { MustEnableIssues(ctx) if ctx.Written() { diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index c7396d44e3..5482780c98 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -15,7 +15,6 @@ import ( "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" - access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -196,56 +195,6 @@ func prepareUpstreamDivergingInfo(ctx *context.Context) { ctx.Data["UpstreamDivergingInfo"] = upstreamDivergingInfo } -func prepareRecentlyPushedNewBranches(ctx *context.Context) { - if ctx.Doer != nil { - if err := ctx.Repo.Repository.GetBaseRepo(ctx); err != nil { - ctx.ServerError("GetBaseRepo", err) - return - } - - opts := &git_model.FindRecentlyPushedNewBranchesOptions{ - Repo: ctx.Repo.Repository, - BaseRepo: ctx.Repo.Repository, - } - if ctx.Repo.Repository.IsFork { - opts.BaseRepo = ctx.Repo.Repository.BaseRepo - } - - baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, opts.BaseRepo, ctx.Doer) - if err != nil { - ctx.ServerError("GetUserRepoPermission", err) - return - } - - if !opts.Repo.IsMirror && !opts.BaseRepo.IsMirror && - opts.BaseRepo.UnitEnabled(ctx, unit_model.TypePullRequests) && - baseRepoPerm.CanRead(unit_model.TypePullRequests) { - var finalBranches []*git_model.RecentlyPushedNewBranch - branches, err := git_model.FindRecentlyPushedNewBranches(ctx, ctx.Doer, opts) - if err != nil { - log.Error("FindRecentlyPushedNewBranches failed: %v", err) - } - - for _, branch := range branches { - divergingInfo, err := repo_service.GetBranchDivergingInfo(ctx, - branch.BranchRepo, branch.BranchName, // "base" repo for diverging info - opts.BaseRepo, opts.BaseRepo.DefaultBranch, // "head" repo for diverging info - ) - if err != nil { - log.Error("GetBranchDivergingInfo failed: %v", err) - continue - } - branchRepoHasNewCommits := divergingInfo.BaseHasNewCommits - baseRepoCommitsBehind := divergingInfo.HeadCommitsBehind - if branchRepoHasNewCommits || baseRepoCommitsBehind > 0 { - finalBranches = append(finalBranches, branch) - } - } - ctx.Data["RecentlyPushedNewBranches"] = finalBranches - } - } -} - func updateContextRepoEmptyAndStatus(ctx *context.Context, empty bool, status repo_model.RepositoryStatus) { if ctx.Repo.Repository.IsEmpty == empty && ctx.Repo.Repository.Status == status { return diff --git a/templates/repo/code/recently_pushed_new_branches.tmpl b/templates/repo/code/recently_pushed_new_branches.tmpl index 4a864ba756..8569bd6c13 100644 --- a/templates/repo/code/recently_pushed_new_branches.tmpl +++ b/templates/repo/code/recently_pushed_new_branches.tmpl @@ -1,12 +1,18 @@ -{{range .RecentlyPushedNewBranches}} -
-
- {{$timeSince := DateUtils.TimeSince .CommitTime}} - {{$branchLink := HTMLFormat `%s` .BranchLink .BranchDisplayName}} +{{/* Template Attributes: +* RecentBranchesPromptData +*/}} +{{$data := .RecentBranchesPromptData}} +{{if $data}} + {{range $recentBranch := $data.RecentlyPushedNewBranches}} +
+
+ {{$timeSince := DateUtils.TimeSince $recentBranch.CommitTime}} + {{$branchLink := HTMLFormat `%s` $recentBranch.BranchLink .BranchDisplayName}} {{ctx.Locale.Tr "repo.pulls.recently_pushed_new_branches" $branchLink $timeSince}}
- + {{ctx.Locale.Tr "repo.pulls.compare_changes"}}
+ {{end}} {{end}} diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index f86b90502d..2a6c0d2fe5 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -15,7 +15,7 @@
{{end}} - {{template "repo/code/recently_pushed_new_branches" .}} + {{template "repo/code/recently_pushed_new_branches" dict "RecentBranchesPromptData" .RecentBranchesPromptData}}
diff --git a/templates/repo/issue/list.tmpl b/templates/repo/issue/list.tmpl index 0ab761e038..1fe220e1b8 100644 --- a/templates/repo/issue/list.tmpl +++ b/templates/repo/issue/list.tmpl @@ -4,6 +4,8 @@
{{template "base/alert" .}} + {{template "repo/code/recently_pushed_new_branches" dict "RecentBranchesPromptData" .RecentBranchesPromptData}} + {{if .PinnedIssues}}
{{range .PinnedIssues}} diff --git a/templates/repo/view.tmpl b/templates/repo/view.tmpl index 85d09d03a1..f99fe2f57a 100644 --- a/templates/repo/view.tmpl +++ b/templates/repo/view.tmpl @@ -14,7 +14,7 @@
{{end}} - {{template "repo/code/recently_pushed_new_branches" .}} + {{template "repo/code/recently_pushed_new_branches" dict "RecentBranchesPromptData" .RecentBranchesPromptData}}
From 6ab6d4e17ff61b4eaf1a88e3d167f0d504b8390e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 11 Jul 2025 02:08:40 +0800 Subject: [PATCH 11/15] Support base64-encoded agit push options (#35037) --- services/agit/agit.go | 18 ++++++++++++++++-- services/agit/agit_test.go | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 services/agit/agit_test.go diff --git a/services/agit/agit.go b/services/agit/agit.go index b27dfc8ecd..0ea8bbfa5d 100644 --- a/services/agit/agit.go +++ b/services/agit/agit.go @@ -5,6 +5,7 @@ package agit import ( "context" + "encoding/base64" "fmt" "os" "strings" @@ -18,17 +19,30 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/private" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" notify_service "code.gitea.io/gitea/services/notify" pull_service "code.gitea.io/gitea/services/pull" ) +func parseAgitPushOptionValue(s string) string { + if base64Value, ok := strings.CutPrefix(s, "{base64}"); ok { + decoded, err := base64.StdEncoding.DecodeString(base64Value) + return util.Iif(err == nil, string(decoded), s) + } + return s +} + // ProcReceive handle proc receive work func ProcReceive(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, opts *private.HookOptions) ([]private.HookProcReceiveRefResult, error) { results := make([]private.HookProcReceiveRefResult, 0, len(opts.OldCommitIDs)) forcePush := opts.GitPushOptions.Bool(private.GitPushOptionForcePush) topicBranch := opts.GitPushOptions["topic"] - title := strings.TrimSpace(opts.GitPushOptions["title"]) - description := strings.TrimSpace(opts.GitPushOptions["description"]) + + // some options are base64-encoded with "{base64}" prefix if they contain new lines + // other agit push options like "issue", "reviewer" and "cc" are not supported + title := parseAgitPushOptionValue(opts.GitPushOptions["title"]) + description := parseAgitPushOptionValue(opts.GitPushOptions["description"]) + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) userName := strings.ToLower(opts.UserName) diff --git a/services/agit/agit_test.go b/services/agit/agit_test.go new file mode 100644 index 0000000000..feaf7dca9b --- /dev/null +++ b/services/agit/agit_test.go @@ -0,0 +1,16 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package agit + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseAgitPushOptionValue(t *testing.T) { + assert.Equal(t, "a", parseAgitPushOptionValue("a")) + assert.Equal(t, "a", parseAgitPushOptionValue("{base64}YQ==")) + assert.Equal(t, "{base64}invalid value", parseAgitPushOptionValue("{base64}invalid value")) +} From a5a3d9b10177e0266a0877936c517f54102b3f91 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 11 Jul 2025 02:35:59 +0800 Subject: [PATCH 12/15] Refactor OpenIDConnect to support SSH/FullName sync (#34978) * Fix #26585 * Fix #28327 * Fix #34932 --- cmd/admin_auth_oauth.go | 16 +++ cmd/admin_auth_oauth_test.go | 64 +++++----- models/asymkey/ssh_key.go | 4 +- models/auth/oauth2.go | 4 +- models/auth/source.go | 2 +- modules/setting/oauth2.go | 2 +- options/locale/locale_en-US.ini | 2 + routers/web/admin/auths.go | 3 + routers/web/auth/2fa.go | 3 +- routers/web/auth/auth.go | 27 ++--- routers/web/auth/linkaccount.go | 66 +++++------ routers/web/auth/oauth.go | 56 +++++---- routers/web/auth/oauth_signin_sync.go | 88 ++++++++++++++ routers/web/auth/openid.go | 2 +- routers/web/auth/webauthn.go | 5 +- services/auth/source/oauth2/providers.go | 1 + services/auth/source/oauth2/providers_base.go | 7 ++ .../auth/source/oauth2/providers_openid.go | 4 + services/auth/source/oauth2/source.go | 3 + services/externalaccount/link.go | 30 ----- services/externalaccount/user.go | 26 ++--- services/forms/auth_form.go | 101 +++++++++------- templates/admin/auth/edit.tmpl | 15 ++- templates/admin/auth/source/oauth.tmpl | 16 ++- tests/integration/oauth_test.go | 110 ++++++++++++++++++ tests/integration/signin_test.go | 5 +- web_src/js/features/admin/common.ts | 3 + 27 files changed, 459 insertions(+), 206 deletions(-) create mode 100644 routers/web/auth/oauth_signin_sync.go delete mode 100644 services/externalaccount/link.go diff --git a/cmd/admin_auth_oauth.go b/cmd/admin_auth_oauth.go index d1aa753500..8848c94fc5 100644 --- a/cmd/admin_auth_oauth.go +++ b/cmd/admin_auth_oauth.go @@ -87,6 +87,14 @@ func oauthCLIFlags() []cli.Flag { Value: nil, Usage: "Scopes to request when to authenticate against this OAuth2 source", }, + &cli.StringFlag{ + Name: "ssh-public-key-claim-name", + Usage: "Claim name that provides SSH public keys", + }, + &cli.StringFlag{ + Name: "full-name-claim-name", + Usage: "Claim name that provides user's full name", + }, &cli.StringFlag{ Name: "required-claim-name", Value: "", @@ -177,6 +185,8 @@ func parseOAuth2Config(c *cli.Command) *oauth2.Source { RestrictedGroup: c.String("restricted-group"), GroupTeamMap: c.String("group-team-map"), GroupTeamMapRemoval: c.Bool("group-team-map-removal"), + SSHPublicKeyClaimName: c.String("ssh-public-key-claim-name"), + FullNameClaimName: c.String("full-name-claim-name"), } } @@ -268,6 +278,12 @@ func (a *authService) runUpdateOauth(ctx context.Context, c *cli.Command) error if c.IsSet("group-team-map-removal") { oAuth2Config.GroupTeamMapRemoval = c.Bool("group-team-map-removal") } + if c.IsSet("ssh-public-key-claim-name") { + oAuth2Config.SSHPublicKeyClaimName = c.String("ssh-public-key-claim-name") + } + if c.IsSet("full-name-claim-name") { + oAuth2Config.FullNameClaimName = c.String("full-name-claim-name") + } // update custom URL mapping customURLMapping := &oauth2.CustomURLMapping{} diff --git a/cmd/admin_auth_oauth_test.go b/cmd/admin_auth_oauth_test.go index df1bd9c1a6..bb9da667fd 100644 --- a/cmd/admin_auth_oauth_test.go +++ b/cmd/admin_auth_oauth_test.go @@ -88,6 +88,8 @@ func TestAddOauth(t *testing.T) { "--restricted-group", "restricted", "--group-team-map", `{"group1": [1,2]}`, "--group-team-map-removal=true", + "--ssh-public-key-claim-name", "attr_ssh_pub_key", + "--full-name-claim-name", "attr_full_name", }, source: &auth_model.Source{ Type: auth_model.OAuth2, @@ -104,15 +106,17 @@ func TestAddOauth(t *testing.T) { EmailURL: "https://example.com/email", Tenant: "some_tenant", }, - IconURL: "https://example.com/icon", - Scopes: []string{"scope1", "scope2"}, - RequiredClaimName: "claim_name", - RequiredClaimValue: "claim_value", - GroupClaimName: "group_name", - AdminGroup: "admin", - RestrictedGroup: "restricted", - GroupTeamMap: `{"group1": [1,2]}`, - GroupTeamMapRemoval: true, + IconURL: "https://example.com/icon", + Scopes: []string{"scope1", "scope2"}, + RequiredClaimName: "claim_name", + RequiredClaimValue: "claim_value", + GroupClaimName: "group_name", + AdminGroup: "admin", + RestrictedGroup: "restricted", + GroupTeamMap: `{"group1": [1,2]}`, + GroupTeamMapRemoval: true, + SSHPublicKeyClaimName: "attr_ssh_pub_key", + FullNameClaimName: "attr_full_name", }, TwoFactorPolicy: "skip", }, @@ -223,15 +227,17 @@ func TestUpdateOauth(t *testing.T) { EmailURL: "https://old.example.com/email", Tenant: "old_tenant", }, - IconURL: "https://old.example.com/icon", - Scopes: []string{"old_scope1", "old_scope2"}, - RequiredClaimName: "old_claim_name", - RequiredClaimValue: "old_claim_value", - GroupClaimName: "old_group_name", - AdminGroup: "old_admin", - RestrictedGroup: "old_restricted", - GroupTeamMap: `{"old_group1": [1,2]}`, - GroupTeamMapRemoval: true, + IconURL: "https://old.example.com/icon", + Scopes: []string{"old_scope1", "old_scope2"}, + RequiredClaimName: "old_claim_name", + RequiredClaimValue: "old_claim_value", + GroupClaimName: "old_group_name", + AdminGroup: "old_admin", + RestrictedGroup: "old_restricted", + GroupTeamMap: `{"old_group1": [1,2]}`, + GroupTeamMapRemoval: true, + SSHPublicKeyClaimName: "old_ssh_pub_key", + FullNameClaimName: "old_full_name", }, TwoFactorPolicy: "", }, @@ -257,6 +263,8 @@ func TestUpdateOauth(t *testing.T) { "--restricted-group", "restricted", "--group-team-map", `{"group1": [1,2]}`, "--group-team-map-removal=false", + "--ssh-public-key-claim-name", "new_ssh_pub_key", + "--full-name-claim-name", "new_full_name", }, authSource: &auth_model.Source{ ID: 1, @@ -274,15 +282,17 @@ func TestUpdateOauth(t *testing.T) { EmailURL: "https://example.com/email", Tenant: "new_tenant", }, - IconURL: "https://example.com/icon", - Scopes: []string{"scope1", "scope2"}, - RequiredClaimName: "claim_name", - RequiredClaimValue: "claim_value", - GroupClaimName: "group_name", - AdminGroup: "admin", - RestrictedGroup: "restricted", - GroupTeamMap: `{"group1": [1,2]}`, - GroupTeamMapRemoval: false, + IconURL: "https://example.com/icon", + Scopes: []string{"scope1", "scope2"}, + RequiredClaimName: "claim_name", + RequiredClaimValue: "claim_value", + GroupClaimName: "group_name", + AdminGroup: "admin", + RestrictedGroup: "restricted", + GroupTeamMap: `{"group1": [1,2]}`, + GroupTeamMapRemoval: false, + SSHPublicKeyClaimName: "new_ssh_pub_key", + FullNameClaimName: "new_full_name", }, TwoFactorPolicy: "skip", }, diff --git a/models/asymkey/ssh_key.go b/models/asymkey/ssh_key.go index 7a18732c32..dd94070fb9 100644 --- a/models/asymkey/ssh_key.go +++ b/models/asymkey/ssh_key.go @@ -355,13 +355,13 @@ func AddPublicKeysBySource(ctx context.Context, usr *user_model.User, s *auth.So return sshKeysNeedUpdate } -// SynchronizePublicKeys updates a users public keys. Returns true if there are changes. +// SynchronizePublicKeys updates a user's public keys. Returns true if there are changes. func SynchronizePublicKeys(ctx context.Context, usr *user_model.User, s *auth.Source, sshPublicKeys []string) bool { var sshKeysNeedUpdate bool log.Trace("synchronizePublicKeys[%s]: Handling Public SSH Key synchronization for user %s", s.Name, usr.Name) - // Get Public Keys from DB with current LDAP source + // Get Public Keys from DB with the current auth source var giteaKeys []string keys, err := db.Find[PublicKey](ctx, FindPublicKeyOptions{ OwnerID: usr.ID, diff --git a/models/auth/oauth2.go b/models/auth/oauth2.go index c2b6690116..55af4e9036 100644 --- a/models/auth/oauth2.go +++ b/models/auth/oauth2.go @@ -612,8 +612,8 @@ func (err ErrOAuthApplicationNotFound) Unwrap() error { return util.ErrNotExist } -// GetActiveOAuth2SourceByName returns a OAuth2 AuthSource based on the given name -func GetActiveOAuth2SourceByName(ctx context.Context, name string) (*Source, error) { +// GetActiveOAuth2SourceByAuthName returns a OAuth2 AuthSource based on the given name +func GetActiveOAuth2SourceByAuthName(ctx context.Context, name string) (*Source, error) { authSource := new(Source) has, err := db.GetEngine(ctx).Where("name = ? and type = ? and is_active = ?", name, OAuth2, true).Get(authSource) if err != nil { diff --git a/models/auth/source.go b/models/auth/source.go index 7d7bc0f03c..08cfc9615b 100644 --- a/models/auth/source.go +++ b/models/auth/source.go @@ -334,7 +334,7 @@ func UpdateSource(ctx context.Context, source *Source) error { err = registerableSource.RegisterSource() if err != nil { - // restore original values since we cannot update the provider it self + // restore original values since we cannot update the provider itself if _, err := db.GetEngine(ctx).ID(source.ID).AllCols().Update(originalSource); err != nil { log.Error("UpdateSource: Error while wrapOpenIDConnectInitializeError: %v", err) } diff --git a/modules/setting/oauth2.go b/modules/setting/oauth2.go index 0d3e63e0b4..1a88f3cb08 100644 --- a/modules/setting/oauth2.go +++ b/modules/setting/oauth2.go @@ -12,7 +12,7 @@ import ( "code.gitea.io/gitea/modules/log" ) -// OAuth2UsernameType is enum describing the way gitea 'name' should be generated from oauth2 data +// OAuth2UsernameType is enum describing the way gitea generates its 'username' from oauth2 data type OAuth2UsernameType string const ( diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 90cc164a60..ff32c94ff9 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3251,6 +3251,8 @@ auths.oauth2_required_claim_name_helper = Set this name to restrict login from t auths.oauth2_required_claim_value = Required Claim Value auths.oauth2_required_claim_value_helper = Set this value to restrict login from this source to users with a claim with this name and value auths.oauth2_group_claim_name = Claim name providing group names for this source. (Optional) +auths.oauth2_full_name_claim_name = Full Name Claim Name. (Optional, if set, the user's full name will always be synchronized with this claim) +auths.oauth2_ssh_public_key_claim_name = SSH Public Key Claim Name auths.oauth2_admin_group = Group Claim value for administrator users. (Optional - requires claim name above) auths.oauth2_restricted_group = Group Claim value for restricted users. (Optional - requires claim name above) auths.oauth2_map_group_to_team = Map claimed groups to Organization teams. (Optional - requires claim name above) diff --git a/routers/web/admin/auths.go b/routers/web/admin/auths.go index 0f6f31b884..56c384b970 100644 --- a/routers/web/admin/auths.go +++ b/routers/web/admin/auths.go @@ -199,6 +199,9 @@ func parseOAuth2Config(form forms.AuthenticationForm) *oauth2.Source { AdminGroup: form.Oauth2AdminGroup, GroupTeamMap: form.Oauth2GroupTeamMap, GroupTeamMapRemoval: form.Oauth2GroupTeamMapRemoval, + + SSHPublicKeyClaimName: form.Oauth2SSHPublicKeyClaimName, + FullNameClaimName: form.Oauth2FullNameClaimName, } } diff --git a/routers/web/auth/2fa.go b/routers/web/auth/2fa.go index d15d33dfd4..1f087a7897 100644 --- a/routers/web/auth/2fa.go +++ b/routers/web/auth/2fa.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/services/context" - "code.gitea.io/gitea/services/externalaccount" "code.gitea.io/gitea/services/forms" ) @@ -75,7 +74,7 @@ func TwoFactorPost(ctx *context.Context) { } if ctx.Session.Get("linkAccount") != nil { - err = externalaccount.LinkAccountFromStore(ctx, ctx.Session, u) + err = linkAccountFromContext(ctx, u) if err != nil { ctx.ServerError("UserSignIn", err) return diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 94f75f69ff..13cd083771 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -329,6 +329,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe "twofaUid", "twofaRemember", "linkAccount", + "linkAccountData", }, map[string]any{ session.KeyUID: u.ID, session.KeyUname: u.Name, @@ -519,7 +520,7 @@ func SignUpPost(ctx *context.Context) { Passwd: form.Password, } - if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, nil, false) { + if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, nil) { // error already handled return } @@ -530,22 +531,22 @@ func SignUpPost(ctx *context.Context) { // createAndHandleCreatedUser calls createUserInContext and // then handleUserCreated. -func createAndHandleCreatedUser(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, gothUser *goth.User, allowLink bool) bool { - if !createUserInContext(ctx, tpl, form, u, overwrites, gothUser, allowLink) { +func createAndHandleCreatedUser(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, possibleLinkAccountData *LinkAccountData) bool { + if !createUserInContext(ctx, tpl, form, u, overwrites, possibleLinkAccountData) { return false } - return handleUserCreated(ctx, u, gothUser) + return handleUserCreated(ctx, u, possibleLinkAccountData) } // createUserInContext creates a user and handles errors within a given context. -// Optionally a template can be specified. -func createUserInContext(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, gothUser *goth.User, allowLink bool) (ok bool) { +// Optionally, a template can be specified. +func createUserInContext(ctx *context.Context, tpl templates.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, possibleLinkAccountData *LinkAccountData) (ok bool) { meta := &user_model.Meta{ InitialIP: ctx.RemoteAddr(), InitialUserAgent: ctx.Req.UserAgent(), } if err := user_model.CreateUser(ctx, u, meta, overwrites); err != nil { - if allowLink && (user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err)) { + if possibleLinkAccountData != nil && (user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err)) { switch setting.OAuth2Client.AccountLinking { case setting.OAuth2AccountLinkingAuto: var user *user_model.User @@ -561,15 +562,15 @@ func createUserInContext(ctx *context.Context, tpl templates.TplName, form any, } // TODO: probably we should respect 'remember' user's choice... - linkAccount(ctx, user, *gothUser, true) + oauth2LinkAccount(ctx, user, possibleLinkAccountData, true) return false // user is already created here, all redirects are handled case setting.OAuth2AccountLinkingLogin: - showLinkingLogin(ctx, *gothUser) + showLinkingLogin(ctx, &possibleLinkAccountData.AuthSource, possibleLinkAccountData.GothUser) return false // user will be created only after linking login } } - // handle error without template + // handle error without a template if len(tpl) == 0 { ctx.ServerError("CreateUser", err) return false @@ -610,7 +611,7 @@ func createUserInContext(ctx *context.Context, tpl templates.TplName, form any, // handleUserCreated does additional steps after a new user is created. // It auto-sets admin for the only user, updates the optional external user and // sends a confirmation email if required. -func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.User) (ok bool) { +func handleUserCreated(ctx *context.Context, u *user_model.User, possibleLinkAccountData *LinkAccountData) (ok bool) { // Auto-set admin for the only user. hasUsers, err := user_model.HasUsers(ctx) if err != nil { @@ -631,8 +632,8 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. } // update external user information - if gothUser != nil { - if err := externalaccount.EnsureLinkExternalToUser(ctx, u, *gothUser); err != nil { + if possibleLinkAccountData != nil { + if err := externalaccount.EnsureLinkExternalToUser(ctx, possibleLinkAccountData.AuthSource.ID, u, possibleLinkAccountData.GothUser); err != nil { log.Error("EnsureLinkExternalToUser failed: %v", err) } } diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index b3c61946b9..cf1aa302c4 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -5,7 +5,6 @@ package auth import ( "errors" - "fmt" "net/http" "strings" @@ -21,8 +20,6 @@ import ( "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/externalaccount" "code.gitea.io/gitea/services/forms" - - "github.com/markbates/goth" ) var tplLinkAccount templates.TplName = "user/auth/link_account" @@ -52,28 +49,28 @@ func LinkAccount(ctx *context.Context) { ctx.Data["SignInLink"] = setting.AppSubURL + "/user/link_account_signin" ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/link_account_signup" - gothUser, ok := ctx.Session.Get("linkAccountGothUser").(goth.User) + linkAccountData := oauth2GetLinkAccountData(ctx) // If you'd like to quickly debug the "link account" page layout, just uncomment the blow line // Don't worry, when the below line exists, the lint won't pass: ineffectual assignment to gothUser (ineffassign) - // gothUser, ok = goth.User{Email: "invalid-email", Name: "."}, true // intentionally use invalid data to avoid pass the registration check + // linkAccountData = &LinkAccountData{authSource, gothUser} // intentionally use invalid data to avoid pass the registration check - if !ok { + if linkAccountData == nil { // no account in session, so just redirect to the login page, then the user could restart the process ctx.Redirect(setting.AppSubURL + "/user/login") return } - if missingFields, ok := gothUser.RawData["__giteaAutoRegMissingFields"].([]string); ok { - ctx.Data["AutoRegistrationFailedPrompt"] = ctx.Tr("auth.oauth_callback_unable_auto_reg", gothUser.Provider, strings.Join(missingFields, ",")) + if missingFields, ok := linkAccountData.GothUser.RawData["__giteaAutoRegMissingFields"].([]string); ok { + ctx.Data["AutoRegistrationFailedPrompt"] = ctx.Tr("auth.oauth_callback_unable_auto_reg", linkAccountData.GothUser.Provider, strings.Join(missingFields, ",")) } - uname, err := extractUserNameFromOAuth2(&gothUser) + uname, err := extractUserNameFromOAuth2(&linkAccountData.GothUser) if err != nil { ctx.ServerError("UserSignIn", err) return } - email := gothUser.Email + email := linkAccountData.GothUser.Email ctx.Data["user_name"] = uname ctx.Data["email"] = email @@ -152,8 +149,8 @@ func LinkAccountPostSignIn(ctx *context.Context) { ctx.Data["SignInLink"] = setting.AppSubURL + "/user/link_account_signin" ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/link_account_signup" - gothUser := ctx.Session.Get("linkAccountGothUser") - if gothUser == nil { + linkAccountData := oauth2GetLinkAccountData(ctx) + if linkAccountData == nil { ctx.ServerError("UserSignIn", errors.New("not in LinkAccount session")) return } @@ -169,11 +166,14 @@ func LinkAccountPostSignIn(ctx *context.Context) { return } - linkAccount(ctx, u, gothUser.(goth.User), signInForm.Remember) + oauth2LinkAccount(ctx, u, linkAccountData, signInForm.Remember) } -func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, remember bool) { - updateAvatarIfNeed(ctx, gothUser.AvatarURL, u) +func oauth2LinkAccount(ctx *context.Context, u *user_model.User, linkAccountData *LinkAccountData, remember bool) { + oauth2SignInSync(ctx, &linkAccountData.AuthSource, u, linkAccountData.GothUser) + if ctx.Written() { + return + } // If this user is enrolled in 2FA, we can't sign the user in just yet. // Instead, redirect them to the 2FA authentication page. @@ -185,7 +185,7 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r return } - err = externalaccount.LinkAccountToUser(ctx, u, gothUser) + err = externalaccount.LinkAccountToUser(ctx, linkAccountData.AuthSource.ID, u, linkAccountData.GothUser) if err != nil { ctx.ServerError("UserLinkAccount", err) return @@ -243,17 +243,11 @@ func LinkAccountPostRegister(ctx *context.Context) { ctx.Data["SignInLink"] = setting.AppSubURL + "/user/link_account_signin" ctx.Data["SignUpLink"] = setting.AppSubURL + "/user/link_account_signup" - gothUserInterface := ctx.Session.Get("linkAccountGothUser") - if gothUserInterface == nil { + linkAccountData := oauth2GetLinkAccountData(ctx) + if linkAccountData == nil { ctx.ServerError("UserSignUp", errors.New("not in LinkAccount session")) return } - gothUser, ok := gothUserInterface.(goth.User) - if !ok { - ctx.ServerError("UserSignUp", fmt.Errorf("session linkAccountGothUser type is %t but not goth.User", gothUserInterface)) - return - } - if ctx.HasError() { ctx.HTML(http.StatusOK, tplLinkAccount) return @@ -296,31 +290,33 @@ func LinkAccountPostRegister(ctx *context.Context) { } } - authSource, err := auth.GetActiveOAuth2SourceByName(ctx, gothUser.Provider) - if err != nil { - ctx.ServerError("CreateUser", err) - return - } - u := &user_model.User{ Name: form.UserName, Email: form.Email, Passwd: form.Password, LoginType: auth.OAuth2, - LoginSource: authSource.ID, - LoginName: gothUser.UserID, + LoginSource: linkAccountData.AuthSource.ID, + LoginName: linkAccountData.GothUser.UserID, } - if !createAndHandleCreatedUser(ctx, tplLinkAccount, form, u, nil, &gothUser, false) { + if !createAndHandleCreatedUser(ctx, tplLinkAccount, form, u, nil, linkAccountData) { // error already handled return } - source := authSource.Cfg.(*oauth2.Source) - if err := syncGroupsToTeams(ctx, source, &gothUser, u); err != nil { + source := linkAccountData.AuthSource.Cfg.(*oauth2.Source) + if err := syncGroupsToTeams(ctx, source, &linkAccountData.GothUser, u); err != nil { ctx.ServerError("SyncGroupsToTeams", err) return } handleSignIn(ctx, u, false) } + +func linkAccountFromContext(ctx *context.Context, user *user_model.User) error { + linkAccountData := oauth2GetLinkAccountData(ctx) + if linkAccountData == nil { + return errors.New("not in LinkAccount session") + } + return externalaccount.LinkAccountToUser(ctx, linkAccountData.AuthSource.ID, user, linkAccountData.GothUser) +} diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index a13b987aab..3df2734bb6 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -20,7 +20,6 @@ import ( "code.gitea.io/gitea/modules/optional" "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/modules/web/middleware" source_service "code.gitea.io/gitea/services/auth/source" "code.gitea.io/gitea/services/auth/source/oauth2" @@ -35,9 +34,8 @@ import ( // SignInOAuth handles the OAuth2 login buttons func SignInOAuth(ctx *context.Context) { - provider := ctx.PathParam("provider") - - authSource, err := auth.GetActiveOAuth2SourceByName(ctx, provider) + authName := ctx.PathParam("provider") + authSource, err := auth.GetActiveOAuth2SourceByAuthName(ctx, authName) if err != nil { ctx.ServerError("SignIn", err) return @@ -74,8 +72,6 @@ func SignInOAuth(ctx *context.Context) { // SignInOAuthCallback handles the callback from the given provider func SignInOAuthCallback(ctx *context.Context) { - provider := ctx.PathParam("provider") - if ctx.Req.FormValue("error") != "" { var errorKeyValues []string for k, vv := range ctx.Req.Form { @@ -88,7 +84,8 @@ func SignInOAuthCallback(ctx *context.Context) { } // first look if the provider is still active - authSource, err := auth.GetActiveOAuth2SourceByName(ctx, provider) + authName := ctx.PathParam("provider") + authSource, err := auth.GetActiveOAuth2SourceByAuthName(ctx, authName) if err != nil { ctx.ServerError("SignIn", err) return @@ -133,7 +130,7 @@ func SignInOAuthCallback(ctx *context.Context) { if u == nil { if ctx.Doer != nil { // attach user to the current signed-in user - err = externalaccount.LinkAccountToUser(ctx, ctx.Doer, gothUser) + err = externalaccount.LinkAccountToUser(ctx, authSource.ID, ctx.Doer, gothUser) if err != nil { ctx.ServerError("UserLinkAccount", err) return @@ -174,12 +171,11 @@ func SignInOAuthCallback(ctx *context.Context) { gothUser.RawData = make(map[string]any) } gothUser.RawData["__giteaAutoRegMissingFields"] = missingFields - showLinkingLogin(ctx, gothUser) + showLinkingLogin(ctx, authSource, gothUser) return } u = &user_model.User{ Name: uname, - FullName: gothUser.Name, Email: gothUser.Email, LoginType: auth.OAuth2, LoginSource: authSource.ID, @@ -196,7 +192,11 @@ func SignInOAuthCallback(ctx *context.Context) { u.IsAdmin = isAdmin.ValueOrDefault(user_service.UpdateOptionField[bool]{FieldValue: false}).FieldValue u.IsRestricted = isRestricted.ValueOrDefault(setting.Service.DefaultUserIsRestricted) - if !createAndHandleCreatedUser(ctx, templates.TplName(""), nil, u, overwriteDefault, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) { + linkAccountData := &LinkAccountData{*authSource, gothUser} + if setting.OAuth2Client.AccountLinking == setting.OAuth2AccountLinkingDisabled { + linkAccountData = nil + } + if !createAndHandleCreatedUser(ctx, "", nil, u, overwriteDefault, linkAccountData) { // error already handled return } @@ -207,7 +207,7 @@ func SignInOAuthCallback(ctx *context.Context) { } } else { // no existing user is found, request attach or new account - showLinkingLogin(ctx, gothUser) + showLinkingLogin(ctx, authSource, gothUser) return } } @@ -271,9 +271,22 @@ func getUserAdminAndRestrictedFromGroupClaims(source *oauth2.Source, gothUser *g return isAdmin, isRestricted } -func showLinkingLogin(ctx *context.Context, gothUser goth.User) { +type LinkAccountData struct { + AuthSource auth.Source + GothUser goth.User +} + +func oauth2GetLinkAccountData(ctx *context.Context) *LinkAccountData { + v, ok := ctx.Session.Get("linkAccountData").(LinkAccountData) + if !ok { + return nil + } + return &v +} + +func showLinkingLogin(ctx *context.Context, authSource *auth.Source, gothUser goth.User) { if err := updateSession(ctx, nil, map[string]any{ - "linkAccountGothUser": gothUser, + "linkAccountData": LinkAccountData{*authSource, gothUser}, }); err != nil { ctx.ServerError("updateSession", err) return @@ -281,7 +294,7 @@ func showLinkingLogin(ctx *context.Context, gothUser goth.User) { ctx.Redirect(setting.AppSubURL + "/user/link_account") } -func updateAvatarIfNeed(ctx *context.Context, url string, u *user_model.User) { +func oauth2UpdateAvatarIfNeed(ctx *context.Context, url string, u *user_model.User) { if setting.OAuth2Client.UpdateAvatar && len(url) > 0 { resp, err := http.Get(url) if err == nil { @@ -299,11 +312,14 @@ func updateAvatarIfNeed(ctx *context.Context, url string, u *user_model.User) { } } -func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model.User, gothUser goth.User) { - updateAvatarIfNeed(ctx, gothUser.AvatarURL, u) +func handleOAuth2SignIn(ctx *context.Context, authSource *auth.Source, u *user_model.User, gothUser goth.User) { + oauth2SignInSync(ctx, authSource, u, gothUser) + if ctx.Written() { + return + } needs2FA := false - if !source.TwoFactorShouldSkip() { + if !authSource.TwoFactorShouldSkip() { _, err := auth.GetTwoFactorByUID(ctx, u.ID) if err != nil && !auth.IsErrTwoFactorNotEnrolled(err) { ctx.ServerError("UserSignIn", err) @@ -312,7 +328,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model needs2FA = err == nil } - oauth2Source := source.Cfg.(*oauth2.Source) + oauth2Source := authSource.Cfg.(*oauth2.Source) groupTeamMapping, err := auth_module.UnmarshalGroupTeamMapping(oauth2Source.GroupTeamMap) if err != nil { ctx.ServerError("UnmarshalGroupTeamMapping", err) @@ -338,7 +354,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model } } - if err := externalaccount.EnsureLinkExternalToUser(ctx, u, gothUser); err != nil { + if err := externalaccount.EnsureLinkExternalToUser(ctx, authSource.ID, u, gothUser); err != nil { ctx.ServerError("EnsureLinkExternalToUser", err) return } diff --git a/routers/web/auth/oauth_signin_sync.go b/routers/web/auth/oauth_signin_sync.go new file mode 100644 index 0000000000..787ea9223c --- /dev/null +++ b/routers/web/auth/oauth_signin_sync.go @@ -0,0 +1,88 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package auth + +import ( + "fmt" + + asymkey_model "code.gitea.io/gitea/models/asymkey" + "code.gitea.io/gitea/models/auth" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" + asymkey_service "code.gitea.io/gitea/services/asymkey" + "code.gitea.io/gitea/services/auth/source/oauth2" + "code.gitea.io/gitea/services/context" + + "github.com/markbates/goth" +) + +func oauth2SignInSync(ctx *context.Context, authSource *auth.Source, u *user_model.User, gothUser goth.User) { + oauth2UpdateAvatarIfNeed(ctx, gothUser.AvatarURL, u) + + oauth2Source, _ := authSource.Cfg.(*oauth2.Source) + if !authSource.IsOAuth2() || oauth2Source == nil { + ctx.ServerError("oauth2SignInSync", fmt.Errorf("source %s is not an OAuth2 source", gothUser.Provider)) + return + } + + // sync full name + fullNameKey := util.IfZero(oauth2Source.FullNameClaimName, "name") + fullName, _ := gothUser.RawData[fullNameKey].(string) + fullName = util.IfZero(fullName, gothUser.Name) + + // need to update if the user has no full name set + shouldUpdateFullName := u.FullName == "" + // force to update if the attribute is set + shouldUpdateFullName = shouldUpdateFullName || oauth2Source.FullNameClaimName != "" + // only update if the full name is different + shouldUpdateFullName = shouldUpdateFullName && u.FullName != fullName + if shouldUpdateFullName { + u.FullName = fullName + if err := user_model.UpdateUserCols(ctx, u, "full_name"); err != nil { + log.Error("Unable to sync OAuth2 user full name %s: %v", gothUser.Provider, err) + } + } + + err := oauth2UpdateSSHPubIfNeed(ctx, authSource, &gothUser, u) + if err != nil { + log.Error("Unable to sync OAuth2 SSH public key %s: %v", gothUser.Provider, err) + } +} + +func oauth2SyncGetSSHKeys(source *oauth2.Source, gothUser *goth.User) ([]string, error) { + value, exists := gothUser.RawData[source.SSHPublicKeyClaimName] + if !exists { + return []string{}, nil + } + rawSlice, ok := value.([]any) + if !ok { + return nil, fmt.Errorf("invalid SSH public key value type: %T", value) + } + + sshKeys := make([]string, 0, len(rawSlice)) + for _, v := range rawSlice { + str, ok := v.(string) + if !ok { + return nil, fmt.Errorf("invalid SSH public key value item type: %T", v) + } + sshKeys = append(sshKeys, str) + } + return sshKeys, nil +} + +func oauth2UpdateSSHPubIfNeed(ctx *context.Context, authSource *auth.Source, gothUser *goth.User, user *user_model.User) error { + oauth2Source, _ := authSource.Cfg.(*oauth2.Source) + if oauth2Source == nil || oauth2Source.SSHPublicKeyClaimName == "" { + return nil + } + sshKeys, err := oauth2SyncGetSSHKeys(oauth2Source, gothUser) + if err != nil { + return err + } + if !asymkey_model.SynchronizePublicKeys(ctx, user, authSource, sshKeys) { + return nil + } + return asymkey_service.RewriteAllPublicKeys(ctx) +} diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go index 2ef4a86022..4ef4c96ccc 100644 --- a/routers/web/auth/openid.go +++ b/routers/web/auth/openid.go @@ -361,7 +361,7 @@ func RegisterOpenIDPost(ctx *context.Context) { Email: form.Email, Passwd: password, } - if !createUserInContext(ctx, tplSignUpOID, form, u, nil, nil, false) { + if !createUserInContext(ctx, tplSignUpOID, form, u, nil, nil) { // error already handled return } diff --git a/routers/web/auth/webauthn.go b/routers/web/auth/webauthn.go index 78f6c3b58e..dacb6be225 100644 --- a/routers/web/auth/webauthn.go +++ b/routers/web/auth/webauthn.go @@ -15,7 +15,6 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" "code.gitea.io/gitea/services/context" - "code.gitea.io/gitea/services/externalaccount" "github.com/go-webauthn/webauthn/protocol" "github.com/go-webauthn/webauthn/webauthn" @@ -150,7 +149,7 @@ func WebAuthnPasskeyLogin(ctx *context.Context) { // Now handle account linking if that's requested if ctx.Session.Get("linkAccount") != nil { - if err := externalaccount.LinkAccountFromStore(ctx, ctx.Session, user); err != nil { + if err := linkAccountFromContext(ctx, user); err != nil { ctx.ServerError("LinkAccountFromStore", err) return } @@ -268,7 +267,7 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) { // Now handle account linking if that's requested if ctx.Session.Get("linkAccount") != nil { - if err := externalaccount.LinkAccountFromStore(ctx, ctx.Session, user); err != nil { + if err := linkAccountFromContext(ctx, user); err != nil { ctx.ServerError("LinkAccountFromStore", err) return } diff --git a/services/auth/source/oauth2/providers.go b/services/auth/source/oauth2/providers.go index f2c1bb4894..75ed41ba66 100644 --- a/services/auth/source/oauth2/providers.go +++ b/services/auth/source/oauth2/providers.go @@ -27,6 +27,7 @@ type Provider interface { DisplayName() string IconHTML(size int) template.HTML CustomURLSettings() *CustomURLSettings + SupportSSHPublicKey() bool } // GothProviderCreator provides a function to create a goth.Provider diff --git a/services/auth/source/oauth2/providers_base.go b/services/auth/source/oauth2/providers_base.go index 9d4ab106e5..d34597d6d9 100644 --- a/services/auth/source/oauth2/providers_base.go +++ b/services/auth/source/oauth2/providers_base.go @@ -14,6 +14,13 @@ import ( type BaseProvider struct { name string displayName string + + // TODO: maybe some providers also support SSH public keys, then they can set this to true + supportSSHPublicKey bool +} + +func (b *BaseProvider) SupportSSHPublicKey() bool { + return b.supportSSHPublicKey } // Name provides the technical name for this provider diff --git a/services/auth/source/oauth2/providers_openid.go b/services/auth/source/oauth2/providers_openid.go index 285876d5ac..e86dc48232 100644 --- a/services/auth/source/oauth2/providers_openid.go +++ b/services/auth/source/oauth2/providers_openid.go @@ -17,6 +17,10 @@ import ( // OpenIDProvider is a GothProvider for OpenID type OpenIDProvider struct{} +func (o *OpenIDProvider) SupportSSHPublicKey() bool { + return true +} + // Name provides the technical name for this provider func (o *OpenIDProvider) Name() string { return "openidConnect" diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 08837de377..00d89b3481 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -27,6 +27,9 @@ type Source struct { GroupTeamMap string GroupTeamMapRemoval bool RestrictedGroup string + + SSHPublicKeyClaimName string + FullNameClaimName string } // FromDB fills up an OAuth2Config from serialized format. diff --git a/services/externalaccount/link.go b/services/externalaccount/link.go deleted file mode 100644 index ab853140cb..0000000000 --- a/services/externalaccount/link.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2021 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package externalaccount - -import ( - "context" - "errors" - - user_model "code.gitea.io/gitea/models/user" - - "github.com/markbates/goth" -) - -// Store represents a thing that stores things -type Store interface { - Get(any) any - Set(any, any) error - Release() error -} - -// LinkAccountFromStore links the provided user with a stored external user -func LinkAccountFromStore(ctx context.Context, store Store, user *user_model.User) error { - gothUser := store.Get("linkAccountGothUser") - if gothUser == nil { - return errors.New("not in LinkAccount session") - } - - return LinkAccountToUser(ctx, user, gothUser.(goth.User)) -} diff --git a/services/externalaccount/user.go b/services/externalaccount/user.go index b53e33654a..1eddc4a5df 100644 --- a/services/externalaccount/user.go +++ b/services/externalaccount/user.go @@ -8,7 +8,6 @@ import ( "strconv" "strings" - "code.gitea.io/gitea/models/auth" issues_model "code.gitea.io/gitea/models/issues" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -17,15 +16,11 @@ import ( "github.com/markbates/goth" ) -func toExternalLoginUser(ctx context.Context, user *user_model.User, gothUser goth.User) (*user_model.ExternalLoginUser, error) { - authSource, err := auth.GetActiveOAuth2SourceByName(ctx, gothUser.Provider) - if err != nil { - return nil, err - } +func toExternalLoginUser(authSourceID int64, user *user_model.User, gothUser goth.User) *user_model.ExternalLoginUser { return &user_model.ExternalLoginUser{ ExternalID: gothUser.UserID, UserID: user.ID, - LoginSourceID: authSource.ID, + LoginSourceID: authSourceID, RawData: gothUser.RawData, Provider: gothUser.Provider, Email: gothUser.Email, @@ -40,15 +35,12 @@ func toExternalLoginUser(ctx context.Context, user *user_model.User, gothUser go AccessTokenSecret: gothUser.AccessTokenSecret, RefreshToken: gothUser.RefreshToken, ExpiresAt: gothUser.ExpiresAt, - }, nil + } } // LinkAccountToUser link the gothUser to the user -func LinkAccountToUser(ctx context.Context, user *user_model.User, gothUser goth.User) error { - externalLoginUser, err := toExternalLoginUser(ctx, user, gothUser) - if err != nil { - return err - } +func LinkAccountToUser(ctx context.Context, authSourceID int64, user *user_model.User, gothUser goth.User) error { + externalLoginUser := toExternalLoginUser(authSourceID, user, gothUser) if err := user_model.LinkExternalToUser(ctx, user, externalLoginUser); err != nil { return err @@ -72,12 +64,8 @@ func LinkAccountToUser(ctx context.Context, user *user_model.User, gothUser goth } // EnsureLinkExternalToUser link the gothUser to the user -func EnsureLinkExternalToUser(ctx context.Context, user *user_model.User, gothUser goth.User) error { - externalLoginUser, err := toExternalLoginUser(ctx, user, gothUser) - if err != nil { - return err - } - +func EnsureLinkExternalToUser(ctx context.Context, authSourceID int64, user *user_model.User, gothUser goth.User) error { + externalLoginUser := toExternalLoginUser(authSourceID, user, gothUser) return user_model.EnsureLinkExternalToUser(ctx, externalLoginUser) } diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index a8f97572b1..886110236c 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -18,45 +18,54 @@ type AuthenticationForm struct { Type int `binding:"Range(2,7)"` Name string `binding:"Required;MaxSize(30)"` TwoFactorPolicy string + IsActive bool + IsSyncEnabled bool - Host string - Port int - BindDN string - BindPassword string - UserBase string - UserDN string - AttributeUsername string - AttributeName string - AttributeSurname string - AttributeMail string - AttributeSSHPublicKey string - AttributeAvatar string - AttributesInBind bool - UsePagedSearch bool - SearchPageSize int - Filter string - AdminFilter string - GroupsEnabled bool - GroupDN string - GroupFilter string - GroupMemberUID string - UserUID string - RestrictedFilter string - AllowDeactivateAll bool - IsActive bool - IsSyncEnabled bool - SMTPAuth string - SMTPHost string - SMTPPort int - AllowedDomains string - SecurityProtocol int `binding:"Range(0,2)"` - TLS bool - SkipVerify bool - HeloHostname string - DisableHelo bool - ForceSMTPS bool - PAMServiceName string - PAMEmailDomain string + // LDAP + Host string + Port int + BindDN string + BindPassword string + UserBase string + UserDN string + AttributeUsername string + AttributeName string + AttributeSurname string + AttributeMail string + AttributeSSHPublicKey string + AttributeAvatar string + AttributesInBind bool + UsePagedSearch bool + SearchPageSize int + Filter string + AdminFilter string + GroupsEnabled bool + GroupDN string + GroupFilter string + GroupMemberUID string + UserUID string + RestrictedFilter string + AllowDeactivateAll bool + GroupTeamMap string `binding:"ValidGroupTeamMap"` + GroupTeamMapRemoval bool + + // SMTP + SMTPAuth string + SMTPHost string + SMTPPort int + AllowedDomains string + SecurityProtocol int `binding:"Range(0,2)"` + TLS bool + SkipVerify bool + HeloHostname string + DisableHelo bool + ForceSMTPS bool + + // PAM + PAMServiceName string + PAMEmailDomain string + + // Oauth2 & OIDC Oauth2Provider string Oauth2Key string Oauth2Secret string @@ -76,13 +85,15 @@ type AuthenticationForm struct { Oauth2RestrictedGroup string Oauth2GroupTeamMap string `binding:"ValidGroupTeamMap"` Oauth2GroupTeamMapRemoval bool - SSPIAutoCreateUsers bool - SSPIAutoActivateUsers bool - SSPIStripDomainNames bool - SSPISeparatorReplacement string `binding:"AlphaDashDot;MaxSize(5)"` - SSPIDefaultLanguage string - GroupTeamMap string `binding:"ValidGroupTeamMap"` - GroupTeamMapRemoval bool + Oauth2SSHPublicKeyClaimName string + Oauth2FullNameClaimName string + + // SSPI + SSPIAutoCreateUsers bool + SSPIAutoActivateUsers bool + SSPIStripDomainNames bool + SSPISeparatorReplacement string `binding:"AlphaDashDot;MaxSize(5)"` + SSPIDefaultLanguage string } // Validate validates fields diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 781f514af4..7b96b4e94f 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -301,19 +301,30 @@
- {{range .OAuth2Providers}}{{if .CustomURLSettings}} + {{range .OAuth2Providers}} + + {{if .CustomURLSettings}} - {{end}}{{end}} + {{end}} + {{end}}
+
+ + +
+
+ + +
diff --git a/templates/admin/auth/source/oauth.tmpl b/templates/admin/auth/source/oauth.tmpl index f02c5bdf30..69590635e4 100644 --- a/templates/admin/auth/source/oauth.tmpl +++ b/templates/admin/auth/source/oauth.tmpl @@ -63,19 +63,31 @@
- {{range .OAuth2Providers}}{{if .CustomURLSettings}} + {{range .OAuth2Providers}} + + {{if .CustomURLSettings}} - {{end}}{{end}} + {{end}} + {{end}}
+ +
+ + +
+
+ + +
diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index f8bc33c32a..a2247801f7 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -9,9 +9,11 @@ import ( "fmt" "io" "net/http" + "net/http/httptest" "strings" "testing" + asymkey_model "code.gitea.io/gitea/models/asymkey" auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" @@ -20,9 +22,13 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/services/auth/source/oauth2" "code.gitea.io/gitea/services/oauth2_provider" "code.gitea.io/gitea/tests" + "github.com/markbates/goth" + "github.com/markbates/goth/gothic" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -931,3 +937,107 @@ func testOAuth2WellKnown(t *testing.T) { defer test.MockVariableValue(&setting.OAuth2.Enabled, false)() MakeRequest(t, NewRequest(t, "GET", urlOpenidConfiguration), http.StatusNotFound) } + +func addOAuth2Source(t *testing.T, authName string, cfg oauth2.Source) { + cfg.Provider = util.IfZero(cfg.Provider, "gitea") + err := auth_model.CreateSource(db.DefaultContext, &auth_model.Source{ + Type: auth_model.OAuth2, + Name: authName, + IsActive: true, + Cfg: &cfg, + }) + require.NoError(t, err) +} + +func TestSignInOauthCallbackSyncSSHKeys(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + var mockServer *httptest.Server + mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/openid-configuration": + _, _ = w.Write([]byte(`{ + "issuer": "` + mockServer.URL + `", + "authorization_endpoint": "` + mockServer.URL + `/authorize", + "token_endpoint": "` + mockServer.URL + `/token", + "userinfo_endpoint": "` + mockServer.URL + `/userinfo" + }`)) + default: + http.NotFound(w, r) + } + })) + defer mockServer.Close() + + ctx := t.Context() + oauth2Source := oauth2.Source{ + Provider: "openidConnect", + ClientID: "test-client-id", + SSHPublicKeyClaimName: "sshpubkey", + FullNameClaimName: "name", + OpenIDConnectAutoDiscoveryURL: mockServer.URL + "/.well-known/openid-configuration", + } + addOAuth2Source(t, "test-oidc-source", oauth2Source) + authSource, err := auth_model.GetActiveOAuth2SourceByAuthName(ctx, "test-oidc-source") + require.NoError(t, err) + + sshKey1 := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAICV0MGX/W9IvLA4FXpIuUcdDcbj5KX4syHgsTy7soVgf" + sshKey2 := "sk-ssh-ed25519@openssh.com AAAAGnNrLXNzaC1lZDI1NTE5QG9wZW5zc2guY29tAAAAIE7kM1R02+4ertDKGKEDcKG0s+2vyDDcIvceJ0Gqv5f1AAAABHNzaDo=" + sshKey3 := "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIEHjnNEfE88W1pvBLdV3otv28x760gdmPao3lVD5uAt9" + cases := []struct { + testName string + mockFullName string + mockRawData map[string]any + expectedSSHPubKeys []string + }{ + { + testName: "Login1", + mockFullName: "FullName1", + mockRawData: map[string]any{"sshpubkey": []any{sshKey1 + " any-comment"}}, + expectedSSHPubKeys: []string{sshKey1}, + }, + { + testName: "Login2", + mockFullName: "FullName2", + mockRawData: map[string]any{"sshpubkey": []any{sshKey2 + " any-comment", sshKey3}}, + expectedSSHPubKeys: []string{sshKey2, sshKey3}, + }, + { + testName: "Login3", + mockFullName: "FullName3", + mockRawData: map[string]any{}, + expectedSSHPubKeys: []string{}, + }, + } + + session := emptyTestSession(t) + for _, c := range cases { + t.Run(c.testName, func(t *testing.T) { + defer test.MockVariableValue(&setting.OAuth2Client.Username, "")() + defer test.MockVariableValue(&setting.OAuth2Client.EnableAutoRegistration, true)() + defer test.MockVariableValue(&gothic.CompleteUserAuth, func(res http.ResponseWriter, req *http.Request) (goth.User, error) { + return goth.User{ + Provider: authSource.Cfg.(*oauth2.Source).Provider, + UserID: "oidc-userid", + Email: "oidc-email@example.com", + RawData: c.mockRawData, + Name: c.mockFullName, + }, nil + })() + req := NewRequest(t, "GET", "/user/oauth2/test-oidc-source/callback?code=XYZ&state=XYZ") + session.MakeRequest(t, req, http.StatusSeeOther) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "oidc-userid"}) + keys, _, err := db.FindAndCount[asymkey_model.PublicKey](ctx, asymkey_model.FindPublicKeyOptions{ + ListOptions: db.ListOptionsAll, + OwnerID: user.ID, + LoginSourceID: authSource.ID, + }) + require.NoError(t, err) + var sshPubKeys []string + for _, key := range keys { + sshPubKeys = append(sshPubKeys, key.Content) + } + assert.ElementsMatch(t, c.expectedSSHPubKeys, sshPubKeys) + assert.Equal(t, c.mockFullName, user.FullName) + }) + } +} diff --git a/tests/integration/signin_test.go b/tests/integration/signin_test.go index 67af5b5877..aa1571c163 100644 --- a/tests/integration/signin_test.go +++ b/tests/integration/signin_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" @@ -17,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers" + "code.gitea.io/gitea/routers/web/auth" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/tests" @@ -103,8 +105,9 @@ func TestEnablePasswordSignInFormAndEnablePasskeyAuth(t *testing.T) { defer tests.PrepareTestEnv(t)() mockLinkAccount := func(ctx *context.Context) { + authSource := auth_model.Source{ID: 1} gothUser := goth.User{Email: "invalid-email", Name: "."} - _ = ctx.Session.Set("linkAccountGothUser", gothUser) + _ = ctx.Session.Set("linkAccountData", auth.LinkAccountData{AuthSource: authSource, GothUser: gothUser}) } t.Run("EnablePasswordSignInForm=false", func(t *testing.T) { diff --git a/web_src/js/features/admin/common.ts b/web_src/js/features/admin/common.ts index 4ed5d62eee..dd5b1f464d 100644 --- a/web_src/js/features/admin/common.ts +++ b/web_src/js/features/admin/common.ts @@ -102,6 +102,9 @@ function initAdminAuthentication() { break; } } + + const supportSshPublicKey = document.querySelector(`#${provider}_SupportSSHPublicKey`)?.value === 'true'; + toggleElem('.field.oauth2_ssh_public_key_claim_name', supportSshPublicKey); onOAuth2UseCustomURLChange(applyDefaultValues); } From 7a15334656cb2b9df0f79a692518ecb38674ba58 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 11 Jul 2025 03:03:36 +0800 Subject: [PATCH 13/15] Fix git commit committer parsing and add some tests (#35007) * Fix #34991 * Fix #34882 --------- Co-authored-by: wxiaoguang --- Dockerfile | 2 - Dockerfile.rootless | 2 - models/asymkey/gpg_key_commit_verification.go | 23 +----- models/user/user.go | 16 ++-- models/user/user_test.go | 5 ++ modules/git/commit.go | 6 +- services/asymkey/commit.go | 62 ++++++++-------- services/asymkey/commit_test.go | 2 +- services/git/commit.go | 7 -- templates/repo/commit_page.tmpl | 2 +- templates/repo/commits_list.tmpl | 2 +- tests/integration/repo_commits_test.go | 74 ++++++++++++------- 12 files changed, 95 insertions(+), 108 deletions(-) diff --git a/Dockerfile b/Dockerfile index c9e6a2d3db..f852cf4235 100644 --- a/Dockerfile +++ b/Dockerfile @@ -39,7 +39,6 @@ RUN chmod 755 /tmp/local/usr/bin/entrypoint \ /tmp/local/etc/s6/.s6-svscan/* \ /go/src/code.gitea.io/gitea/gitea \ /go/src/code.gitea.io/gitea/environment-to-ini -RUN chmod 644 /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete FROM docker.io/library/alpine:3.22 LABEL maintainer="maintainers@gitea.io" @@ -83,4 +82,3 @@ CMD ["/usr/bin/s6-svscan", "/etc/s6"] COPY --from=build-env /tmp/local / COPY --from=build-env /go/src/code.gitea.io/gitea/gitea /app/gitea/gitea COPY --from=build-env /go/src/code.gitea.io/gitea/environment-to-ini /usr/local/bin/environment-to-ini -COPY --from=build-env /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete /etc/profile.d/gitea_bash_autocomplete.sh diff --git a/Dockerfile.rootless b/Dockerfile.rootless index 558e6cf73b..f955edc667 100644 --- a/Dockerfile.rootless +++ b/Dockerfile.rootless @@ -37,7 +37,6 @@ RUN chmod 755 /tmp/local/usr/local/bin/docker-entrypoint.sh \ /tmp/local/usr/local/bin/gitea \ /go/src/code.gitea.io/gitea/gitea \ /go/src/code.gitea.io/gitea/environment-to-ini -RUN chmod 644 /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete FROM docker.io/library/alpine:3.22 LABEL maintainer="maintainers@gitea.io" @@ -72,7 +71,6 @@ RUN chown git:git /var/lib/gitea /etc/gitea COPY --from=build-env /tmp/local / COPY --from=build-env --chown=root:root /go/src/code.gitea.io/gitea/gitea /app/gitea/gitea COPY --from=build-env --chown=root:root /go/src/code.gitea.io/gitea/environment-to-ini /usr/local/bin/environment-to-ini -COPY --from=build-env /go/src/code.gitea.io/gitea/contrib/autocompletion/bash_autocomplete /etc/profile.d/gitea_bash_autocomplete.sh # git:git USER 1000:1000 diff --git a/models/asymkey/gpg_key_commit_verification.go b/models/asymkey/gpg_key_commit_verification.go index 39ec893606..b85374e073 100644 --- a/models/asymkey/gpg_key_commit_verification.go +++ b/models/asymkey/gpg_key_commit_verification.go @@ -15,25 +15,6 @@ import ( "github.com/ProtonMail/go-crypto/openpgp/packet" ) -// __________________ ________ ____ __. -// / _____/\______ \/ _____/ | |/ _|____ ___.__. -// / \ ___ | ___/ \ ___ | <_/ __ < | | -// \ \_\ \| | \ \_\ \ | | \ ___/\___ | -// \______ /|____| \______ / |____|__ \___ > ____| -// \/ \/ \/ \/\/ -// _________ .__ __ -// \_ ___ \ ____ _____ _____ |__|/ |_ -// / \ \/ / _ \ / \ / \| \ __\ -// \ \___( <_> ) Y Y \ Y Y \ || | -// \______ /\____/|__|_| /__|_| /__||__| -// \/ \/ \/ -// ____ ____ .__ _____.__ __ .__ -// \ \ / /___________|__|/ ____\__| ____ _____ _/ |_|__| ____ ____ -// \ Y // __ \_ __ \ \ __\| |/ ___\\__ \\ __\ |/ _ \ / \ -// \ /\ ___/| | \/ || | | \ \___ / __ \| | | ( <_> ) | \ -// \___/ \___ >__| |__||__| |__|\___ >____ /__| |__|\____/|___| / -// \/ \/ \/ \/ - // This file provides functions relating commit verification // CommitVerification represents a commit validation of signature @@ -41,8 +22,8 @@ type CommitVerification struct { Verified bool Warning bool Reason string - SigningUser *user_model.User - CommittingUser *user_model.User + SigningUser *user_model.User // if Verified, then SigningUser is non-nil + CommittingUser *user_model.User // if Verified, then CommittingUser is non-nil SigningEmail string SigningKey *GPGKey SigningSSHKey *PublicKey diff --git a/models/user/user.go b/models/user/user.go index 7c871bf575..c362cbc6d2 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1166,12 +1166,6 @@ func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) ([ for _, c := range oldCommits { user := emailUserMap.GetByEmail(c.Author.Email) // FIXME: why ValidateCommitsWithEmails uses "Author", but ParseCommitsWithSignature uses "Committer"? - if user == nil { - user = &User{ - Name: c.Author.Name, - Email: c.Author.Email, - } - } newCommits = append(newCommits, &UserCommit{ User: user, Commit: c, @@ -1195,12 +1189,14 @@ func GetUsersByEmails(ctx context.Context, emails []string) (*EmailUserMap, erro needCheckEmails := make(container.Set[string]) needCheckUserNames := make(container.Set[string]) + noReplyAddressSuffix := "@" + strings.ToLower(setting.Service.NoReplyAddress) for _, email := range emails { - if strings.HasSuffix(email, "@"+setting.Service.NoReplyAddress) { - username := strings.TrimSuffix(email, "@"+setting.Service.NoReplyAddress) - needCheckUserNames.Add(strings.ToLower(username)) + emailLower := strings.ToLower(email) + if noReplyUserNameLower, ok := strings.CutSuffix(emailLower, noReplyAddressSuffix); ok { + needCheckUserNames.Add(noReplyUserNameLower) + needCheckEmails.Add(emailLower) } else { - needCheckEmails.Add(strings.ToLower(email)) + needCheckEmails.Add(emailLower) } } diff --git a/models/user/user_test.go b/models/user/user_test.go index a2597ba3f5..7944fc4b73 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -85,6 +85,11 @@ func TestUserEmails(t *testing.T) { testGetUserByEmail(t, c.Email, c.UID) }) } + + t.Run("NoReplyConflict", func(t *testing.T) { + setting.Service.NoReplyAddress = "example.com" + testGetUserByEmail(t, "user1-2@example.COM", 1) + }) }) } diff --git a/modules/git/commit.go b/modules/git/commit.go index ed4876e7b3..aae40c575b 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -22,9 +22,9 @@ import ( type Commit struct { Tree // FIXME: bad design, this field can be nil if the commit is from "last commit cache" - ID ObjectID // The ID of this commit object - Author *Signature - Committer *Signature + ID ObjectID + Author *Signature // never nil + Committer *Signature // never nil CommitMessage string Signature *CommitSignature diff --git a/services/asymkey/commit.go b/services/asymkey/commit.go index 148f51fd10..773e7ca83c 100644 --- a/services/asymkey/commit.go +++ b/services/asymkey/commit.go @@ -24,47 +24,43 @@ import ( // ParseCommitWithSignature check if signature is good against keystore. func ParseCommitWithSignature(ctx context.Context, c *git.Commit) *asymkey_model.CommitVerification { - var committer *user_model.User - if c.Committer != nil { - var err error - // Find Committer account - committer, err = user_model.GetUserByEmail(ctx, c.Committer.Email) // This finds the user by primary email or activated email so commit will not be valid if email is not - if err != nil { // Skipping not user for committer - committer = &user_model.User{ - Name: c.Committer.Name, - Email: c.Committer.Email, - } - // We can expect this to often be an ErrUserNotExist. in the case - // it is not, however, it is important to log it. - if !user_model.IsErrUserNotExist(err) { - log.Error("GetUserByEmail: %v", err) - return &asymkey_model.CommitVerification{ - CommittingUser: committer, - Verified: false, - Reason: "gpg.error.no_committer_account", - } - } + committer, err := user_model.GetUserByEmail(ctx, c.Committer.Email) + if err != nil && !user_model.IsErrUserNotExist(err) { + log.Error("GetUserByEmail: %v", err) + return &asymkey_model.CommitVerification{ + Verified: false, + Reason: "gpg.error.no_committer_account", // this error is not right, but such error should seldom happen } } - return ParseCommitWithSignatureCommitter(ctx, c, committer) } +// ParseCommitWithSignatureCommitter parses a commit's GPG or SSH signature. +// If the commit is singed by an instance key, then committer can be nil. +// If the signature exists, even if committer is nil, the returned CommittingUser will be a non-nil fake user. func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { - // If no signature just report the committer + // If no signature, just report the committer if c.Signature == nil { return &asymkey_model.CommitVerification{ CommittingUser: committer, - Verified: false, // Default value - Reason: "gpg.error.not_signed_commit", // Default value + Verified: false, + Reason: "gpg.error.not_signed_commit", } } - - // If this a SSH signature handle it differently - if strings.HasPrefix(c.Signature.Signature, "-----BEGIN SSH SIGNATURE-----") { - return ParseCommitWithSSHSignature(ctx, c, committer) + // to support instance key, we need a fake committer user (not really needed, but legacy code accesses the committer without nil-check) + if committer == nil { + committer = &user_model.User{ + Name: c.Committer.Name, + Email: c.Committer.Email, + } } + if strings.HasPrefix(c.Signature.Signature, "-----BEGIN SSH SIGNATURE-----") { + return parseCommitWithSSHSignature(ctx, c, committer) + } + return parseCommitWithGPGSignature(ctx, c, committer) +} +func parseCommitWithGPGSignature(ctx context.Context, c *git.Commit, committer *user_model.User) *asymkey_model.CommitVerification { // Parsing signature sig, err := asymkey_model.ExtractSignature(c.Signature.Signature) if err != nil { // Skipping failed to extract sign @@ -165,7 +161,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } if err := gpgSettings.LoadPublicKeyContent(); err != nil { log.Error("Error getting default signing key: %s %v", gpgSettings.KeyID, err) - } else if commitVerification := VerifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { + } else if commitVerification := verifyWithGPGSettings(ctx, &gpgSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -180,7 +176,7 @@ func ParseCommitWithSignatureCommitter(ctx context.Context, c *git.Commit, commi } else if defaultGPGSettings == nil { log.Warn("Unable to get defaultGPGSettings for unattached commit: %s", c.ID.String()) } else if defaultGPGSettings.Sign { - if commitVerification := VerifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { + if commitVerification := verifyWithGPGSettings(ctx, defaultGPGSettings, sig, c.Signature.Payload, committer, keyID); commitVerification != nil { if commitVerification.Reason == asymkey_model.BadSignature { defaultReason = asymkey_model.BadSignature } else { @@ -295,7 +291,7 @@ func HashAndVerifyForKeyID(ctx context.Context, sig *packet.Signature, payload s } } -func VerifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string) *asymkey_model.CommitVerification { +func verifyWithGPGSettings(ctx context.Context, gpgSettings *git.GPGSettings, sig *packet.Signature, payload string, committer *user_model.User, keyID string) *asymkey_model.CommitVerification { // First try to find the key in the db if commitVerification := HashAndVerifyForKeyID(ctx, sig, payload, committer, gpgSettings.KeyID, gpgSettings.Name, gpgSettings.Email); commitVerification != nil { return commitVerification @@ -375,8 +371,8 @@ func verifySSHCommitVerificationByInstanceKey(c *git.Commit, committerUser, sign return verifySSHCommitVerification(c.Signature.Signature, c.Signature.Payload, sshPubKey, committerUser, signerUser, committerGitEmail) } -// ParseCommitWithSSHSignature check if signature is good against keystore. -func ParseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUser *user_model.User) *asymkey_model.CommitVerification { +// parseCommitWithSSHSignature check if signature is good against keystore. +func parseCommitWithSSHSignature(ctx context.Context, c *git.Commit, committerUser *user_model.User) *asymkey_model.CommitVerification { // Now try to associate the signature with the committer, if present if committerUser.ID != 0 { keys, err := db.Find[asymkey_model.PublicKey](ctx, asymkey_model.FindPublicKeyOptions{ diff --git a/services/asymkey/commit_test.go b/services/asymkey/commit_test.go index 0438209a61..6bcb6997f4 100644 --- a/services/asymkey/commit_test.go +++ b/services/asymkey/commit_test.go @@ -41,7 +41,7 @@ Initial commit with signed file Name: "User Two", Email: "user2@example.com", } - ret := ParseCommitWithSSHSignature(t.Context(), commit, committingUser) + ret := parseCommitWithSSHSignature(t.Context(), commit, committingUser) require.NotNil(t, ret) assert.True(t, ret.Verified) assert.False(t, ret.Warning) diff --git a/services/git/commit.go b/services/git/commit.go index 2e0e8a5096..e4755ef93d 100644 --- a/services/git/commit.go +++ b/services/git/commit.go @@ -35,13 +35,6 @@ func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository, for _, c := range oldCommits { committerUser := emailUsers.GetByEmail(c.Committer.Email) // FIXME: why ValidateCommitsWithEmails uses "Author", but ParseCommitsWithSignature uses "Committer"? - if committerUser == nil { - committerUser = &user_model.User{ - Name: c.Committer.Name, - Email: c.Committer.Email, - } - } - signCommit := &asymkey_model.SignCommit{ UserCommit: c, Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committerUser), diff --git a/templates/repo/commit_page.tmpl b/templates/repo/commit_page.tmpl index 46f641824b..68ccf9d275 100644 --- a/templates/repo/commit_page.tmpl +++ b/templates/repo/commit_page.tmpl @@ -147,7 +147,7 @@
{{if or (ne .Commit.Committer.Name .Commit.Author.Name) (ne .Commit.Committer.Email .Commit.Author.Email)}} {{ctx.Locale.Tr "repo.diff.committed_by"}} - {{if ne .Verification.CommittingUser.ID 0}} + {{if and .Verification.CommittingUser .Verification.CommittingUser.ID}} {{ctx.AvatarUtils.Avatar .Verification.CommittingUser 20}} {{.Commit.Committer.Name}} {{else}} diff --git a/templates/repo/commits_list.tmpl b/templates/repo/commits_list.tmpl index 9dae6594b9..959f2a9398 100644 --- a/templates/repo/commits_list.tmpl +++ b/templates/repo/commits_list.tmpl @@ -16,7 +16,7 @@
{{$userName := .Author.Name}} - {{if and .User (gt .User.ID 0)}} /* User with id == 0 is a fake user from git author */ + {{if .User}} {{if and .User.FullName DefaultShowFullName}} {{$userName = .User.FullName}} {{end}} diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index 0097a7f62e..b8f086e2b1 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -24,39 +24,59 @@ import ( func TestRepoCommits(t *testing.T) { defer tests.PrepareTestEnv(t)() - session := loginUser(t, "user2") - // Request repository commits page - req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master") - resp := session.MakeRequest(t, req, http.StatusOK) + t.Run("CommitList", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo16/commits/branch/master") + resp := session.MakeRequest(t, req, http.StatusOK) - doc := NewHTMLParser(t, resp.Body) - commitURL, exists := doc.doc.Find("#commits-table .commit-id-short").Attr("href") - assert.True(t, exists) - assert.NotEmpty(t, commitURL) -} - -func Test_ReposGitCommitListNotMaster(t *testing.T) { - defer tests.PrepareTestEnv(t)() - session := loginUser(t, "user2") - req := NewRequest(t, "GET", "/user2/repo16/commits/branch/master") - resp := session.MakeRequest(t, req, http.StatusOK) - - doc := NewHTMLParser(t, resp.Body) - var commits []string - doc.doc.Find("#commits-table .commit-id-short").Each(func(i int, s *goquery.Selection) { - commitURL, _ := s.Attr("href") - commits = append(commits, path.Base(commitURL)) + var commits, userHrefs []string + doc := NewHTMLParser(t, resp.Body) + doc.doc.Find("#commits-table .commit-id-short").Each(func(i int, s *goquery.Selection) { + commits = append(commits, path.Base(s.AttrOr("href", ""))) + }) + doc.doc.Find("#commits-table .author-wrapper").Each(func(i int, s *goquery.Selection) { + userHrefs = append(userHrefs, s.AttrOr("href", "")) + }) + assert.Equal(t, []string{"69554a64c1e6030f051e5c3f94bfbd773cd6a324", "27566bd5738fc8b4e3fef3c5e72cce608537bd95", "5099b81332712fe655e34e8dd63574f503f61811"}, commits) + assert.Equal(t, []string{"/user2", "/user21", "/user2"}, userHrefs) }) - assert.Equal(t, []string{"69554a64c1e6030f051e5c3f94bfbd773cd6a324", "27566bd5738fc8b4e3fef3c5e72cce608537bd95", "5099b81332712fe655e34e8dd63574f503f61811"}, commits) - var userHrefs []string - doc.doc.Find("#commits-table .author-wrapper").Each(func(i int, s *goquery.Selection) { - userHref, _ := s.Attr("href") - userHrefs = append(userHrefs, userHref) + t.Run("LastCommit", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo16") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + commitHref := doc.doc.Find(".latest-commit .commit-id-short").AttrOr("href", "") + authorHref := doc.doc.Find(".latest-commit .author-wrapper").AttrOr("href", "") + assert.Equal(t, "/user2/repo16/commit/69554a64c1e6030f051e5c3f94bfbd773cd6a324", commitHref) + assert.Equal(t, "/user2", authorHref) + }) + + t.Run("CommitListNonExistingCommiter", func(t *testing.T) { + // check the commit list for a repository with no gitea user + // * commit 985f0301dba5e7b34be866819cd15ad3d8f508ee (branch2) + // * Author: 6543 <6543@obermui.de> + req := NewRequest(t, "GET", "/user2/repo1/commits/branch/branch2") + resp := session.MakeRequest(t, req, http.StatusOK) + + doc := NewHTMLParser(t, resp.Body) + commitHref := doc.doc.Find("#commits-table tr:first-child .commit-id-short").AttrOr("href", "") + assert.Equal(t, "/user2/repo1/commit/985f0301dba5e7b34be866819cd15ad3d8f508ee", commitHref) + authorElem := doc.doc.Find("#commits-table tr:first-child .author-wrapper") + assert.Equal(t, "6543", authorElem.Text()) + assert.Equal(t, "span", authorElem.Nodes[0].Data) + }) + + t.Run("LastCommitNonExistingCommiter", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo1/src/branch/branch2") + resp := session.MakeRequest(t, req, http.StatusOK) + doc := NewHTMLParser(t, resp.Body) + commitHref := doc.doc.Find(".latest-commit .commit-id-short").AttrOr("href", "") + assert.Equal(t, "/user2/repo1/commit/985f0301dba5e7b34be866819cd15ad3d8f508ee", commitHref) + authorElem := doc.doc.Find(".latest-commit .author-wrapper") + assert.Equal(t, "6543", authorElem.Text()) + assert.Equal(t, "span", authorElem.Nodes[0].Data) }) - assert.Equal(t, []string{"/user2", "/user21", "/user2"}, userHrefs) } func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) { From b46623f6a5aa8beb3613bd89abde8d77b1e66758 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 11 Jul 2025 07:17:28 +0800 Subject: [PATCH 14/15] Fix updating user visibility (#35036) Fix #35030 --------- Co-authored-by: wxiaoguang --- modules/optional/option.go | 7 +++++++ modules/optional/option_test.go | 6 ++++++ routers/api/v1/admin/user.go | 2 +- routers/api/v1/org/org.go | 2 +- 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/modules/optional/option.go b/modules/optional/option.go index 6075c6347e..cbecf86987 100644 --- a/modules/optional/option.go +++ b/modules/optional/option.go @@ -28,6 +28,13 @@ func FromPtr[T any](v *T) Option[T] { return Some(*v) } +func FromMapLookup[K comparable, V any](m map[K]V, k K) Option[V] { + if v, ok := m[k]; ok { + return Some(v) + } + return None[V]() +} + func FromNonDefault[T comparable](v T) Option[T] { var zero T if v == zero { diff --git a/modules/optional/option_test.go b/modules/optional/option_test.go index f600ff5a2c..ea80a2e3cb 100644 --- a/modules/optional/option_test.go +++ b/modules/optional/option_test.go @@ -56,6 +56,12 @@ func TestOption(t *testing.T) { opt3 := optional.FromNonDefault(1) assert.True(t, opt3.Has()) assert.Equal(t, int(1), opt3.Value()) + + opt4 := optional.FromMapLookup(map[string]int{"a": 1}, "a") + assert.True(t, opt4.Has()) + assert.Equal(t, 1, opt4.Value()) + opt4 = optional.FromMapLookup(map[string]int{"a": 1}, "b") + assert.False(t, opt4.Has()) } func Test_ParseBool(t *testing.T) { diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 8a267cc418..494bace585 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -240,7 +240,7 @@ func EditUser(ctx *context.APIContext) { Description: optional.FromPtr(form.Description), IsActive: optional.FromPtr(form.Active), IsAdmin: user_service.UpdateOptionFieldFromPtr(form.Admin), - Visibility: optional.FromNonDefault(api.VisibilityModes[form.Visibility]), + Visibility: optional.FromMapLookup(api.VisibilityModes, form.Visibility), AllowGitHook: optional.FromPtr(form.AllowGitHook), AllowImportLocal: optional.FromPtr(form.AllowImportLocal), MaxRepoCreation: optional.FromPtr(form.MaxRepoCreation), diff --git a/routers/api/v1/org/org.go b/routers/api/v1/org/org.go index 05744ba155..cd67686065 100644 --- a/routers/api/v1/org/org.go +++ b/routers/api/v1/org/org.go @@ -391,7 +391,7 @@ func Edit(ctx *context.APIContext) { Description: optional.Some(form.Description), Website: optional.Some(form.Website), Location: optional.Some(form.Location), - Visibility: optional.FromNonDefault(api.VisibilityModes[form.Visibility]), + Visibility: optional.FromMapLookup(api.VisibilityModes, form.Visibility), RepoAdminChangeTeamAccess: optional.FromPtr(form.RepoAdminChangeTeamAccess), } if err := user_service.UpdateUser(ctx, ctx.Org.Organization.AsUser(), opts); err != nil { From 56eccb49954dbb561f4360481c3e52de92080f20 Mon Sep 17 00:00:00 2001 From: NorthRealm <155140859+NorthRealm@users.noreply.github.com> Date: Fri, 11 Jul 2025 10:17:52 +0800 Subject: [PATCH 15/15] Add Notifications section in User Settings (#35008) Related: #34982 --------- Signed-off-by: NorthRealm <155140859+NorthRealm@users.noreply.github.com> Co-authored-by: wxiaoguang --- routers/web/user/setting/account.go | 31 +---------- routers/web/user/setting/notifications.go | 62 ++++++++++++++++++++++ routers/web/web.go | 6 ++- templates/user/settings/account.tmpl | 27 +--------- templates/user/settings/navbar.tmpl | 7 ++- templates/user/settings/notifications.tmpl | 34 ++++++++++++ 6 files changed, 109 insertions(+), 58 deletions(-) create mode 100644 routers/web/user/setting/notifications.go create mode 100644 templates/user/settings/notifications.tmpl diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go index b124d5e1de..6b17da50e5 100644 --- a/routers/web/user/setting/account.go +++ b/routers/web/user/setting/account.go @@ -35,7 +35,7 @@ const ( // Account renders change user's password, user's email and user suicide page func Account(ctx *context.Context) { - if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageCredentials, setting.UserFeatureDeletion) && !setting.Service.EnableNotifyMail { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageCredentials, setting.UserFeatureDeletion) { ctx.NotFound(errors.New("account setting are not allowed to be changed")) return } @@ -43,7 +43,6 @@ func Account(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings.account") ctx.Data["PageIsSettingsAccount"] = true ctx.Data["Email"] = ctx.Doer.Email - ctx.Data["EnableNotifyMail"] = setting.Service.EnableNotifyMail loadAccountData(ctx) @@ -61,7 +60,6 @@ func AccountPost(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true ctx.Data["Email"] = ctx.Doer.Email - ctx.Data["EnableNotifyMail"] = setting.Service.EnableNotifyMail if ctx.HasError() { loadAccountData(ctx) @@ -112,7 +110,6 @@ func EmailPost(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true ctx.Data["Email"] = ctx.Doer.Email - ctx.Data["EnableNotifyMail"] = setting.Service.EnableNotifyMail // Make email address primary. if ctx.FormString("_method") == "PRIMARY" { @@ -172,30 +169,6 @@ func EmailPost(ctx *context.Context) { ctx.Redirect(setting.AppSubURL + "/user/settings/account") return } - // Set Email Notification Preference - if ctx.FormString("_method") == "NOTIFICATION" { - preference := ctx.FormString("preference") - if !(preference == user_model.EmailNotificationsEnabled || - preference == user_model.EmailNotificationsOnMention || - preference == user_model.EmailNotificationsDisabled || - preference == user_model.EmailNotificationsAndYourOwn) { - log.Error("Email notifications preference change returned unrecognized option %s: %s", preference, ctx.Doer.Name) - ctx.ServerError("SetEmailPreference", errors.New("option unrecognized")) - return - } - opts := &user.UpdateOptions{ - EmailNotificationsPreference: optional.Some(preference), - } - if err := user.UpdateUser(ctx, ctx.Doer, opts); err != nil { - log.Error("Set Email Notifications failed: %v", err) - ctx.ServerError("UpdateUser", err) - return - } - log.Trace("Email notifications preference made %s: %s", preference, ctx.Doer.Name) - ctx.Flash.Success(ctx.Tr("settings.email_preference_set_success")) - ctx.Redirect(setting.AppSubURL + "/user/settings/account") - return - } if ctx.HasError() { loadAccountData(ctx) @@ -267,7 +240,6 @@ func DeleteAccount(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("settings") ctx.Data["PageIsSettingsAccount"] = true ctx.Data["Email"] = ctx.Doer.Email - ctx.Data["EnableNotifyMail"] = setting.Service.EnableNotifyMail if _, _, err := auth.UserSignIn(ctx, ctx.Doer.Name, ctx.FormString("password")); err != nil { switch { @@ -342,7 +314,6 @@ func loadAccountData(ctx *context.Context) { emails[i] = &email } ctx.Data["Emails"] = emails - ctx.Data["EmailNotificationsPreference"] = ctx.Doer.EmailNotificationsPreference ctx.Data["ActivationsPending"] = pendingActivation ctx.Data["CanAddEmails"] = !pendingActivation || !setting.Service.RegisterEmailConfirm ctx.Data["UserDisabledFeatures"] = user_model.DisabledFeaturesWithLoginType(ctx.Doer) diff --git a/routers/web/user/setting/notifications.go b/routers/web/user/setting/notifications.go new file mode 100644 index 0000000000..16e58a0481 --- /dev/null +++ b/routers/web/user/setting/notifications.go @@ -0,0 +1,62 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import ( + "errors" + "net/http" + + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/services/context" + "code.gitea.io/gitea/services/user" +) + +const tplSettingsNotifications templates.TplName = "user/settings/notifications" + +// Notifications render user's notifications settings +func Notifications(ctx *context.Context) { + if !setting.Service.EnableNotifyMail { + ctx.NotFound(nil) + return + } + + ctx.Data["Title"] = ctx.Tr("notifications") + ctx.Data["PageIsSettingsNotifications"] = true + ctx.Data["EmailNotificationsPreference"] = ctx.Doer.EmailNotificationsPreference + + ctx.HTML(http.StatusOK, tplSettingsNotifications) +} + +// NotificationsEmailPost set user's email notification preference +func NotificationsEmailPost(ctx *context.Context) { + if !setting.Service.EnableNotifyMail { + ctx.NotFound(nil) + return + } + + preference := ctx.FormString("preference") + if !(preference == user_model.EmailNotificationsEnabled || + preference == user_model.EmailNotificationsOnMention || + preference == user_model.EmailNotificationsDisabled || + preference == user_model.EmailNotificationsAndYourOwn) { + log.Error("Email notifications preference change returned unrecognized option %s: %s", preference, ctx.Doer.Name) + ctx.ServerError("SetEmailPreference", errors.New("option unrecognized")) + return + } + opts := &user.UpdateOptions{ + EmailNotificationsPreference: optional.Some(preference), + } + if err := user.UpdateUser(ctx, ctx.Doer, opts); err != nil { + log.Error("Set Email Notifications failed: %v", err) + ctx.ServerError("UpdateUser", err) + return + } + log.Trace("Email notifications preference made %s: %s", preference, ctx.Doer.Name) + ctx.Flash.Success(ctx.Tr("settings.email_preference_set_success")) + ctx.Redirect(setting.AppSubURL + "/user/settings/notifications") +} diff --git a/routers/web/web.go b/routers/web/web.go index ddea468d17..b9c7013f63 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -595,6 +595,10 @@ func registerWebRoutes(m *web.Router) { m.Post("/hidden_comments", user_setting.UpdateUserHiddenComments) m.Post("/theme", web.Bind(forms.UpdateThemeForm{}), user_setting.UpdateUIThemePost) }) + m.Group("/notifications", func() { + m.Get("", user_setting.Notifications) + m.Post("/email", user_setting.NotificationsEmailPost) + }) m.Group("/security", func() { m.Get("", security.Security) m.Group("/two_factor", func() { @@ -682,7 +686,7 @@ func registerWebRoutes(m *web.Router) { m.Get("", user_setting.BlockedUsers) m.Post("", web.Bind(forms.BlockUserForm{}), user_setting.BlockedUsersPost) }) - }, reqSignIn, ctxDataSet("PageIsUserSettings", true, "EnablePackages", setting.Packages.Enabled)) + }, reqSignIn, ctxDataSet("PageIsUserSettings", true, "EnablePackages", setting.Packages.Enabled, "EnableNotifyMail", setting.Service.EnableNotifyMail)) m.Group("/user", func() { m.Get("/activate", auth.Activate) diff --git a/templates/user/settings/account.tmpl b/templates/user/settings/account.tmpl index 2c01c88d47..7dbac0ebd4 100644 --- a/templates/user/settings/account.tmpl +++ b/templates/user/settings/account.tmpl @@ -35,37 +35,12 @@ {{end}}
- {{if not (and ($.UserDisabledFeatures.Contains "manage_credentials") (not $.EnableNotifyMail))}} + {{if not ($.UserDisabledFeatures.Contains "manage_credentials")}}

{{ctx.Locale.Tr "settings.manage_emails"}}

- {{if $.EnableNotifyMail}} -
-
- {{$.CsrfTokenHtml}} - -
- -
- - {{svg "octicon-triangle-down" 14 "dropdown icon"}} -
-
-
{{ctx.Locale.Tr "settings.email_notifications.enable"}}
-
{{ctx.Locale.Tr "settings.email_notifications.andyourown"}}
-
{{ctx.Locale.Tr "settings.email_notifications.onmention"}}
-
{{ctx.Locale.Tr "settings.email_notifications.disable"}}
-
-
-
-
- -
-
-
- {{end}} {{if not ($.UserDisabledFeatures.Contains "manage_credentials")}} {{range .Emails}}
diff --git a/templates/user/settings/navbar.tmpl b/templates/user/settings/navbar.tmpl index c6c15512ab..34e089a68a 100644 --- a/templates/user/settings/navbar.tmpl +++ b/templates/user/settings/navbar.tmpl @@ -4,11 +4,16 @@ {{ctx.Locale.Tr "settings.profile"}} - {{if not (and ($.UserDisabledFeatures.Contains "manage_credentials" "deletion") (not $.EnableNotifyMail))}} + {{if not ($.UserDisabledFeatures.Contains "manage_credentials" "deletion")}} {{ctx.Locale.Tr "settings.account"}} {{end}} + {{if $.EnableNotifyMail}} + + {{ctx.Locale.Tr "notifications"}} + + {{end}} {{ctx.Locale.Tr "settings.appearance"}} diff --git a/templates/user/settings/notifications.tmpl b/templates/user/settings/notifications.tmpl new file mode 100644 index 0000000000..4694bbb30a --- /dev/null +++ b/templates/user/settings/notifications.tmpl @@ -0,0 +1,34 @@ +{{template "user/settings/layout_head" (dict "ctxData" . "pageClass" "user settings")}} +
+

+ {{ctx.Locale.Tr "notifications"}} +

+
+
+
+
+ {{$.CsrfTokenHtml}} +
+ +
+ + {{svg "octicon-triangle-down" 14 "dropdown icon"}} +
+
+
{{ctx.Locale.Tr "settings.email_notifications.enable"}}
+
{{ctx.Locale.Tr "settings.email_notifications.andyourown"}}
+
{{ctx.Locale.Tr "settings.email_notifications.onmention"}}
+
{{ctx.Locale.Tr "settings.email_notifications.disable"}}
+
+
+
+
+ +
+
+
+
+
+
+ +{{template "user/settings/layout_footer" .}}