Repository: oauth2-proxy/oauth2-proxy
Pull request #953 (2020-12-12 21:28:02)

Refactor Keycloak Provider Methods
Move Keycloak to use standard EnrichSession & Authorize methods

## Description

Migrate Keycloak to use universal AuthZ logic and migrate to EnrichSession & Authorize functionality.

Existing singular group AuthZ piggybacks on `SetAllowedGroups` and `session.Groups` functionality to get Keycloak groups included in upstream headers. Multiple groups now supported

Support a ProfileURL for the `EnrichSession` call to the userinfo endpoint. Previously this was done with the `ValidateURL` which is not correct. `ValidateURL` still supported when `ProfileURL` is nil for backwards compatibility.

Migrate existing unit tests to Ginkgo

## Motivation and Context

Streamline Keycloak provider with our global provider conventions.

## How Has This Been Tested?

Unit tests.  Seeking community Keycloak help for manual integration testing

## Checklist:

- [x] My change requires a change to the documentation or CHANGELOG.
- [x] I have updated the documentation/CHANGELOG accordingly.
- [x] I have created a feature (non-master) branch for my PR.


NickMeves: @khawaga This should help standardize things for your PR #767 to build off of.

Do you mind making sure this works in a realistic Keycloak environment?
khawaga: @NickMeves sure thing, let me test this out and get back to you!
NickMeves: Related issue with `keycloak` provider in general to have on our minds: https://github.com/oauth2-proxy/oauth2-proxy/issues/956
khawaga: Not sure what's causing this is but I've noticed that my config file is no longer being read, and passing config values via command-line args is also problematic as it looks like some of the URLs are not being parsed correctly resulting in an empty string and getting errors such as `[2020/12/22 21:44:55] [keycloak.go:78] failed making request error performing request: Get "": unsupported protocol scheme ""`
NickMeves: > Not sure what's causing this is but I've noticed that my config file is no longer being read, and passing config values via command-line args is also problematic as it looks like some of the URLs are not being parsed correctly resulting in an empty string and getting errors such as `[2020/12/22 21:44:55] [keycloak.go:78] failed making request error performing request: Get "": unsupported protocol scheme ""`

Hmmm - `keycloak_group` to `keycloak_groups` would be a breaking change in this PR, was that it?

This also streamlined how the `profileURL` worked and uses that as the userinfo endpoint if set and not the `validateURL`?

Potentially either of those changes? I don't think I touched how you'd input your global provider URLs 🤔 
khawaga: > > Not sure what's causing this is but I've noticed that my config file is no longer being read, and passing config values via command-line args is also problematic as it looks like some of the URLs are not being parsed correctly resulting in an empty string and getting errors such as `[2020/12/22 21:44:55] [keycloak.go:78] failed making request error performing request: Get "": unsupported protocol scheme ""`
> 
> Hmmm - `keycloak_group` to `keycloak_groups` would be a breaking change in this PR, was that it?
> 
> This also streamlined how the `profileURL` worked and uses that as the userinfo endpoint if set and not the `validateURL`?
> 
> Potentially either of those changes? I don't think I touched how you'd input your global provider URLs 🤔

I used the `keycloak-group` command-line flag, since the config file wasn't being read, so that shouldn't be the issue. I find this kind of strange as well. I ran the same setup using the v6.1.1 image and it worked fine. I'll look into this again in the next couple of days, maybe I'm missing something.
khawaga: I'm also getting this error in addition to the one I mentioned before.
`[2020/12/23 01:12:15] [oauthproxy.go:878] Error creating session during OAuth2 callback: error performing request: Get "": unsupported protocol scheme ""`
khawaga: Okay, I can confirm that `--profile-url` works but `--validate-url` does not and results in the above errors. Still unsure why the config file does not get read.
khawaga: Keeping in mind what I said above, and after switching to `--profile-url`, using either `--keycloak-group` and `--allowed-group` works as intended, I believe `--allowed-group` should to be added to the docs as well.
NickMeves: @khawaga Thanks for the help! It looks like I need to dig into this more: https://github.com/oauth2-proxy/oauth2-proxy/pull/953/files#diff-859d0170ad5425660ad9ef59fdf81e34ff86592bade92ea3976c3dd651491765R68

