From bc9ba63b06ac698dd5c341cc32e56a1e93b7570f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Franke?= Date: Thu, 30 Mar 2023 11:12:00 +0200 Subject: [PATCH] Fix tests for invitations service. --- ocis-pkg/keycloak/client.go | 47 ++----- ocis-pkg/keycloak/types.go | 39 ++++++ services/invitations/Makefile | 2 +- .../pkg/backends/keycloak/backend.go | 4 +- .../pkg/backends/keycloak/backend_test.go | 114 ++++------------ services/invitations/pkg/mocks/client.go | 122 ++++++++++++++++++ services/invitations/pkg/mocks/go_cloak.go | 112 ---------------- 7 files changed, 203 insertions(+), 237 deletions(-) create mode 100644 ocis-pkg/keycloak/types.go create mode 100644 services/invitations/pkg/mocks/client.go delete mode 100644 services/invitations/pkg/mocks/go_cloak.go diff --git a/ocis-pkg/keycloak/client.go b/ocis-pkg/keycloak/client.go index 1544cddc2f..303acb908b 100644 --- a/ocis-pkg/keycloak/client.go +++ b/ocis-pkg/keycloak/client.go @@ -17,8 +17,8 @@ const ( userTypeAttr = "OWNCLOUD_USER_TYPE" ) -// Client represents a keycloak client -type Client struct { +// ConcreteClient represents a concrete implementation of a keycloak client +type ConcreteClient struct { keycloak GoCloak clientID string clientSecret string @@ -26,34 +26,11 @@ type Client struct { baseURL string } -// PIIReport is a structure of all the PersonalIdentifiableInformation contained in keycloak. -type PIIReport struct { - UserData *libregraph.User - Credentials []*gocloak.CredentialRepresentation -} - -// UserAction defines a type for user actions -type UserAction int8 - -// An incomplete list of UserActions -const ( - // UserActionUpdatePassword sets it that the user needs to change their password. - UserActionUpdatePassword UserAction = iota - // UserActionVerifyEmail sets it that the user needs to verify their email address. - UserActionVerifyEmail -) - -// A lookup table to translate user actions to their string equivalents -var userActionsToString = map[UserAction]string{ - UserActionUpdatePassword: "UPDATE_PASSWORD", - UserActionVerifyEmail: "VERIFY_EMAIL", -} - // New instantiates a new keycloak.Backend with a default gocloak client. func New( baseURL, clientID, clientSecret, realm string, insecureSkipVerify bool, -) *Client { +) *ConcreteClient { gc := gocloak.NewClient(baseURL) restyClient := gc.RestyClient() restyClient.SetTLSClientConfig(&tls.Config{InsecureSkipVerify: insecureSkipVerify}) //nolint:gosec @@ -64,8 +41,8 @@ func New( func NewWithClient( gocloakClient GoCloak, baseURL, clientID, clientSecret, realm string, -) *Client { - return &Client{ +) *ConcreteClient { + return &ConcreteClient{ keycloak: gocloakClient, baseURL: baseURL, clientID: clientID, @@ -78,7 +55,7 @@ func NewWithClient( // TODO: For now we only call this from the invitation service where all the attributes are set correctly. // // For more wider use, do some sanity checking on the user instance. -func (c *Client) CreateUser(ctx context.Context, realm string, user *libregraph.User, userActions []UserAction) (string, error) { +func (c *ConcreteClient) CreateUser(ctx context.Context, realm string, user *libregraph.User, userActions []UserAction) (string, error) { token, err := c.getToken(ctx) if err != nil { return "", err @@ -100,7 +77,7 @@ func (c *Client) CreateUser(ctx context.Context, realm string, user *libregraph. } // SendActionsMail sends a mail to the user with userID instructing them to do the actions defined in userActions. -func (c *Client) SendActionsMail(ctx context.Context, realm, userID string, userActions []UserAction) error { +func (c *ConcreteClient) SendActionsMail(ctx context.Context, realm, userID string, userActions []UserAction) error { token, err := c.getToken(ctx) if err != nil { return err @@ -114,7 +91,7 @@ func (c *Client) SendActionsMail(ctx context.Context, realm, userID string, user } // GetUserByEmail looks up a user by email. -func (c *Client) GetUserByEmail(ctx context.Context, realm, mail string) (*libregraph.User, error) { +func (c *ConcreteClient) GetUserByEmail(ctx context.Context, realm, mail string) (*libregraph.User, error) { token, err := c.getToken(ctx) if err != nil { return nil, err @@ -139,7 +116,7 @@ func (c *Client) GetUserByEmail(ctx context.Context, realm, mail string) (*libre } // GetPIIReport returns a structure with all the PII for the user. -func (c *Client) GetPIIReport(ctx context.Context, realm string, user *libregraph.User) (*PIIReport, error) { +func (c *ConcreteClient) GetPIIReport(ctx context.Context, realm string, user *libregraph.User) (*PIIReport, error) { u, err := c.GetUserByEmail(ctx, realm, *user.Mail) if err != nil { return nil, err @@ -168,7 +145,7 @@ func (c *Client) GetPIIReport(ctx context.Context, realm string, user *libregrap // getToken gets a fresh token for the request. // TODO: set a token on the struct and check if it's still valid before requesting a new one. -func (c *Client) getToken(ctx context.Context) (*gocloak.JWT, error) { +func (c *ConcreteClient) getToken(ctx context.Context) (*gocloak.JWT, error) { token, err := c.keycloak.LoginClient(ctx, c.clientID, c.clientSecret, c.realm) if err != nil { return nil, fmt.Errorf("failed to get token: %w", err) @@ -186,7 +163,7 @@ func (c *Client) getToken(ctx context.Context) (*gocloak.JWT, error) { return token, nil } -func (c *Client) keycloakUserToLibregraph(u *gocloak.User) *libregraph.User { +func (c *ConcreteClient) keycloakUserToLibregraph(u *gocloak.User) *libregraph.User { attrs := *u.Attributes ldapID := "" ldapIDs, ok := attrs[idAttr] @@ -216,7 +193,7 @@ func (c *Client) keycloakUserToLibregraph(u *gocloak.User) *libregraph.User { } } -func (c *Client) getKeyCloakID(u *libregraph.User) (string, error) { +func (c *ConcreteClient) getKeyCloakID(u *libregraph.User) (string, error) { for _, i := range u.Identities { if *i.Issuer == c.baseURL { return *i.IssuerAssignedId, nil diff --git a/ocis-pkg/keycloak/types.go b/ocis-pkg/keycloak/types.go new file mode 100644 index 0000000000..1f5ddfa642 --- /dev/null +++ b/ocis-pkg/keycloak/types.go @@ -0,0 +1,39 @@ +package keycloak + +import ( + "context" + + "github.com/Nerzal/gocloak/v13" + libregraph "github.com/owncloud/libre-graph-api-go" +) + +// UserAction defines a type for user actions +type UserAction int8 + +// An incomplete list of UserActions +const ( + // UserActionUpdatePassword sets it that the user needs to change their password. + UserActionUpdatePassword UserAction = iota + // UserActionVerifyEmail sets it that the user needs to verify their email address. + UserActionVerifyEmail +) + +// A lookup table to translate user actions to their string equivalents +var userActionsToString = map[UserAction]string{ + UserActionUpdatePassword: "UPDATE_PASSWORD", + UserActionVerifyEmail: "VERIFY_EMAIL", +} + +// PIIReport is a structure of all the PersonalIdentifiableInformation contained in keycloak. +type PIIReport struct { + UserData *libregraph.User + Credentials []*gocloak.CredentialRepresentation +} + +// Client represents a keycloak client. +type Client interface { + CreateUser(ctx context.Context, realm string, user *libregraph.User, userActions []UserAction) (string, error) + SendActionsMail(ctx context.Context, realm, userID string, userActions []UserAction) error + GetUserByEmail(ctx context.Context, realm, email string) (*libregraph.User, error) + GetPIIReport(ctx context.Context, realm string, user *libregraph.User) (*PIIReport, error) +} diff --git a/services/invitations/Makefile b/services/invitations/Makefile index 717e7ff5d2..5e06194da7 100644 --- a/services/invitations/Makefile +++ b/services/invitations/Makefile @@ -25,7 +25,7 @@ include ../../.make/generate.mk .PHONY: ci-go-generate ci-go-generate: $(MOCKERY) # CI runs ci-node-generate automatically before this target - $(MOCKERY) --dir pkg/backends/keycloak --output pkg/mocks --case underscore --name GoCloak + $(MOCKERY) --output pkg/mocks --case underscore --name Client --srcpkg "github.com/owncloud/ocis/v2/ocis-pkg/keycloak" .PHONY: ci-node-generate diff --git a/services/invitations/pkg/backends/keycloak/backend.go b/services/invitations/pkg/backends/keycloak/backend.go index 3050363631..7d718de4d4 100644 --- a/services/invitations/pkg/backends/keycloak/backend.go +++ b/services/invitations/pkg/backends/keycloak/backend.go @@ -23,7 +23,7 @@ var userRequiredActions = []keycloak.UserAction{ // Backend represents the keycloak backend. type Backend struct { logger log.Logger - client *keycloak.Client + client keycloak.Client userRealm string } @@ -48,7 +48,7 @@ func New( // NewWithClient creates a new backend with the supplied keycloak client. func NewWithClient( logger log.Logger, - client *keycloak.Client, + client keycloak.Client, userRealm string, ) *Backend { return &Backend{ diff --git a/services/invitations/pkg/backends/keycloak/backend_test.go b/services/invitations/pkg/backends/keycloak/backend_test.go index 6b8ef80805..019469b529 100644 --- a/services/invitations/pkg/backends/keycloak/backend_test.go +++ b/services/invitations/pkg/backends/keycloak/backend_test.go @@ -7,7 +7,7 @@ import ( "context" "testing" - "github.com/Nerzal/gocloak/v13" + kcpkg "github.com/owncloud/ocis/v2/ocis-pkg/keycloak" "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/invitations/pkg/backends/keycloak" "github.com/owncloud/ocis/v2/services/invitations/pkg/invitations" @@ -34,11 +34,11 @@ func TestBackend_CreateUser(t *testing.T) { returns []interface{} } tests := []struct { - name string - args args - want string - keycloakMocks []mockInputs - assertion assert.ErrorAssertionFunc + name string + args args + want string + clientMocks []mockInputs + assertion assert.ErrorAssertionFunc }{ { name: "Test without diplay name", @@ -48,46 +48,17 @@ func TestBackend_CreateUser(t *testing.T) { }, }, want: "test-id", - keycloakMocks: []mockInputs{ - { - funcName: "LoginClient", - args: []interface{}{ - mock.Anything, - clientID, - clientSecret, - clientRealm, - }, - returns: []interface{}{ - &gocloak.JWT{ - AccessToken: jwtToken, - }, - nil, - }, - }, - { - funcName: "RetrospectToken", - args: []interface{}{ - mock.Anything, - jwtToken, - clientID, - clientSecret, - clientRealm, - }, - returns: []interface{}{ - &gocloak.IntroSpectTokenResult{ - Active: gocloak.BoolP(true), - }, - nil, - }, - }, + clientMocks: []mockInputs{ { funcName: "CreateUser", args: []interface{}{ mock.Anything, - jwtToken, userRealm, mock.Anything, // can't match on the user because it generates a UUID internally. - // might be worth refactoring the UUID generation to outside of the func + []kcpkg.UserAction{ + kcpkg.UserActionUpdatePassword, + kcpkg.UserActionVerifyEmail, + }, }, returns: []interface{}{ "test-id", @@ -103,11 +74,11 @@ func TestBackend_CreateUser(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - c := &mocks.GoCloak{} - for _, m := range tt.keycloakMocks { + c := &mocks.Client{} + for _, m := range tt.clientMocks { c.On(m.funcName, m.args...).Return(m.returns...) } - b := keycloak.NewWithClient(log.NopLogger(), c, clientID, clientSecret, clientRealm, userRealm) + b := keycloak.NewWithClient(log.NopLogger(), c, userRealm) got, err := b.CreateUser(ctx, tt.args.invitation) tt.assertion(t, err) assert.Equal(t, tt.want, got) @@ -125,57 +96,26 @@ func TestBackend_SendMail(t *testing.T) { returns []interface{} } tests := []struct { - name string - args args - keycloakMocks []mockInputs - assertion assert.ErrorAssertionFunc + name string + args args + clientMocks []mockInputs + assertion assert.ErrorAssertionFunc }{ { name: "Mail successfully sent", args: args{ id: "test-id", }, - keycloakMocks: []mockInputs{ + clientMocks: []mockInputs{ { - funcName: "LoginClient", + funcName: "SendActionsMail", args: []interface{}{ mock.Anything, - clientID, - clientSecret, - clientRealm, - }, - returns: []interface{}{ - &gocloak.JWT{ - AccessToken: jwtToken, - }, - nil, - }, - }, - { - funcName: "RetrospectToken", - args: []interface{}{ - mock.Anything, - jwtToken, - clientID, - clientSecret, - clientRealm, - }, - returns: []interface{}{ - &gocloak.IntroSpectTokenResult{ - Active: gocloak.BoolP(true), - }, - nil, - }, - }, - { - funcName: "ExecuteActionsEmail", - args: []interface{}{ - mock.Anything, - jwtToken, userRealm, - gocloak.ExecuteActionsEmail{ - UserID: gocloak.StringP("test-id"), - Actions: &[]string{"UPDATE_PASSWORD", "VERIFY_EMAIL"}, + "test-id", + []kcpkg.UserAction{ + kcpkg.UserActionUpdatePassword, + kcpkg.UserActionVerifyEmail, }, }, returns: []interface{}{ @@ -191,11 +131,11 @@ func TestBackend_SendMail(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - c := &mocks.GoCloak{} - for _, m := range tt.keycloakMocks { + c := &mocks.Client{} + for _, m := range tt.clientMocks { c.On(m.funcName, m.args...).Return(m.returns...) } - b := keycloak.NewWithClient(log.NopLogger(), c, clientID, clientSecret, clientRealm, userRealm) + b := keycloak.NewWithClient(log.NopLogger(), c, userRealm) tt.assertion(t, b.SendMail(ctx, tt.args.id)) }) } diff --git a/services/invitations/pkg/mocks/client.go b/services/invitations/pkg/mocks/client.go new file mode 100644 index 0000000000..eddaf9cec8 --- /dev/null +++ b/services/invitations/pkg/mocks/client.go @@ -0,0 +1,122 @@ +// Code generated by mockery v2.22.1. DO NOT EDIT. + +package mocks + +import ( + context "context" + + libregraph "github.com/owncloud/libre-graph-api-go" + keycloak "github.com/owncloud/ocis/v2/ocis-pkg/keycloak" + + mock "github.com/stretchr/testify/mock" +) + +// Client is an autogenerated mock type for the Client type +type Client struct { + mock.Mock +} + +// CreateUser provides a mock function with given fields: ctx, realm, user, userActions +func (_m *Client) CreateUser(ctx context.Context, realm string, user *libregraph.User, userActions []keycloak.UserAction) (string, error) { + ret := _m.Called(ctx, realm, user, userActions) + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, *libregraph.User, []keycloak.UserAction) (string, error)); ok { + return rf(ctx, realm, user, userActions) + } + if rf, ok := ret.Get(0).(func(context.Context, string, *libregraph.User, []keycloak.UserAction) string); ok { + r0 = rf(ctx, realm, user, userActions) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(context.Context, string, *libregraph.User, []keycloak.UserAction) error); ok { + r1 = rf(ctx, realm, user, userActions) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetPIIReport provides a mock function with given fields: ctx, realm, user +func (_m *Client) GetPIIReport(ctx context.Context, realm string, user *libregraph.User) (*keycloak.PIIReport, error) { + ret := _m.Called(ctx, realm, user) + + var r0 *keycloak.PIIReport + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, *libregraph.User) (*keycloak.PIIReport, error)); ok { + return rf(ctx, realm, user) + } + if rf, ok := ret.Get(0).(func(context.Context, string, *libregraph.User) *keycloak.PIIReport); ok { + r0 = rf(ctx, realm, user) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*keycloak.PIIReport) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string, *libregraph.User) error); ok { + r1 = rf(ctx, realm, user) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetUserByEmail provides a mock function with given fields: ctx, realm, email +func (_m *Client) GetUserByEmail(ctx context.Context, realm string, email string) (*libregraph.User, error) { + ret := _m.Called(ctx, realm, email) + + var r0 *libregraph.User + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, string) (*libregraph.User, error)); ok { + return rf(ctx, realm, email) + } + if rf, ok := ret.Get(0).(func(context.Context, string, string) *libregraph.User); ok { + r0 = rf(ctx, realm, email) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*libregraph.User) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { + r1 = rf(ctx, realm, email) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// SendActionsMail provides a mock function with given fields: ctx, realm, userID, userActions +func (_m *Client) SendActionsMail(ctx context.Context, realm string, userID string, userActions []keycloak.UserAction) error { + ret := _m.Called(ctx, realm, userID, userActions) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, string, string, []keycloak.UserAction) error); ok { + r0 = rf(ctx, realm, userID, userActions) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +type mockConstructorTestingTNewClient interface { + mock.TestingT + Cleanup(func()) +} + +// NewClient creates a new instance of Client. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewClient(t mockConstructorTestingTNewClient) *Client { + mock := &Client{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/services/invitations/pkg/mocks/go_cloak.go b/services/invitations/pkg/mocks/go_cloak.go deleted file mode 100644 index 6289046719..0000000000 --- a/services/invitations/pkg/mocks/go_cloak.go +++ /dev/null @@ -1,112 +0,0 @@ -// Code generated by mockery v2.14.1. DO NOT EDIT. - -package mocks - -import ( - context "context" - - gocloak "github.com/Nerzal/gocloak/v13" - - mock "github.com/stretchr/testify/mock" -) - -// GoCloak is an autogenerated mock type for the GoCloak type -type GoCloak struct { - mock.Mock -} - -// CreateUser provides a mock function with given fields: ctx, token, realm, user -func (_m *GoCloak) CreateUser(ctx context.Context, token string, realm string, user gocloak.User) (string, error) { - ret := _m.Called(ctx, token, realm, user) - - var r0 string - if rf, ok := ret.Get(0).(func(context.Context, string, string, gocloak.User) string); ok { - r0 = rf(ctx, token, realm, user) - } else { - r0 = ret.Get(0).(string) - } - - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, string, string, gocloak.User) error); ok { - r1 = rf(ctx, token, realm, user) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// ExecuteActionsEmail provides a mock function with given fields: ctx, token, realm, params -func (_m *GoCloak) ExecuteActionsEmail(ctx context.Context, token string, realm string, params gocloak.ExecuteActionsEmail) error { - ret := _m.Called(ctx, token, realm, params) - - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, string, string, gocloak.ExecuteActionsEmail) error); ok { - r0 = rf(ctx, token, realm, params) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// LoginClient provides a mock function with given fields: ctx, clientID, clientSecret, realm -func (_m *GoCloak) LoginClient(ctx context.Context, clientID string, clientSecret string, realm string) (*gocloak.JWT, error) { - ret := _m.Called(ctx, clientID, clientSecret, realm) - - var r0 *gocloak.JWT - if rf, ok := ret.Get(0).(func(context.Context, string, string, string) *gocloak.JWT); ok { - r0 = rf(ctx, clientID, clientSecret, realm) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*gocloak.JWT) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, string, string, string) error); ok { - r1 = rf(ctx, clientID, clientSecret, realm) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// RetrospectToken provides a mock function with given fields: ctx, accessToken, clientID, clientSecret, realm -func (_m *GoCloak) RetrospectToken(ctx context.Context, accessToken string, clientID string, clientSecret string, realm string) (*gocloak.IntroSpectTokenResult, error) { - ret := _m.Called(ctx, accessToken, clientID, clientSecret, realm) - - var r0 *gocloak.IntroSpectTokenResult - if rf, ok := ret.Get(0).(func(context.Context, string, string, string, string) *gocloak.IntroSpectTokenResult); ok { - r0 = rf(ctx, accessToken, clientID, clientSecret, realm) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*gocloak.IntroSpectTokenResult) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, string, string, string, string) error); ok { - r1 = rf(ctx, accessToken, clientID, clientSecret, realm) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -type mockConstructorTestingTNewGoCloak interface { - mock.TestingT - Cleanup(func()) -} - -// NewGoCloak creates a new instance of GoCloak. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewGoCloak(t mockConstructorTestingTNewGoCloak) *GoCloak { - mock := &GoCloak{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -}