Corrected logic in group verification

connector/openshift/openshift.go

@@ -165,10 +165,12 @@ func (c *openshiftConnector) HandleCallback(s connector.Scopes, r *http.Request)
 		return identity, fmt.Errorf("openshift: get user: %v", err)
 	}
 
-	validGroups := validateRequiredGroups(user.Groups, c.groups)
+	if len(c.groups) > 0 {
+		validGroups := validateAllowedGroups(user.Groups, c.groups)
 
-	if !validGroups {
-		return identity, fmt.Errorf("openshift: user %q is not in any of the required groups", user.Name)
+		if !validGroups {
+			return identity, fmt.Errorf("openshift: user %q is not in any of the required groups", user.Name)
+		}
 	}
 
 	identity = connector.Identity{
@@ -211,10 +213,10 @@ func (c *openshiftConnector) user(ctx context.Context, client *http.Client) (u u
 	return u, err
 }
 
-func validateRequiredGroups(userGroups, requiredGroups []string) bool {
-	matchingGroups := groups.Filter(userGroups, requiredGroups)
+func validateAllowedGroups(userGroups, allowedGroups []string) bool {
+	matchingGroups := groups.Filter(userGroups, allowedGroups)
 
-	return len(requiredGroups) == len(matchingGroups)
+	return len(matchingGroups) != 0
 }
 
 // newHTTPClient returns a new HTTP client

connector/openshift/openshift_test.go

@@ -83,11 +83,29 @@ func TestGetUser(t *testing.T) {
 	expectEquals(t, len(u.Groups), 1)
 }
 
-func TestVerifyGroupFn(t *testing.T) {
-	requiredGroups := []string{"users"}
+func TestVerifySingleGroupFn(t *testing.T) {
+	allowedGroups := []string{"users"}
 	groupMembership := []string{"users", "org1"}
 
-	validGroupMembership := validateRequiredGroups(groupMembership, requiredGroups)
+	validGroupMembership := validateAllowedGroups(groupMembership, allowedGroups)
+
+	expectEquals(t, validGroupMembership, true)
+}
+
+func TestVerifySingleGroupFailureFn(t *testing.T) {
+	allowedGroups := []string{"admins"}
+	groupMembership := []string{"users"}
+
+	validGroupMembership := validateAllowedGroups(groupMembership, allowedGroups)
+
+	expectEquals(t, validGroupMembership, false)
+}
+
+func TestVerifyMultipleGroupFn(t *testing.T) {
+	allowedGroups := []string{"users", "admins"}
+	groupMembership := []string{"users", "org1"}
+
+	validGroupMembership := validateAllowedGroups(groupMembership, allowedGroups)
 
 	expectEquals(t, validGroupMembership, true)
 }