OAuth: configurable user name attribute (#28286)

* OAuth: more user-frienly logging

* OAuth: custom user name attribute

* OAuth: remove deprecated nameAttributeName option

* OAuth: nameAttributePath tests

* OAuth: add name_attribute_path config option

* OAuth: docs for name_attribute_path option

* move docs to the separate branch

conf/defaults.ini

@@ -442,6 +442,7 @@ scopes = user:email
 email_attribute_name = email:primary
 email_attribute_path =
 login_attribute_path =
+name_attribute_path =
 role_attribute_path =
 id_token_attribute_name =
 auth_url =

conf/sample.ini

@@ -432,6 +432,7 @@
 ;email_attribute_name = email:primary
 ;email_attribute_path =
 ;login_attribute_path =
+;name_attribute_path =
 ;id_token_attribute_name =
 ;auth_url = https://foo.bar/login/oauth/authorize
 ;token_url = https://foo.bar/login/oauth/access_token

pkg/login/social/generic_oauth.go

@@ -24,6 +24,7 @@ type SocialGenericOAuth struct {
 	emailAttributeName   string
 	emailAttributePath   string
 	loginAttributePath   string
+	nameAttributePath    string
 	roleAttributePath    string
 	idTokenAttributeName string
 	teamIds              []int
@@ -107,13 +108,7 @@ func (s *SocialGenericOAuth) UserInfo(client *http.Client, token *oauth2.Token)
 		s.log.Debug("Processing external user info", "source", data.source, "data", data)
 
 		if userInfo.Name == "" {
-			if data.Name != "" {
+			userInfo.Name = s.extractUserName(data)
-				s.log.Debug("Setting user info name from name field")
-				userInfo.Name = data.Name
-			} else if data.DisplayName != "" {
-				s.log.Debug("Setting user info name from display name field")
-				userInfo.Name = data.DisplayName
-			}
 		}
 
 		if userInfo.Login == "" {
@@ -250,7 +245,7 @@ func (s *SocialGenericOAuth) extractFromToken(token *oauth2.Token) *UserInfoJson
 
 	data.rawJSON = rawJSON
 	data.source = "token"
-	s.log.Debug("Received id_token", "raw_json", string(data.rawJSON), "data", data)
+	s.log.Debug("Received id_token", "raw_json", string(data.rawJSON), "data", data.String())
 	return &data
 }
 
@@ -272,7 +267,7 @@ func (s *SocialGenericOAuth) extractFromAPI(client *http.Client) *UserInfoJson {
 
 	data.rawJSON = rawJSON
 	data.source = "API"
-	s.log.Debug("Received user info response from API", "raw_json", string(rawJSON), "data", data)
+	s.log.Debug("Received user info response from API", "raw_json", string(rawJSON), "data", data.String())
 	return &data
 }
 
@@ -306,6 +301,31 @@ func (s *SocialGenericOAuth) extractEmail(data *UserInfoJson) string {
 	return ""
 }
 
+func (s *SocialGenericOAuth) extractUserName(data *UserInfoJson) string {
+	if s.nameAttributePath != "" {
+		name, err := s.searchJSONForAttr(s.nameAttributePath, data.rawJSON)
+		if err != nil {
+			s.log.Error("Failed to search JSON for attribute", "error", err)
+		} else if name != "" {
+			s.log.Debug("Setting user info name from nameAttributePath", "nameAttributePath", s.nameAttributePath)
+			return name
+		}
+	}
+
+	if data.Name != "" {
+		s.log.Debug("Setting user info name from name field")
+		return data.Name
+	}
+
+	if data.DisplayName != "" {
+		s.log.Debug("Setting user info name from display name field")
+		return data.DisplayName
+	}
+
+	s.log.Debug("Unable to find user info name")
+	return ""
+}
+
 func (s *SocialGenericOAuth) extractRole(data *UserInfoJson) (string, error) {
 	if s.roleAttributePath == "" {
 		return "", nil

pkg/login/social/generic_oauth_test.go

@@ -428,6 +428,104 @@ func TestUserInfoSearchesForLogin(t *testing.T) {
 	})
 }
 
+func TestUserInfoSearchesForName(t *testing.T) {
+	t.Run("Given a generic OAuth provider", func(t *testing.T) {
+		provider := SocialGenericOAuth{
+			SocialBase: &SocialBase{
+				log: log.NewWithLevel("generic_oauth_test", log15.LvlDebug),
+			},
+			nameAttributePath: "name",
+		}
+
+		tests := []struct {
+			Name              string
+			ResponseBody      interface{}
+			OAuth2Extra       interface{}
+			NameAttributePath string
+			ExpectedName      string
+		}{
+			{
+				Name: "Given a valid id_token, a valid name path, no API response, use id_token",
+				OAuth2Extra: map[string]interface{}{
+					// { "name": "John Doe", "login": "johndoe", "email": "john.doe@example.com" }
+					"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJsb2dpbiI6ImpvaG5kb2UiLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwibmFtZSI6IkpvaG4gRG9lIn0.oMsXH0mHxUSYMXh6FonZIWh8LgNIcYbKRLSO1bwnfSI",
+				},
+				NameAttributePath: "name",
+				ExpectedName:      "John Doe",
+			},
+			{
+				Name: "Given a valid id_token, no name path, no API response, use id_token",
+				OAuth2Extra: map[string]interface{}{
+					// { "name": "John Doe", "login": "johndoe", "email": "john.doe@example.com" }
+					"id_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJsb2dpbiI6ImpvaG5kb2UiLCJlbWFpbCI6ImpvaG4uZG9lQGV4YW1wbGUuY29tIiwibmFtZSI6IkpvaG4gRG9lIn0.oMsXH0mHxUSYMXh6FonZIWh8LgNIcYbKRLSO1bwnfSI",
+				},
+				NameAttributePath: "",
+				ExpectedName:      "John Doe",
+			},
+			{
+				Name: "Given no id_token, a valid name path, a valid API response, use API response",
+				ResponseBody: map[string]interface{}{
+					"user_name": "John Doe",
+					"login":     "johndoe",
+					"email":     "john.doe@example.com",
+				},
+				NameAttributePath: "user_name",
+				ExpectedName:      "John Doe",
+			},
+			{
+				Name: "Given no id_token, no name path, a valid API response, use API response",
+				ResponseBody: map[string]interface{}{
+					"display_name": "John Doe",
+					"login":        "johndoe",
+				},
+				NameAttributePath: "",
+				ExpectedName:      "John Doe",
+			},
+			{
+				Name: "Given no id_token, a name path, a valid API response without a name, use API response",
+				ResponseBody: map[string]interface{}{
+					"display_name": "John Doe",
+					"username":     "john.doe",
+				},
+				NameAttributePath: "name",
+				ExpectedName:      "John Doe",
+			},
+			{
+				Name:              "Given no id_token, a valid name path, no API response, no data",
+				NameAttributePath: "name",
+				ExpectedName:      "",
+			},
+		}
+
+		for _, test := range tests {
+			provider.nameAttributePath = test.NameAttributePath
+			t.Run(test.Name, func(t *testing.T) {
+				body, err := json.Marshal(test.ResponseBody)
+				require.NoError(t, err)
+				ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.WriteHeader(http.StatusOK)
+					w.Header().Set("Content-Type", "application/json")
+					t.Log("Writing fake API response body", "body", test.ResponseBody)
+					_, err = w.Write(body)
+					require.NoError(t, err)
+				}))
+				provider.apiUrl = ts.URL
+				staticToken := oauth2.Token{
+					AccessToken:  "",
+					TokenType:    "",
+					RefreshToken: "",
+					Expiry:       time.Now(),
+				}
+
+				token := staticToken.WithExtra(test.OAuth2Extra)
+				actualResult, err := provider.UserInfo(ts.Client(), token)
+				require.NoError(t, err)
+				require.Equal(t, test.ExpectedName, actualResult.Name)
+			})
+		}
+	})
+}
+
 func TestPayloadCompression(t *testing.T) {
 	provider := SocialGenericOAuth{
 		SocialBase: &SocialBase{

pkg/login/social/social.go

@@ -181,6 +181,7 @@ func NewOAuthService() {
 				apiUrl:               info.ApiUrl,
 				emailAttributeName:   info.EmailAttributeName,
 				emailAttributePath:   info.EmailAttributePath,
+				nameAttributePath:    sec.Key("name_attribute_path").String(),
 				roleAttributePath:    info.RoleAttributePath,
 				loginAttributePath:   sec.Key("login_attribute_path").String(),
 				idTokenAttributeName: sec.Key("id_token_attribute_name").String(),