Skip to content

Commit

Permalink
#2895: Add Support for Multiple Admin Emails to Retrieve Group Lists (#…
Browse files Browse the repository at this point in the history
…2911)

Signed-off-by: Viacheslav Sychov <[email protected]>
  • Loading branch information
vsychov authored Jun 7, 2023
1 parent 47a0e06 commit 6cd5c8b
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 37 deletions.
93 changes: 71 additions & 22 deletions connector/google/google.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"os"
"strings"
"time"

"github.com/coreos/go-oidc/v3/oidc"
Expand All @@ -22,7 +23,8 @@ import (
)

const (
issuerURL = "https://accounts.google.com"
issuerURL = "https://accounts.google.com"
wildcardDomainToAdminEmail = "*"
)

// Config holds configuration options for Google logins.
Expand All @@ -46,17 +48,28 @@ type Config struct {
// check groups with the admin directory api
ServiceAccountFilePath string `json:"serviceAccountFilePath"`

// Deprecated: Use DomainToAdminEmail
AdminEmail string

// Required if ServiceAccountFilePath
// The email of a GSuite super user which the service account will impersonate
// The map workspace domain to email of a GSuite super user which the service account will impersonate
// when listing groups
AdminEmail string
DomainToAdminEmail map[string]string

// If this field is true, fetch direct group membership and transitive group membership
FetchTransitiveGroupMembership bool `json:"fetchTransitiveGroupMembership"`
}

// Open returns a connector which can be used to login users through Google.
func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, err error) {
if c.AdminEmail != "" {
log.Deprecated(logger, `google: use "domainToAdminEmail.*: %s" option instead of "adminEmail: %s".`, c.AdminEmail, c.AdminEmail)
if c.DomainToAdminEmail == nil {
c.DomainToAdminEmail = make(map[string]string)
}

c.DomainToAdminEmail[wildcardDomainToAdminEmail] = c.AdminEmail
}
ctx, cancel := context.WithCancel(context.Background())

provider, err := oidc.NewProvider(ctx, issuerURL)
Expand All @@ -72,17 +85,26 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
scopes = append(scopes, "profile", "email")
}

var adminSrv *admin.Service
adminSrv := make(map[string]*admin.Service)

// We know impersonation is required when using a service account credential
// TODO: or is it?
if len(c.DomainToAdminEmail) == 0 && c.ServiceAccountFilePath != "" {
cancel()
return nil, fmt.Errorf("directory service requires the domainToAdminEmail option to be configured")
}

// Fixing a regression caused by default config fallback: https://github.com/dexidp/dex/issues/2699
if (c.ServiceAccountFilePath != "" && c.AdminEmail != "") || slices.Contains(scopes, "groups") {
srv, err := createDirectoryService(c.ServiceAccountFilePath, c.AdminEmail, logger)
if err != nil {
cancel()
return nil, fmt.Errorf("could not create directory service: %v", err)
}
if (c.ServiceAccountFilePath != "" && len(c.DomainToAdminEmail) > 0) || slices.Contains(scopes, "groups") {
for domain, adminEmail := range c.DomainToAdminEmail {
srv, err := createDirectoryService(c.ServiceAccountFilePath, adminEmail, logger)
if err != nil {
cancel()
return nil, fmt.Errorf("could not create directory service: %v", err)
}

adminSrv = srv
adminSrv[domain] = srv
}
}

clientID := c.ClientID
Expand All @@ -103,7 +125,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
hostedDomains: c.HostedDomains,
groups: c.Groups,
serviceAccountFilePath: c.ServiceAccountFilePath,
adminEmail: c.AdminEmail,
domainToAdminEmail: c.DomainToAdminEmail,
fetchTransitiveGroupMembership: c.FetchTransitiveGroupMembership,
adminSrv: adminSrv,
}, nil
Expand All @@ -123,9 +145,9 @@ type googleConnector struct {
hostedDomains []string
groups []string
serviceAccountFilePath string
adminEmail string
domainToAdminEmail map[string]string
fetchTransitiveGroupMembership bool
adminSrv *admin.Service
adminSrv map[string]*admin.Service
}

