From 7555aeee0919629a7714c3fcdda37e4416648dd7 Mon Sep 17 00:00:00 2001 From: sispeo <42068883+fperot74@users.noreply.github.com> Date: Fri, 27 Mar 2020 15:54:11 +0100 Subject: [PATCH] [CLOUDTRUST-2438] Remove parameter groupIDs from GET /kyc/users --- api/kyc/swagger-api_kyc.yaml | 6 ----- pkg/kyc/authorization.go | 18 +++++++-------- pkg/kyc/authorization_test.go | 11 +++++----- pkg/kyc/component.go | 34 +++++++++++++++++++++++------ pkg/kyc/component_test.go | 41 +++++++++++++++++++++++++++++------ pkg/kyc/endpoint.go | 9 +------- pkg/kyc/endpoint_test.go | 15 +++---------- 7 files changed, 78 insertions(+), 56 deletions(-) diff --git a/api/kyc/swagger-api_kyc.yaml b/api/kyc/swagger-api_kyc.yaml index ab3adf16..3c0a613c 100644 --- a/api/kyc/swagger-api_kyc.yaml +++ b/api/kyc/swagger-api_kyc.yaml @@ -41,12 +41,6 @@ paths: required: true schema: type: string - - name: groupIds - in: query - description: list of groupId the users may belong to (list comma seperated) - required: true - schema: - type: string responses: 200: description: Successful operation. Returns the generated username diff --git a/pkg/kyc/authorization.go b/pkg/kyc/authorization.go index f728ef2e..372a98b3 100644 --- a/pkg/kyc/authorization.go +++ b/pkg/kyc/authorization.go @@ -25,7 +25,7 @@ func newAction(as string, scope security.Scope) security.Action { var ( KYCGetActions = newAction("KYC_GetActions", security.ScopeGlobal) KYCGetUser = newAction("KYC_GetUser", security.ScopeGroup) - KYCGetUserByUsername = newAction("KYC_GetUserByUsername", security.ScopeGroup) + KYCGetUserByUsername = newAction("KYC_GetUserByUsername", security.ScopeRealm) KYCValidateUser = newAction("KYC_ValidateUser", security.ScopeGroup) ) @@ -63,22 +63,20 @@ func (c *authorizationComponentMW) GetActions(ctx context.Context) ([]apikyc.Act return c.next.GetActions(ctx) } -func (c *authorizationComponentMW) GetUserByUsername(ctx context.Context, username string, groupIDs []string) (apikyc.UserRepresentation, error) { +func (c *authorizationComponentMW) GetUserByUsername(ctx context.Context, username string) (apikyc.UserRepresentation, error) { var action = KYCGetUserByUsername.String() - var targetRealm = c.realmName + var targetRealm = ctx.Value(cs.CtContextRealm).(string) - for _, groupID := range groupIDs { - if err := c.authManager.CheckAuthorizationOnTargetGroupID(ctx, action, targetRealm, groupID); err != nil { - return apikyc.UserRepresentation{}, err - } + if err := c.authManager.CheckAuthorizationOnTargetRealm(ctx, action, targetRealm); err != nil { + return apikyc.UserRepresentation{}, err } - return c.next.GetUserByUsername(ctx, username, groupIDs) + return c.next.GetUserByUsername(ctx, username) } func (c *authorizationComponentMW) GetUser(ctx context.Context, userID string) (apikyc.UserRepresentation, error) { var action = KYCGetUser.String() - var targetRealm = c.realmName + var targetRealm = ctx.Value(cs.CtContextRealm).(string) if err := c.authManager.CheckAuthorizationOnTargetUser(ctx, action, targetRealm, userID); err != nil { return apikyc.UserRepresentation{}, err @@ -89,7 +87,7 @@ func (c *authorizationComponentMW) GetUser(ctx context.Context, userID string) ( func (c *authorizationComponentMW) ValidateUser(ctx context.Context, userID string, user apikyc.UserRepresentation) error { var action = KYCValidateUser.String() - var targetRealm = c.realmName + var targetRealm = ctx.Value(cs.CtContextRealm).(string) if err := c.authManager.CheckAuthorizationOnTargetUser(ctx, action, targetRealm, userID); err != nil { return err diff --git a/pkg/kyc/authorization_test.go b/pkg/kyc/authorization_test.go index 87910278..f1ab8fdb 100644 --- a/pkg/kyc/authorization_test.go +++ b/pkg/kyc/authorization_test.go @@ -24,7 +24,6 @@ func TestMakeAuthorizationRegisterComponentMW(t *testing.T) { var ctx = context.WithValue(context.Background(), cs.CtContextRealm, realm) var user = apikyc.UserRepresentation{} var userID = "user4673" - var groupIDs = []string{"group1", "group2"} var username = "username" var expectedErr = errors.New("") @@ -57,16 +56,16 @@ func TestMakeAuthorizationRegisterComponentMW(t *testing.T) { }) t.Run("GetUserByUsername - not authorized", func(t *testing.T) { - mockAuthManager.EXPECT().CheckAuthorizationOnTargetGroupID(ctx, KYCGetUserByUsername.String(), realm, gomock.Any()). + mockAuthManager.EXPECT().CheckAuthorizationOnTargetRealm(ctx, KYCGetUserByUsername.String(), realm). Return(nil).Return(expectedErr) - var _, err = component.GetUserByUsername(ctx, username, groupIDs) + var _, err = component.GetUserByUsername(ctx, username) assert.Equal(t, expectedErr, err) }) t.Run("GetUserByUsername - authorized", func(t *testing.T) { - mockAuthManager.EXPECT().CheckAuthorizationOnTargetGroupID(ctx, KYCGetUserByUsername.String(), realm, gomock.Any()).Return(nil).Times(2) - mockComponent.EXPECT().GetUserByUsername(ctx, username, groupIDs).Return(apikyc.UserRepresentation{}, expectedErr).Times(1) - var _, err = component.GetUserByUsername(ctx, username, groupIDs) + mockAuthManager.EXPECT().CheckAuthorizationOnTargetRealm(ctx, KYCGetUserByUsername.String(), realm).Return(nil) + mockComponent.EXPECT().GetUserByUsername(ctx, username).Return(apikyc.UserRepresentation{}, expectedErr).Times(1) + var _, err = component.GetUserByUsername(ctx, username) assert.Equal(t, expectedErr, err) }) diff --git a/pkg/kyc/component.go b/pkg/kyc/component.go index f4e26534..30d79eb4 100644 --- a/pkg/kyc/component.go +++ b/pkg/kyc/component.go @@ -5,6 +5,7 @@ import ( "time" "github.com/cloudtrust/common-service/configuration" + "github.com/cloudtrust/keycloak-client" cs "github.com/cloudtrust/common-service" "github.com/cloudtrust/common-service/database" @@ -22,6 +23,7 @@ type KeycloakClient interface { UpdateUser(accessToken string, realmName, userID string, user kc.UserRepresentation) error GetUser(accessToken string, realmName, userID string) (kc.UserRepresentation, error) GetUsers(accessToken string, reqRealmName, targetRealmName string, paramKV ...string) (kc.UsersPageRepresentation, error) + GetGroups(accessToken string, realmName string) ([]kc.GroupRepresentation, error) } // UsersDBModule is the interface from the users module @@ -41,7 +43,7 @@ type EventsDBModule interface { type Component interface { GetActions(ctx context.Context) ([]apikyc.ActionRepresentation, error) GetUser(ctx context.Context, userID string) (apikyc.UserRepresentation, error) - GetUserByUsername(ctx context.Context, username string, groupIDs []string) (apikyc.UserRepresentation, error) + GetUserByUsername(ctx context.Context, username string) (apikyc.UserRepresentation, error) ValidateUser(ctx context.Context, userID string, user apikyc.UserRepresentation) error } @@ -91,12 +93,17 @@ func (c *component) GetActions(ctx context.Context) ([]apikyc.ActionRepresentati return apiActions, nil } -func (c *component) GetUserByUsername(ctx context.Context, username string, _ []string) (apikyc.UserRepresentation, error) { +func (c *component) GetUserByUsername(ctx context.Context, username string) (apikyc.UserRepresentation, error) { var accessToken = ctx.Value(cs.CtContextAccessToken).(string) - var kcUser, err = c.getUserByUsername(accessToken, c.socialRealmName, c.socialRealmName, username) + var group, err = c.getGroupByName(accessToken, c.socialRealmName, "end_user") + if err != nil { + return apikyc.UserRepresentation{}, err + } + var kcUser keycloak.UserRepresentation + kcUser, err = c.getUserByUsername(accessToken, c.socialRealmName, c.socialRealmName, username, *group.Id) if err != nil { c.logger.Info(ctx, "msg", "GetUser: can't find user in Keycloak", "err", err.Error()) - return apikyc.UserRepresentation{}, errorhandler.CreateInternalServerError("keycloak") + return apikyc.UserRepresentation{}, err } keycloakb.ConvertLegacyAttribute(&kcUser) return c.getUser(ctx, *kcUser.Id, kcUser) @@ -228,13 +235,26 @@ func (c *component) ValidateUser(ctx context.Context, userID string, user apikyc return nil } -func (c *component) getUserByUsername(accessToken, reqRealmName, targetRealmName, username string) (kc.UserRepresentation, error) { - var kcUsers, err = c.keycloakClient.GetUsers(accessToken, reqRealmName, targetRealmName, "username", username) +func (c *component) getGroupByName(accessToken, realmName, groupName string) (kc.GroupRepresentation, error) { + var groups, err = c.keycloakClient.GetGroups(accessToken, realmName) + if err != nil { + return kc.GroupRepresentation{}, err + } + for _, grp := range groups { + if *grp.Name == groupName { + return grp, nil + } + } + return kc.GroupRepresentation{}, errorhandler.CreateNotFoundError("group") +} + +func (c *component) getUserByUsername(accessToken, reqRealmName, targetRealmName, username, groupID string) (kc.UserRepresentation, error) { + var kcUsers, err = c.keycloakClient.GetUsers(accessToken, reqRealmName, targetRealmName, "username", username, "groupId", groupID) if err != nil { return kc.UserRepresentation{}, errorhandler.CreateInternalServerError("keycloak") } if kcUsers.Count == nil || *kcUsers.Count != 1 || kcUsers.Users[0].Username == nil || *kcUsers.Users[0].Username != username { - return kc.UserRepresentation{}, errorhandler.CreateNotFoundError("username") + return kc.UserRepresentation{}, errorhandler.CreateNotFoundError("user") } var res = kcUsers.Users[0] diff --git a/pkg/kyc/component_test.go b/pkg/kyc/component_test.go index 019432aa..6c022e66 100644 --- a/pkg/kyc/component_test.go +++ b/pkg/kyc/component_test.go @@ -75,36 +75,63 @@ func TestGetUserByUsernameComponent(t *testing.T) { var realm = "my-realm" var username = "utr167x" var userID = "1234567890" - var groupIDs = []string{"not", "used", "values"} + var grpEndUserID = "11111-22222" + var grpEndUserName = "end_user" + var grpOtherID = "33333-44444" + var grpOtherName = "other_group" var kcUser = kc.UserRepresentation{ Id: &userID, Username: &username, } + var kcGroup1 = kc.GroupRepresentation{ + Id: &grpOtherID, + Name: &grpOtherName, + } + var kcGroup2 = kc.GroupRepresentation{ + Id: &grpEndUserID, + Name: &grpEndUserName, + } var one = 1 var kcUsersSearch = kc.UsersPageRepresentation{Count: &one, Users: []kc.UserRepresentation{kcUser}} + var kcGroupSearch = []kc.GroupRepresentation{kcGroup1, kcGroup2} var ctx = context.WithValue(context.Background(), cs.CtContextAccessToken, accessToken) var component = NewComponent(realm, mockKeycloakClient, mockUsersDB, mockEventsDB, mockAccreditations, log.NewNopLogger()) + t.Run("GetGroups from Keycloak fails", func(t *testing.T) { + var kcError = errors.New("kc error") + mockKeycloakClient.EXPECT().GetGroups(accessToken, realm).Return(kcGroupSearch, kcError) + var _, err = component.GetUserByUsername(ctx, username) + assert.NotNil(t, err) + }) + t.Run("GetGroups: unknown group", func(t *testing.T) { + mockKeycloakClient.EXPECT().GetGroups(accessToken, realm).Return([]kc.GroupRepresentation{}, nil) + var _, err = component.GetUserByUsername(ctx, username) + assert.NotNil(t, err) + }) + t.Run("GetUserByUsername from Keycloak fails", func(t *testing.T) { var kcError = errors.New("kc error") - mockKeycloakClient.EXPECT().GetUsers(accessToken, realm, realm, "username", username).Return(kcUsersSearch, kcError) - var _, err = component.GetUserByUsername(ctx, username, groupIDs) + mockKeycloakClient.EXPECT().GetGroups(accessToken, realm).Return(kcGroupSearch, nil) + mockKeycloakClient.EXPECT().GetUsers(accessToken, realm, realm, "username", username, "groupId", grpEndUserID).Return(kcUsersSearch, kcError) + var _, err = component.GetUserByUsername(ctx, username) assert.NotNil(t, err) }) t.Run("GetUserByUsername from Keycloak fails", func(t *testing.T) { var none = 0 var searchNoResult = kc.UsersPageRepresentation{Count: &none, Users: []kc.UserRepresentation{}} - mockKeycloakClient.EXPECT().GetUsers(accessToken, realm, realm, "username", username).Return(searchNoResult, nil) - var _, err = component.GetUserByUsername(ctx, username, groupIDs) + mockKeycloakClient.EXPECT().GetGroups(accessToken, realm).Return(kcGroupSearch, nil) + mockKeycloakClient.EXPECT().GetUsers(accessToken, realm, realm, "username", username, "groupId", grpEndUserID).Return(searchNoResult, nil) + var _, err = component.GetUserByUsername(ctx, username) assert.NotNil(t, err) }) t.Run("GetUserByUsername success", func(t *testing.T) { - mockKeycloakClient.EXPECT().GetUsers(accessToken, realm, realm, "username", username).Return(kcUsersSearch, nil) + mockKeycloakClient.EXPECT().GetGroups(accessToken, realm).Return(kcGroupSearch, nil) + mockKeycloakClient.EXPECT().GetUsers(accessToken, realm, realm, "username", username, "groupId", grpEndUserID).Return(kcUsersSearch, nil) mockUsersDB.EXPECT().GetUser(ctx, realm, *kcUser.Id).Return(&dto.DBUser{}, nil) - var user, err = component.GetUserByUsername(ctx, username, groupIDs) + var user, err = component.GetUserByUsername(ctx, username) assert.Nil(t, err) assert.NotNil(t, user) }) diff --git a/pkg/kyc/endpoint.go b/pkg/kyc/endpoint.go index bac8e3a6..06f2e929 100644 --- a/pkg/kyc/endpoint.go +++ b/pkg/kyc/endpoint.go @@ -2,7 +2,6 @@ package kyc import ( "context" - "strings" cs "github.com/cloudtrust/common-service" commonerrors "github.com/cloudtrust/common-service/errors" @@ -32,13 +31,7 @@ func MakeGetUserByUsernameEndpoint(component Component) cs.Endpoint { var m = req.(map[string]string) var user = m["username"] - _, ok := m["groupIds"] - if !ok { - return nil, commonerrors.CreateMissingParameterError(msg.GroupIDs) - } - groupIDs := strings.Split(m["groupIds"], ",") - - return component.GetUserByUsername(ctx, user, groupIDs) + return component.GetUserByUsername(ctx, user) } } diff --git a/pkg/kyc/endpoint_test.go b/pkg/kyc/endpoint_test.go index 51c2d80b..ef2e676e 100644 --- a/pkg/kyc/endpoint_test.go +++ b/pkg/kyc/endpoint_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "errors" - "strings" "testing" apikyc "github.com/cloudtrust/keycloak-bridge/api/kyc" @@ -67,28 +66,20 @@ func TestMakeGetUserByUsernameEndpoint(t *testing.T) { var realm = "master" var username = "user1234" - var groupIDList = "group1,group2,group3" - var groupIDs = strings.Split(groupIDList, ",") - var m = map[string]string{"realm": realm, "username": username, "groupIds": groupIDList} + var m = map[string]string{"realm": realm, "username": username} var expectedError = errors.New("get-user") t.Run("GetUserByUsername - success case", func(t *testing.T) { - mockKYCComponent.EXPECT().GetUserByUsername(gomock.Any(), username, groupIDs).Return(apikyc.UserRepresentation{}, nil) + mockKYCComponent.EXPECT().GetUserByUsername(gomock.Any(), username).Return(apikyc.UserRepresentation{}, nil) _, err := MakeGetUserByUsernameEndpoint(mockKYCComponent)(context.Background(), m) assert.Nil(t, err) }) t.Run("GetUserByUsername - failure case", func(t *testing.T) { - mockKYCComponent.EXPECT().GetUserByUsername(gomock.Any(), username, groupIDs).Return(apikyc.UserRepresentation{}, expectedError) + mockKYCComponent.EXPECT().GetUserByUsername(gomock.Any(), username).Return(apikyc.UserRepresentation{}, expectedError) _, err := MakeGetUserByUsernameEndpoint(mockKYCComponent)(context.Background(), m) assert.Equal(t, expectedError, err) }) - - t.Run("GetUserByUsername - missing groupIDs", func(t *testing.T) { - var missingGroupIDs = map[string]string{"realm": realm, "username": username} - _, err := MakeGetUserByUsernameEndpoint(mockKYCComponent)(context.Background(), missingGroupIDs) - assert.NotNil(t, err) - }) } func TestMakeValidateUserEndpoint(t *testing.T) {