From c8f48335d4de92f64cb7d7b3a2b7d85d57aa5c0c Mon Sep 17 00:00:00 2001 From: Trong Huu Nguyen Date: Fri, 2 Sep 2022 15:13:44 +0200 Subject: [PATCH] refactor(openid/config): extract getter for ingresses --- pkg/handler/handler.go | 7 +++++++ pkg/handler/handler_standard.go | 7 ++++++- pkg/handler/url/url_test.go | 8 ++++---- pkg/mock/client.go | 23 ----------------------- pkg/mock/config.go | 10 ++++++++++ pkg/mock/openid.go | 20 ++++++++++++++------ pkg/mock/request.go | 6 +++--- pkg/openid/client/login_test.go | 7 +++++-- pkg/openid/client/logout_callback_test.go | 3 ++- pkg/openid/client/logout_test.go | 3 ++- pkg/openid/config/client.go | 13 ------------- 11 files changed, 53 insertions(+), 54 deletions(-) diff --git a/pkg/handler/handler.go b/pkg/handler/handler.go index 0beec8c..752707d 100644 --- a/pkg/handler/handler.go +++ b/pkg/handler/handler.go @@ -10,6 +10,7 @@ import ( "github.com/nais/wonderwall/pkg/crypto" "github.com/nais/wonderwall/pkg/handler/autologin" "github.com/nais/wonderwall/pkg/handler/reverseproxy" + "github.com/nais/wonderwall/pkg/ingress" "github.com/nais/wonderwall/pkg/loginstatus" "github.com/nais/wonderwall/pkg/openid/client" openidconfig "github.com/nais/wonderwall/pkg/openid/config" @@ -46,12 +47,18 @@ func NewHandler( return nil, err } + ingresses, err := ingress.ParseIngresses(cfg) + if err != nil { + return nil, err + } + return &StandardHandler{ autoLogin: autoLogin, client: openidClient, config: cfg, cookieOptions: cookieOpts, crypter: crypter, + ingresses: ingresses, loginstatus: loginstatus.NewClient(cfg.Loginstatus, httpClient), openidConfig: openidConfig, provider: openidProvider, diff --git a/pkg/handler/handler_standard.go b/pkg/handler/handler_standard.go index 5475b16..90ef211 100644 --- a/pkg/handler/handler_standard.go +++ b/pkg/handler/handler_standard.go @@ -34,6 +34,7 @@ type StandardHandler struct { config *config.Config cookieOptions cookie.Options crypter crypto.Crypter + ingresses *ingress.Ingresses loginstatus loginstatus.Loginstatus openidConfig openidconfig.Config provider provider.Provider @@ -71,7 +72,11 @@ func (s *StandardHandler) GetErrorRedirectURI() string { } func (s *StandardHandler) GetIngresses() *ingress.Ingresses { - return s.openidConfig.Client().Ingresses() + return s.ingresses +} + +func (s *StandardHandler) SetIngresses(ingresses *ingress.Ingresses) { + s.ingresses = ingresses } func (s *StandardHandler) GetLoginstatus() loginstatus.Loginstatus { diff --git a/pkg/handler/url/url_test.go b/pkg/handler/url/url_test.go index c2518e4..1a83ee7 100644 --- a/pkg/handler/url/url_test.go +++ b/pkg/handler/url/url_test.go @@ -195,7 +195,7 @@ func TestLoginCallbackURL(t *testing.T) { "https://nav.no/dagpenger", "https://nav.no/dagpenger/soknad", } - openidConfig := mock.NewTestConfiguration(cfg) + ingresses := mock.Ingresses(cfg) for _, test := range []struct { input string @@ -224,7 +224,7 @@ func TestLoginCallbackURL(t *testing.T) { }, } { t.Run(test.input, func(t *testing.T) { - req := mock.NewGetRequest(test.input, openidConfig) + req := mock.NewGetRequest(test.input, ingresses) actual, err := urlpkg.LoginCallbackURL(req) if test.err != nil { @@ -245,7 +245,7 @@ func TestLogoutCallbackURL(t *testing.T) { "https://nav.no/dagpenger", "https://nav.no/dagpenger/soknad", } - openidConfig := mock.NewTestConfiguration(cfg) + ingresses := mock.Ingresses(cfg) for _, test := range []struct { input string @@ -274,7 +274,7 @@ func TestLogoutCallbackURL(t *testing.T) { }, } { t.Run(test.input, func(t *testing.T) { - req := mock.NewGetRequest(test.input, openidConfig) + req := mock.NewGetRequest(test.input, ingresses) actual, err := urlpkg.LogoutCallbackURL(req) if test.err != nil { diff --git a/pkg/mock/client.go b/pkg/mock/client.go index bffc219..7aedeca 100644 --- a/pkg/mock/client.go +++ b/pkg/mock/client.go @@ -5,14 +5,12 @@ import ( "github.com/nais/wonderwall/pkg/config" "github.com/nais/wonderwall/pkg/crypto" - "github.com/nais/wonderwall/pkg/ingress" "github.com/nais/wonderwall/pkg/openid/scopes" ) type TestClientConfiguration struct { *config.Config clientJwk jwk.Key - ingresses *ingress.Ingresses } func (c *TestClientConfiguration) ACRValues() string { @@ -27,21 +25,6 @@ func (c *TestClientConfiguration) ClientJWK() jwk.Key { return c.clientJwk } -func (c *TestClientConfiguration) Ingresses() *ingress.Ingresses { - return c.ingresses -} - -func (c *TestClientConfiguration) SetIngresses(ingresses ...string) { - c.Config.Ingresses = ingresses - - parsed, err := ingress.ParseIngresses(c.Config) - if err != nil { - panic(err) - } - - c.ingresses = parsed -} - func (c *TestClientConfiguration) SetPostLogoutRedirectURI(uri string) { c.Config.OpenID.PostLogoutRedirectURI = uri } @@ -70,14 +53,8 @@ func clientConfiguration(cfg *config.Config) *TestClientConfiguration { panic(err) } - ingresses, err := ingress.ParseIngresses(cfg) - if err != nil { - panic(err) - } - return &TestClientConfiguration{ Config: cfg, clientJwk: key, - ingresses: ingresses, } } diff --git a/pkg/mock/config.go b/pkg/mock/config.go index ffad3b3..cb1f2bb 100644 --- a/pkg/mock/config.go +++ b/pkg/mock/config.go @@ -4,6 +4,7 @@ import ( "time" "github.com/nais/wonderwall/pkg/config" + "github.com/nais/wonderwall/pkg/ingress" openidconfig "github.com/nais/wonderwall/pkg/openid/config" ) @@ -48,3 +49,12 @@ func NewTestConfiguration(cfg *config.Config) *TestConfiguration { TestProvider: providerConfiguration(cfg), } } + +func Ingresses(cfg *config.Config) *ingress.Ingresses { + parsed, err := ingress.ParseIngresses(cfg) + if err != nil { + panic(err) + } + + return parsed +} diff --git a/pkg/mock/openid.go b/pkg/mock/openid.go index 57b7457..ce5d558 100644 --- a/pkg/mock/openid.go +++ b/pkg/mock/openid.go @@ -21,6 +21,7 @@ import ( "github.com/nais/wonderwall/pkg/cookie" "github.com/nais/wonderwall/pkg/crypto" handlerpkg "github.com/nais/wonderwall/pkg/handler" + "github.com/nais/wonderwall/pkg/ingress" "github.com/nais/wonderwall/pkg/openid" openidclient "github.com/nais/wonderwall/pkg/openid/client" openidconfig "github.com/nais/wonderwall/pkg/openid/config" @@ -62,11 +63,17 @@ func (in *IdentityProvider) RelyingPartyClient() *http.Client { func (in *IdentityProvider) SetIngresses(ingresses ...string) { in.Cfg.Ingresses = ingresses - in.OpenIDConfig.TestClient.SetIngresses(ingresses...) + + parsed, err := ingress.ParseIngresses(in.Cfg) + if err != nil { + panic(err) + } + + in.RelyingPartyHandler.SetIngresses(parsed) } func (in *IdentityProvider) GetRequest(target string) *http.Request { - return NewGetRequest(target, in.OpenIDConfig) + return NewGetRequest(target, in.RelyingPartyHandler.GetIngresses()) } func NewIdentityProvider(cfg *config.Config) *IdentityProvider { @@ -94,10 +101,7 @@ func NewIdentityProvider(cfg *config.Config) *IdentityProvider { rpRouter := router.New(rpHandler) rpServer := httptest.NewServer(rpRouter) - // reconfigure client after Relying Party server is started - openidConfig.TestClient.SetIngresses(rpServer.URL) - - return &IdentityProvider{ + ip := &IdentityProvider{ cancelFunc: cancel, Cfg: cfg, RelyingPartyHandler: rpHandler, @@ -107,6 +111,10 @@ func NewIdentityProvider(cfg *config.Config) *IdentityProvider { ProviderHandler: handler, ProviderServer: server, } + + // reconfigure ingresses after Relying Party server is started + ip.SetIngresses(rpServer.URL) + return ip } func identityProviderRouter(ip *IdentityProviderHandler) chi.Router { diff --git a/pkg/mock/request.go b/pkg/mock/request.go index ce119de..c4982bd 100644 --- a/pkg/mock/request.go +++ b/pkg/mock/request.go @@ -4,13 +4,13 @@ import ( "net/http" "net/http/httptest" + "github.com/nais/wonderwall/pkg/ingress" mw "github.com/nais/wonderwall/pkg/middleware" - openidconfig "github.com/nais/wonderwall/pkg/openid/config" ) -func NewGetRequest(target string, openidConfig openidconfig.Config) *http.Request { +func NewGetRequest(target string, ingresses *ingress.Ingresses) *http.Request { req := httptest.NewRequest(http.MethodGet, target, nil) - match, ok := openidConfig.Client().Ingresses().MatchingIngress(req) + match, ok := ingresses.MatchingIngress(req) if ok { req = mw.RequestWithIngress(req, match) req = mw.RequestWithPath(req, match.Path()) diff --git a/pkg/openid/client/login_test.go b/pkg/openid/client/login_test.go index ab7055d..91273cf 100644 --- a/pkg/openid/client/login_test.go +++ b/pkg/openid/client/login_test.go @@ -64,10 +64,12 @@ func TestLogin_URL(t *testing.T) { t.Run(test.url, func(t *testing.T) { cfg := mock.Config() openidConfig := mock.NewTestConfiguration(cfg) + ingresses := mock.Ingresses(cfg) + c := client.NewClient(openidConfig) lsc := loginstatus.NewClient(cfg.Loginstatus, http.DefaultClient) - req := mock.NewGetRequest(test.url, openidConfig) + req := mock.NewGetRequest(test.url, ingresses) result, err := c.Login(req, lsc) if test.error != nil { @@ -125,8 +127,9 @@ func TestLoginURL_WithResourceIndicator(t *testing.T) { openidConfig.TestProvider.SetAuthorizationEndpoint("https://provider/authorize") c := client.NewClient(openidConfig) + ingresses := mock.Ingresses(cfg) - req := mock.NewGetRequest(mock.Ingress+"/oauth2/login", openidConfig) + req := mock.NewGetRequest(mock.Ingress+"/oauth2/login", ingresses) result, err := c.Login(req, lsc) assert.NoError(t, err) diff --git a/pkg/openid/client/logout_callback_test.go b/pkg/openid/client/logout_callback_test.go index 50ddc78..4faa3dd 100644 --- a/pkg/openid/client/logout_callback_test.go +++ b/pkg/openid/client/logout_callback_test.go @@ -36,6 +36,7 @@ func TestLogoutCallback_PostLogoutRedirectURI(t *testing.T) { func newLogoutCallback(cfg *config.Config) client.LogoutCallback { openidCfg := mock.NewTestConfiguration(cfg) - req := mock.NewGetRequest(mock.Ingress+"/oauth2/logout/callback", openidCfg) + ingresses := mock.Ingresses(cfg) + req := mock.NewGetRequest(mock.Ingress+"/oauth2/logout/callback", ingresses) return newTestClientWithConfig(openidCfg).LogoutCallback(req) } diff --git a/pkg/openid/client/logout_test.go b/pkg/openid/client/logout_test.go index 539a5a4..a201bb7 100644 --- a/pkg/openid/client/logout_test.go +++ b/pkg/openid/client/logout_test.go @@ -70,8 +70,9 @@ func newLogout(t *testing.T) client.Logout { openidCfg := mock.NewTestConfiguration(cfg) openidCfg.TestClient.SetPostLogoutRedirectURI(PostLogoutRedirectURI) openidCfg.TestProvider.SetEndSessionEndpoint(EndSessionEndpoint) + ingresses := mock.Ingresses(cfg) - req := mock.NewGetRequest(mock.Ingress+"/oauth2/logout", openidCfg) + req := mock.NewGetRequest(mock.Ingress+"/oauth2/logout", ingresses) logout, err := newTestClientWithConfig(openidCfg).Logout(req) assert.NoError(t, err) diff --git a/pkg/openid/config/client.go b/pkg/openid/config/client.go index a3474e2..e9fdcf8 100644 --- a/pkg/openid/config/client.go +++ b/pkg/openid/config/client.go @@ -7,7 +7,6 @@ import ( log "github.com/sirupsen/logrus" wonderwallconfig "github.com/nais/wonderwall/pkg/config" - "github.com/nais/wonderwall/pkg/ingress" "github.com/nais/wonderwall/pkg/openid/scopes" ) @@ -15,7 +14,6 @@ type Client interface { ACRValues() string ClientID() string ClientJWK() jwk.Key - Ingresses() *ingress.Ingresses PostLogoutRedirectURI() string Scopes() scopes.Scopes UILocales() string @@ -27,7 +25,6 @@ type Client interface { type client struct { wonderwallconfig.OpenID clientJwk jwk.Key - ingresses *ingress.Ingresses } func (in *client) ACRValues() string { @@ -42,10 +39,6 @@ func (in *client) ClientJWK() jwk.Key { return in.clientJwk } -func (in *client) Ingresses() *ingress.Ingresses { - return in.ingresses -} - func (in *client) PostLogoutRedirectURI() string { return in.OpenID.PostLogoutRedirectURI } @@ -84,15 +77,9 @@ func NewClientConfig(cfg *wonderwallconfig.Config) (Client, error) { return nil, fmt.Errorf("parsing client JWK: %w", err) } - ingresses, err := ingress.ParseIngresses(cfg) - if err != nil { - return nil, err - } - c := &client{ OpenID: cfg.OpenID, clientJwk: clientJwk, - ingresses: ingresses, } var clientConfig Client