[ARVADOS] updated: 2.1.0-904-g47be45afd

[Git user]

Summary of changes:
 lib/controller/localdb/login_oidc.go      | 12 +++++-------
 lib/controller/localdb/login_oidc_test.go | 11 +++++++++--
 2 files changed, 14 insertions(+), 9 deletions(-)

       via  47be45afdbe10cfb0dd5af5cf7ee6427b66e53b3 (commit)
      from  ae1f6a36ad655640138378657f1bc33c657ad56b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 47be45afdbe10cfb0dd5af5cf7ee6427b66e53b3
Author: Nico Cesar <nico at nicocesar.com>
Date:   Thu Jul 8 10:59:53 2021 -0400

    Addressing note #11 part 1
    
    Arvados-DCO-1.1-Signed-off-by: Nico Cesar <nico at curii.com>

diff --git a/lib/controller/localdb/login_oidc.go b/lib/controller/localdb/login_oidc.go
index 2cae14e39..6182469ac 100644
--- a/lib/controller/localdb/login_oidc.go
+++ b/lib/controller/localdb/login_oidc.go
@@ -175,16 +175,14 @@ func (ctrl *oidcLoginController) getAuthInfo(ctx context.Context, token *oauth2.
 	if err := claimser.Claims(&claims); err != nil {
 		return nil, fmt.Errorf("error extracting claims from token: %s", err)
 	} else if verified, _ := claims[ctrl.EmailVerifiedClaim].(bool); verified || ctrl.EmailVerifiedClaim == "" {
-		givenName, ok := claims["given_name"].(string)
-		if ok && givenName != "" {
+		// Fall back to this info if the People API call
+		// (below) doesn't return a primary && verified email.
+		givenName, _ := claims["given_name"].(string)
+		familyName, _ := claims["family_name"].(string)
+		if givenName != "" && familyName != "" {
 			ret.FirstName = givenName
-		}
-		familyName, ok := claims["family_name"].(string)
-		if ok && familyName != "" {
 			ret.LastName = familyName
 		} else {
-			// Fall back to this info if the People API call
-			// (below) doesn't return a primary && verified email.
 			name, _ := claims["name"].(string)
 			if names := strings.Fields(strings.TrimSpace(name)); len(names) > 1 {
 				ret.FirstName = strings.Join(names[0:len(names)-1], " ")
diff --git a/lib/controller/localdb/login_oidc_test.go b/lib/controller/localdb/login_oidc_test.go
index df18e380c..50ea31d5e 100644
--- a/lib/controller/localdb/login_oidc_test.go
+++ b/lib/controller/localdb/login_oidc_test.go
@@ -51,6 +51,13 @@ func (s *OIDCLoginSuite) TearDownSuite(c *check.C) {
 	c.Check(arvados.NewClientFromEnv().RequestAndDecode(nil, "POST", "database/reset", nil, nil), check.IsNil)
 }
 
+// TODO: In the interest of reducing redundant tests: It looks like if we change
+// Given/Family names to {"Fake", "User Name"} in SetUpTest, and update
+// TestGoogleLogin_Success accordingly, then we wouldn't even need a separate
+// TestGoogleLogin_OIDCClamisWithGivenNames test to check the normal case, and
+// the old/renamed OIDCNameWithoutGivenAndFamilyNames test would check the case
+// where we fall back to splitting on whitespace.
+
 func (s *OIDCLoginSuite) SetUpTest(c *check.C) {
 	s.fakeProvider = arvadostest.NewOIDCProvider(c)
 	s.fakeProvider.AuthEmail = "active-user at arvados.local"
@@ -474,7 +481,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_RealName(c *check.C) {
 	c.Check(authinfo.LastName, check.Equals, "Psmith")
 }
 
-func (s *OIDCLoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_OIDCNameWithoutGivenAndFamilyNames(c *check.C) {
 	s.fakeProvider.AuthName = "Joe P. Smith"
 	s.fakeProvider.AuthGivenName = ""
 	s.fakeProvider.AuthFamilyName = ""
@@ -490,7 +497,7 @@ func (s *OIDCLoginSuite) TestGoogleLogin_OIDCRealName(c *check.C) {
 	c.Check(authinfo.LastName, check.Equals, "Smith")
 }
 
-func (s *OIDCLoginSuite) TestGoogleLogin_OIDCClamisWithGivenNames(c *check.C) {
+func (s *OIDCLoginSuite) TestGoogleLogin_OIDCClaimsWithGivenNames(c *check.C) {
 	s.fakeProvider.AuthName = "Federico Garcia Lorca"
 	s.fakeProvider.AuthGivenName = "Federico"
 	s.fakeProvider.AuthFamilyName = "Garcia Lorca"

-----------------------------------------------------------------------


hooks/post-receive
--