From de619c6e8946d300da33a8aa4b515f5ee4b499fc Mon Sep 17 00:00:00 2001 From: Trong Huu Nguyen Date: Wed, 25 Aug 2021 09:21:40 +0200 Subject: [PATCH] refactor: add constructor for routing handler to deduplicate config --- cmd/wonderwall/main.go | 29 +---- pkg/config/config.go | 5 + pkg/router/idporten_mock_server_test.go | 16 ++- pkg/router/router.go | 36 +++++- pkg/router/router_test.go | 159 ++++++++++++------------ 5 files changed, 135 insertions(+), 110 deletions(-) diff --git a/cmd/wonderwall/main.go b/cmd/wonderwall/main.go index 632dc9e..47482d4 100644 --- a/cmd/wonderwall/main.go +++ b/cmd/wonderwall/main.go @@ -12,11 +12,8 @@ import ( "github.com/nais/wonderwall/pkg/session" - "github.com/nais/wonderwall/pkg/token" - "github.com/nais/liberator/pkg/conftools" log "github.com/sirupsen/logrus" - "golang.org/x/oauth2" "github.com/nais/wonderwall/pkg/config" "github.com/nais/wonderwall/pkg/cryptutil" @@ -49,8 +46,6 @@ func run() error { log.Info(line) } - scopes := []string{token.ScopeOpenID} - key, err := base64.StdEncoding.DecodeString(cfg.EncryptionKey) if err != nil { if len(cfg.EncryptionKey) > 0 { @@ -80,29 +75,17 @@ func run() error { log.Warnf("Redis not configured, using in-memory session backing store; not suitable for multi-pod deployments!") } - oauthConfig := oauth2.Config{ - ClientID: cfg.IDPorten.ClientID, - Endpoint: oauth2.Endpoint{ - AuthURL: cfg.IDPorten.WellKnown.AuthorizationEndpoint, - TokenURL: cfg.IDPorten.WellKnown.TokenEndpoint, - }, - RedirectURL: cfg.IDPorten.RedirectURI, - Scopes: scopes, - } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() - jwkSet, err := jwk.Fetch(context.Background(), cfg.IDPorten.WellKnown.JwksURI) + jwkSet, err := jwk.Fetch(ctx, cfg.IDPorten.WellKnown.JwksURI) if err != nil { return fmt.Errorf("fetching jwks: %w", err) } - handler := &router.Handler{ - Config: cfg.IDPorten, - Crypter: crypt, - OauthConfig: oauthConfig, - UpstreamHost: cfg.UpstreamHost, - SecureCookies: true, - Sessions: sessionStore, - JwkSet: jwkSet, + handler, err := router.NewHandler(cfg.IDPorten, crypt, jwkSet, sessionStore, cfg.UpstreamHost) + if err != nil { + return fmt.Errorf("initializing routing handler: %w", err) } r := router.New(handler) diff --git a/pkg/config/config.go b/pkg/config/config.go index 9276931..5c83408 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -5,6 +5,8 @@ import ( "github.com/nais/liberator/pkg/conftools" "github.com/spf13/viper" + + "github.com/nais/wonderwall/pkg/token" ) type Config struct { @@ -26,6 +28,7 @@ type IDPorten struct { Locale string `json:"locale"` SecurityLevel string `json:"security-level"` PostLogoutRedirectURI string `json:"post-logout-redirect-uri"` + Scopes []string `json:"scopes"` } const ( @@ -42,6 +45,7 @@ const ( IDPortenLocale = "idporten.locale" IDPortenSecurityLevel = "idporten.security-level" IDPortenPostLogoutRedirectURI = "idporten.post-logout-redirect-uri" + IDPortenScopes = "idporten.scopes" ) func bindNAIS() { @@ -64,6 +68,7 @@ func Initialize() *Config { flag.String(IDPortenSecurityLevel, "Level4", "Requested security level, either Level3 or Level4.") flag.String(IDPortenLocale, "nb", "Locale for OAuth2 consent screen.") flag.String(IDPortenPostLogoutRedirectURI, "https://nav.no", "URI for redirecting the user after successful logout at IDPorten.") + flag.StringSlice(IDPortenScopes, []string{token.ScopeOpenID}, "List of scopes that should be used during the Auth Code flow.") return &Config{} } diff --git a/pkg/router/idporten_mock_server_test.go b/pkg/router/idporten_mock_server_test.go index fc7f35e..fd63b7f 100644 --- a/pkg/router/idporten_mock_server_test.go +++ b/pkg/router/idporten_mock_server_test.go @@ -14,10 +14,13 @@ import ( "github.com/lestrrat-go/jwx/jwa" "github.com/lestrrat-go/jwx/jwk" "github.com/lestrrat-go/jwx/jwt" + + "github.com/nais/wonderwall/pkg/config" ) type IDPorten struct { Clients map[string]string + Config config.IDPorten Codes map[string]AuthRequest Keys jwk.Set Sessions map[string]string @@ -30,7 +33,7 @@ type AuthRequest struct { Nonce string } -func NewIDPorten(clients map[string]string) *IDPorten { +func NewIDPorten(clients map[string]string, config config.IDPorten) *IDPorten { privateKey, err := rsa.GenerateKey(rand.Reader, 2048) if err != nil { panic(err) @@ -56,9 +59,10 @@ func NewIDPorten(clients map[string]string) *IDPorten { return &IDPorten{ Clients: clients, - Sessions: make(map[string]string), Codes: make(map[string]AuthRequest), + Config: config, Keys: keys, + Sessions: make(map[string]string), } } @@ -114,7 +118,7 @@ func (ip *IDPorten) Token(w http.ResponseWriter, r *http.Request) { accessToken := jwt.New() accessToken.Set("sub", sub) - accessToken.Set("iss", cfg.WellKnown.Issuer) + accessToken.Set("iss", ip.Config.WellKnown.Issuer) accessToken.Set("acr", auth.AcrLevel) accessToken.Set("iat", time.Now().Unix()) accessToken.Set("exp", time.Now().Unix()+expires) @@ -127,8 +131,8 @@ func (ip *IDPorten) Token(w http.ResponseWriter, r *http.Request) { idToken := jwt.New() idToken.Set("sub", sub) - idToken.Set("iss", cfg.WellKnown.Issuer) - idToken.Set("aud", cfg.ClientID) + idToken.Set("iss", ip.Config.WellKnown.Issuer) + idToken.Set("aud", ip.Config.ClientID) idToken.Set("locale", auth.Locale) idToken.Set("nonce", auth.Nonce) idToken.Set("acr", auth.AcrLevel) @@ -143,7 +147,7 @@ func (ip *IDPorten) Token(w http.ResponseWriter, r *http.Request) { return } - ip.Sessions[sid] = cfg.ClientID + ip.Sessions[sid] = ip.Config.ClientID // fixme: generate valid access token and id token; sign them with the correct key token := &TokenJSON{ AccessToken: string(signedAccessToken), diff --git a/pkg/router/router.go b/pkg/router/router.go index 42f97cb..dc6a817 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -37,15 +37,43 @@ const ( type Handler struct { Config config.IDPorten - OauthConfig oauth2.Config Crypter cryptutil.Crypter - UpstreamHost string - JwkSet jwk.Set + OauthConfig oauth2.Config SecureCookies bool Sessions session.Store + UpstreamHost string + jwkSet jwk.Set lock sync.Mutex } +func NewHandler(cfg config.IDPorten, crypter cryptutil.Crypter, jwkSet jwk.Set, sessionStore session.Store, upstreamHost string) (*Handler, error) { + oauthConfig := oauth2.Config{ + ClientID: cfg.ClientID, + Endpoint: oauth2.Endpoint{ + AuthURL: cfg.WellKnown.AuthorizationEndpoint, + TokenURL: cfg.WellKnown.TokenEndpoint, + }, + RedirectURL: cfg.RedirectURI, + Scopes: cfg.Scopes, + } + + return &Handler{ + Config: cfg, + Crypter: crypter, + jwkSet: jwkSet, + lock: sync.Mutex{}, + OauthConfig: oauthConfig, + Sessions: sessionStore, + SecureCookies: true, + UpstreamHost: upstreamHost, + }, nil +} + +func (h *Handler) WithSecureCookie(enabled bool) *Handler { + h.SecureCookies = enabled + return h +} + type loginParams struct { state string codeVerifier string @@ -169,7 +197,7 @@ func (h *Handler) Callback(w http.ResponseWriter, r *http.Request) { return } - idToken, err := token.ParseIDToken(r.Context(), h.JwkSet, tokens) + idToken, err := token.ParseIDToken(r.Context(), h.jwkSet, tokens) if err != nil { log.Error(err) w.WriteHeader(http.StatusUnauthorized) diff --git a/pkg/router/router_test.go b/pkg/router/router_test.go index 13ffb3c..57cc548 100644 --- a/pkg/router/router_test.go +++ b/pkg/router/router_test.go @@ -2,7 +2,10 @@ package router_test import ( "context" + "crypto/rand" + "crypto/rsa" "encoding/base64" + "encoding/json" "fmt" "net/http" "net/http/cookiejar" @@ -12,86 +15,86 @@ import ( "time" "github.com/lestrrat-go/jwx/jwk" - - "github.com/nais/wonderwall/pkg/session" - - "golang.org/x/oauth2" - - "github.com/nais/wonderwall/pkg/cryptutil" - "github.com/stretchr/testify/assert" "github.com/nais/wonderwall/pkg/config" + "github.com/nais/wonderwall/pkg/cryptutil" "github.com/nais/wonderwall/pkg/router" + "github.com/nais/wonderwall/pkg/session" ) const clientID = "clientid" var encryptionKey = []byte(`G8Roe6AcoBpdr5GhO3cs9iORl4XIC8eq`) // 256 bits AES -var cfg = config.IDPorten{ - ClientID: clientID, - ClientJWK: ` -{ - "kty": "RSA", - "kid": "9rJ_0ziKoGNjSS_l11hn0yQxEqg", - "n": "siUszyp3NOJlMMhguHxh6vLpMrFLRUSOx0FfjcBSIZ5QCPh6D4IpwhOW5yprKbApLPIse6qCo-cRwwzYhkXiSF7U8BSnpdp6orhtfHKBBfbYTljHXvQLQ7ADquFNXOl1KlK8A26ut6goYzxgOJ_QYOzshjTx_kNwYJb1DzKPNBxzAg-pOjwEbPKp3ErTlv46yE43gOYi_9wAmFBsA-oX8SiDBaYSmi6XDdaUx5XRpWkXwSdaJ2Hh5pky2fRRwLzQGTyyxAW_u4iyDgRf48C2eOtCc_fORc7vQkojrXWS366vQXNjp605al1H4zbq2YTOQI9YEL18EaQyoxUaxrUqVw", - "e": "AQAB", - "d": "eVbm6YjUP1pBcHPbpW1bSKwB-PxX96tV0RSPID8x8iIiA6ozgaK4DLBJJdV3vqJ1uV6OvAENENTP_VoflX2-PmsRgSGge1CQHYufT5eymDxlYyAHVH7HuWgHZ3oktrdxjc1isLfQG9pXABjctVTtm0dlZ5hiiDypK7FG4_4dGnFud74suZU1kXi-fQsf7W-VxO4LZ6BLyTrvjPecu91GtKpHezjNoP__cEhGF2KISspO9bvvTuwguJCaM0bg_nW1POlXggTUbV3tJbD5PYs929ExKgWpq3cZXRq0fG3bApJsquCFAOJjdJ_dF37sh5gveUEhFbciXUeVXMSYKdnsAQ", - "p": "5v-xD0CqUlgIF51Q2puG8CEZNxVBtqJN-xrs9HH9Do6ObYrxjPPWVx_TJc7Os_q9hgAtMKhxEC-ssf3t5bOD6IUsiyiSJKNT5RZEgTdMAAKD_5p-BkXi9lhDKySGRf2r5V1qSYTTYzEx0HOn-ZSrOPK6psR3PI2fCdb_TfRn3gE", - "q": "xW0QRv4Jd9KefgyF53nmeQ0eALcAIiWreKz8u96xdByl0RCbo6OAmoobgaWFT0_TdPzCIz1qwa_xT3_6xGhBBW5BXoLUaf86j6_WxQesIelC9ZfwNWdP0V17VBd8L94Y4kGw6VvI42P8FKrXA0MXSNmAMVMb8PrLvl9rrL_YuFc", - "dp": "g67fUMKcVbS5aDzWCsj-c4VqymvjuilsKul-ixswF0xNBUVfzepzFdeelr7-NruJrwoKuOJNEd0bpZwMMhXT7Il-ixXlud0hxkabZs4PFTJZ7Sw1C35rk-Nc5ws7QEsL4wUNwjtmBfXVX--OokiOEzjMDqWRE4PoVcOqZtYdIAE", - "dq": "PYtIPblHnlDME6M3wvcfP7E1HyftJLf1gkL67l33l6iukEPLIPIBTyuqc3nz2suZsahxpKaqtwJwCUZuF_gf_N9oBVxndzuXN9-q5fUEVfXvZ7wbp6ozGaM4pPhFQG7N9wpfaf-w2iH7HT48lMm_YnhbHAU6ep7UEN6SJGIR3zU", - "qi": "juJsMNCw9y1aTCGxkGW-LkyumX5VfcigOr893gzYMkX7XuCupEp5Yk9IlDDjnLrbd6KU_ytZHK1ErPCekt3LDd7CnsYNYkWvHpFAS3tqF5DVGWUtZ82Z-0dDYZvjbCojHgG3eFUk5bJZkHxgNql6dKX9ro_LfaGwIJ4-beVQHG0", - "x5t": "9rJ_0ziKoGNjSS_l11hn0yQxEqg" -} -`, - RedirectURI: "http://localhost/callback", - WellKnownURL: "", - WellKnown: config.IDPortenWellKnown{ - Issuer: "issuer", - AuthorizationEndpoint: "http://localhost:1234/authorize", - }, - Locale: "nb", - SecurityLevel: "Level4", - PostLogoutRedirectURI: "", -} - var clients = map[string]string{ clientID: "http://localhost/oauth2/logout/frontchannel", } -var idp = NewIDPorten(clients) -func handler() *router.Handler { - handler := router.Handler{ - Config: cfg, - Sessions: session.NewMemory(), - OauthConfig: oauth2.Config{ - ClientID: "client-id", - ClientSecret: "client-secret", - Endpoint: oauth2.Endpoint{ - AuthURL: "auth-url", - TokenURL: "token-url", - }, - RedirectURL: "redirect-url", - Scopes: []string{"scopes"}, - }, - Crypter: cryptutil.New(encryptionKey), - UpstreamHost: "", +func defaultConfig() config.IDPorten { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + panic(err) } - return &handler + + key, err := jwk.New(privateKey) + if err != nil { + panic(err) + } + + clientJwk, err := json.Marshal(key) + if err != nil { + panic(err) + } + + return config.IDPorten{ + ClientID: clientID, + ClientJWK: string(clientJwk), + RedirectURI: "http://localhost/callback", + WellKnownURL: "", + WellKnown: config.IDPortenWellKnown{ + Issuer: "issuer", + AuthorizationEndpoint: "http://localhost:1234/authorize", + }, + Locale: "nb", + SecurityLevel: "Level4", + PostLogoutRedirectURI: "", + } +} + +func handler(cfg config.IDPorten) *router.Handler { + var jwkSet jwk.Set + var err error + + if len(cfg.WellKnown.JwksURI) == 0 { + jwk.NewSet() + } else { + jwkSet, err = jwk.Fetch(context.Background(), cfg.WellKnown.JwksURI) + } + if err != nil { + panic(err) + } + + crypter := cryptutil.New(encryptionKey) + sessionStore := session.NewMemory() + + handler, err := router.NewHandler(cfg, crypter, jwkSet, sessionStore, "") + if err != nil { + panic(err) + } + return handler.WithSecureCookie(false) } func TestLoginURL(t *testing.T) { - handler := &router.Handler{ - Config: cfg, - } + handler := handler(defaultConfig()) _, err := handler.LoginURL() assert.NoError(t, err) } func TestHandler_Login(t *testing.T) { - h := handler() + cfg := defaultConfig() + + h := handler(cfg) r := router.New(h) server := httptest.NewServer(r) @@ -100,7 +103,7 @@ func TestHandler_Login(t *testing.T) { return http.ErrUseLastResponse } - idprouter := idportenRouter(idp) + idprouter := idportenRouter(NewIDPorten(clients, cfg)) idpserver := httptest.NewServer(idprouter) h.Config.WellKnown.AuthorizationEndpoint = idpserver.URL + "/authorize" @@ -136,20 +139,21 @@ func TestHandler_Login(t *testing.T) { } func TestHandler_Callback_and_Logout(t *testing.T) { - h := handler() + cfg := defaultConfig() + + idprouter := idportenRouter(NewIDPorten(clients, cfg)) + idpserver := httptest.NewServer(idprouter) + cfg.WellKnown.JwksURI = idpserver.URL + "/jwks" + cfg.WellKnown.AuthorizationEndpoint = idpserver.URL + "/authorize" + cfg.WellKnown.TokenEndpoint = idpserver.URL + "/token" + cfg.WellKnown.EndSessionEndpoint = idpserver.URL + "/endsession" + + h := handler(cfg) r := router.New(h) server := httptest.NewServer(r) - idprouter := idportenRouter(idp) - idpserver := httptest.NewServer(idprouter) - h.OauthConfig.Endpoint.TokenURL = idpserver.URL + "/token" - h.Config.WellKnown.AuthorizationEndpoint = idpserver.URL + "/authorize" - h.Config.WellKnown.EndSessionEndpoint = idpserver.URL + "/endsession" h.Config.RedirectURI = server.URL + "/oauth2/callback" h.Config.PostLogoutRedirectURI = server.URL - jwkSet, err := jwk.Fetch(context.Background(), idpserver.URL+"/jwks") - assert.NoError(t, err) - h.JwkSet = jwkSet jar, err := cookiejar.New(nil) assert.NoError(t, err) @@ -228,22 +232,23 @@ func TestHandler_Callback_and_Logout(t *testing.T) { } func TestHandler_FrontChannelLogout(t *testing.T) { - h := handler() + cfg := defaultConfig() + + idp := NewIDPorten(clients, cfg) + idprouter := idportenRouter(idp) + idpserver := httptest.NewServer(idprouter) + + cfg.WellKnown.JwksURI = idpserver.URL + "/jwks" + cfg.WellKnown.AuthorizationEndpoint = idpserver.URL + "/authorize" + cfg.WellKnown.TokenEndpoint = idpserver.URL + "/token" + + h := handler(cfg) r := router.New(h) server := httptest.NewServer(r) - idprouter := idportenRouter(idp) - idpserver := httptest.NewServer(idprouter) - h.OauthConfig.Endpoint.TokenURL = idpserver.URL + "/token" - h.Config.WellKnown.AuthorizationEndpoint = idpserver.URL + "/authorize" - h.Config.WellKnown.EndSessionEndpoint = idpserver.URL + "/endsession" h.Config.RedirectURI = server.URL + "/oauth2/callback" h.Config.PostLogoutRedirectURI = server.URL - jwkSet, err := jwk.Fetch(context.Background(), idpserver.URL+"/jwks") - assert.NoError(t, err) - h.JwkSet = jwkSet - jar, err := cookiejar.New(nil) assert.NoError(t, err)