From 3066e3baffd95f5cae3d3692b8829deaa104dd55 Mon Sep 17 00:00:00 2001 From: Derek Trider Date: Fri, 24 Jul 2020 12:42:13 -0400 Subject: [PATCH] fix: Separate providers for transient and persistent storage Signed-off-by: Derek Trider --- cmd/auth-rest/startcmd/start.go | 2 +- pkg/restapi/controller_test.go | 5 +- pkg/restapi/operation/operations.go | 19 ++++---- pkg/restapi/operation/operations_test.go | 59 ++++++++++++++---------- 4 files changed, 49 insertions(+), 36 deletions(-) diff --git a/cmd/auth-rest/startcmd/start.go b/cmd/auth-rest/startcmd/start.go index 1f6412d..398236e 100644 --- a/cmd/auth-rest/startcmd/start.go +++ b/cmd/auth-rest/startcmd/start.go @@ -245,7 +245,7 @@ func startAuthService(parameters *authRestParameters, srv server) error { } // TODO #34 add the handlers from this controller to the router so that the endpoints are reachable - _, err = restapi.New(&operation.Config{Provider: provider}) + _, err = restapi.New(&operation.Config{TransientStoreProvider: memstore.NewProvider(), StoreProvider: provider}) if err != nil { return err } diff --git a/pkg/restapi/controller_test.go b/pkg/restapi/controller_test.go index 430c936..2495d70 100644 --- a/pkg/restapi/controller_test.go +++ b/pkg/restapi/controller_test.go @@ -55,8 +55,9 @@ func config() (*operation.Config, func()) { path, cleanup := newTestOIDCProvider() return &operation.Config{ - OIDCProviderURL: path, - Provider: memstore.NewProvider(), + OIDCProviderURL: path, + TransientStoreProvider: memstore.NewProvider(), + StoreProvider: memstore.NewProvider(), }, cleanup } diff --git a/pkg/restapi/operation/operations.go b/pkg/restapi/operation/operations.go index 5047a9d..9fc6861 100644 --- a/pkg/restapi/operation/operations.go +++ b/pkg/restapi/operation/operations.go @@ -121,13 +121,14 @@ type Operation struct { // Config defines configuration for rp operations. type Config struct { - TLSConfig *tls.Config - RequestTokens map[string]string - OIDCProviderURL string - OIDCClientID string - OIDCClientSecret string - OIDCCallbackURL string - Provider storage.Provider + TLSConfig *tls.Config + RequestTokens map[string]string + OIDCProviderURL string + OIDCClientID string + OIDCClientSecret string + OIDCCallbackURL string + TransientStoreProvider storage.Provider + StoreProvider storage.Provider } type createOIDCRequestResponse struct { @@ -161,7 +162,7 @@ func New(config *Config) (*Operation, error) { svc.oidcProvider = &oidcProviderImpl{op: idp} - svc.transientStore, err = createStore(config.Provider) + svc.transientStore, err = createStore(config.TransientStoreProvider) if err != nil { return nil, fmt.Errorf("failed to create store : %w", err) } @@ -183,7 +184,7 @@ func New(config *Config) (*Operation, error) { } } - bootstrapStore, err := openBootstrapStore(config.Provider) + bootstrapStore, err := openBootstrapStore(config.StoreProvider) if err != nil { return nil, err } diff --git a/pkg/restapi/operation/operations_test.go b/pkg/restapi/operation/operations_test.go index 4cd3987..8ea3fe4 100644 --- a/pkg/restapi/operation/operations_test.go +++ b/pkg/restapi/operation/operations_test.go @@ -42,9 +42,9 @@ func TestNew(t *testing.T) { config, cleanup := config() defer cleanup() - config.Provider = memstore.NewProvider() + config.TransientStoreProvider = memstore.NewProvider() - err := config.Provider.CreateStore(bootstrapStoreName) + err := config.TransientStoreProvider.CreateStore(bootstrapStoreName) require.NoError(t, err) svc, err := New(config) @@ -64,7 +64,7 @@ func TestNew(t *testing.T) { t.Run("error if unable to open transient store", func(t *testing.T) { config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{ + config.TransientStoreProvider = &mockstore.Provider{ ErrOpenStoreHandle: errors.New("test"), } _, err := New(config) @@ -74,7 +74,7 @@ func TestNew(t *testing.T) { t.Run("error if unable to create transient store", func(t *testing.T) { config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{ + config.TransientStoreProvider = &mockstore.Provider{ ErrCreateStore: errors.New("generic"), } _, err := New(config) @@ -120,7 +120,7 @@ func TestCreateOIDCRequest(t *testing.T) { t.Run("internal server error if transient store fails", func(t *testing.T) { config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{ + config.TransientStoreProvider = &mockstore.Provider{ Store: &mockstore.MockStore{ Store: make(map[string][]byte), ErrPut: errors.New("test"), @@ -144,7 +144,7 @@ func TestHandleOIDCCallback(t *testing.T) { config, configCleanup := config() defer configCleanup() - config.Provider = &mockstore.Provider{ + config.TransientStoreProvider = &mockstore.Provider{ Store: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), @@ -215,7 +215,7 @@ func TestHandleOIDCCallback(t *testing.T) { config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{ + config.TransientStoreProvider = &mockstore.Provider{ Store: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), @@ -232,21 +232,26 @@ func TestHandleOIDCCallback(t *testing.T) { }) t.Run("generic bootstrap store FETCH error", func(t *testing.T) { - sub := uuid.New().String() + id := uuid.New().String() state := uuid.New().String() config, cleanup := config() defer cleanup() - config.Provider = &mockstorage.Provider{ + config.TransientStoreProvider = &mockstorage.Provider{ Stores: map[string]storage.Store{ transientStoreName: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), }, }, + }, + } + + config.StoreProvider = &mockstorage.Provider{ + Stores: map[string]storage.Store{ bootstrapStoreName: &mockstore.MockStore{ Store: map[string][]byte{ - sub: {}, + id: {}, }, ErrGet: errors.New("generic"), }, @@ -268,7 +273,7 @@ func TestHandleOIDCCallback(t *testing.T) { oidcClaimsFunc: func(v interface{}) error { c, ok := v.(*oidcClaims) require.True(t, ok) - c.Sub = sub + c.Sub = id return nil }, @@ -282,21 +287,26 @@ func TestHandleOIDCCallback(t *testing.T) { }) t.Run("generic bootstrap store PUT error while onboarding user", func(t *testing.T) { - sub := uuid.New().String() + id := uuid.New().String() state := uuid.New().String() config, cleanup := config() defer cleanup() - config.Provider = &mockstorage.Provider{ + config.TransientStoreProvider = &mockstorage.Provider{ Stores: map[string]storage.Store{ transientStoreName: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), }, }, + }, + } + + config.StoreProvider = &mockstorage.Provider{ + Stores: map[string]storage.Store{ bootstrapStoreName: &mockstore.MockStore{ Store: map[string][]byte{ - sub: []byte("{}"), + id: []byte("{}"), }, ErrGet: storage.ErrValueNotFound, ErrPut: errors.New("generic"), @@ -319,7 +329,7 @@ func TestHandleOIDCCallback(t *testing.T) { oidcClaimsFunc: func(v interface{}) error { c, ok := v.(*oidcClaims) require.True(t, ok) - c.Sub = sub + c.Sub = id return nil }, @@ -336,7 +346,7 @@ func TestHandleOIDCCallback(t *testing.T) { state := uuid.New().String() config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{Store: &mockstore.MockStore{ + config.TransientStoreProvider = &mockstore.Provider{Store: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), }, @@ -357,7 +367,7 @@ func TestHandleOIDCCallback(t *testing.T) { state := uuid.New().String() config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{Store: &mockstore.MockStore{ + config.TransientStoreProvider = &mockstore.Provider{Store: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), }, @@ -379,7 +389,7 @@ func TestHandleOIDCCallback(t *testing.T) { state := uuid.New().String() config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{Store: &mockstore.MockStore{ + config.TransientStoreProvider = &mockstore.Provider{Store: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), }, @@ -401,7 +411,7 @@ func TestHandleOIDCCallback(t *testing.T) { state := uuid.New().String() config, cleanup := config() defer cleanup() - config.Provider = &mockstore.Provider{Store: &mockstore.MockStore{ + config.TransientStoreProvider = &mockstore.Provider{Store: &mockstore.MockStore{ Store: map[string][]byte{ state: []byte(state), }, @@ -458,11 +468,12 @@ func config() (*Config, func()) { path, oidcCleanup := newTestOIDCProvider() return &Config{ - OIDCProviderURL: path, - OIDCClientID: uuid.New().String(), - OIDCClientSecret: uuid.New().String(), - OIDCCallbackURL: "http://test.com", - Provider: memstore.NewProvider(), + OIDCProviderURL: path, + OIDCClientID: uuid.New().String(), + OIDCClientSecret: uuid.New().String(), + OIDCCallbackURL: "http://test.com", + TransientStoreProvider: memstore.NewProvider(), + StoreProvider: memstore.NewProvider(), }, func() { oidcCleanup() }