From 6c1cb9e41586198acd63cebca4678c7c68384c0d Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Mon, 20 Jan 2020 23:27:41 +0800 Subject: [PATCH 1/3] configurable oidc groups key --- connector/oidc/oidc.go | 11 ++++++++++- connector/oidc/oidc_test.go | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index ee6b24a860..9b1c55ab15 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -54,6 +54,9 @@ type Config struct { // Configurable key which contains the user name claim UserNameKey string `json:"userNameKey"` + + // Configurable key which contains the groups claims + GroupsKey string `json:"groupsKey"` // defaults to "groups" } // Domains that don't support basic auth. golang.org/x/oauth2 has an internal @@ -135,6 +138,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e getUserInfo: c.GetUserInfo, userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, + groupsKey: c.GroupsKey, }, nil } @@ -156,6 +160,7 @@ type oidcConnector struct { getUserInfo bool userIDKey string userNameKey string + groupsKey string } func (c *oidcConnector) Close() error { @@ -326,7 +331,11 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } if c.insecureEnableGroups { - vs, ok := claims["groups"].([]interface{}) + if c.groupsKey == "" { + c.groupsKey = "groups" + } + + vs, ok := claims[c.groupsKey].([]interface{}) if ok { for _, v := range vs { if s, ok := v.(string); ok { diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 52afa15804..f393160ff1 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -49,10 +49,13 @@ func TestHandleCallback(t *testing.T) { name string userIDKey string userNameKey string + groupsKey string insecureSkipEmailVerified bool + insecureEnableGroups bool scopes []string expectUserID string expectUserName string + expectGroups []string expectedEmailField string token map[string]interface{} }{ @@ -108,6 +111,22 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "withGroupsKey", + insecureEnableGroups: true, + groupsKey: "groups_key", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectGroups: []string{"group"}, + expectedEmailField: "emailvalue", + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "groups_key": []string{"group"}, + "email": "emailvalue", + "email_verified": true, + }, + }, { name: "emptyEmailScope", expectUserID: "subvalue", @@ -161,7 +180,9 @@ func TestHandleCallback(t *testing.T) { RedirectURI: fmt.Sprintf("%s/callback", serverURL), UserIDKey: tc.userIDKey, UserNameKey: tc.userNameKey, + GroupsKey: tc.groupsKey, InsecureSkipEmailVerified: tc.insecureSkipEmailVerified, + InsecureEnableGroups: tc.insecureEnableGroups, BasicAuthUnsupported: &basicAuth, } @@ -182,6 +203,7 @@ func TestHandleCallback(t *testing.T) { expectEquals(t, identity.UserID, tc.expectUserID) expectEquals(t, identity.Username, tc.expectUserName) + expectEquals(t, identity.Groups, tc.expectGroups) expectEquals(t, identity.Email, tc.expectedEmailField) expectEquals(t, identity.EmailVerified, true) }) From b7d161a8c6ccb2ea2bfb3ab03be00298d6c1e5df Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Tue, 21 Jan 2020 01:31:09 +0800 Subject: [PATCH 2/3] add tests for Open and LoginURL --- connector/oidc/oidc_test.go | 69 +++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index f393160ff1..ad7d16dac2 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -11,7 +11,9 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "reflect" + "sort" "strings" "testing" "time" @@ -42,6 +44,73 @@ func TestKnownBrokenAuthHeaderProvider(t *testing.T) { } } +func TestOpen(t *testing.T) { + testServer, err := setupServer(make(map[string]interface{})) + if err != nil { + t.Fatal("failed to setup test server", err) + } + defer testServer.Close() + + conn, err := newConnector(Config{ + Issuer: testServer.URL, + ClientID: "clientID", + ClientSecret: "clientSecret", + Scopes: []string{"groups"}, + RedirectURI: fmt.Sprintf("%s/callback", testServer.URL), + }) + + if err != nil { + t.Fatal("failed to create new connector", err) + } + + sort.Strings(conn.oauth2Config.Scopes) + + expectEquals(t, conn.oauth2Config.ClientID, "clientID") + expectEquals(t, conn.oauth2Config.ClientSecret, "clientSecret") + expectEquals(t, conn.oauth2Config.RedirectURL, testServer.URL+"/callback") + expectEquals(t, conn.oauth2Config.Endpoint.TokenURL, testServer.URL+"/token") + expectEquals(t, conn.oauth2Config.Endpoint.AuthURL, testServer.URL+"/authorize") + expectEquals(t, len(conn.oauth2Config.Scopes), 2) + expectEquals(t, conn.oauth2Config.Scopes[0], "groups") + expectEquals(t, conn.oauth2Config.Scopes[1], "openid") +} + +func TestLoginURL(t *testing.T) { + testServer, err := setupServer(make(map[string]interface{})) + if err != nil { + t.Fatal("failed to setup test server", err) + } + defer testServer.Close() + + conn, err := newConnector(Config{ + Issuer: testServer.URL, + ClientID: "clientID", + ClientSecret: "clientSecret", + Scopes: []string{"groups"}, + RedirectURI: fmt.Sprintf("%s/callback", testServer.URL), + }) + + if err != nil { + t.Fatal("failed to create new connector", err) + } + + loginURL, err := conn.LoginURL(connector.Scopes{}, conn.redirectURI, "some-state") + expectEquals(t, err, nil) + + expectedURL, err := url.Parse(testServer.URL + "/authorize") + expectEquals(t, err, nil) + + values := url.Values{} + values.Add("client_id", "clientID") + values.Add("redirect_uri", conn.redirectURI) + values.Add("response_type", "code") + values.Add("scope", "openid groups") + values.Add("state", "some-state") + expectedURL.RawQuery = values.Encode() + + expectEquals(t, loginURL, expectedURL.String()) +} + func TestHandleCallback(t *testing.T) { t.Helper() From e06890390965eb2d4ee913bb820f425759e5aca8 Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Tue, 3 Mar 2020 20:58:53 -0500 Subject: [PATCH 3/3] linter error fix --- connector/oidc/oidc.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index d22578c3d3..e98f95dbd2 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -58,7 +58,7 @@ type Config struct { // PromptType will be used fot the prompt parameter (when offline_access, by default prompt=consent) PromptType string `json:"promptType"` - // Configurable key which contains the groups claims + // Configurable key which contains the groups claims GroupsKey string `json:"groupsKey"` // defaults to "groups" } @@ -146,7 +146,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e getUserInfo: c.GetUserInfo, userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, - promptType: c.PromptType, + promptType: c.PromptType, groupsKey: c.GroupsKey, }, nil } @@ -358,4 +358,4 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } return identity, nil -} \ No newline at end of file +}