Merge pull request #141 from openai/googleGroupEmail

Check google group membership based on email address

CHANGELOG.md

@@ -57,6 +57,9 @@
 - [#170](https://github.com/pusher/oauth2_proxy/pull/170) Restore binary tarball contents to be compatible with bitlys original tarballs (@zeha)
 - [#185](https://github.com/pusher/oauth2_proxy/pull/185) Fix an unsupported protocol scheme error during token validation when using the Azure provider (@jonas)
 
+- [#141](https://github.com/pusher/oauth2_proxy/pull/141) Check google group membership based on email address (@bchess)
+  - Google Group membership is additionally checked via email address, allowing users outside a GSuite domain to be authorized.
+
 # v3.2.0
 
 ## Release highlights

providers/google.go

@@ -191,11 +191,9 @@ func getAdminService(adminEmail string, credentialsReader io.Reader) *admin.Serv
 func userInGroup(service *admin.Service, groups []string, email string) bool {
 	user, err := fetchUser(service, email)
 	if err != nil {
-		logger.Printf("error fetching user: %v", err)
+		logger.Printf("Warning: unable to fetch user: %v", err)
-		return false
+		user = nil
 	}
-	id := user.Id
-	custID := user.CustomerId
 
 	for _, group := range groups {
 		members, err := fetchGroupMembers(service, group)
@@ -209,13 +207,19 @@ func userInGroup(service *admin.Service, groups []string, email string) bool {
 		}
 
 		for _, member := range members {
+			if member.Email == email {
+				return true
+			}
+			if user == nil {
+				continue
+			}
 			switch member.Type {
 			case "CUSTOMER":
-				if member.Id == custID {
+				if member.Id == user.CustomerId {
 					return true
 				}
 			case "USER":
-				if member.Id == id {
+				if member.Id == user.Id {
 					return true
 				}
 			}

providers/google_test.go

@@ -3,12 +3,15 @@ package providers
 import (
 	"encoding/base64"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	admin "google.golang.org/api/admin/directory/v1"
 )
 
 func newRedeemServer(body []byte) (*url.URL, *httptest.Server) {
@@ -179,3 +182,37 @@ func TestGoogleProviderGetEmailAddressEmailMissing(t *testing.T) {
 	}
 
 }
+
+func TestGoogleProviderUserInGroup(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/users/member-by-email@example.com" {
+			fmt.Fprintln(w, "{}")
+		} else if r.URL.Path == "/users/non-member-by-email@example.com" {
+			fmt.Fprintln(w, "{}")
+		} else if r.URL.Path == "/users/member-by-id@example.com" {
+			fmt.Fprintln(w, "{\"id\": \"member-id\"}")
+		} else if r.URL.Path == "/users/non-member-by-id@example.com" {
+			fmt.Fprintln(w, "{\"id\": \"non-member-id\"}")
+		} else if r.URL.Path == "/groups/group@example.com/members" {
+			fmt.Fprintln(w, "{\"members\": [{\"email\": \"member-by-email@example.com\"}, {\"id\": \"member-id\", \"type\": \"USER\"}]}")
+		}
+	}))
+	defer ts.Close()
+
+	client := ts.Client()
+	service, err := admin.New(client)
+	service.BasePath = ts.URL
+	assert.Equal(t, nil, err)
+
+	result := userInGroup(service, []string{"group@example.com"}, "member-by-email@example.com")
+	assert.True(t, result)
+
+	result = userInGroup(service, []string{"group@example.com"}, "member-by-id@example.com")
+	assert.True(t, result)
+
+	result = userInGroup(service, []string{"group@example.com"}, "non-member-by-id@example.com")
+	assert.False(t, result)
+
+	result = userInGroup(service, []string{"group@example.com"}, "non-member-by-email@example.com")
+	assert.False(t, result)
+}