Merge pull request #2696 from dexidp/backport-2694
Backport #2694 to v2.35.x
This commit is contained in:
		| @@ -71,14 +71,11 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e | |||||||
| 		scopes = append(scopes, "profile", "email") | 		scopes = append(scopes, "profile", "email") | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	var srv *admin.Service | 	srv, err := createDirectoryService(c.ServiceAccountFilePath, c.AdminEmail, logger) | ||||||
| 	if len(c.Groups) > 0 { |  | ||||||
| 		srv, err = createDirectoryService(c.ServiceAccountFilePath, c.AdminEmail, logger) |  | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		cancel() | 		cancel() | ||||||
| 		return nil, fmt.Errorf("could not create directory service: %v", err) | 		return nil, fmt.Errorf("could not create directory service: %v", err) | ||||||
| 	} | 	} | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	clientID := c.ClientID | 	clientID := c.ClientID | ||||||
| 	return &googleConnector{ | 	return &googleConnector{ | ||||||
| @@ -286,7 +283,9 @@ func (c *googleConnector) getGroups(email string, fetchTransitiveGroupMembership | |||||||
| // the google admin api. If no serviceAccountFilePath is defined, the application default credential | // the google admin api. If no serviceAccountFilePath is defined, the application default credential | ||||||
| // is used. | // is used. | ||||||
| func createDirectoryService(serviceAccountFilePath, email string, logger log.Logger) (*admin.Service, error) { | func createDirectoryService(serviceAccountFilePath, email string, logger log.Logger) (*admin.Service, error) { | ||||||
| 	if email == "" { | 	// We know impersonation is required when using a service account credential | ||||||
|  | 	// TODO: or is it? | ||||||
|  | 	if email == "" && serviceAccountFilePath != "" { | ||||||
| 		return nil, fmt.Errorf("directory service requires adminEmail") | 		return nil, fmt.Errorf("directory service requires adminEmail") | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| @@ -311,7 +310,12 @@ func createDirectoryService(serviceAccountFilePath, email string, logger log.Log | |||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, fmt.Errorf("unable to parse credentials to config: %v", err) | 		return nil, fmt.Errorf("unable to parse credentials to config: %v", err) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | 	// Only attempt impersonation when there is a user configured | ||||||
|  | 	if email != "" { | ||||||
| 		config.Subject = email | 		config.Subject = email | ||||||
|  | 	} | ||||||
|  |  | ||||||
| 	return admin.NewService(ctx, option.WithHTTPClient(config.Client(ctx))) | 	return admin.NewService(ctx, option.WithHTTPClient(config.Client(ctx))) | ||||||
| } | } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -72,22 +72,13 @@ func TestOpen(t *testing.T) { | |||||||
| 	assert.Nil(t, err) | 	assert.Nil(t, err) | ||||||
|  |  | ||||||
| 	for name, reference := range map[string]testCase{ | 	for name, reference := range map[string]testCase{ | ||||||
| 		"not_requesting_groups": { |  | ||||||
| 			config: &Config{ |  | ||||||
| 				ClientID:     "testClient", |  | ||||||
| 				ClientSecret: "testSecret", |  | ||||||
| 				RedirectURI:  ts.URL + "/callback", |  | ||||||
| 				Scopes:       []string{"openid"}, |  | ||||||
| 			}, |  | ||||||
| 			expectedErr: "", |  | ||||||
| 		}, |  | ||||||
| 		"missing_admin_email": { | 		"missing_admin_email": { | ||||||
| 			config: &Config{ | 			config: &Config{ | ||||||
| 				ClientID:               "testClient", | 				ClientID:               "testClient", | ||||||
| 				ClientSecret:           "testSecret", | 				ClientSecret:           "testSecret", | ||||||
| 				RedirectURI:            ts.URL + "/callback", | 				RedirectURI:            ts.URL + "/callback", | ||||||
| 				Scopes:                 []string{"openid", "groups"}, | 				Scopes:                 []string{"openid", "groups"}, | ||||||
| 				Groups:       []string{"someGroup"}, | 				ServiceAccountFilePath: serviceAccountFilePath, | ||||||
| 			}, | 			}, | ||||||
| 			expectedErr: "requires adminEmail", | 			expectedErr: "requires adminEmail", | ||||||
| 		}, | 		}, | ||||||
| @@ -99,7 +90,6 @@ func TestOpen(t *testing.T) { | |||||||
| 				Scopes:                 []string{"openid", "groups"}, | 				Scopes:                 []string{"openid", "groups"}, | ||||||
| 				AdminEmail:             "foo@bar.com", | 				AdminEmail:             "foo@bar.com", | ||||||
| 				ServiceAccountFilePath: "not_found.json", | 				ServiceAccountFilePath: "not_found.json", | ||||||
| 				Groups:                 []string{"someGroup"}, |  | ||||||
| 			}, | 			}, | ||||||
| 			expectedErr: "error reading credentials", | 			expectedErr: "error reading credentials", | ||||||
| 		}, | 		}, | ||||||
| @@ -111,7 +101,6 @@ func TestOpen(t *testing.T) { | |||||||
| 				Scopes:                 []string{"openid", "groups"}, | 				Scopes:                 []string{"openid", "groups"}, | ||||||
| 				AdminEmail:             "foo@bar.com", | 				AdminEmail:             "foo@bar.com", | ||||||
| 				ServiceAccountFilePath: serviceAccountFilePath, | 				ServiceAccountFilePath: serviceAccountFilePath, | ||||||
| 				Groups:                 []string{"someGroup"}, |  | ||||||
| 			}, | 			}, | ||||||
| 			expectedErr: "", | 			expectedErr: "", | ||||||
| 		}, | 		}, | ||||||
| @@ -122,7 +111,6 @@ func TestOpen(t *testing.T) { | |||||||
| 				RedirectURI:  ts.URL + "/callback", | 				RedirectURI:  ts.URL + "/callback", | ||||||
| 				Scopes:       []string{"openid", "groups"}, | 				Scopes:       []string{"openid", "groups"}, | ||||||
| 				AdminEmail:   "foo@bar.com", | 				AdminEmail:   "foo@bar.com", | ||||||
| 				Groups:       []string{"someGroup"}, |  | ||||||
| 			}, | 			}, | ||||||
| 			adc:         serviceAccountFilePath, | 			adc:         serviceAccountFilePath, | ||||||
| 			expectedErr: "", | 			expectedErr: "", | ||||||
| @@ -135,7 +123,6 @@ func TestOpen(t *testing.T) { | |||||||
| 				Scopes:                 []string{"openid", "groups"}, | 				Scopes:                 []string{"openid", "groups"}, | ||||||
| 				AdminEmail:             "foo@bar.com", | 				AdminEmail:             "foo@bar.com", | ||||||
| 				ServiceAccountFilePath: serviceAccountFilePath, | 				ServiceAccountFilePath: serviceAccountFilePath, | ||||||
| 				Groups:                 []string{"someGroup"}, |  | ||||||
| 			}, | 			}, | ||||||
| 			adc:         "/dev/null", | 			adc:         "/dev/null", | ||||||
| 			expectedErr: "", | 			expectedErr: "", | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user