Its seeming like the default case is not `nil` 🤔 
NickMeves: > @khawaga Thanks for the help! It looks like I need to dig into this more: https://github.com/oauth2-proxy/oauth2-proxy/pull/953/files#diff-859d0170ad5425660ad9ef59fdf81e34ff86592bade92ea3976c3dd651491765R68
> 
> Its seeming like the default case is not `nil` 🤔

Ah - this is probabaly the culprit: https://github.com/oauth2-proxy/oauth2-proxy/blob/22f60e9b63e54e8a0fc6b625280f05159b5560a5/providers/provider_data.go#L95
NickMeves: @khawaga I have the fix in the latest commit -- do you mind giving it another quick test with your setup?
NickMeves: > LGTM

Thanks @JoelSpeed! I'll wait until @khawaga has a chance to test this fix to confirm it works in their live Keycloak installations before merging.
khawaga: @NickMeves I can confirm that `--validate-url` now works.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index edb631d6..6af45d6a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 ## Important Notes
 
+- [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Keycloak will now use `--profile-url` if set for the userinfo endpoint
+  instead of `--validate-url`. `--validate-url` will still work for backwards compatibility.
 - [#936](https://github.com/oauth2-proxy/oauth2-proxy/pull/936) `--user-id-claim` option is deprecated and replaced by `--oidc-email-claim`
 - [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Gitlab projects needs a Gitlab application with the extra `read_api` enabled
 - [#849](https://github.com/oauth2-proxy/oauth2-proxy/pull/849) `/oauth2/auth` `allowed_groups` querystring parameter can be paired with the `allowed-groups` configuration option.
@@ -33,6 +35,8 @@
 
 ## Breaking Changes
 
+- [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) In config files & envvar configs, `keycloak_group` is now the plural `keycloak_groups`.
+  Flag configs are still `--keycloak-group` but it can be passed multiple times.
 - [#911](https://github.com/oauth2-proxy/oauth2-proxy/pull/911) Specifying a non-existent provider will cause OAuth2-Proxy to fail on startup instead of defaulting to "google".
 - [#797](https://github.com/oauth2-proxy/oauth2-proxy/pull/797) Security changes to Google provider group authorization flow
   - If you change the list of allowed groups, existing sessions that now don't have a valid group will be logged out immediately.
@@ -54,6 +58,7 @@
 
 ## Changes since v6.1.1
 
+- [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Migrate Keycloak to EnrichSession & support multiple groups for authorization (@NickMeves)
 - [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Add support for Gitlab project based authentication (@factorysh)
 - [#907](https://github.com/oauth2-proxy/oauth2-proxy/pull/907) Introduce alpha configuration option to enable testing of structured configuration (@JoelSpeed)
 - [#938](https://github.com/oauth2-proxy/oauth2-proxy/pull/938) Cleanup missed provider renaming refactor methods (@NickMeves)
diff --git a/docs/docs/configuration/auth.md b/docs/docs/configuration/auth.md
index 7c5bac39..f16f2e26 100644
--- a/docs/docs/configuration/auth.md
+++ b/docs/docs/configuration/auth.md
@@ -135,15 +135,25 @@ If you are using GitHub enterprise, make sure you set the following to the appro
 
 Make sure you set the following to the appropriate url:
 
-    -provider=keycloak
-    -client-id=<client you have created>
-    -client-secret=<your client's secret>
-    -login-url="http(s)://<keycloak host>/auth/realms/<your realm>/protocol/openid-connect/auth"
-    -redeem-url="http(s)://<keycloak host>/auth/realms/<your realm>/protocol/openid-connect/token"
-    -validate-url="http(s)://<keycloak host>/auth/realms/<your realm>/protocol/openid-connect/userinfo"
-    -keycloak-group=<user_group>
-
-The group management in keycloak is using a tree. If you create a group named admin in keycloak you should define the 'keycloak-group' value to /admin.
+    --provider=keycloak
+    --client-id=<client you have created>
+    --client-secret=<your client's secret>
+    --login-url="http(s)://<keycloak host>/auth/realms/<your realm>/protocol/openid-connect/auth"
+    --redeem-url="http(s)://<keycloak host>/auth/realms/<your realm>/protocol/openid-connect/token"
+    --profile-url="http(s)://<keycloak host>/auth/realms/<your realm>/protocol/openid-connect/userinfo"
+    --validate-url="http(s)://<keycloak host>/auth/realms/<your realm>/protocol/openid-connect/userinfo"
+    --keycloak-group=<first_allowed_user_group>
+    --keycloak-group=<second_allowed_user_group>
+    
+For group based authorization, the optional `--keycloak-group` (legacy) or `--allowed-group` (global standard)
+flags can be used to specify which groups to limit access to.
+
+If these are unset but a `groups` mapper is set up above in step (3), the provider will still
+populate the `X-Forwarded-Groups` header to your upstream server with the `groups` data in the
+Keycloak userinfo endpoint response.
+
+The group management in keycloak is using a tree. If you create a group named admin in keycloak
+you should define the 'keycloak-group' value to /admin.
 
 ### GitLab Auth Provider
 
diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go
index 46cdcedb..c0f91422 100644
--- a/pkg/apis/options/options.go
+++ b/pkg/apis/options/options.go
@@ -36,7 +36,7 @@ type Options struct {
 	TLSKeyFile         string   `flag:"tls-key-file" cfg:"tls_key_file"`
 
 	AuthenticatedEmailsFile  string   `flag:"authenticated-emails-file" cfg:"authenticated_emails_file"`
-	KeycloakGroup            string   `flag:"keycloak-group" cfg:"keycloak_group"`
+	KeycloakGroups           []string `flag:"keycloak-group" cfg:"keycloak_groups"`
 	AzureTenant              string   `flag:"azure-tenant" cfg:"azure_tenant"`
 	BitbucketTeam            string   `flag:"bitbucket-team" cfg:"bitbucket_team"`
 	BitbucketRepository      string   `flag:"bitbucket-repository" cfg:"bitbucket_repository"`
@@ -181,7 +181,7 @@ func NewFlagSet() *pflag.FlagSet {
 
 	flagSet.StringSlice("email-domain", []string{}, "authenticate emails with the specified domain (may be given multiple times). Use * to authenticate any email")
 	flagSet.StringSlice("whitelist-domain", []string{}, "allowed domains for redirection after authentication. Prefix domain with a . to allow subdomains (eg .example.com)")
-	flagSet.String("keycloak-group", "", "restrict login to members of this group.")
+	flagSet.StringSlice("keycloak-group", []string{}, "restrict logins to members of these groups (may be given multiple times)")
 	flagSet.String("azure-tenant", "common", "go to a tenant-specific or common (tenant-independent) endpoint.")
 	flagSet.String("bitbucket-team", "", "restrict logins to members of this team")
 	flagSet.String("bitbucket-repository", "", "restrict logins to user with access to this repository")
diff --git a/pkg/validation/options.go b/pkg/validation/options.go
index 4fc0b0a4..52b0fb69 100644
--- a/pkg/validation/options.go
+++ b/pkg/validation/options.go
@@ -263,7 +263,10 @@ func parseProviderInfo(o *options.Options, msgs []string) []string {
 		p.SetRepo(o.GitHubRepo, o.GitHubToken)
 		p.SetUsers(o.GitHubUsers)
 	case *providers.KeycloakProvider:
-		p.SetGroup(o.KeycloakGroup)
+		// Backwards compatibility with `--keycloak-group` option
+		if len(o.KeycloakGroups) > 0 {
+			p.SetAllowedGroups(o.KeycloakGroups)
+		}
 	case *providers.GoogleProvider:
 		if o.GoogleServiceAccountJSON != "" {
 			file, err := os.Open(o.GoogleServiceAccountJSON)
diff --git a/providers/keycloak.go b/providers/keycloak.go
index 60b3eaca..a1e4f064 100644
--- a/providers/keycloak.go
+++ b/providers/keycloak.go
@@ -2,6 +2,7 @@ package providers
 
 import (
 	"context"
+	"fmt"
 	"net/url"
 
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
@@ -11,7 +12,6 @@ import (
 
 type KeycloakProvider struct {
 	*ProviderData
-	Group string
 }
 
 var _ Provider = (*KeycloakProvider)(nil)
@@ -47,6 +47,7 @@ var (
 	}
 )
 
+// NewKeycloakProvider creates a KeyCloakProvider using the passed ProviderData
 func NewKeycloakProvider(p *ProviderData) *KeycloakProvider {
 	p.setProviderDefaults(providerDefaults{
 		name:        keycloakProviderName,
@@ -59,41 +60,39 @@ func NewKeycloakProvider(p *ProviderData) *KeycloakProvider {
 	return &KeycloakProvider{ProviderData: p}
 }
 
-func (p *KeycloakProvider) SetGroup(group string) {
-	p.Group = group
-}
+// EnrichSession uses the Keycloak userinfo endpoint to populate the session's
+// email and groups.
+func (p *KeycloakProvider) EnrichSession(ctx context.Context, s *sessions.SessionState) error {
+	// Fallback to ValidateURL if ProfileURL not set for legacy compatibility
+	profileURL := p.ValidateURL.String()
+	if p.ProfileURL.String() != "" {
+		profileURL = p.ProfileURL.String()
+	}
 
-func (p *KeycloakProvider) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) {
-	json, err := requests.New(p.ValidateURL.String()).
+	json, err := requests.New(profileURL).
 		WithContext(ctx).
 		SetHeader("Authorization", "Bearer "+s.AccessToken).
 		Do().
 		UnmarshalJSON()
 	if err != nil {
-		logger.Errorf("failed making request %s", err)
-		return "", err
+		logger.Errorf("failed making request %v", err)
+		return err
 	}
 
-	if p.Group != "" {
-		var groups, err = json.Get("groups").Array()
-		if err != nil {
-			logger.Printf("groups not found %s", err)
-			return "", err
-		}
-
-		var found = false
-		for i := range groups {
-			if groups[i].(string) == p.Group {
-				found = true
-				break
+	groups, err := json.Get("groups").StringArray()
+	if err == nil {
+		for _, group := range groups {
+			if group != "" {
+				s.Groups = append(s.Groups, group)
 			}
 		}
+	}
 
-		if !found {
-			logger.Printf("group not found, access denied")
-			return "", nil
-		}
+	email, err := json.Get("email").String()
+	if err != nil {
+		return fmt.Errorf("unable to extract email from userinfo endpoint: %v", err)
 	}
+	s.Email = email
 
-	return json.Get("email").String()
+	return nil
 }


Test patch:
diff --git a/providers/keycloak_test.go b/providers/keycloak_test.go
index 3f419f2e..513be643 100644
--- a/providers/keycloak_test.go
+++ b/providers/keycloak_test.go
@@ -2,17 +2,24 @@ package providers
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"testing"
 
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
 	. "github.com/onsi/gomega"
-	"github.com/stretchr/testify/assert"
 )
 
-func testKeycloakProvider(hostname, group string) *KeycloakProvider {
+const (
+	keycloakAccessToken  = "eyJKeycloak.eyJAccess.Token"
+	keycloakUserinfoPath = "/api/v3/user"
+)
+
+func testKeycloakProvider(backend *httptest.Server) (*KeycloakProvider, error) {
 	p := NewKeycloakProvider(
 		&ProviderData{
 			ProviderName: "",
@@ -22,142 +29,182 @@ func testKeycloakProvider(hostname, group string) *KeycloakProvider {
 			ValidateURL:  &url.URL{},
 			Scope:        ""})
 
-	if group != "" {
-		p.SetGroup(group)
-	}
+	if backend != nil {
+		bURL, err := url.Parse(backend.URL)
+		if err != nil {
+			return nil, err
+		}
+		hostname := bURL.Host
 
-	if hostname != "" {
 		updateURL(p.Data().LoginURL, hostname)
 		updateURL(p.Data().RedeemURL, hostname)
 		updateURL(p.Data().ProfileURL, hostname)
 		updateURL(p.Data().ValidateURL, hostname)
 	}
-	return p
-}
 
-func testKeycloakBackend(payload string) *httptest.Server {
-	path := "/api/v3/user"
-
-	return httptest.NewServer(http.HandlerFunc(
-		func(w http.ResponseWriter, r *http.Request) {
-			url := r.URL
-			if url.Path != path {
-				w.WriteHeader(404)
-			} else if !IsAuthorizedInHeader(r.Header) {
-				w.WriteHeader(403)
-			} else {
-				w.WriteHeader(200)
-				w.Write([]byte(payload))
-			}
-		}))
+	return p, nil
 }
 
-func TestKeycloakProviderDefaults(t *testing.T) {
-	p := testKeycloakProvider("", "")
-	assert.NotEqual(t, nil, p)
-	assert.Equal(t, "Keycloak", p.Data().ProviderName)
-	assert.Equal(t, "https://keycloak.org/oauth/authorize",
-		p.Data().LoginURL.String())
-	assert.Equal(t, "https://keycloak.org/oauth/token",
-		p.Data().RedeemURL.String())
-	assert.Equal(t, "https://keycloak.org/api/v3/user",
-		p.Data().ValidateURL.String())
-	assert.Equal(t, "api", p.Data().Scope)
-}
-
-func TestNewKeycloakProvider(t *testing.T) {
-	g := NewWithT(t)
-
-	// Test that defaults are set when calling for a new provider with nothing set
-	providerData := NewKeycloakProvider(&ProviderData{}).Data()
-	g.Expect(providerData.ProviderName).To(Equal("Keycloak"))
-	g.Expect(providerData.LoginURL.String()).To(Equal("https://keycloak.org/oauth/authorize"))
-	g.Expect(providerData.RedeemURL.String()).To(Equal("https://keycloak.org/oauth/token"))
-	g.Expect(providerData.ProfileURL.String()).To(Equal(""))
-	g.Expect(providerData.ValidateURL.String()).To(Equal("https://keycloak.org/api/v3/user"))
-	g.Expect(providerData.Scope).To(Equal("api"))
-}
-
-func TestKeycloakProviderOverrides(t *testing.T) {
-	p := NewKeycloakProvider(
-		&ProviderData{
-			LoginURL: &url.URL{
-				Scheme: "https",
-				Host:   "example.com",
-				Path:   "/oauth/auth"},
-			RedeemURL: &url.URL{
-				Scheme: "https",
-				Host:   "example.com",
-				Path:   "/oauth/token"},
-			ValidateURL: &url.URL{
-				Scheme: "https",
-				Host:   "example.com",
-				Path:   "/api/v3/user"},
-			Scope: "profile"})
-	assert.NotEqual(t, nil, p)
-	assert.Equal(t, "Keycloak", p.Data().ProviderName)
-	assert.Equal(t, "https://example.com/oauth/auth",
-		p.Data().LoginURL.String())
-	assert.Equal(t, "https://example.com/oauth/token",
-		p.Data().RedeemURL.String())
-	assert.Equal(t, "https://example.com/api/v3/user",
-		p.Data().ValidateURL.String())
-	assert.Equal(t, "profile", p.Data().Scope)
-}
-
-func TestKeycloakProviderGetEmailAddress(t *testing.T) {
-	b := testKeycloakBackend("{\"email\": \"michael.bland@gsa.gov\"}")
-	defer b.Close()
-
-	bURL, _ := url.Parse(b.URL)
-	p := testKeycloakProvider(bURL.Host, "")
-
-	session := CreateAuthorizedSession()
-	email, err := p.GetEmailAddress(context.Background(), session)
-	assert.Equal(t, nil, err)
-	assert.Equal(t, "michael.bland@gsa.gov", email)
-}
-
-func TestKeycloakProviderGetEmailAddressAndGroup(t *testing.T) {
-	b := testKeycloakBackend("{\"email\": \"michael.bland@gsa.gov\", \"groups\": [\"test-grp1\", \"test-grp2\"]}")
-	defer b.Close()
-
-	bURL, _ := url.Parse(b.URL)
-	p := testKeycloakProvider(bURL.Host, "test-grp1")
-
-	session := CreateAuthorizedSession()
-	email, err := p.GetEmailAddress(context.Background(), session)
-	assert.Equal(t, nil, err)
-	assert.Equal(t, "michael.bland@gsa.gov", email)
-}
-
-// Note that trying to trigger the "failed building request" case is not
-// practical, since the only way it can fail is if the URL fails to parse.
-func TestKeycloakProviderGetEmailAddressFailedRequest(t *testing.T) {
-	b := testKeycloakBackend("unused payload")
-	defer b.Close()
-
-	bURL, _ := url.Parse(b.URL)
-	p := testKeycloakProvider(bURL.Host, "")
-
-	// We'll trigger a request failure by using an unexpected access
-	// token. Alternatively, we could allow the parsing of the payload as
-	// JSON to fail.
-	session := &sessions.SessionState{AccessToken: "unexpected_access_token"}
-	email, err := p.GetEmailAddress(context.Background(), session)
-	assert.NotEqual(t, nil, err)
-	assert.Equal(t, "", email)
-}
-
-func TestKeycloakProviderGetEmailAddressEmailNotPresentInPayload(t *testing.T) {
-	b := testKeycloakBackend("{\"foo\": \"bar\"}")
-	defer b.Close()
-
-	bURL, _ := url.Parse(b.URL)
-	p := testKeycloakProvider(bURL.Host, "")
-
-	session := CreateAuthorizedSession()
-	email, err := p.GetEmailAddress(context.Background(), session)
-	assert.NotEqual(t, nil, err)
-	assert.Equal(t, "", email)
-}
+var _ = Describe("Keycloak Provider Tests", func() {
+	Context("New Provider Init", func() {
+		It("uses defaults", func() {
+			providerData := NewKeycloakProvider(&ProviderData{}).Data()
+			Expect(providerData.ProviderName).To(Equal("Keycloak"))
+			Expect(providerData.LoginURL.String()).To(Equal("https://keycloak.org/oauth/authorize"))
+			Expect(providerData.RedeemURL.String()).To(Equal("https://keycloak.org/oauth/token"))
+			Expect(providerData.ProfileURL.String()).To(Equal(""))
+			Expect(providerData.ValidateURL.String()).To(Equal("https://keycloak.org/api/v3/user"))
+			Expect(providerData.Scope).To(Equal("api"))
+		})
+
+		It("overrides defaults", func() {
+			p := NewKeycloakProvider(
+				&ProviderData{
+					LoginURL: &url.URL{
+						Scheme: "https",
+						Host:   "example.com",
+						Path:   "/oauth/auth"},
+					RedeemURL: &url.URL{
+						Scheme: "https",
+						Host:   "example.com",
+						Path:   "/oauth/token"},
+					ProfileURL: &url.URL{
+						Scheme: "https",
+						Host:   "example.com",
+						Path:   "/api/v3/user"},
+					ValidateURL: &url.URL{
+						Scheme: "https",
+						Host:   "example.com",
+						Path:   "/api/v3/user"},
+					Scope: "profile"})
+			providerData := p.Data()
+
+			Expect(providerData.ProviderName).To(Equal("Keycloak"))
+			Expect(providerData.LoginURL.String()).To(Equal("https://example.com/oauth/auth"))
+			Expect(providerData.RedeemURL.String()).To(Equal("https://example.com/oauth/token"))
+			Expect(providerData.ProfileURL.String()).To(Equal("https://example.com/api/v3/user"))
+			Expect(providerData.ValidateURL.String()).To(Equal("https://example.com/api/v3/user"))
+			Expect(providerData.Scope).To(Equal("profile"))
+		})
+	})
+
+	Context("EnrichSession", func() {
+		type enrichSessionTableInput struct {
+			backendHandler http.HandlerFunc
+			expectedError  error
+			expectedEmail  string
+			expectedGroups []string
+		}
+
+		DescribeTable("should return expected results",
+			func(in enrichSessionTableInput) {
+				backend := httptest.NewServer(in.backendHandler)
+				p, err := testKeycloakProvider(backend)
+				Expect(err).To(BeNil())
+
+				p.ProfileURL, err = url.Parse(
+					fmt.Sprintf("%s%s", backend.URL, keycloakUserinfoPath),
+				)
+				Expect(err).To(BeNil())
+
+				session := &sessions.SessionState{AccessToken: keycloakAccessToken}
+				err = p.EnrichSession(context.Background(), session)
+
+				if in.expectedError != nil {
+					Expect(err).To(Equal(in.expectedError))
+				} else {
+					Expect(err).To(BeNil())
+				}
+
+				Expect(session.Email).To(Equal(in.expectedEmail))
+
+				if in.expectedGroups != nil {
+					Expect(session.Groups).To(Equal(in.expectedGroups))
+				} else {
+					Expect(session.Groups).To(BeNil())
+				}
+			},
+			Entry("email and multiple groups", enrichSessionTableInput{
+				backendHandler: func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(200)
+					_, err := w.Write([]byte(`
+						{
+							"email": "michael.bland@gsa.gov",
+							"groups": [
+								"test-grp1",
+								"test-grp2"
+							]
+						}
+					`))
+					if err != nil {
+						panic(err)
+					}
+				},
+				expectedError:  nil,
+				expectedEmail:  "michael.bland@gsa.gov",
+				expectedGroups: []string{"test-grp1", "test-grp2"},
+			}),
+			Entry("email and single group", enrichSessionTableInput{
+				backendHandler: func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(200)
+					_, err := w.Write([]byte(`
+						{
+							"email": "michael.bland@gsa.gov",
+							"groups": ["test-grp1"]
+						}
+					`))
+					if err != nil {
+						panic(err)
+					}
+				},
+				expectedError:  nil,
+				expectedEmail:  "michael.bland@gsa.gov",
+				expectedGroups: []string{"test-grp1"},
+			}),
+			Entry("email and no groups", enrichSessionTableInput{
+				backendHandler: func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(200)
+					_, err := w.Write([]byte(`
+						{
+							"email": "michael.bland@gsa.gov"
+						}
+					`))
+					if err != nil {
+						panic(err)
+					}
+				},
+				expectedError:  nil,
+				expectedEmail:  "michael.bland@gsa.gov",
+				expectedGroups: nil,
+			}),
+			Entry("missing email", enrichSessionTableInput{
+				backendHandler: func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(200)
+					_, err := w.Write([]byte(`
+						{
+							"groups": [
+								"test-grp1",
+								"test-grp2"
+							]
+						}
+					`))
+					if err != nil {
+						panic(err)
+					}
+				},
+				expectedError: errors.New(
+					"unable to extract email from userinfo endpoint: type assertion to string failed"),
+				expectedEmail:  "",
+				expectedGroups: []string{"test-grp1", "test-grp2"},
+			}),
+			Entry("request failure", enrichSessionTableInput{
+				backendHandler: func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(500)
+				},
+				expectedError:  errors.New(`unexpected status "500": `),
+				expectedEmail:  "",
+				expectedGroups: nil,
+			}),
+		)
+	})
+})