feat: make google-groups argument optional (#3138)

add test cases

update documentation

refactor code and some cleanup

update changelog

Signed-off-by: Jan Larwig <jan@larwig.com>

CHANGELOG.md

@@ -15,6 +15,7 @@
 - [#2524](https://github.com/oauth2-proxy/oauth2-proxy/pull/2524) fix: regex substitution for $ signs in upstream path handling before running envsubst (@dashkan / @tuunit)
 - [#3104](https://github.com/oauth2-proxy/oauth2-proxy/pull/3104) feat(cookie): add feature support for cookie-secret-file (@sandy2008)
 - [#3055](https://github.com/oauth2-proxy/oauth2-proxy/pull/3055) feat: support non-default authorization request response mode also for OIDC providers (@stieler-it)
+- [#3138](https://github.com/oauth2-proxy/oauth2-proxy/pull/3138) feat: make google_groups argument optional when using google provider (@sourava01)
 
 # V7.10.0
 

docs/docs/configuration/providers/google.md

@@ -8,7 +8,7 @@ title: Google (default)
 | Flag                                           | Toml Field                                   | Type   | Description                                                                                      | Default                                            |
 | ---------------------------------------------- | -------------------------------------------- | ------ | ------------------------------------------------------------------------------------------------ | -------------------------------------------------- |
 | `--google-admin-email`                         | `google_admin_email`                         | string | the google admin to impersonate for api calls                                                    |                                                    |
-| `--google-group`                               | `google_groups`                              | string | restrict logins to members of this google group (may be given multiple times).                   |                                                    |
+| `--google-group`                               | `google_groups`                              | string | restrict logins to members of this google group (may be given multiple times). If not specified and service account or default credentials are configured, all user groups will be allowed.                   |                                                    |
 | `--google-service-account-json`                | `google_service_account_json`                | string | the path to the service account json credentials                                                 |                                                    |
 | `--google-use-application-default-credentials` | `google_use_application_default_credentials` | bool   | use application default credentials instead of service account json (i.e. GKE Workload Identity) |                                                    |
 | `--google-target-principal`                    | `google_target_principal`                    | bool   | the target principal to impersonate when using ADC                                               | defaults to the service account configured for ADC |

pkg/validation/options_test.go

@@ -55,18 +55,38 @@ func TestNewOptions(t *testing.T) {
 	assert.Equal(t, expected, err.Error())
 }
 
-func TestGoogleGroupOptions(t *testing.T) {
+func TestGoogleGroupOptionsWithoutServiceAccountJSON(t *testing.T) {
 	o := testOptions()
-	o.Providers[0].GoogleConfig.Groups = []string{"googlegroup"}
+	o.Providers[0].GoogleConfig.AdminEmail = "admin@example.com"
 	err := Validate(o)
 	assert.NotEqual(t, nil, err)
 
 	expected := errorMsg([]string{
-		"missing setting: google-admin-email",
 		"missing setting: google-service-account-json or google-use-application-default-credentials"})
 	assert.Equal(t, expected, err.Error())
 }
 
+func TestGoogleGroupOptionsWithoutAdminEmail(t *testing.T) {
+	o := testOptions()
+	o.Providers[0].GoogleConfig.UseApplicationDefaultCredentials = true
+	err := Validate(o)
+	assert.NotEqual(t, nil, err)
+
+	expected := errorMsg([]string{
+		"missing setting: google-admin-email"})
+	assert.Equal(t, expected, err.Error())
+}
+
+func TestGoogleGroupOptionsWithoutGroups(t *testing.T) {
+	o := testOptions()
+	// Set admin email and application default credentials but no groups - should still require them
+	o.Providers[0].GoogleConfig.AdminEmail = "admin@example.com"
+	o.Providers[0].GoogleConfig.UseApplicationDefaultCredentials = true
+	err := Validate(o)
+	// Should pass validation since google-group is now optional
+	assert.Equal(t, nil, err)
+}
+
 func TestGoogleGroupInvalidFile(t *testing.T) {
 	o := testOptions()
 	o.Providers[0].GoogleConfig.Groups = []string{"test_group"}

pkg/validation/providers.go

@@ -94,18 +94,14 @@ func validateClientSecret(provider options.Provider) []string {
 func validateGoogleConfig(provider options.Provider) []string {
 	msgs := []string{}
 
-	hasGoogleGroups := len(provider.GoogleConfig.Groups) >= 1
 	hasAdminEmail := provider.GoogleConfig.AdminEmail != ""
 	hasSAJSON := provider.GoogleConfig.ServiceAccountJSON != ""
 	useADC := provider.GoogleConfig.UseApplicationDefaultCredentials
 
-	if !hasGoogleGroups && !hasAdminEmail && !hasSAJSON && !useADC {
+	if !hasAdminEmail && !hasSAJSON && !useADC {
 		return msgs
 	}
 
-	if !hasGoogleGroups {
-		msgs = append(msgs, "missing setting: google-group")
-	}
 	if !hasAdminEmail {
 		msgs = append(msgs, "missing setting: google-admin-email")
 	}

providers/google.go

@@ -103,17 +103,24 @@ func NewGoogleProvider(p *ProviderData, opts options.GoogleOptions) (*GoogleProv
 	}
 
 	if opts.ServiceAccountJSON != "" || opts.UseApplicationDefaultCredentials {
-		// Backwards compatibility with `--google-group` option
-		if len(opts.Groups) > 0 {
-			provider.setAllowedGroups(opts.Groups)
-		}
-
-		provider.setGroupRestriction(opts)
+		provider.configureGroups(opts)
 	}
 
 	return provider, nil
 }
 
+func (p *GoogleProvider) configureGroups(opts options.GoogleOptions) {
+	adminService := getAdminService(opts)
+	// Backwards compatibility with `--google-group` option
+	if len(opts.Groups) > 0 {
+		p.setAllowedGroups(opts.Groups)
+		p.groupValidator = p.setGroupRestriction(opts.Groups, adminService)
+		return
+	}
+
+	p.groupValidator = p.populateAllGroups(adminService)
+}
+
 func claimsFromIDToken(idToken string) (*claims, error) {
 
 	// id_token is a base64 encode ID token payload
@@ -209,18 +216,13 @@ func (p *GoogleProvider) EnrichSession(_ context.Context, s *sessions.SessionSta
 }
 
 // SetGroupRestriction configures the GoogleProvider to restrict access to the
-// specified group(s). AdminEmail has to be an administrative email on the domain that is
-// checked. CredentialsFile is the path to a json file containing a Google service
-// account credentials.
-//
-// TODO (@NickMeves) - Unit Test this OR refactor away from groupValidator func
-func (p *GoogleProvider) setGroupRestriction(opts options.GoogleOptions) {
-	adminService := getAdminService(opts)
-	p.groupValidator = func(s *sessions.SessionState) bool {
+// specified group(s).
+func (p *GoogleProvider) setGroupRestriction(groups []string, adminService *admin.Service) func(*sessions.SessionState) bool {
+	return func(s *sessions.SessionState) bool {
 		// Reset our saved Groups in case membership changed
 		// This is used by `Authorize` on every request
-		s.Groups = make([]string, 0, len(opts.Groups))
-		for _, group := range opts.Groups {
+		s.Groups = make([]string, 0, len(groups))
+		for _, group := range groups {
 			if userInGroup(adminService, group, s.Email) {
 				s.Groups = append(s.Groups, group)
 			}
@@ -229,6 +231,25 @@ func (p *GoogleProvider) setGroupRestriction(opts options.GoogleOptions) {
 	}
 }
 
+// populateAllGroups configures the GoogleProvider to allow access with all
+// groups and populate session with all groups of the user when no specific
+// groups are configured.
+func (p *GoogleProvider) populateAllGroups(adminService *admin.Service) func(s *sessions.SessionState) bool {
+	return func(s *sessions.SessionState) bool {
+		// Get all groups of the user
+		groups, err := getUserGroups(adminService, s.Email)
+		if err != nil {
+			logger.Errorf("Failed to get user groups for %s: %v", s.Email, err)
+			s.Groups = []string{}
+			return true // Allow access even if we can't get groups
+		}
+
+		// Populate session with all user groups
+		s.Groups = groups
+		return true // Always allow access when no specific groups are configured
+	}
+}
+
 // https://developers.google.com/admin-sdk/directory/reference/rest/v1/members/hasMember#authorization-scopes
 var possibleScopesList = [...]string{
 	admin.AdminDirectoryGroupMemberReadonlyScope,
@@ -269,6 +290,10 @@ func getOauth2TokenSource(ctx context.Context, opts options.GoogleOptions, scope
 	return conf.TokenSource(ctx)
 }
 
+// getAdminService retrieves an oauth token for the admin api of Google
+// AdminEmail has to be an administrative email on the domain that is
+// checked. CredentialsFile is the path to a json file containing a Google service
+// account credentials.
 func getAdminService(opts options.GoogleOptions) *admin.Service {
 	ctx := context.Background()
 	var client *http.Client
@@ -339,6 +364,38 @@ func getTargetPrincipal(ctx context.Context, opts options.GoogleOptions) (target
 	return targetPrincipal
 }
 
+// getUserGroups retrieves all groups that a user is a member of using the Google Admin Directory API
+func getUserGroups(service *admin.Service, email string) ([]string, error) {
+	var allGroups []string
+	var pageToken string
+
+	for {
+		req := service.Groups.List().UserKey(email).MaxResults(200)
+		if pageToken != "" {
+			req = req.PageToken(pageToken)
+		}
+
+		groupsResp, err := req.Do()
+		if err != nil {
+			return nil, fmt.Errorf("failed to list groups for user %s: %v", email, err)
+		}
+
+		for _, group := range groupsResp.Groups {
+			if group.Email != "" {
+				allGroups = append(allGroups, group.Email)
+			}
+		}
+
+		// Check if there are more pages
+		if groupsResp.NextPageToken == "" {
+			break
+		}
+		pageToken = groupsResp.NextPageToken
+	}
+
+	return allGroups, nil
+}
+
 func userInGroup(service *admin.Service, group string, email string) bool {
 	// Use the HasMember API to checking for the user's presence in each group or nested subgroups
 	req := service.Members.HasMember(group, email)

providers/google_test.go

@@ -289,3 +289,39 @@ func TestGoogleProvider_userInGroup(t *testing.T) {
 	result = userInGroup(service, "group@example.com", "non-member-out-of-domain@otherexample.com")
 	assert.False(t, result)
 }
+
+func TestGoogleProvider_getUserGroups(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/admin/directory/v1/groups" && r.URL.Query().Get("userKey") == "test@example.com" {
+			response := `{
+				"kind": "admin#directory#groups",
+				"groups": [
+					{
+						"kind": "admin#directory#group",
+						"id": "1",
+						"email": "group1@example.com",
+						"name": "Group 1"
+					},
+					{
+						"kind": "admin#directory#group", 
+						"id": "2",
+						"email": "group2@example.com",
+						"name": "Group 2"
+					}
+				]
+			}`
+			fmt.Fprintln(w, response)
+		} else {
+			http.NotFound(w, r)
+		}
+	}))
+	defer ts.Close()
+
+	client := &http.Client{}
+	adminService, err := admin.NewService(context.Background(), option.WithHTTPClient(client), option.WithEndpoint(ts.URL))
+	assert.NoError(t, err)
+
+	groups, err := getUserGroups(adminService, "test@example.com")
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"group1@example.com", "group2@example.com"}, groups)
+}