From 6ac04b8c7dcedb9c6d994bb2a8cd37580394d9dd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 14 Nov 2024 18:13:01 -0800 Subject: [PATCH 1/5] Fix oauth2 error handle not return immediately (#32514) (cherry picked from commit 4121f952d18a4c3a3c08ae645af3458ef08b439d) --- routers/web/auth/oauth.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index ee08b514b1..651c9fb474 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1013,6 +1013,8 @@ func SignInOAuthCallback(ctx *context.Context) { } if err, ok := err.(*go_oauth2.RetrieveError); ok { ctx.Flash.Error("OAuth2 RetrieveError: "+err.Error(), true) + ctx.Redirect(setting.AppSubURL + "/user/login") + return } ctx.ServerError("UserSignIn", err) return From 9f05c76b7b84f3cfafd4de22f5f18b87e4c79775 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Nov 2024 12:17:58 +0800 Subject: [PATCH 2/5] Fix nil panic if repo doesn't exist (#32501) fix #32496 (cherry picked from commit 985e2a8af3d6468bac3ab178148c38bdbd8414f5) --- models/activities/action.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/models/activities/action.go b/models/activities/action.go index b6c816f096..dd67b98242 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -250,6 +250,9 @@ func (a *Action) GetActDisplayNameTitle(ctx context.Context) string { // GetRepoUserName returns the name of the action repository owner. func (a *Action) GetRepoUserName(ctx context.Context) string { a.loadRepo(ctx) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.OwnerName } @@ -262,6 +265,9 @@ func (a *Action) ShortRepoUserName(ctx context.Context) string { // GetRepoName returns the name of the action repository. func (a *Action) GetRepoName(ctx context.Context) string { a.loadRepo(ctx) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.Name } From 8cec637d08542535d1dc9689c22943cd3ffe1c45 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 12 Nov 2024 13:33:35 -0800 Subject: [PATCH 3/5] Disable Oauth check if oauth disabled (#32368) Fix #32367 --------- Co-authored-by: Giteabot Co-authored-by: wxiaoguang (cherry picked from commit 840ad7eefe2b49ab453b9a89b153a264a8c9f8a2) Conflicts: services/auth/oauth2.go trivial context conflict --- routers/web/web.go | 68 ++++++++++++++++++++++------------------- services/auth/oauth2.go | 3 ++ 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index b93192143e..ab73ef9b36 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -327,6 +327,13 @@ func registerRoutes(m *web.Route) { } } + oauth2Enabled := func(ctx *context.Context) { + if !setting.OAuth2.Enabled { + ctx.Error(http.StatusForbidden) + return + } + } + reqMilestonesDashboardPageEnabled := func(ctx *context.Context) { if !setting.Service.ShowMilestonesDashboardPage { ctx.Error(http.StatusForbidden) @@ -516,16 +523,18 @@ func registerRoutes(m *web.Route) { m.Any("/user/events", routing.MarkLongPolling, events.Events) m.Group("/login/oauth", func() { - m.Get("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) - m.Post("/grant", web.Bind(forms.GrantApplicationForm{}), auth.GrantApplicationOAuth) - // TODO manage redirection - m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) - }, ignSignInAndCsrf, reqSignIn) + m.Group("", func() { + m.Get("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) + m.Post("/grant", web.Bind(forms.GrantApplicationForm{}), auth.GrantApplicationOAuth) + // TODO manage redirection + m.Post("/authorize", web.Bind(forms.AuthorizationForm{}), auth.AuthorizeOAuth) + }, ignSignInAndCsrf, reqSignIn) - m.Methods("GET, OPTIONS", "/login/oauth/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth) - m.Methods("POST, OPTIONS", "/login/oauth/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth) - m.Methods("GET, OPTIONS", "/login/oauth/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys) - m.Methods("POST, OPTIONS", "/login/oauth/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth) + m.Methods("GET, OPTIONS", "/userinfo", optionsCorsHandler(), ignSignInAndCsrf, auth.InfoOAuth) + m.Methods("POST, OPTIONS", "/access_token", optionsCorsHandler(), web.Bind(forms.AccessTokenForm{}), ignSignInAndCsrf, auth.AccessTokenOAuth) + m.Methods("GET, OPTIONS", "/keys", optionsCorsHandler(), ignSignInAndCsrf, auth.OIDCKeys) + m.Methods("POST, OPTIONS", "/introspect", optionsCorsHandler(), web.Bind(forms.IntrospectTokenForm{}), ignSignInAndCsrf, auth.IntrospectOAuth) + }, oauth2Enabled) m.Group("/user/settings", func() { m.Get("", user_setting.Profile) @@ -567,17 +576,24 @@ func registerRoutes(m *web.Route) { }, openIDSignInEnabled) m.Post("/account_link", linkAccountEnabled, security.DeleteAccountLink) }) - m.Group("/applications/oauth2", func() { - m.Get("/{id}", user_setting.OAuth2ApplicationShow) - m.Post("/{id}", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsEdit) - m.Post("/{id}/regenerate_secret", user_setting.OAuthApplicationsRegenerateSecret) - m.Post("", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsPost) - m.Post("/{id}/delete", user_setting.DeleteOAuth2Application) - m.Post("/{id}/revoke/{grantId}", user_setting.RevokeOAuth2Grant) + + m.Group("/applications", func() { + // oauth2 applications + m.Group("/oauth2", func() { + m.Get("/{id}", user_setting.OAuth2ApplicationShow) + m.Post("/{id}", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsEdit) + m.Post("/{id}/regenerate_secret", user_setting.OAuthApplicationsRegenerateSecret) + m.Post("", web.Bind(forms.EditOAuth2ApplicationForm{}), user_setting.OAuthApplicationsPost) + m.Post("/{id}/delete", user_setting.DeleteOAuth2Application) + m.Post("/{id}/revoke/{grantId}", user_setting.RevokeOAuth2Grant) + }, oauth2Enabled) + + // access token applications + m.Combo("").Get(user_setting.Applications). + Post(web.Bind(forms.NewAccessTokenForm{}), user_setting.ApplicationsPost) + m.Post("/delete", user_setting.DeleteApplication) }) - m.Combo("/applications").Get(user_setting.Applications). - Post(web.Bind(forms.NewAccessTokenForm{}), user_setting.ApplicationsPost) - m.Post("/applications/delete", user_setting.DeleteApplication) + m.Combo("/keys").Get(user_setting.Keys). Post(web.Bind(forms.AddKeyForm{}), user_setting.KeysPost) m.Post("/keys/delete", user_setting.DeleteKey) @@ -755,12 +771,7 @@ func registerRoutes(m *web.Route) { m.Post("/regenerate_secret", admin.ApplicationsRegenerateSecret) m.Post("/delete", admin.DeleteApplication) }) - }, func(ctx *context.Context) { - if !setting.OAuth2.Enabled { - ctx.Error(http.StatusForbidden) - return - } - }) + }, oauth2Enabled) m.Group("/actions", func() { m.Get("", admin.RedirectToDefaultSetting) @@ -883,12 +894,7 @@ func registerRoutes(m *web.Route) { m.Post("/regenerate_secret", org.OAuthApplicationsRegenerateSecret) m.Post("/delete", org.DeleteOAuth2Application) }) - }, func(ctx *context.Context) { - if !setting.OAuth2.Enabled { - ctx.Error(http.StatusForbidden) - return - } - }) + }, oauth2Enabled) m.Group("/hooks", func() { m.Get("", org.Webhooks) diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 6a63c62796..8b625a193e 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -68,6 +68,9 @@ func grantAdditionalScopes(grantScopes string) string { // CheckOAuthAccessToken returns uid of user from oauth token // + non default openid scopes requested func CheckOAuthAccessToken(ctx context.Context, accessToken string) (int64, string) { + if !setting.OAuth2.Enabled { + return 0, "" + } // JWT tokens require a "." if !strings.Contains(accessToken, ".") { return 0, "" From 53c546951115d9e269a2778f90e43b0cb413eab6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 16 Nov 2024 16:41:44 +0800 Subject: [PATCH 4/5] Fix and refactor markdown rendering (#32522) (cherry picked from commit 5eebe1dc5fb29a162c51d050396fce7b14e47f4e) Conflicts: models/repo/repo.go models/repo/repo_test.go modules/markup/html.go modules/markup/html_commit.go modules/markup/html_email.go modules/markup/html_emoji.go modules/markup/html_internal_test.go modules/markup/html_issue.go modules/markup/html_link.go modules/markup/html_node.go modules/markup/html_test.go modules/markup/markdown/goldmark.go modules/markup/markdown/markdown_test.go modules/markup/markdown/transform_image.go modules/markup/orgmode/orgmode.go modules/markup/orgmode/orgmode_test.go modules/markup/render.go modules/markup/render_links.go modules/templates/util_render.go modules/templates/util_render_test.go routers/common/markup.go routers/web/feed/convert.go routers/web/repo/wiki.go but a few lines survived and are useful --- modules/markup/asciicast/asciicast.go | 2 +- modules/markup/csv/csv.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/markup/asciicast/asciicast.go b/modules/markup/asciicast/asciicast.go index 0678062340..873029c1bd 100644 --- a/modules/markup/asciicast/asciicast.go +++ b/modules/markup/asciicast/asciicast.go @@ -39,7 +39,7 @@ const ( // SanitizerRules implements markup.Renderer func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{ - {Element: "div", AllowAttr: "class", Regexp: regexp.MustCompile(playerClassName)}, + {Element: "div", AllowAttr: "class", Regexp: regexp.MustCompile("^" + playerClassName + "$")}, {Element: "div", AllowAttr: playerSrcAttr}, } } diff --git a/modules/markup/csv/csv.go b/modules/markup/csv/csv.go index 3d952b0de4..092eec7098 100644 --- a/modules/markup/csv/csv.go +++ b/modules/markup/csv/csv.go @@ -37,9 +37,9 @@ func (Renderer) Extensions() []string { // SanitizerRules implements markup.Renderer func (Renderer) SanitizerRules() []setting.MarkupSanitizerRule { return []setting.MarkupSanitizerRule{ - {Element: "table", AllowAttr: "class", Regexp: regexp.MustCompile(`data-table`)}, - {Element: "th", AllowAttr: "class", Regexp: regexp.MustCompile(`line-num`)}, - {Element: "td", AllowAttr: "class", Regexp: regexp.MustCompile(`line-num`)}, + {Element: "table", AllowAttr: "class", Regexp: regexp.MustCompile(`^data-table$`)}, + {Element: "th", AllowAttr: "class", Regexp: regexp.MustCompile(`^line-num$`)}, + {Element: "td", AllowAttr: "class", Regexp: regexp.MustCompile(`^line-num$`)}, } } From 5b2db9d3ca7c80c5f4a56d70c7b7bba406122965 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Sun, 17 Nov 2024 20:52:58 +0100 Subject: [PATCH 5/5] chore(release-notes): notes for the week 2024-47-v9.0 weekly cherry pick --- release-notes/5998.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 release-notes/5998.md diff --git a/release-notes/5998.md b/release-notes/5998.md new file mode 100644 index 0000000000..b1b7d56d08 --- /dev/null +++ b/release-notes/5998.md @@ -0,0 +1,4 @@ +fix(security): [commit](https://codeberg.org/forgejo/forgejo/commit/53c546951115d9e269a2778f90e43b0cb413eab6) Fix and refactor markdown rendering +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/6ac04b8c7dcedb9c6d994bb2a8cd37580394d9dd) Fix oauth2 error handle not return immediately +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/9f05c76b7b84f3cfafd4de22f5f18b87e4c79775) Fix nil panic if repo doesn't exist +fix: [commit](https://codeberg.org/forgejo/forgejo/commit/8cec637d08542535d1dc9689c22943cd3ffe1c45) Disable Oauth check if oauth disabled