func (c *googleConnector) Close() error {
Expand Down Expand Up @@ -226,7 +248,7 @@ func (c *googleConnector) createIdentity(ctx context.Context, identity connector
}

var groups []string
if s.Groups && c.adminSrv != nil {
if s.Groups && len(c.adminSrv) > 0 {
checkedGroups := make(map[string]struct{})
groups, err = c.getGroups(claims.Email, c.fetchTransitiveGroupMembership, checkedGroups)
if err != nil {
Expand Down Expand Up @@ -258,8 +280,14 @@ func (c *googleConnector) getGroups(email string, fetchTransitiveGroupMembership
var userGroups []string
var err error
groupsList := &admin.Groups{}
domain := c.extractDomainFromEmail(email)
adminSrv, err := c.findAdminService(domain)
if err != nil {
return nil, err
}

for {
groupsList, err = c.adminSrv.Groups.List().
groupsList, err = adminSrv.Groups.List().
UserKey(email).PageToken(groupsList.NextPageToken).Do()
if err != nil {
return nil, fmt.Errorf("could not list groups: %v", err)
Expand Down Expand Up @@ -295,16 +323,37 @@ func (c *googleConnector) getGroups(email string, fetchTransitiveGroupMembership
return userGroups, nil
}

func (c *googleConnector) findAdminService(domain string) (*admin.Service, error) {
adminSrv, ok := c.adminSrv[domain]
if !ok {
adminSrv, ok = c.adminSrv[wildcardDomainToAdminEmail]
c.logger.Debugf("using wildcard (%s) admin email to fetch groups", c.domainToAdminEmail[wildcardDomainToAdminEmail])
}

if !ok {
return nil, fmt.Errorf("unable to find super admin email, domainToAdminEmail for domain: %s not set, %s is also empty", domain, wildcardDomainToAdminEmail)
}

return adminSrv, nil
}

// extracts the domain name from an email input. If the email is valid, it returns the domain name after the "@" symbol.
// However, in the case of a broken or invalid email, it returns a wildcard symbol.
func (c *googleConnector) extractDomainFromEmail(email string) string {
at := strings.LastIndex(email, "@")
if at >= 0 {
_, domain := email[:at], email[at+1:]

return domain
}

return wildcardDomainToAdminEmail
}

// createDirectoryService sets up super user impersonation and creates an admin client for calling
// the google admin api. If no serviceAccountFilePath is defined, the application default credential
// is used.
func createDirectoryService(serviceAccountFilePath, email string, logger log.Logger) (*admin.Service, error) {
// 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")
}

var jsonCredentials []byte
var err error

Expand Down
85 changes: 70 additions & 15 deletions connector/google/google_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,15 @@ func TestOpen(t *testing.T) {
Scopes: []string{"openid", "groups"},
ServiceAccountFilePath: serviceAccountFilePath,
},
expectedErr: "requires adminEmail",
expectedErr: "requires the domainToAdminEmail",
},
"service_account_key_not_found": {
config: &Config{
ClientID: "testClient",
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
AdminEmail: "[email protected]",
DomainToAdminEmail: map[string]string{"*": "[email protected]"},
ServiceAccountFilePath: "not_found.json",
},
expectedErr: "error reading credentials",
Expand All @@ -121,18 +121,18 @@ func TestOpen(t *testing.T) {
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
AdminEmail: "[email protected]",
DomainToAdminEmail: map[string]string{"bar.com": "[email protected]"},
ServiceAccountFilePath: serviceAccountFilePath,
},
expectedErr: "",
},
"adc": {
config: &Config{
ClientID: "testClient",
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
AdminEmail: "[email protected]",
ClientID: "testClient",
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
DomainToAdminEmail: map[string]string{"*": "[email protected]"},
},
adc: serviceAccountFilePath,
expectedErr: "",
Expand All @@ -143,7 +143,7 @@ func TestOpen(t *testing.T) {
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
AdminEmail: "[email protected]",
DomainToAdminEmail: map[string]string{"*": "[email protected]"},
ServiceAccountFilePath: serviceAccountFilePath,
},
adc: "/dev/null",
Expand Down Expand Up @@ -176,15 +176,15 @@ func TestGetGroups(t *testing.T) {

os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", serviceAccountFilePath)
conn, err := newConnector(&Config{
ClientID: "testClient",
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
AdminEmail: "[email protected]",
ClientID: "testClient",
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
DomainToAdminEmail: map[string]string{"*": "[email protected]"},
})
assert.Nil(t, err)

conn.adminSrv, err = admin.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(ts.URL))
conn.adminSrv[wildcardDomainToAdminEmail], err = admin.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(ts.URL))
assert.Nil(t, err)
type testCase struct {
userKey string
Expand Down Expand Up @@ -236,3 +236,58 @@ func TestGetGroups(t *testing.T) {
})
}
}

func TestDomainToAdminEmailConfig(t *testing.T) {
ts := testSetup()
defer ts.Close()

serviceAccountFilePath, err := tempServiceAccountKey()
assert.Nil(t, err)

os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", serviceAccountFilePath)
conn, err := newConnector(&Config{
ClientID: "testClient",
ClientSecret: "testSecret",
RedirectURI: ts.URL + "/callback",
Scopes: []string{"openid", "groups"},
DomainToAdminEmail: map[string]string{"dexidp.com": "[email protected]"},
})
assert.Nil(t, err)

conn.adminSrv["dexidp.com"], err = admin.NewService(context.Background(), option.WithoutAuthentication(), option.WithEndpoint(ts.URL))
assert.Nil(t, err)
type testCase struct {
userKey string
expectedErr string
}

for name, testCase := range map[string]testCase{
"correct_user_request": {
userKey: "[email protected]",
expectedErr: "",
},
"wrong_user_request": {
userKey: "[email protected]",
expectedErr: "unable to find super admin email",
},
"wrong_connector_response": {
userKey: "user_1_foo.bar",
expectedErr: "unable to find super admin email",
},
} {
testCase := testCase
callCounter = map[string]int{}
t.Run(name, func(t *testing.T) {
assert := assert.New(t)
lookup := make(map[string]struct{})

_, err := conn.getGroups(testCase.userKey, true, lookup)
if testCase.expectedErr != "" {
assert.ErrorContains(err, testCase.expectedErr)
} else {
assert.Nil(err)
}
t.Logf("[%s] Amount of API calls per userKey: %+v\n", t.Name(), callCounter)
})
}
}

0 comments on commit 6cd5c8b

Please sign in to comment.