From 9fbbe613649331243b3777955cf2818862583f7e Mon Sep 17 00:00:00 2001
From: Antonin Delpeuch <antonin@delpeuch.eu>
Date: Sun, 19 Nov 2023 11:50:05 +0000
Subject: [PATCH] [GITEA] oauth2: use link_account page when email/username is
 missing (#1757)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/1757
Co-authored-by: Antonin Delpeuch <antonin@delpeuch.eu>
Co-committed-by: Antonin Delpeuch <antonin@delpeuch.eu>
(cherry picked from commit 0f6e0f90359b4b669d297a533de18b41e3293df2)
(cherry picked from commit 779168a572c521507a35ba624dbd032ec28f272e)
(cherry picked from commit 29a2457321e4587f55b333d0c5698925e403f026)
(cherry picked from commit a1edc2314d2687c9320d884f8a584d8b539eec96)
(cherry picked from commit cd015946109d39c6e30091de2fff47eba01eb937)
(cherry picked from commit 74db46b0f50a5b465269688ef83e170b7584e2be)
(cherry picked from commit fd98f55204f1cec66c3941d85b45dc84f8ab9ecd)
(cherry picked from commit 3099d0e2818d1de763a686b6a23dcf5d55ba75ef)
---
 routers/web/auth/oauth.go       | 20 ++++++++++++--------
 tests/integration/oauth_test.go | 27 +++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go
index 2dee93a11f..0a8b9f6a4b 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 4a00d73a02..fed73696e3 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")
+}