Backport #37327 by @prettysunflower Nyallo~ In pull request #36901, a change is made so that the link to authentication sources is now escaped with the QueryEscape filter. https://github.com/go-gitea/gitea/pull/36901/changes#diff-34c39c9736a8b62e293c0c0b24c4b5b8c1c792790018c5809f9ff2cbc12b16b1R4 The problem is that [QueryEscape replace spaces with the `+` character](https://cs.opensource.google/go/go/+/refs/tags/go1.26.2:src/net/url/url.go;l=234;drc=917949cc1d16c652cb09ba369718f45e5d814d8f), and this is not unescaped when a user tries to log in with an authentication source that contains a space, which throws an error. This commit fixes that by unescaping the provider name in the URL. --- Example of the error, on my instance, when I try to log in with `prettysunflower's auth` ``` 2026/04/21 00:11:41 routers/web/auth/oauth.go:42:SignInOAuth() [E] SignIn: oauth2 source not found, name: "prettysunflower's+auth" /go/src/code.gitea.io/gitea/routers/web/auth/oauth.go:42 (0x2cfa5c5) /usr/local/go/src/reflect/value.go:586 (0x51e245) /usr/local/go/src/reflect/value.go:369 (0x51d0f8) /go/src/code.gitea.io/gitea/modules/web/handler.go:181 (0x1a6aaf6) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:188 (0x1a6ab65) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:188 (0x1a6ab65) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:188 (0x1a6ab65) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/services/context/context.go:217 (0x2df1b23) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/gitea.com/go-chi/session@v0.0.0-20251124165456-68e0254e989e/session.go:258 (0x197eb82) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/chain.go:31 (0x1a61d05) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:479 (0x1a64fae) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:73 (0x1a628c2) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:321 (0x1a6421a) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/chain.go:31 (0x1a61d05) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:479 (0x1a64fae) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/middleware/get_head.go:37 (0x2c33a67) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:73 (0x1a628c2) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:321 (0x1a6421a) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/routers/common/maintenancemode.go:50 (0x2b752da) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/chain.go:31 (0x1a61d05) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:479 (0x1a64fae) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/routing/logger_manager.go:124 (0x127d1ec) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/chi-middleware/proxy@v1.1.1/middleware.go:37 (0x2b76acf) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/routers/common/middleware.go:89 (0x2b78cd6) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/routers/common/middleware.go:104 (0x2b7890f) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/src/code.gitea.io/gitea/modules/web/handler.go:145 (0x1a6afb5) /usr/local/go/src/net/http/server.go:2286 (0x94dc88) /go/pkg/mod/github.com/go-chi/chi/v5@v5.2.5/mux.go:90 (0x1a62881) /go/src/code.gitea.io/gitea/modules/web/router.go:286 (0x1a6d2a2) /go/src/code.gitea.io/gitea/modules/web/router.go:221 (0x1a6cbc6) /usr/local/go/src/net/http/server.go:3311 (0x96e36d) /usr/local/go/src/net/http/server.go:2073 (0x94bd6f) /usr/local/go/src/runtime/asm_amd64.s:1771 (0x49af20) ``` Signed-off-by: prettysunflower <me@prettysunflower.moe> Signed-off-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: prettysunflower <me@prettysunflower.moe> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
@@ -36,7 +36,9 @@ import (
|
|||||||
|
|
||||||
// SignInOAuth handles the OAuth2 login buttons
|
// SignInOAuth handles the OAuth2 login buttons
|
||||||
func SignInOAuth(ctx *context.Context) {
|
func SignInOAuth(ctx *context.Context) {
|
||||||
authName := ctx.PathParam("provider")
|
// the provider is escaped by backend QueryEscape and frontend urlQueryEscape
|
||||||
|
// so always use QueryUnescape to decode it
|
||||||
|
authName, _ := url.QueryUnescape(ctx.PathParamRaw("provider"))
|
||||||
authSource, err := auth.GetActiveOAuth2SourceByAuthName(ctx, authName)
|
authSource, err := auth.GetActiveOAuth2SourceByAuthName(ctx, authName)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ctx.ServerError("SignIn", err)
|
ctx.ServerError("SignIn", err)
|
||||||
|
|||||||
@@ -44,6 +44,8 @@ func TestOAuth2Provider(t *testing.T) {
|
|||||||
t.Run("AuthorizeLoginRedirect", testAuthorizeLoginRedirect)
|
t.Run("AuthorizeLoginRedirect", testAuthorizeLoginRedirect)
|
||||||
|
|
||||||
t.Run("OAuth2WellKnown", testOAuth2WellKnown)
|
t.Run("OAuth2WellKnown", testOAuth2WellKnown)
|
||||||
|
t.Run("OAuthSourceWithSpace", testOAuthSourceWithSpace)
|
||||||
|
// TODO: move more tests as sub-tests here, avoid unnecessary PrepareTestEnv
|
||||||
}
|
}
|
||||||
|
|
||||||
func testAuthorizeNoClientID(t *testing.T) {
|
func testAuthorizeNoClientID(t *testing.T) {
|
||||||
@@ -999,9 +1001,7 @@ func addOAuth2Source(t *testing.T, authName string, cfg oauth2.Source) {
|
|||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSignInOauthCallbackSyncSSHKeys(t *testing.T) {
|
func createMockServer() *httptest.Server {
|
||||||
defer tests.PrepareTestEnv(t)()
|
|
||||||
|
|
||||||
var mockServer *httptest.Server
|
var mockServer *httptest.Server
|
||||||
mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
mockServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
switch r.URL.Path {
|
switch r.URL.Path {
|
||||||
@@ -1016,6 +1016,14 @@ func TestSignInOauthCallbackSyncSSHKeys(t *testing.T) {
|
|||||||
http.NotFound(w, r)
|
http.NotFound(w, r)
|
||||||
}
|
}
|
||||||
}))
|
}))
|
||||||
|
|
||||||
|
return mockServer
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSignInOauthCallbackSyncSSHKeys(t *testing.T) {
|
||||||
|
defer tests.PrepareTestEnv(t)()
|
||||||
|
|
||||||
|
mockServer := createMockServer()
|
||||||
defer mockServer.Close()
|
defer mockServer.Close()
|
||||||
|
|
||||||
ctx := t.Context()
|
ctx := t.Context()
|
||||||
@@ -1091,3 +1099,22 @@ func TestSignInOauthCallbackSyncSSHKeys(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Checks if an OAuth provider with spaces within the name does work,
|
||||||
|
// with the encoding of its names in the URL (PR#37327)
|
||||||
|
func testOAuthSourceWithSpace(t *testing.T) {
|
||||||
|
mockServer := createMockServer()
|
||||||
|
defer mockServer.Close()
|
||||||
|
|
||||||
|
authName := "oauth test with spaces"
|
||||||
|
oauth2Source := oauth2.Source{
|
||||||
|
Provider: "openidConnect",
|
||||||
|
OpenIDConnectAutoDiscoveryURL: mockServer.URL + "/.well-known/openid-configuration",
|
||||||
|
}
|
||||||
|
addOAuth2Source(t, authName, oauth2Source)
|
||||||
|
|
||||||
|
session := emptyTestSession(t)
|
||||||
|
req := NewRequest(t, "GET", "/user/oauth2/"+url.QueryEscape(authName))
|
||||||
|
resp := session.MakeRequest(t, req, http.StatusTemporaryRedirect)
|
||||||
|
assert.Contains(t, resp.Header().Get("Location"), mockServer.URL+"/authorize")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user