From d0227c236aa195bd03990210f968b8e52eb20b79 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 14 Jul 2024 07:38:45 -0700 Subject: [PATCH 1/6] Issue Templates: add option to have dropdown printed list (#31577) Issue template dropdown can have many entries, and it could be better to have them rendered as list later on if multi-select is enabled. so this adds an option to the issue template engine to do so. DOCS: https://gitea.com/gitea/docs/pulls/19 --- ## demo: ```yaml name: Name title: Title about: About labels: ["label1", "label2"] ref: Ref body: - type: dropdown id: id6 attributes: label: Label of dropdown (list) description: Description of dropdown multiple: true list: true options: - Option 1 of dropdown - Option 2 of dropdown - Option 3 of dropdown - Option 4 of dropdown - Option 5 of dropdown - Option 6 of dropdown - Option 7 of dropdown - Option 8 of dropdown - Option 9 of dropdown ``` ![image](https://github.com/user-attachments/assets/102ed0f4-89da-420b-ab2a-1788b59676f9) ![image](https://github.com/user-attachments/assets/a2bdb14e-43ff-4cc6-9bbe-20244830453c) --- *Sponsored by Kithara Software GmbH* (cherry picked from commit 1064e817c4a6fa6eb5170143150505503c4ef6ed) --- modules/issue/template/template.go | 11 ++++++- modules/issue/template/template_test.go | 43 ++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/modules/issue/template/template.go b/modules/issue/template/template.go index cf5fcf28e5..0a105c723c 100644 --- a/modules/issue/template/template.go +++ b/modules/issue/template/template.go @@ -88,6 +88,9 @@ func validateYaml(template *api.IssueTemplate) error { if err := validateBoolItem(position, field.Attributes, "multiple"); err != nil { return err } + if err := validateBoolItem(position, field.Attributes, "list"); err != nil { + return err + } if err := validateOptions(field, idx); err != nil { return err } @@ -340,7 +343,13 @@ func (f *valuedField) WriteTo(builder *strings.Builder) { } } if len(checkeds) > 0 { - _, _ = fmt.Fprintf(builder, "%s\n", strings.Join(checkeds, ", ")) + if list, ok := f.Attributes["list"].(bool); ok && list { + for _, check := range checkeds { + _, _ = fmt.Fprintf(builder, "- %s\n", check) + } + } else { + _, _ = fmt.Fprintf(builder, "%s\n", strings.Join(checkeds, ", ")) + } } else { _, _ = fmt.Fprint(builder, blankPlaceholder) } diff --git a/modules/issue/template/template_test.go b/modules/issue/template/template_test.go index 481058754d..349dbeabb0 100644 --- a/modules/issue/template/template_test.go +++ b/modules/issue/template/template_test.go @@ -216,6 +216,20 @@ body: `, wantErr: "body[0](dropdown): 'multiple' should be a bool", }, + { + name: "dropdown invalid list", + content: ` +name: "test" +about: "this is about" +body: + - type: "dropdown" + id: "1" + attributes: + label: "a" + list: "on" +`, + wantErr: "body[0](dropdown): 'list' should be a bool", + }, { name: "checkboxes invalid description", content: ` @@ -807,7 +821,7 @@ body: - type: dropdown id: id5 attributes: - label: Label of dropdown + label: Label of dropdown (one line) description: Description of dropdown multiple: true options: @@ -816,8 +830,21 @@ body: - Option 3 of dropdown validations: required: true - - type: checkboxes + - type: dropdown id: id6 + attributes: + label: Label of dropdown (list) + description: Description of dropdown + multiple: true + list: true + options: + - Option 1 of dropdown + - Option 2 of dropdown + - Option 3 of dropdown + validations: + required: true + - type: checkboxes + id: id7 attributes: label: Label of checkboxes description: Description of checkboxes @@ -836,8 +863,9 @@ body: "form-field-id3": {"Value of id3"}, "form-field-id4": {"Value of id4"}, "form-field-id5": {"0,1"}, - "form-field-id6-0": {"on"}, - "form-field-id6-2": {"on"}, + "form-field-id6": {"1,2"}, + "form-field-id7-0": {"on"}, + "form-field-id7-2": {"on"}, }, }, @@ -849,10 +877,15 @@ body: Value of id4 -### Label of dropdown +### Label of dropdown (one line) Option 1 of dropdown, Option 2 of dropdown +### Label of dropdown (list) + +- Option 2 of dropdown +- Option 3 of dropdown + ### Label of checkboxes - [x] Option 1 of checkboxes From 54f2dcff9d4bdb15f099efd4258c5c492f78b238 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 15 Jul 2024 05:15:59 +0800 Subject: [PATCH 2/6] Upgrade xorm to v1.3.9 and improve some migrations Sync (#29899) Co-authored-by: 6543 <6543@obermui.de> (cherry picked from commit 0d08bb6112884411eb4f58b056278d3c824a8fc0) --- models/migrations/v1_21/v279.go | 6 +++++- models/migrations/v1_22/v284.go | 6 +++++- models/migrations/v1_22/v285.go | 6 +++++- models/migrations/v1_22/v286.go | 5 ++++- models/migrations/v1_22/v289.go | 5 ++++- models/migrations/v1_22/v290.go | 9 ++++++++- models/migrations/v1_22/v291.go | 6 +++++- 7 files changed, 36 insertions(+), 7 deletions(-) diff --git a/models/migrations/v1_21/v279.go b/models/migrations/v1_21/v279.go index 19647225c9..2abd1bbe84 100644 --- a/models/migrations/v1_21/v279.go +++ b/models/migrations/v1_21/v279.go @@ -12,5 +12,9 @@ func AddIndexToActionUserID(x *xorm.Engine) error { UserID int64 `xorm:"INDEX"` } - return x.Sync(new(Action)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreDropIndices: true, + IgnoreConstrains: true, + }, new(Action)) + return err } diff --git a/models/migrations/v1_22/v284.go b/models/migrations/v1_22/v284.go index 1a4c786964..2b95078980 100644 --- a/models/migrations/v1_22/v284.go +++ b/models/migrations/v1_22/v284.go @@ -10,5 +10,9 @@ func AddIgnoreStaleApprovalsColumnToProtectedBranchTable(x *xorm.Engine) error { type ProtectedBranch struct { IgnoreStaleApprovals bool `xorm:"NOT NULL DEFAULT false"` } - return x.Sync(new(ProtectedBranch)) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreIndices: true, + IgnoreConstrains: true, + }, new(ProtectedBranch)) + return err } diff --git a/models/migrations/v1_22/v285.go b/models/migrations/v1_22/v285.go index c0dacd40bc..a55cc17c04 100644 --- a/models/migrations/v1_22/v285.go +++ b/models/migrations/v1_22/v285.go @@ -14,5 +14,9 @@ func AddPreviousDurationToActionRun(x *xorm.Engine) error { PreviousDuration time.Duration } - return x.Sync(&ActionRun{}) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreIndices: true, + IgnoreConstrains: true, + }, &ActionRun{}) + return err } diff --git a/models/migrations/v1_22/v286.go b/models/migrations/v1_22/v286.go index 6a5f45122a..97ff649dca 100644 --- a/models/migrations/v1_22/v286.go +++ b/models/migrations/v1_22/v286.go @@ -54,7 +54,10 @@ func addObjectFormatNameToRepository(x *xorm.Engine) error { ObjectFormatName string `xorm:"VARCHAR(6) NOT NULL DEFAULT 'sha1'"` } - if err := x.Sync(new(Repository)); err != nil { + if _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreIndices: true, + IgnoreConstrains: true, + }, new(Repository)); err != nil { return err } diff --git a/models/migrations/v1_22/v289.go b/models/migrations/v1_22/v289.go index e2dfc48715..b9941aadd9 100644 --- a/models/migrations/v1_22/v289.go +++ b/models/migrations/v1_22/v289.go @@ -10,7 +10,10 @@ func AddDefaultWikiBranch(x *xorm.Engine) error { ID int64 DefaultWikiBranch string } - if err := x.Sync(&Repository{}); err != nil { + if _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreIndices: true, + IgnoreConstrains: true, + }, &Repository{}); err != nil { return err } _, err := x.Exec("UPDATE `repository` SET default_wiki_branch = 'master' WHERE (default_wiki_branch IS NULL) OR (default_wiki_branch = '')") diff --git a/models/migrations/v1_22/v290.go b/models/migrations/v1_22/v290.go index e9c471b3dd..e3c58b0515 100644 --- a/models/migrations/v1_22/v290.go +++ b/models/migrations/v1_22/v290.go @@ -35,5 +35,12 @@ type HookTask struct { func AddPayloadVersionToHookTaskTable(x *xorm.Engine) error { // create missing column - return x.Sync(new(HookTask)) + if _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreIndices: true, + IgnoreConstrains: true, + }, new(HookTask)); err != nil { + return err + } + _, err := x.Exec("UPDATE hook_task SET payload_version = 1 WHERE payload_version IS NULL") + return err } diff --git a/models/migrations/v1_22/v291.go b/models/migrations/v1_22/v291.go index 0bfffe5d05..74726fae96 100644 --- a/models/migrations/v1_22/v291.go +++ b/models/migrations/v1_22/v291.go @@ -10,5 +10,9 @@ func AddCommentIDIndexofAttachment(x *xorm.Engine) error { CommentID int64 `xorm:"INDEX"` } - return x.Sync(&Attachment{}) + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreDropIndices: true, + IgnoreConstrains: true, + }, &Attachment{}) + return err } From 004cc6dc0ab7cc9c324ccb4ecd420c6aeeb20500 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 14 Jul 2024 14:27:00 -0700 Subject: [PATCH 3/6] Add option to change mail from user display name (#31528) Make it posible to let mails show e.g.: `Max Musternam (via gitea.kithara.com) ` Docs: https://gitea.com/gitea/docs/pulls/23 --- *Sponsored by Kithara Software GmbH* (cherry picked from commit 0f533241829d0d48aa16a91e7dc0614fe50bc317) Conflicts: - services/mailer/mail_release.go services/mailer/mail_test.go In both cases, applied the changes manually. --- custom/conf/app.example.ini | 4 +++ modules/setting/mailer.go | 15 +++++++++++ services/mailer/mail.go | 18 ++++++++++++- services/mailer/mail_release.go | 2 +- services/mailer/mail_repo.go | 2 +- services/mailer/mail_test.go | 48 +++++++++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 3 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index cdb7629887..bbf383b065 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1727,6 +1727,10 @@ LEVEL = Info ;; Sometimes it is helpful to use a different address on the envelope. Set this to use ENVELOPE_FROM as the from on the envelope. Set to `<>` to send an empty address. ;ENVELOPE_FROM = ;; +;; If gitea sends mails on behave of users, it will just use the name also displayed in the WebUI. If you want e.g. `Mister X (by CodeIt) `, +;; set it to `{{ .DisplayName }} (by {{ .AppName }})`. Available Variables: `.DisplayName`, `.AppName` and `.Domain`. +;FROM_DISPLAY_NAME_FORMAT = {{ .DisplayName }} +;; ;; Mailer user name and password, if required by provider. ;USER = ;; diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index cfedb4613f..136d932b8d 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -8,6 +8,7 @@ import ( "net" "net/mail" "strings" + "text/template" "time" "code.gitea.io/gitea/modules/log" @@ -46,6 +47,10 @@ type Mailer struct { SendmailArgs []string `ini:"-"` SendmailTimeout time.Duration `ini:"SENDMAIL_TIMEOUT"` SendmailConvertCRLF bool `ini:"SENDMAIL_CONVERT_CRLF"` + + // Customization + FromDisplayNameFormat string `ini:"FROM_DISPLAY_NAME_FORMAT"` + FromDisplayNameFormatTemplate *template.Template `ini:"-"` } // MailService the global mailer @@ -234,6 +239,16 @@ func loadMailerFrom(rootCfg ConfigProvider) { log.Error("no mailer.FROM provided, email system may not work.") } + MailService.FromDisplayNameFormatTemplate, _ = template.New("mailFrom").Parse("{{ .DisplayName }}") + if MailService.FromDisplayNameFormat != "" { + template, err := template.New("mailFrom").Parse(MailService.FromDisplayNameFormat) + if err != nil { + log.Error("mailer.FROM_DISPLAY_NAME_FORMAT is no valid template: %v", err) + } else { + MailService.FromDisplayNameFormatTemplate = template + } + } + switch MailService.EnvelopeFrom { case "": MailService.OverrideEnvelopeFrom = false diff --git a/services/mailer/mail.go b/services/mailer/mail.go index ca8fb34ef1..38e292e6a1 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -313,7 +313,7 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient for _, recipient := range recipients { msg := NewMessageFrom( recipient.Email, - ctx.Doer.GetCompleteName(), + fromDisplayName(ctx.Doer), setting.MailService.FromEmail, subject, mailBody.String(), @@ -545,3 +545,19 @@ func actionToTemplate(issue *issues_model.Issue, actionType activities_model.Act } return typeName, name, template } + +func fromDisplayName(u *user_model.User) string { + if setting.MailService.FromDisplayNameFormatTemplate != nil { + var ctx bytes.Buffer + err := setting.MailService.FromDisplayNameFormatTemplate.Execute(&ctx, map[string]any{ + "DisplayName": u.DisplayName(), + "AppName": setting.AppName, + "Domain": setting.Domain, + }) + if err == nil { + return mime.QEncoding.Encode("utf-8", ctx.String()) + } + log.Error("fromDisplayName: %w", err) + } + return u.GetCompleteName() +} diff --git a/services/mailer/mail_release.go b/services/mailer/mail_release.go index 445e58724c..0b8b97e9cd 100644 --- a/services/mailer/mail_release.go +++ b/services/mailer/mail_release.go @@ -85,7 +85,7 @@ func mailNewRelease(ctx context.Context, lang string, tos []*user_model.User, re } msgs := make([]*Message, 0, len(tos)) - publisherName := rel.Publisher.DisplayName() + publisherName := fromDisplayName(rel.Publisher) msgID := createMessageIDForRelease(rel) for _, to := range tos { msg := NewMessageFrom(to.EmailTo(), publisherName, setting.MailService.FromEmail, subject, mailBody.String()) diff --git a/services/mailer/mail_repo.go b/services/mailer/mail_repo.go index 28b9cef8a7..7003584786 100644 --- a/services/mailer/mail_repo.go +++ b/services/mailer/mail_repo.go @@ -79,7 +79,7 @@ func sendRepoTransferNotifyMailPerLang(lang string, newOwner, doer *user_model.U } for _, to := range emailTos { - msg := NewMessage(to.EmailTo(), subject, content.String()) + msg := NewMessageFrom(to.EmailTo(), fromDisplayName(doer), setting.MailService.FromEmail, subject, content.String()) msg.Info = fmt.Sprintf("UID: %d, repository pending transfer notification", newOwner.ID) SendAsync(msg) diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index 8fa45fd593..54b9c177e8 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -489,3 +489,51 @@ func Test_createReference(t *testing.T) { }) } } + +func TestFromDisplayName(t *testing.T) { + template, err := texttmpl.New("mailFrom").Parse("{{ .DisplayName }}") + assert.NoError(t, err) + setting.MailService = &setting.Mailer{FromDisplayNameFormatTemplate: template} + defer func() { setting.MailService = nil }() + + tests := []struct { + userDisplayName string + fromDisplayName string + }{{ + userDisplayName: "test", + fromDisplayName: "test", + }, { + userDisplayName: "Hi Its ", + fromDisplayName: "Hi Its ", + }, { + userDisplayName: "Æsir", + fromDisplayName: "=?utf-8?q?=C3=86sir?=", + }, { + userDisplayName: "new😀user", + fromDisplayName: "=?utf-8?q?new=F0=9F=98=80user?=", + }} + + for _, tc := range tests { + t.Run(tc.userDisplayName, func(t *testing.T) { + user := &user_model.User{FullName: tc.userDisplayName, Name: "tmp"} + got := fromDisplayName(user) + assert.EqualValues(t, tc.fromDisplayName, got) + }) + } + + t.Run("template with all available vars", func(t *testing.T) { + template, err = texttmpl.New("mailFrom").Parse("{{ .DisplayName }} (by {{ .AppName }} on [{{ .Domain }}])") + assert.NoError(t, err) + setting.MailService = &setting.Mailer{FromDisplayNameFormatTemplate: template} + oldAppName := setting.AppName + setting.AppName = "Code IT" + oldDomain := setting.Domain + setting.Domain = "code.it" + defer func() { + setting.AppName = oldAppName + setting.Domain = oldDomain + }() + + assert.EqualValues(t, "Mister X (by Code IT on [code.it])", fromDisplayName(&user_model.User{FullName: "Mister X", Name: "tmp"})) + }) +} From 21fdd28f084e7f1aef309c9ebd7599ffa6986453 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Tue, 16 Jul 2024 13:33:16 -0500 Subject: [PATCH 4/6] allow synchronizing user status from OAuth2 login providers (#31572) This leverages the existing `sync_external_users` cron job to synchronize the `IsActive` flag on users who use an OAuth2 provider set to synchronize. This synchronization is done by checking for expired access tokens, and using the stored refresh token to request a new access token. If the response back from the OAuth2 provider is the `invalid_grant` error code, the user is marked as inactive. However, the user is able to reactivate their account by logging in the web browser through their OAuth2 flow. Also changed to support this is that a linked `ExternalLoginUser` is always created upon a login or signup via OAuth2. Ideally, we would also refresh permissions from the configured OAuth provider (e.g., admin, restricted and group mappings) to match the implementation of LDAP. However, the OAuth library used for this `goth`, doesn't seem to support issuing a session via refresh tokens. The interface provides a [`RefreshToken` method](https://github.com/markbates/goth/blob/master/provider.go#L20), but the returned `oauth.Token` doesn't implement the `goth.Session` we would need to call `FetchUser`. Due to specific implementations, we would need to build a compatibility function for every provider, since they cast to concrete types (e.g. [Azure](https://github.com/markbates/goth/blob/master/providers/azureadv2/azureadv2.go#L132)) --------- Co-authored-by: Kyle D (cherry picked from commit 416c36f3034e228a27258b5a8a15eec4e5e426ba) Conflicts: - tests/integration/auth_ldap_test.go Trivial conflict resolved by manually applying the change. - routers/web/auth/oauth.go Technically not a conflict, but the original PR removed the modules/util import, which in our version, is still in use. Added it back. --- models/auth/source.go | 2 +- models/user/external_login_user.go | 41 ++++++- routers/web/auth/auth.go | 6 +- routers/web/auth/oauth.go | 64 +++++----- services/auth/source/oauth2/main_test.go | 14 +++ services/auth/source/oauth2/providers_test.go | 62 ++++++++++ services/auth/source/oauth2/source.go | 2 +- services/auth/source/oauth2/source_sync.go | 114 ++++++++++++++++++ .../auth/source/oauth2/source_sync_test.go | 100 +++++++++++++++ services/externalaccount/user.go | 6 +- templates/admin/auth/edit.tmpl | 2 +- templates/admin/auth/new.tmpl | 2 +- tests/integration/auth_ldap_test.go | 3 +- 13 files changed, 370 insertions(+), 48 deletions(-) create mode 100644 services/auth/source/oauth2/main_test.go create mode 100644 services/auth/source/oauth2/providers_test.go create mode 100644 services/auth/source/oauth2/source_sync.go create mode 100644 services/auth/source/oauth2/source_sync_test.go diff --git a/models/auth/source.go b/models/auth/source.go index d03d4975dc..8f7c2a89db 100644 --- a/models/auth/source.go +++ b/models/auth/source.go @@ -216,7 +216,7 @@ func CreateSource(ctx context.Context, source *Source) error { return ErrSourceAlreadyExist{source.Name} } // Synchronization is only available with LDAP for now - if !source.IsLDAP() { + if !source.IsLDAP() && !source.IsOAuth2() { source.IsSyncEnabled = false } diff --git a/models/user/external_login_user.go b/models/user/external_login_user.go index 965b7a5ed1..0e764efb9f 100644 --- a/models/user/external_login_user.go +++ b/models/user/external_login_user.go @@ -160,12 +160,34 @@ func UpdateExternalUserByExternalID(ctx context.Context, external *ExternalLogin return err } +// EnsureLinkExternalToUser link the external user to the user +func EnsureLinkExternalToUser(ctx context.Context, external *ExternalLoginUser) error { + has, err := db.Exist[ExternalLoginUser](ctx, builder.Eq{ + "external_id": external.ExternalID, + "login_source_id": external.LoginSourceID, + }) + if err != nil { + return err + } + + if has { + _, err = db.GetEngine(ctx).Where("external_id=? AND login_source_id=?", external.ExternalID, external.LoginSourceID).AllCols().Update(external) + return err + } + + _, err = db.GetEngine(ctx).Insert(external) + return err +} + // FindExternalUserOptions represents an options to find external users type FindExternalUserOptions struct { db.ListOptions - Provider string - UserID int64 - OrderBy string + Provider string + UserID int64 + LoginSourceID int64 + HasRefreshToken bool + Expired bool + OrderBy string } func (opts FindExternalUserOptions) ToConds() builder.Cond { @@ -176,9 +198,22 @@ func (opts FindExternalUserOptions) ToConds() builder.Cond { if opts.UserID > 0 { cond = cond.And(builder.Eq{"user_id": opts.UserID}) } + if opts.Expired { + cond = cond.And(builder.Lt{"expires_at": time.Now()}) + } + if opts.HasRefreshToken { + cond = cond.And(builder.Neq{"refresh_token": ""}) + } + if opts.LoginSourceID != 0 { + cond = cond.And(builder.Eq{"login_source_id": opts.LoginSourceID}) + } return cond } func (opts FindExternalUserOptions) ToOrders() string { return opts.OrderBy } + +func IterateExternalLogin(ctx context.Context, opts FindExternalUserOptions, f func(ctx context.Context, u *ExternalLoginUser) error) error { + return db.Iterate(ctx, opts.ToConds(), f) +} diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index a738de380a..3181800211 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -619,10 +619,8 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth. notify_service.NewUserSignUp(ctx, u) // update external user information if gothUser != nil { - if err := externalaccount.UpdateExternalUser(ctx, u, *gothUser); err != nil { - if !errors.Is(err, util.ErrNotExist) { - log.Error("UpdateExternalUser failed: %v", err) - } + if err := externalaccount.EnsureLinkExternalToUser(ctx, u, *gothUser); err != nil { + log.Error("EnsureLinkExternalToUser failed: %v", err) } } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index d133d9dae1..8ffc2b711c 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1154,9 +1154,39 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model groups := getClaimedGroups(oauth2Source, &gothUser) + opts := &user_service.UpdateOptions{} + + // Reactivate user if they are deactivated + if !u.IsActive { + opts.IsActive = optional.Some(true) + } + + // Update GroupClaims + opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) + + if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval { + if err := source_service.SyncGroupsToTeams(ctx, u, groups, groupTeamMapping, oauth2Source.GroupTeamMapRemoval); err != nil { + ctx.ServerError("SyncGroupsToTeams", err) + return + } + } + + if err := externalaccount.EnsureLinkExternalToUser(ctx, u, gothUser); err != nil { + ctx.ServerError("EnsureLinkExternalToUser", err) + return + } + // If this user is enrolled in 2FA and this source doesn't override it, // we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page. if !needs2FA { + // Register last login + opts.SetLastLogin = true + + if err := user_service.UpdateUser(ctx, u, opts); err != nil { + ctx.ServerError("UpdateUser", err) + return + } + if err := updateSession(ctx, nil, map[string]any{ "uid": u.ID, }); err != nil { @@ -1167,29 +1197,6 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model // Clear whatever CSRF cookie has right now, force to generate a new one ctx.Csrf.DeleteCookie(ctx) - opts := &user_service.UpdateOptions{ - SetLastLogin: true, - } - opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) - if err := user_service.UpdateUser(ctx, u, opts); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - - if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval { - if err := source_service.SyncGroupsToTeams(ctx, u, groups, groupTeamMapping, oauth2Source.GroupTeamMapRemoval); err != nil { - ctx.ServerError("SyncGroupsToTeams", err) - return - } - } - - // update external user information - if err := externalaccount.UpdateExternalUser(ctx, u, gothUser); err != nil { - if !errors.Is(err, util.ErrNotExist) { - log.Error("UpdateExternalUser failed: %v", err) - } - } - if err := resetLocale(ctx, u); err != nil { ctx.ServerError("resetLocale", err) return @@ -1205,22 +1212,13 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model return } - opts := &user_service.UpdateOptions{} - opts.IsAdmin, opts.IsRestricted = getUserAdminAndRestrictedFromGroupClaims(oauth2Source, &gothUser) - if opts.IsAdmin.Has() || opts.IsRestricted.Has() { + if opts.IsActive.Has() || opts.IsAdmin.Has() || opts.IsRestricted.Has() { if err := user_service.UpdateUser(ctx, u, opts); err != nil { ctx.ServerError("UpdateUser", err) return } } - if oauth2Source.GroupTeamMap != "" || oauth2Source.GroupTeamMapRemoval { - if err := source_service.SyncGroupsToTeams(ctx, u, groups, groupTeamMapping, oauth2Source.GroupTeamMapRemoval); err != nil { - ctx.ServerError("SyncGroupsToTeams", err) - return - } - } - if err := updateSession(ctx, nil, map[string]any{ // User needs to use 2FA, save data and redirect to 2FA page. "twofaUid": u.ID, diff --git a/services/auth/source/oauth2/main_test.go b/services/auth/source/oauth2/main_test.go new file mode 100644 index 0000000000..57c74fd3e7 --- /dev/null +++ b/services/auth/source/oauth2/main_test.go @@ -0,0 +1,14 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package oauth2 + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m, &unittest.TestOptions{}) +} diff --git a/services/auth/source/oauth2/providers_test.go b/services/auth/source/oauth2/providers_test.go new file mode 100644 index 0000000000..353816c71e --- /dev/null +++ b/services/auth/source/oauth2/providers_test.go @@ -0,0 +1,62 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package oauth2 + +import ( + "time" + + "github.com/markbates/goth" + "golang.org/x/oauth2" +) + +type fakeProvider struct{} + +func (p *fakeProvider) Name() string { + return "fake" +} + +func (p *fakeProvider) SetName(name string) {} + +func (p *fakeProvider) BeginAuth(state string) (goth.Session, error) { + return nil, nil +} + +func (p *fakeProvider) UnmarshalSession(string) (goth.Session, error) { + return nil, nil +} + +func (p *fakeProvider) FetchUser(goth.Session) (goth.User, error) { + return goth.User{}, nil +} + +func (p *fakeProvider) Debug(bool) { +} + +func (p *fakeProvider) RefreshToken(refreshToken string) (*oauth2.Token, error) { + switch refreshToken { + case "expired": + return nil, &oauth2.RetrieveError{ + ErrorCode: "invalid_grant", + } + default: + return &oauth2.Token{ + AccessToken: "token", + TokenType: "Bearer", + RefreshToken: "refresh", + Expiry: time.Now().Add(time.Hour), + }, nil + } +} + +func (p *fakeProvider) RefreshTokenAvailable() bool { + return true +} + +func init() { + RegisterGothProvider( + NewSimpleProvider("fake", "Fake", []string{"account"}, + func(clientKey, secret, callbackURL string, scopes ...string) goth.Provider { + return &fakeProvider{} + })) +} diff --git a/services/auth/source/oauth2/source.go b/services/auth/source/oauth2/source.go index 675005e55a..3454c9ad55 100644 --- a/services/auth/source/oauth2/source.go +++ b/services/auth/source/oauth2/source.go @@ -36,7 +36,7 @@ func (source *Source) FromDB(bs []byte) error { return json.UnmarshalHandleDoubleEncode(bs, &source) } -// ToDB exports an SMTPConfig to a serialized format. +// ToDB exports an OAuth2Config to a serialized format. func (source *Source) ToDB() ([]byte, error) { return json.Marshal(source) } diff --git a/services/auth/source/oauth2/source_sync.go b/services/auth/source/oauth2/source_sync.go new file mode 100644 index 0000000000..5e30313c8f --- /dev/null +++ b/services/auth/source/oauth2/source_sync.go @@ -0,0 +1,114 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package oauth2 + +import ( + "context" + "time" + + "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/db" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/log" + + "github.com/markbates/goth" + "golang.org/x/oauth2" +) + +// Sync causes this OAuth2 source to synchronize its users with the db. +func (source *Source) Sync(ctx context.Context, updateExisting bool) error { + log.Trace("Doing: SyncExternalUsers[%s] %d", source.authSource.Name, source.authSource.ID) + + if !updateExisting { + log.Info("SyncExternalUsers[%s] not running since updateExisting is false", source.authSource.Name) + return nil + } + + provider, err := createProvider(source.authSource.Name, source) + if err != nil { + return err + } + + if !provider.RefreshTokenAvailable() { + log.Trace("SyncExternalUsers[%s] provider doesn't support refresh tokens, can't synchronize", source.authSource.Name) + return nil + } + + opts := user_model.FindExternalUserOptions{ + HasRefreshToken: true, + Expired: true, + LoginSourceID: source.authSource.ID, + } + + return user_model.IterateExternalLogin(ctx, opts, func(ctx context.Context, u *user_model.ExternalLoginUser) error { + return source.refresh(ctx, provider, u) + }) +} + +func (source *Source) refresh(ctx context.Context, provider goth.Provider, u *user_model.ExternalLoginUser) error { + log.Trace("Syncing login_source_id=%d external_id=%s expiration=%s", u.LoginSourceID, u.ExternalID, u.ExpiresAt) + + shouldDisable := false + + token, err := provider.RefreshToken(u.RefreshToken) + if err != nil { + if err, ok := err.(*oauth2.RetrieveError); ok && err.ErrorCode == "invalid_grant" { + // this signals that the token is not valid and the user should be disabled + shouldDisable = true + } else { + return err + } + } + + user := &user_model.User{ + LoginName: u.ExternalID, + LoginType: auth.OAuth2, + LoginSource: u.LoginSourceID, + } + + hasUser, err := user_model.GetUser(ctx, user) + if err != nil { + return err + } + + // If the grant is no longer valid, disable the user and + // delete local tokens. If the OAuth2 provider still + // recognizes them as a valid user, they will be able to login + // via their provider and reactivate their account. + if shouldDisable { + log.Info("SyncExternalUsers[%s] disabling user %d", source.authSource.Name, user.ID) + + return db.WithTx(ctx, func(ctx context.Context) error { + if hasUser { + user.IsActive = false + err := user_model.UpdateUserCols(ctx, user, "is_active") + if err != nil { + return err + } + } + + // Delete stored tokens, since they are invalid. This + // also provents us from checking this in subsequent runs. + u.AccessToken = "" + u.RefreshToken = "" + u.ExpiresAt = time.Time{} + + return user_model.UpdateExternalUserByExternalID(ctx, u) + }) + } + + // Otherwise, update the tokens + u.AccessToken = token.AccessToken + u.ExpiresAt = token.Expiry + + // Some providers only update access tokens provide a new + // refresh token, so avoid updating it if it's empty + if token.RefreshToken != "" { + u.RefreshToken = token.RefreshToken + } + + err = user_model.UpdateExternalUserByExternalID(ctx, u) + + return err +} diff --git a/services/auth/source/oauth2/source_sync_test.go b/services/auth/source/oauth2/source_sync_test.go new file mode 100644 index 0000000000..e2f04bcb25 --- /dev/null +++ b/services/auth/source/oauth2/source_sync_test.go @@ -0,0 +1,100 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package oauth2 + +import ( + "context" + "testing" + + "code.gitea.io/gitea/models/auth" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func TestSource(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + source := &Source{ + Provider: "fake", + authSource: &auth.Source{ + ID: 12, + Type: auth.OAuth2, + Name: "fake", + IsActive: true, + IsSyncEnabled: true, + }, + } + + user := &user_model.User{ + LoginName: "external", + LoginType: auth.OAuth2, + LoginSource: source.authSource.ID, + Name: "test", + Email: "external@example.com", + } + + err := user_model.CreateUser(context.Background(), user, &user_model.CreateUserOverwriteOptions{}) + assert.NoError(t, err) + + e := &user_model.ExternalLoginUser{ + ExternalID: "external", + UserID: user.ID, + LoginSourceID: user.LoginSource, + RefreshToken: "valid", + } + err = user_model.LinkExternalToUser(context.Background(), user, e) + assert.NoError(t, err) + + provider, err := createProvider(source.authSource.Name, source) + assert.NoError(t, err) + + t.Run("refresh", func(t *testing.T) { + t.Run("valid", func(t *testing.T) { + err := source.refresh(context.Background(), provider, e) + assert.NoError(t, err) + + e := &user_model.ExternalLoginUser{ + ExternalID: e.ExternalID, + LoginSourceID: e.LoginSourceID, + } + + ok, err := user_model.GetExternalLogin(context.Background(), e) + assert.NoError(t, err) + assert.True(t, ok) + assert.Equal(t, e.RefreshToken, "refresh") + assert.Equal(t, e.AccessToken, "token") + + u, err := user_model.GetUserByID(context.Background(), user.ID) + assert.NoError(t, err) + assert.True(t, u.IsActive) + }) + + t.Run("expired", func(t *testing.T) { + err := source.refresh(context.Background(), provider, &user_model.ExternalLoginUser{ + ExternalID: "external", + UserID: user.ID, + LoginSourceID: user.LoginSource, + RefreshToken: "expired", + }) + assert.NoError(t, err) + + e := &user_model.ExternalLoginUser{ + ExternalID: e.ExternalID, + LoginSourceID: e.LoginSourceID, + } + + ok, err := user_model.GetExternalLogin(context.Background(), e) + assert.NoError(t, err) + assert.True(t, ok) + assert.Equal(t, e.RefreshToken, "") + assert.Equal(t, e.AccessToken, "") + + u, err := user_model.GetUserByID(context.Background(), user.ID) + assert.NoError(t, err) + assert.False(t, u.IsActive) + }) + }) +} diff --git a/services/externalaccount/user.go b/services/externalaccount/user.go index 3cfd8c81f9..b53e33654a 100644 --- a/services/externalaccount/user.go +++ b/services/externalaccount/user.go @@ -71,14 +71,14 @@ func LinkAccountToUser(ctx context.Context, user *user_model.User, gothUser goth return nil } -// UpdateExternalUser updates external user's information -func UpdateExternalUser(ctx context.Context, user *user_model.User, gothUser goth.User) error { +// 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 } - return user_model.UpdateExternalUserByExternalID(ctx, externalLoginUser) + return user_model.EnsureLinkExternalToUser(ctx, externalLoginUser) } // UpdateMigrationsByType updates all migrated repositories' posterid from gitServiceType to replace originalAuthorID to posterID diff --git a/templates/admin/auth/edit.tmpl b/templates/admin/auth/edit.tmpl index 8a8bd61148..a8b2049f92 100644 --- a/templates/admin/auth/edit.tmpl +++ b/templates/admin/auth/edit.tmpl @@ -416,7 +416,7 @@

{{ctx.Locale.Tr "admin.auths.sspi_default_language_helper"}}

{{end}} - {{if .Source.IsLDAP}} + {{if (or .Source.IsLDAP .Source.IsOAuth2)}}
diff --git a/templates/admin/auth/new.tmpl b/templates/admin/auth/new.tmpl index f6a14e1f7d..0ba207215d 100644 --- a/templates/admin/auth/new.tmpl +++ b/templates/admin/auth/new.tmpl @@ -59,7 +59,7 @@
-
+
diff --git a/tests/integration/auth_ldap_test.go b/tests/integration/auth_ldap_test.go index 06677287c0..f0492015b8 100644 --- a/tests/integration/auth_ldap_test.go +++ b/tests/integration/auth_ldap_test.go @@ -244,7 +244,8 @@ func TestLDAPUserSync(t *testing.T) { } defer tests.PrepareTestEnv(t)() addAuthSourceLDAP(t, "", "", "", "") - auth.SyncExternalUsers(context.Background(), true) + err := auth.SyncExternalUsers(context.Background(), true) + assert.NoError(t, err) // Check if users exists for _, gitLDAPUser := range gitLDAPUsers { From 0792f81e0447229ae2b5dced70235006da9e53fa Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 22 Jul 2024 10:30:28 +0200 Subject: [PATCH 5/6] Add a release note for cherry-picked features This adds a release note file for features cherry picked during the 2024-30 weekly gitea->forgejo cherry pick. Thanks @earl-warren for the notes themselves! Signed-off-by: Gergely Nagy --- release-notes/4607.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 release-notes/4607.md diff --git a/release-notes/4607.md b/release-notes/4607.md new file mode 100644 index 0000000000..586225bcde --- /dev/null +++ b/release-notes/4607.md @@ -0,0 +1,3 @@ +feat: [commit](https://codeberg.org/forgejo/forgejo/commit/21fdd28f084e7f1aef309c9ebd7599ffa6986453) allow synchronizing user status from OAuth2 login providers. +feat: [commit](https://codeberg.org/forgejo/forgejo/commit/004cc6dc0ab7cc9c324ccb4ecd420c6aeeb20500) add option to change mail from user display name. +feat: [commit](https://codeberg.org/forgejo/forgejo/commit/d0227c236aa195bd03990210f968b8e52eb20b79) issue Templates: add option to have dropdown printed list. From f37d8fc0ed5ce7973e83e37603466a99935fd619 Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 17 Jul 2024 12:04:28 +0200 Subject: [PATCH 6/6] Remove unneccessary uses of `word-break: break-all` (#31637) Fixes: https://github.com/go-gitea/gitea/issues/31636 1. Issue sidebar topic is disussed in https://github.com/go-gitea/gitea/issues/31636 2. Org description already has `overflow-wrap: anywhere` to ensure no overflow. Co-authored-by: Giteabot (cherry picked from commit 0c1127a2fb4c07576b4a2e4cffbcd2b0c8670a27) --- web_src/css/org.css | 1 - web_src/css/repo.css | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/web_src/css/org.css b/web_src/css/org.css index e96b2bb83b..6853a26bf7 100644 --- a/web_src/css/org.css +++ b/web_src/css/org.css @@ -96,7 +96,6 @@ .page-content.organization #org-info { overflow-wrap: anywhere; flex: 1; - word-break: break-all; } .page-content.organization #org-info .ui.header { diff --git a/web_src/css/repo.css b/web_src/css/repo.css index 56525aca35..3b1735f27e 100644 --- a/web_src/css/repo.css +++ b/web_src/css/repo.css @@ -2635,7 +2635,7 @@ tbody.commit-list { .sidebar-item-link { display: inline-flex; align-items: center; - word-break: break-all; + overflow-wrap: anywhere; } .diff-file-header {