From 533900cd234dc51fd1c7d380569abf1469a87f91 Mon Sep 17 00:00:00 2001 From: levotrea <38857756+levotrea@users.noreply.github.com> Date: Tue, 16 Jun 2020 16:19:58 +0200 Subject: [PATCH] Refactoring VerifyToken and its underlying functions --- Gopkg.lock | 6 +++--- Gopkg.toml | 2 +- api/keycloak_client.go | 11 +++++------ integration/integration_test.go | 7 ++----- toolbox/issuer.go | 28 +++++++++++----------------- toolbox/issuer_test.go | 13 ++++++------- 6 files changed, 28 insertions(+), 39 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index ed7b2c5..233de3f 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -2,12 +2,12 @@ [[projects]] - digest = "1:fcbbb02d897902b3460476fcc6723f21a1674c0b6bd4a5579de11d53983d3d70" + digest = "1:2c7abdf1364ef3db046635ca5bb2cc26838ccb45a2b6608849e7b7fc6493423d" name = "github.com/cloudtrust/common-service" packages = ["errors"] pruneopts = "" - revision = "0dac96e146315562f548272f90e3f0ccf9ea7ddb" - version = "v2.2.0" + revision = "c9a387c12b76cd979ddf8be40168f3e19f643184" + version = "v2.2.1" [[projects]] digest = "1:bb7f91ab4d1c44a3bb2651c613463c134165bda0282fca891a63b88d1b501997" diff --git a/Gopkg.toml b/Gopkg.toml index e03946d..715d04f 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -22,7 +22,7 @@ [[constraint]] name = "github.com/cloudtrust/common-service" - version = "v2.2.0" + version = "2.2.1" [[constraint]] name = "github.com/pkg/errors" diff --git a/api/keycloak_client.go b/api/keycloak_client.go index 02d8d37..ec702bc 100644 --- a/api/keycloak_client.go +++ b/api/keycloak_client.go @@ -1,7 +1,6 @@ package api import ( - "context" "encoding/json" "regexp" @@ -35,11 +34,11 @@ type AccountClient struct { } // New returns a keycloak client. -func New(config keycloak.Config, keyContextIssuerDomain interface{}) (*Client, error) { +func New(config keycloak.Config) (*Client, error) { var issuerMgr toolbox.IssuerManager { var err error - issuerMgr, err = toolbox.NewIssuerManager(config, keyContextIssuerDomain) + issuerMgr, err = toolbox.NewIssuerManager(config) if err != nil { return nil, errors.Wrap(err, keycloak.MsgErrCannotParse+"."+keycloak.TokenProviderURL) } @@ -120,13 +119,13 @@ func (c *Client) GetToken(realm string, username string, password string) (strin } // VerifyToken verifies a token. It returns an error it is malformed, expired,... -func (c *Client) VerifyToken(ctx context.Context, realmName string, accessToken string) error { - issuer, err := c.issuerManager.GetIssuer(ctx) +func (c *Client) VerifyToken(issuer string, realmName string, accessToken string) error { + oidcVerifierProvider, err := c.issuerManager.GetOidcVerifierProvider(issuer) if err != nil { return err } - verifier, err := issuer.GetOidcVerifier(realmName) + verifier, err := oidcVerifierProvider.GetOidcVerifier(realmName) if err != nil { return err } diff --git a/integration/integration_test.go b/integration/integration_test.go index 26616a9..f537c35 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -1,7 +1,6 @@ package main import ( - "context" "fmt" "log" "strings" @@ -18,13 +17,11 @@ const ( tstRealm = "__internal" reqRealm = "master" user = "version" - - keyContextIssuerDomain keyContext = iota ) func main() { var conf = getKeycloakConfig() - var client, err = api.New(*conf, keyContextIssuerDomain) + var client, err = api.New(*conf) if err != nil { log.Fatalf("could not create keycloak client: %v", err) } @@ -35,7 +32,7 @@ func main() { log.Fatalf("could not get access token: %v", err) } - err = client.VerifyToken(context.Background(), "master", accessToken) + err = client.VerifyToken("issuer", "master", accessToken) if err != nil { log.Fatalf("could not validate access token: %v", err) } diff --git a/toolbox/issuer.go b/toolbox/issuer.go index 339f26e..0d04cf3 100644 --- a/toolbox/issuer.go +++ b/toolbox/issuer.go @@ -1,7 +1,6 @@ package toolbox import ( - "context" "errors" "net/url" "regexp" @@ -13,12 +12,11 @@ import ( // IssuerManager provides URL according to a given context type IssuerManager interface { - GetIssuer(ctx context.Context) (OidcVerifierProvider, error) + GetOidcVerifierProvider(issuer string) (OidcVerifierProvider, error) } type issuerManager struct { - domainToIssuer map[string]OidcVerifierProvider - keyContextIssuerDomain interface{} + domainToVerifier map[string]OidcVerifierProvider } func getProtocolAndDomain(URL string) string { @@ -32,7 +30,7 @@ func getProtocolAndDomain(URL string) string { } // NewIssuerManager creates a new URLProvider -func NewIssuerManager(config keycloak.Config, keyContextIssuerDomain interface{}) (IssuerManager, error) { +func NewIssuerManager(config keycloak.Config) (IssuerManager, error) { URLs := config.AddrTokenProvider // Use default values when clients are not initializing these values cacheTTL := config.CacheTTL @@ -44,29 +42,25 @@ func NewIssuerManager(config keycloak.Config, keyContextIssuerDomain interface{} errTolerance = time.Minute } - var domainToIssuer = make(map[string]OidcVerifierProvider) + var domainToVerifier = make(map[string]OidcVerifierProvider) for _, value := range strings.Split(URLs, " ") { uToken, err := url.Parse(value) if err != nil { return nil, err } - issuer := NewVerifierCache(uToken, cacheTTL, errTolerance) - domainToIssuer[getProtocolAndDomain(value)] = issuer + verifier := NewVerifierCache(uToken, cacheTTL, errTolerance) + domainToVerifier[getProtocolAndDomain(value)] = verifier } return &issuerManager{ - domainToIssuer: domainToIssuer, - keyContextIssuerDomain: keyContextIssuerDomain, + domainToVerifier: domainToVerifier, }, nil } -func (im *issuerManager) GetIssuer(ctx context.Context) (OidcVerifierProvider, error) { - if rawValue := ctx.Value(im.keyContextIssuerDomain); rawValue != nil { - // The issuer domain has been found in the context - issuerDomain := getProtocolAndDomain(rawValue.(string)) - if issuer, ok := im.domainToIssuer[issuerDomain]; ok { - return issuer, nil - } +func (im *issuerManager) GetOidcVerifierProvider(issuer string) (OidcVerifierProvider, error) { + issuerDomain := getProtocolAndDomain(issuer) + if verifier, ok := im.domainToVerifier[issuerDomain]; ok { + return verifier, nil } return nil, errors.New("Unknown issuer") } diff --git a/toolbox/issuer_test.go b/toolbox/issuer_test.go index cf60043..4f988cf 100644 --- a/toolbox/issuer_test.go +++ b/toolbox/issuer_test.go @@ -1,7 +1,6 @@ package toolbox import ( - "context" "fmt" "testing" @@ -23,7 +22,7 @@ func TestGetProtocolAndDomain(t *testing.T) { func TestNewIssuerManager(t *testing.T) { t.Run("Invalid URL", func(t *testing.T) { - _, err := NewIssuerManager(keycloak.Config{AddrTokenProvider: ":"}, keyContextIssuerDomain) + _, err := NewIssuerManager(keycloak.Config{AddrTokenProvider: ":"}) assert.NotNil(t, err) }) @@ -32,18 +31,18 @@ func TestNewIssuerManager(t *testing.T) { otherDomainPath := "http://other.domain.com:2120/" allDomains := fmt.Sprintf("%s %s %s", defaultPath, myDomainPath, otherDomainPath) - prov, err := NewIssuerManager(keycloak.Config{AddrTokenProvider: allDomains}, keyContextIssuerDomain) + prov, err := NewIssuerManager(keycloak.Config{AddrTokenProvider: allDomains}) assert.Nil(t, err) assert.NotNil(t, prov) // No issuer provided with context - issuerNoContext, _ := prov.GetIssuer(context.Background()) + issuerNoContext, _ := prov.GetOidcVerifierProvider("") // Unrecognized issuer provided in context - issuerDefault, _ := prov.GetIssuer(context.WithValue(context.Background(), keyContextIssuerDomain, "http://unknown.issuer.com/one/path")) + issuerDefault, _ := prov.GetOidcVerifierProvider("http://unknown.issuer.com/one/path") // Case insensitive - issuerMyDomain, _ := prov.GetIssuer(context.WithValue(context.Background(), keyContextIssuerDomain, "http://MY.DOMAIN.COM/issuer")) + issuerMyDomain, _ := prov.GetOidcVerifierProvider("http://MY.DOMAIN.COM/issuer") // Other domain - issuerOtherDomain, _ := prov.GetIssuer(context.WithValue(context.Background(), keyContextIssuerDomain, "http://other.domain.com:2120/any/thing/here")) + issuerOtherDomain, _ := prov.GetOidcVerifierProvider("http://other.domain.com:2120/any/thing/here") assert.Equal(t, issuerNoContext, issuerDefault) assert.NotEqual(t, issuerNoContext, issuerMyDomain)