diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index a53e718aa9..89d2fd37ce 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1024,7 +1024,7 @@ func SignInOAuthCallback(ctx *context.Context) { return } if err, ok := err.(*go_oauth2.RetrieveError); ok { - ctx.Flash.Error("OAuth2 RetrieveError: "+err.Error(), true) + ctx.Flash.Error("OAuth2 RetrieveError: " + err.Error()) ctx.Redirect(setting.AppSubURL + "/user/login") return } diff --git a/services/forms/auth_form.go b/services/forms/auth_form.go index 59c5f5b80f..9d60c1561f 100644 --- a/services/forms/auth_form.go +++ b/services/forms/auth_form.go @@ -57,8 +57,8 @@ type AuthenticationForm struct { PAMServiceName string PAMEmailDomain string Oauth2Provider string - Oauth2Key string - Oauth2Secret string + Oauth2Key string `preprocess:"TrimSpace"` + Oauth2Secret string `preprocess:"TrimSpace"` OpenIDConnectAutoDiscoveryURL string Oauth2UseCustomURL bool Oauth2TokenURL string diff --git a/tests/integration/admin_auth_source_test.go b/tests/integration/admin_auth_source_test.go index 4b46541318..d655ec0c39 100644 --- a/tests/integration/admin_auth_source_test.go +++ b/tests/integration/admin_auth_source_test.go @@ -10,6 +10,8 @@ import ( "forgejo.org/models/auth" "forgejo.org/tests" + + "github.com/stretchr/testify/assert" ) func TestAdminAuthAllowUsernameChangeSetting(t *testing.T) { @@ -30,3 +32,24 @@ func TestAdminAuthAllowUsernameChangeSetting(t *testing.T) { htmlDoc.AssertElement(t, "#allow_username_change[checked]", true) } + +func TestAdminAuthTrimSpace(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + session := loginUser(t, "user1") + + source := addAuthSource(t, map[string]string{ + "type": fmt.Sprintf("%d", auth.OAuth2), + "name": "some-name", + "is_active": "on", + "oauth2_provider": "gitlab", + "oauth2_key": " public_id ", + "oauth2_secret": " secret_key ", + }) + + response := session.MakeRequest(t, NewRequestf(t, "GET", "/admin/auths/%d", source.ID), http.StatusOK) + htmlDoc := NewHTMLParser(t, response.Body) + + assert.Equal(t, "public_id", htmlDoc.GetInputValueByName("oauth2_key")) + assert.Equal(t, "secret_key", htmlDoc.GetInputValueByName("oauth2_secret")) +} diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 2533a64a3d..27c66f0e5a 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -26,11 +26,13 @@ import ( api "forgejo.org/modules/structs" "forgejo.org/modules/test" "forgejo.org/routers/web/auth" + app_context "forgejo.org/services/context" "forgejo.org/tests" "github.com/markbates/goth" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + go_oauth2 "golang.org/x/oauth2" ) func TestAuthorizeNoClientID(t *testing.T) { @@ -1803,3 +1805,38 @@ func TestSignInOAuthCallbackGothUserFields(t *testing.T) { assert.True(t, logFiltered[1], "Expected trace log with IDToken") }) } + +func TestSignInOAuthCallbackSignInRetrieveError(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + gitlabName := "gitlab" + gitlab := addAuthSource(t, authSourcePayloadGitLabCustom(gitlabName)) + + userGitLabUserID := "5678" + userGitLab := &user_model.User{ + Name: "gitlabuser", + Email: "gitlabuser@example.com", + Passwd: "gitlabuserpassword", + Type: user_model.UserTypeIndividual, + LoginType: auth_model.OAuth2, + LoginSource: gitlab.ID, + LoginName: userGitLabUserID, + } + defer createUser(t.Context(), t, userGitLab)() + + defer mockCompleteUserAuth(func(res http.ResponseWriter, req *http.Request) (goth.User, error) { + return goth.User{}, &go_oauth2.RetrieveError{ + Response: &http.Response{ + Status: "404 Not Found", + }, + Body: []byte("cooked"), + } + })() + sess := emptyTestSession(t) + resp := sess.MakeRequest(t, NewRequest(t, "GET", fmt.Sprintf("/user/oauth2/%s/callback?code=XYZ&state=XYZ", gitlabName)), http.StatusSeeOther) + + assert.Equal(t, "/user/login", test.RedirectURL(resp)) + flashCookie := sess.GetCookie(app_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.Equal(t, "error%3DOAuth2%2BRetrieveError%253A%2Boauth2%253A%2Bcannot%2Bfetch%2Btoken%253A%2B404%2BNot%2BFound%250AResponse%253A%2Bcooked", flashCookie.Value) +}