From 30edab9f693216426b2fa1aea7ad1ded114c4bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20Franke?= Date: Mon, 17 Apr 2023 17:03:05 +0200 Subject: [PATCH] Search users by username instead of by email As some setups don't have email addresses setup or reuse email addresses, the keycloak search has to be done by username as that is guaranteed to always be unique and defined. This PR changes that. --- ocis-pkg/keycloak/client.go | 30 +++++--- ocis-pkg/keycloak/types.go | 4 +- services/graph/pkg/service/v0/personaldata.go | 2 +- services/invitations/pkg/mocks/client.go | 68 +++++++++++++++++-- 4 files changed, 86 insertions(+), 18 deletions(-) diff --git a/ocis-pkg/keycloak/client.go b/ocis-pkg/keycloak/client.go index f88f2b142..e01faa2ce 100644 --- a/ocis-pkg/keycloak/client.go +++ b/ocis-pkg/keycloak/client.go @@ -90,34 +90,46 @@ func (c *ConcreteClient) SendActionsMail(ctx context.Context, realm, userID stri return c.keycloak.ExecuteActionsEmail(ctx, token.AccessToken, realm, params) } -// GetUserByEmail looks up a user by email. -func (c *ConcreteClient) GetUserByEmail(ctx context.Context, realm, mail string) (*libregraph.User, error) { +// GetUserByParams looks up a user by the given parameters. +func (c *ConcreteClient) GetUserByParams(ctx context.Context, realm string, params gocloak.GetUsersParams) (*libregraph.User, error) { token, err := c.getToken(ctx) if err != nil { return nil, err } - users, err := c.keycloak.GetUsers(ctx, token.AccessToken, realm, gocloak.GetUsersParams{ - Email: &mail, - }) + users, err := c.keycloak.GetUsers(ctx, token.AccessToken, realm, params) if err != nil { return nil, err } if len(users) == 0 { - return nil, fmt.Errorf("no users found with mail address %s", mail) + return nil, fmt.Errorf("no users found") } if len(users) > 1 { - return nil, fmt.Errorf("%d users found with mail address %s, expected 1", len(users), mail) + return nil, fmt.Errorf("%d users found", len(users)) } return c.keycloakUserToLibregraph(users[0]), nil } +// GetUserByEmail looks up a user by email. +func (c *ConcreteClient) GetUserByEmail(ctx context.Context, realm, mail string) (*libregraph.User, error) { + return c.GetUserByParams(ctx, realm, gocloak.GetUsersParams{ + Email: &mail, + }) +} + +// GetUserByUsername looks up a user by username. +func (c *ConcreteClient) GetUserByUsername(ctx context.Context, realm, username string) (*libregraph.User, error) { + return c.GetUserByParams(ctx, realm, gocloak.GetUsersParams{ + Username: &username, + }) +} + // GetPIIReport returns a structure with all the PII for the user. -func (c *ConcreteClient) GetPIIReport(ctx context.Context, realm string, email string) (*PIIReport, error) { - u, err := c.GetUserByEmail(ctx, realm, email) +func (c *ConcreteClient) GetPIIReport(ctx context.Context, realm, username string) (*PIIReport, error) { + u, err := c.GetUserByUsername(ctx, realm, username) if err != nil { return nil, err } diff --git a/ocis-pkg/keycloak/types.go b/ocis-pkg/keycloak/types.go index 901893823..f51a583bd 100644 --- a/ocis-pkg/keycloak/types.go +++ b/ocis-pkg/keycloak/types.go @@ -34,6 +34,8 @@ type PIIReport struct { 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 + GetUserByParams(ctx context.Context, realm string, params gocloak.GetUsersParams) (*libregraph.User, error) GetUserByEmail(ctx context.Context, realm, email string) (*libregraph.User, error) - GetPIIReport(ctx context.Context, realm string, email string) (*PIIReport, error) + GetUserByUsername(ctx context.Context, realm, username string) (*libregraph.User, error) + GetPIIReport(ctx context.Context, realm, username string) (*PIIReport, error) } diff --git a/services/graph/pkg/service/v0/personaldata.go b/services/graph/pkg/service/v0/personaldata.go index 2017c6031..474e00f78 100644 --- a/services/graph/pkg/service/v0/personaldata.go +++ b/services/graph/pkg/service/v0/personaldata.go @@ -90,7 +90,7 @@ func (g Graph) GatherPersonalData(usr *user.User, ref *provider.Reference, token // Check if we have a keycloak client, and if so, get the keycloak export. if ctx != nil && g.keycloakClient != nil { - kcd, err := g.keycloakClient.GetPIIReport(ctx, g.config.Keycloak.UserRealm, usr.GetMail()) + kcd, err := g.keycloakClient.GetPIIReport(ctx, g.config.Keycloak.UserRealm, usr.GetUsername()) if err != nil { g.logger.Error().Err(err).Str("userID", usr.GetId().GetOpaqueId()).Msg("cannot get keycloak personal data") } diff --git a/services/invitations/pkg/mocks/client.go b/services/invitations/pkg/mocks/client.go index 42efdfbf0..82c9c811e 100644 --- a/services/invitations/pkg/mocks/client.go +++ b/services/invitations/pkg/mocks/client.go @@ -5,9 +5,11 @@ package mocks import ( context "context" - libregraph "github.com/owncloud/libre-graph-api-go" + gocloak "github.com/Nerzal/gocloak/v13" keycloak "github.com/owncloud/ocis/v2/ocis-pkg/keycloak" + libregraph "github.com/owncloud/libre-graph-api-go" + mock "github.com/stretchr/testify/mock" ) @@ -40,17 +42,17 @@ func (_m *Client) CreateUser(ctx context.Context, realm string, user *libregraph return r0, r1 } -// GetPIIReport provides a mock function with given fields: ctx, realm, email -func (_m *Client) GetPIIReport(ctx context.Context, realm string, email string) (*keycloak.PIIReport, error) { - ret := _m.Called(ctx, realm, email) +// GetPIIReport provides a mock function with given fields: ctx, realm, username +func (_m *Client) GetPIIReport(ctx context.Context, realm string, username string) (*keycloak.PIIReport, error) { + ret := _m.Called(ctx, realm, username) var r0 *keycloak.PIIReport var r1 error if rf, ok := ret.Get(0).(func(context.Context, string, string) (*keycloak.PIIReport, error)); ok { - return rf(ctx, realm, email) + return rf(ctx, realm, username) } if rf, ok := ret.Get(0).(func(context.Context, string, string) *keycloak.PIIReport); ok { - r0 = rf(ctx, realm, email) + r0 = rf(ctx, realm, username) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*keycloak.PIIReport) @@ -58,7 +60,7 @@ func (_m *Client) GetPIIReport(ctx context.Context, realm string, email string) } if rf, ok := ret.Get(1).(func(context.Context, string, string) error); ok { - r1 = rf(ctx, realm, email) + r1 = rf(ctx, realm, username) } else { r1 = ret.Error(1) } @@ -92,6 +94,58 @@ func (_m *Client) GetUserByEmail(ctx context.Context, realm string, email string return r0, r1 } +// GetUserByParams provides a mock function with given fields: ctx, realm, params +func (_m *Client) GetUserByParams(ctx context.Context, realm string, params gocloak.GetUsersParams) (*libregraph.User, error) { + ret := _m.Called(ctx, realm, params) + + var r0 *libregraph.User + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, string, gocloak.GetUsersParams) (*libregraph.User, error)); ok { + return rf(ctx, realm, params) + } + if rf, ok := ret.Get(0).(func(context.Context, string, gocloak.GetUsersParams) *libregraph.User); ok { + r0 = rf(ctx, realm, params) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*libregraph.User) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, string, gocloak.GetUsersParams) error); ok { + r1 = rf(ctx, realm, params) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetUserByUsername provides a mock function with given fields: ctx, realm, username +func (_m *Client) GetUserByUsername(ctx context.Context, realm string, username string) (*libregraph.User, error) { + ret := _m.Called(ctx, realm, username) + + 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, username) + } + if rf, ok := ret.Get(0).(func(context.Context, string, string) *libregraph.User); ok { + r0 = rf(ctx, realm, username) + } 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, username) + } 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)