Skip to content

Disable Oauth check if oauth disabled #32368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 12, 2024
Merged

Disable Oauth check if oauth disabled #32368

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4597aeb
Disable Oauth check if oauth disabled
lunny Oct 29, 2024
ee7116a
Fix test and refactor web routers
lunny Nov 2, 2024
ff56466
Merge branch 'main' into lunny/fix-oauth2-disable
lunny Nov 2, 2024
f1b3305
Merge branch 'main' into lunny/fix-oauth2-disable
GiteaBot Nov 10, 2024
72c3025
Merge branch 'main' into lunny/fix-oauth2-disable
GiteaBot Nov 10, 2024
25ebfe4
Merge branch 'main' into lunny/fix-oauth2-disable
GiteaBot Nov 11, 2024
2d8d8aa
Merge branch 'main' into lunny/fix-oauth2-disable
GiteaBot Nov 11, 2024
da0b2cb
Merge branch 'main' into lunny/fix-oauth2-disable
wxiaoguang Nov 12, 2024
3e74010
fix
wxiaoguang Nov 12, 2024
9cb651b
Merge branch 'main' into lunny/fix-oauth2-disable
GiteaBot Nov 12, 2024
d4f9654
Merge branch 'main' into lunny/fix-oauth2-disable
GiteaBot Nov 12, 2024
5d26567
Merge branch 'main' into lunny/fix-oauth2-disable
GiteaBot Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 38 additions & 32 deletions routers/web/web.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -547,16 +554,18 @@ 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)
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", "/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)
Expand Down Expand Up @@ -597,17 +606,24 @@ 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.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)
Expand Down Expand Up @@ -781,12 +797,7 @@ func registerRoutes(m *web.Router) {
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)
Expand Down Expand Up @@ -910,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)
Expand Down
4 changes: 4 additions & 0 deletions services/auth/oauth2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions tests/integration/oauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it test? It tests nothing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ee7116a

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, what does it test? If revert all other code, the test still passes? And I guess such "basic auth" should already have been tested somewhere else?

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))
})
}
Loading