Skip to content

Commit

Permalink
[CLOUDTRUST-2438] Remove parameter groupIDs from GET /kyc/users
Browse files Browse the repository at this point in the history
  • Loading branch information
@fperot74
fperot74 committed Mar 27, 2020
1 parent e577abb commit 7555aee
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 56 deletions.
6 changes: 0 additions & 6 deletions api/kyc/swagger-api_kyc.yaml
View file
Expand Up @@ -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
Expand Down
18 changes: 8 additions & 10 deletions pkg/kyc/authorization.go
View file
Expand Up @@ -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)
)

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
11 changes: 5 additions & 6 deletions pkg/kyc/authorization_test.go
View file
Expand Up @@ -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("")

Expand Down Expand Up @@ -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)
})

Expand Down
34 changes: 27 additions & 7 deletions pkg/kyc/component.go
View file
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand Down
41 changes: 34 additions & 7 deletions pkg/kyc/component_test.go
View file
Expand Up @@ -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)
})
Expand Down
9 changes: 1 addition & 8 deletions pkg/kyc/endpoint.go
View file
Expand Up @@ -2,7 +2,6 @@ package kyc

import (
"context"
"strings"

cs "github.com/cloudtrust/common-service"
commonerrors "github.com/cloudtrust/common-service/errors"
Expand Down Expand Up @@ -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)
}
}

Expand Down
15 changes: 3 additions & 12 deletions pkg/kyc/endpoint_test.go
View file
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"encoding/json"
"errors"
"strings"
"testing"

apikyc "github.com/cloudtrust/keycloak-bridge/api/kyc"
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 7555aee

Please sign in to comment.