From 4597aeb017801c9263b1d5fe5ffb435379ab2567 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 29 Oct 2024 08:21:58 -0700 Subject: [PATCH 1/3] Disable Oauth check if oauth disabled --- routers/web/web.go | 71 +++++++++++++++++---------------- services/auth/oauth2.go | 4 ++ tests/integration/oauth_test.go | 23 +++++++++++ 3 files changed, 64 insertions(+), 34 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index a6ccb7a7924e9..ddb3dabdae767 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -546,17 +546,19 @@ func registerRoutes(m *web.Router) { 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.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) + if setting.OAuth2.Enabled { + 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.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.Group("/user/settings", func() { m.Get("", user_setting.Profile) @@ -597,16 +599,20 @@ func registerRoutes(m *web.Router) { }, 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.Combo("/applications").Get(user_setting.Applications). - Post(web.Bind(forms.NewAccessTokenForm{}), user_setting.ApplicationsPost) + + if setting.OAuth2.Enabled { + 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.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) @@ -773,20 +779,17 @@ func registerRoutes(m *web.Router) { m.Post("/empty", admin.EmptyNotices) }) - m.Group("/applications", func() { - m.Get("", admin.Applications) - m.Post("/oauth2", web.Bind(forms.EditOAuth2ApplicationForm{}), admin.ApplicationsPost) - m.Group("/oauth2/{id}", func() { - m.Combo("").Get(admin.EditApplication).Post(web.Bind(forms.EditOAuth2ApplicationForm{}), admin.EditApplicationPost) - m.Post("/regenerate_secret", admin.ApplicationsRegenerateSecret) - m.Post("/delete", admin.DeleteApplication) + if setting.OAuth2.Enabled { + m.Group("/applications", func() { + m.Get("", admin.Applications) + m.Post("/oauth2", web.Bind(forms.EditOAuth2ApplicationForm{}), admin.ApplicationsPost) + m.Group("/oauth2/{id}", func() { + m.Combo("").Get(admin.EditApplication).Post(web.Bind(forms.EditOAuth2ApplicationForm{}), admin.EditApplicationPost) + m.Post("/regenerate_secret", admin.ApplicationsRegenerateSecret) + m.Post("/delete", admin.DeleteApplication) + }) }) - }, func(ctx *context.Context) { - if !setting.OAuth2.Enabled { - ctx.Error(http.StatusForbidden) - return - } - }) + } m.Group("/actions", func() { m.Get("", admin.RedirectToDefaultSetting) diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index 523998a634522..d1b3fbb5cb452 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -31,6 +31,10 @@ func CheckOAuthAccessToken(ctx context.Context, accessToken string) int64 { if !strings.Contains(accessToken, ".") { return 0 } + if !setting.OAuth2.Enabled { + return 0 + } + token, err := oauth2_provider.ParseToken(accessToken, oauth2_provider.DefaultSigningKey) if err != nil { log.Trace("oauth2.ParseToken: %v", err) diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index b32d365b04d15..08164df54643e 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -7,8 +7,10 @@ import ( "bytes" "io" "net/http" + "net/url" "testing" + auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" oauth2_provider "code.gitea.io/gitea/services/oauth2_provider" @@ -477,3 +479,24 @@ func TestOAuthIntrospection(t *testing.T) { resp = MakeRequest(t, req, http.StatusUnauthorized) assert.Contains(t, resp.Body.String(), "no valid authorization") } + +func TestGitOpWithOAuthDisabled(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + setting.OAuth2.Enabled = true + defer func() { + setting.OAuth2.Enabled = false + }() + + onGiteaRun(t, func(t *testing.T, u *url.URL) { + httpContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository) + + u.Path = httpContext.GitPath() + dstPath := t.TempDir() + + u.Path = httpContext.GitPath() + u.User = url.UserPassword("user2", userPassword) + + t.Run("Clone", doGitClone(dstPath, u)) + }) +} From ee7116a7fea40a8ccfd2f18e9a2de5585517f766 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 2 Nov 2024 12:37:32 -0700 Subject: [PATCH 2/3] Fix test and refactor web routers --- routers/web/web.go | 61 +++++++++++++++++---------------- tests/integration/oauth_test.go | 4 +-- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/routers/web/web.go b/routers/web/web.go index ddb3dabdae767..fc1c7dbbfb6a0 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -325,6 +325,13 @@ func registerRoutes(m *web.Router) { } } + 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) @@ -546,19 +553,19 @@ func registerRoutes(m *web.Router) { m.Any("/user/events", routing.MarkLongPolling, events.Events) - if setting.OAuth2.Enabled { - m.Group("/login/oauth", func() { + m.Group("/login/oauth", func() { + 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) @@ -600,20 +607,23 @@ func registerRoutes(m *web.Router) { m.Post("/account_link", linkAccountEnabled, security.DeleteAccountLink) }) - if setting.OAuth2.Enabled { - m.Group("/applications/oauth2", func() { + 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) - }) - m.Combo("/applications").Get(user_setting.Applications). + }, 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.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) @@ -779,17 +789,15 @@ func registerRoutes(m *web.Router) { m.Post("/empty", admin.EmptyNotices) }) - if setting.OAuth2.Enabled { - m.Group("/applications", func() { - m.Get("", admin.Applications) - m.Post("/oauth2", web.Bind(forms.EditOAuth2ApplicationForm{}), admin.ApplicationsPost) - m.Group("/oauth2/{id}", func() { - m.Combo("").Get(admin.EditApplication).Post(web.Bind(forms.EditOAuth2ApplicationForm{}), admin.EditApplicationPost) - m.Post("/regenerate_secret", admin.ApplicationsRegenerateSecret) - m.Post("/delete", admin.DeleteApplication) - }) + m.Group("/applications", func() { + m.Get("", admin.Applications) + m.Post("/oauth2", web.Bind(forms.EditOAuth2ApplicationForm{}), admin.ApplicationsPost) + m.Group("/oauth2/{id}", func() { + m.Combo("").Get(admin.EditApplication).Post(web.Bind(forms.EditOAuth2ApplicationForm{}), admin.EditApplicationPost) + m.Post("/regenerate_secret", admin.ApplicationsRegenerateSecret) + m.Post("/delete", admin.DeleteApplication) }) - } + }, oauth2Enabled) m.Group("/actions", func() { m.Get("", admin.RedirectToDefaultSetting) @@ -913,12 +921,7 @@ func registerRoutes(m *web.Router) { 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/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 08164df54643e..b8cce36155b18 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -483,9 +483,9 @@ func TestOAuthIntrospection(t *testing.T) { func TestGitOpWithOAuthDisabled(t *testing.T) { defer tests.PrepareTestEnv(t)() - setting.OAuth2.Enabled = true + setting.OAuth2.Enabled = false defer func() { - setting.OAuth2.Enabled = false + setting.OAuth2.Enabled = true }() onGiteaRun(t, func(t *testing.T, u *url.URL) { From 3e740107942c098f4c98952d50ea6d11512bbb04 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 12 Nov 2024 13:21:19 +0800 Subject: [PATCH 3/3] fix --- services/auth/oauth2.go | 7 ++++--- tests/integration/oauth_test.go | 23 ----------------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/services/auth/oauth2.go b/services/auth/oauth2.go index d1b3fbb5cb452..d0aec085b107d 100644 --- a/services/auth/oauth2.go +++ b/services/auth/oauth2.go @@ -27,11 +27,12 @@ var ( // CheckOAuthAccessToken returns uid of user from oauth token func CheckOAuthAccessToken(ctx context.Context, accessToken string) int64 { - // JWT tokens require a "." - if !strings.Contains(accessToken, ".") { + if !setting.OAuth2.Enabled { return 0 } - if !setting.OAuth2.Enabled { + + // JWT tokens require a ".", if the token isn't like that, return early + if !strings.Contains(accessToken, ".") { return 0 } diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index b8cce36155b18..b32d365b04d15 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -7,10 +7,8 @@ import ( "bytes" "io" "net/http" - "net/url" "testing" - auth_model "code.gitea.io/gitea/models/auth" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" oauth2_provider "code.gitea.io/gitea/services/oauth2_provider" @@ -479,24 +477,3 @@ func TestOAuthIntrospection(t *testing.T) { resp = MakeRequest(t, req, http.StatusUnauthorized) assert.Contains(t, resp.Body.String(), "no valid authorization") } - -func TestGitOpWithOAuthDisabled(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - setting.OAuth2.Enabled = false - defer func() { - setting.OAuth2.Enabled = true - }() - - onGiteaRun(t, func(t *testing.T, u *url.URL) { - httpContext := NewAPITestContext(t, "user2", "repo1", auth_model.AccessTokenScopeWriteRepository) - - u.Path = httpContext.GitPath() - dstPath := t.TempDir() - - u.Path = httpContext.GitPath() - u.User = url.UserPassword("user2", userPassword) - - t.Run("Clone", doGitClone(dstPath, u)) - }) -}