diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 2dee93a11..0a8b9f6a4 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -951,10 +951,16 @@ func SignInOAuthCallback(ctx *context.Context) { return } else if !setting.Service.AllowOnlyInternalRegistration && setting.OAuth2Client.EnableAutoRegistration { // create new user with details from oauth2 provider - var missingFields []string if gothUser.UserID == "" { - missingFields = append(missingFields, "sub") + log.Error("OAuth2 Provider %s returned empty or missing field: UserID", authSource.Name) + if authSource.IsOAuth2() && authSource.Cfg.(*oauth2.Source).Provider == "openidConnect" { + log.Error("You may need to change the 'OPENID_CONNECT_SCOPES' setting to request all required fields") + } + err = fmt.Errorf("OAuth2 Provider %s returned empty or missing field: UserID", authSource.Name) + ctx.ServerError("CreateUser", err) + return } + var missingFields []string if gothUser.Email == "" { missingFields = append(missingFields, "email") } @@ -962,12 +968,10 @@ func SignInOAuthCallback(ctx *context.Context) { missingFields = append(missingFields, "nickname") } if len(missingFields) > 0 { - log.Error("OAuth2 Provider %s returned empty or missing fields: %s", authSource.Name, missingFields) - if authSource.IsOAuth2() && authSource.Cfg.(*oauth2.Source).Provider == "openidConnect" { - log.Error("You may need to change the 'OPENID_CONNECT_SCOPES' setting to request all required fields") - } - err = fmt.Errorf("OAuth2 Provider %s returned empty or missing fields: %s", authSource.Name, missingFields) - ctx.ServerError("CreateUser", err) + // we don't have enough information to create an account automatically, + // so we prompt the user for the remaining bits + log.Trace("OAuth2 Provider %s returned empty or missing fields: %s, prompting the user for them", authSource.Name, missingFields) + showLinkingLogin(ctx, gothUser) return } uname, err := getUserName(&gothUser) diff --git a/tests/integration/oauth_test.go b/tests/integration/oauth_test.go index 4a00d73a0..fed73696e 100644 --- a/tests/integration/oauth_test.go +++ b/tests/integration/oauth_test.go @@ -469,3 +469,30 @@ func TestSignInOAuthCallbackSignIn(t *testing.T) { userAfterLogin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userGitLab.ID}) assert.Greater(t, userAfterLogin.LastLoginUnix, userGitLab.LastLoginUnix) } + +func TestSignUpViaOAuthWithMissingFields(t *testing.T) { + defer tests.PrepareTestEnv(t)() + // enable auto-creation of accounts via OAuth2 + enableAutoRegistration := setting.OAuth2Client.EnableAutoRegistration + setting.OAuth2Client.EnableAutoRegistration = true + defer func() { + setting.OAuth2Client.EnableAutoRegistration = enableAutoRegistration + }() + + // OAuth2 authentication source GitLab + gitlabName := "gitlab" + addAuthSource(t, authSourcePayloadGitLabCustom(gitlabName)) + userGitLabUserID := "5678" + + // The Goth User returned by the oauth2 integration is missing + // an email address, so we won't be able to automatically create a local account for it. + defer mockCompleteUserAuth(func(res http.ResponseWriter, req *http.Request) (goth.User, error) { + return goth.User{ + Provider: gitlabName, + UserID: userGitLabUserID, + }, nil + })() + req := NewRequest(t, "GET", fmt.Sprintf("/user/oauth2/%s/callback?code=XYZ&state=XYZ", gitlabName)) + resp := MakeRequest(t, req, http.StatusSeeOther) + assert.Equal(t, test.RedirectURL(resp), "/user/link_account") +}