mirror of
https://github.com/zitadel/zitadel.git
synced 2025-02-28 20:17:23 +00:00
fix(permissions): return current user when calling ListUsers() when user does not have permissions (#9374)
# Which Problems Are Solved When running `ListUsers()` with no permissions, the calling user shoud be returned # How the Problems Are Solved Added additional clause to SQL search statement # Additional Changes n/a # Additional Context - Closes https://github.com/zitadel/zitadel/issues/9355 --------- Co-authored-by: Iraq Jaber <IraqJaber@gmail.com>
This commit is contained in:
parent
93466055ee
commit
9aad207ee4
@ -17,6 +17,7 @@ import (
|
|||||||
|
|
||||||
"github.com/zitadel/zitadel/internal/integration"
|
"github.com/zitadel/zitadel/internal/integration"
|
||||||
"github.com/zitadel/zitadel/pkg/grpc/object/v2"
|
"github.com/zitadel/zitadel/pkg/grpc/object/v2"
|
||||||
|
"github.com/zitadel/zitadel/pkg/grpc/session/v2"
|
||||||
"github.com/zitadel/zitadel/pkg/grpc/user/v2"
|
"github.com/zitadel/zitadel/pkg/grpc/user/v2"
|
||||||
)
|
)
|
||||||
|
|
||||||
@ -384,7 +385,7 @@ func TestServer_ListUsers(t *testing.T) {
|
|||||||
wantErr bool
|
wantErr bool
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: "list user by id, no permission",
|
name: "list user by id, no permission machine user",
|
||||||
args: args{
|
args: args{
|
||||||
UserCTX,
|
UserCTX,
|
||||||
&user.ListUsersRequest{},
|
&user.ListUsersRequest{},
|
||||||
@ -403,6 +404,68 @@ func TestServer_ListUsers(t *testing.T) {
|
|||||||
Result: []*user.User{},
|
Result: []*user.User{},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "list user by id, no permission human user",
|
||||||
|
args: func() args {
|
||||||
|
info := createUser(IamCTX, orgResp.OrganizationId, true)
|
||||||
|
// create session to get token
|
||||||
|
userID := info.UserID
|
||||||
|
createResp, err := Instance.Client.SessionV2.CreateSession(IamCTX, &session.CreateSessionRequest{
|
||||||
|
Checks: &session.Checks{
|
||||||
|
User: &session.CheckUser{
|
||||||
|
Search: &session.CheckUser_UserId{UserId: userID},
|
||||||
|
},
|
||||||
|
Password: &session.CheckPassword{
|
||||||
|
Password: integration.UserPassword,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
// use token to get ctx
|
||||||
|
HumanCTX := integration.WithAuthorizationToken(IamCTX, createResp.GetSessionToken())
|
||||||
|
return args{
|
||||||
|
HumanCTX,
|
||||||
|
&user.ListUsersRequest{},
|
||||||
|
func(ctx context.Context, request *user.ListUsersRequest) userAttrs {
|
||||||
|
return []userAttr{info}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}(),
|
||||||
|
want: &user.ListUsersResponse{ // human user should return itself when calling ListUsers() even if it has no permissions
|
||||||
|
Details: &object.ListDetails{
|
||||||
|
TotalResult: 1,
|
||||||
|
Timestamp: timestamppb.Now(),
|
||||||
|
},
|
||||||
|
SortingColumn: 0,
|
||||||
|
Result: []*user.User{
|
||||||
|
{
|
||||||
|
State: user.UserState_USER_STATE_ACTIVE,
|
||||||
|
Type: &user.User_Human{
|
||||||
|
Human: &user.HumanUser{
|
||||||
|
Profile: &user.HumanProfile{
|
||||||
|
GivenName: "Mickey",
|
||||||
|
FamilyName: "Mouse",
|
||||||
|
NickName: gu.Ptr("Mickey"),
|
||||||
|
DisplayName: gu.Ptr("Mickey Mouse"),
|
||||||
|
PreferredLanguage: gu.Ptr("nl"),
|
||||||
|
Gender: user.Gender_GENDER_MALE.Enum(),
|
||||||
|
},
|
||||||
|
Email: &user.HumanEmail{
|
||||||
|
IsVerified: true,
|
||||||
|
},
|
||||||
|
Phone: &user.HumanPhone{
|
||||||
|
IsVerified: true,
|
||||||
|
},
|
||||||
|
PasswordChangeRequired: true,
|
||||||
|
PasswordChanged: timestamppb.Now(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "list user by id, ok",
|
name: "list user by id, ok",
|
||||||
args: args{
|
args: args{
|
||||||
|
@ -18,6 +18,7 @@ import (
|
|||||||
"github.com/zitadel/zitadel/internal/integration"
|
"github.com/zitadel/zitadel/internal/integration"
|
||||||
"github.com/zitadel/zitadel/pkg/grpc/object/v2"
|
"github.com/zitadel/zitadel/pkg/grpc/object/v2"
|
||||||
object_v2beta "github.com/zitadel/zitadel/pkg/grpc/object/v2beta"
|
object_v2beta "github.com/zitadel/zitadel/pkg/grpc/object/v2beta"
|
||||||
|
"github.com/zitadel/zitadel/pkg/grpc/session/v2"
|
||||||
user "github.com/zitadel/zitadel/pkg/grpc/user/v2beta"
|
user "github.com/zitadel/zitadel/pkg/grpc/user/v2beta"
|
||||||
)
|
)
|
||||||
|
|
||||||
@ -394,7 +395,7 @@ func TestServer_ListUsers(t *testing.T) {
|
|||||||
wantErr bool
|
wantErr bool
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: "list user by id, no permission",
|
name: "list user by id, no permission machine user",
|
||||||
args: args{
|
args: args{
|
||||||
UserCTX,
|
UserCTX,
|
||||||
&user.ListUsersRequest{},
|
&user.ListUsersRequest{},
|
||||||
@ -413,6 +414,68 @@ func TestServer_ListUsers(t *testing.T) {
|
|||||||
Result: []*user.User{},
|
Result: []*user.User{},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "list user by id, no permission human user",
|
||||||
|
args: func() args {
|
||||||
|
info := createUser(IamCTX, orgResp.OrganizationId, true)
|
||||||
|
// create session to get token
|
||||||
|
userID := info.UserID
|
||||||
|
createResp, err := Instance.Client.SessionV2.CreateSession(IamCTX, &session.CreateSessionRequest{
|
||||||
|
Checks: &session.Checks{
|
||||||
|
User: &session.CheckUser{
|
||||||
|
Search: &session.CheckUser_UserId{UserId: userID},
|
||||||
|
},
|
||||||
|
Password: &session.CheckPassword{
|
||||||
|
Password: integration.UserPassword,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
// use token to get ctx
|
||||||
|
HumanCTX := integration.WithAuthorizationToken(IamCTX, createResp.GetSessionToken())
|
||||||
|
return args{
|
||||||
|
HumanCTX,
|
||||||
|
&user.ListUsersRequest{},
|
||||||
|
func(ctx context.Context, request *user.ListUsersRequest) userAttrs {
|
||||||
|
return []userAttr{info}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}(),
|
||||||
|
want: &user.ListUsersResponse{ // human user should return itself when calling ListUsers() even if it has no permissions
|
||||||
|
Details: &object_v2beta.ListDetails{
|
||||||
|
TotalResult: 1,
|
||||||
|
Timestamp: timestamppb.Now(),
|
||||||
|
},
|
||||||
|
SortingColumn: 0,
|
||||||
|
Result: []*user.User{
|
||||||
|
{
|
||||||
|
State: user.UserState_USER_STATE_ACTIVE,
|
||||||
|
Type: &user.User_Human{
|
||||||
|
Human: &user.HumanUser{
|
||||||
|
Profile: &user.HumanProfile{
|
||||||
|
GivenName: "Mickey",
|
||||||
|
FamilyName: "Mouse",
|
||||||
|
NickName: gu.Ptr("Mickey"),
|
||||||
|
DisplayName: gu.Ptr("Mickey Mouse"),
|
||||||
|
PreferredLanguage: gu.Ptr("nl"),
|
||||||
|
Gender: user.Gender_GENDER_MALE.Enum(),
|
||||||
|
},
|
||||||
|
Email: &user.HumanEmail{
|
||||||
|
IsVerified: true,
|
||||||
|
},
|
||||||
|
Phone: &user.HumanPhone{
|
||||||
|
IsVerified: true,
|
||||||
|
},
|
||||||
|
PasswordChangeRequired: true,
|
||||||
|
PasswordChanged: timestamppb.Now(),
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
{
|
{
|
||||||
name: "list user by id, ok",
|
name: "list user by id, ok",
|
||||||
args: args{
|
args: args{
|
||||||
|
@ -12,7 +12,8 @@ import (
|
|||||||
|
|
||||||
const (
|
const (
|
||||||
// eventstore.permitted_orgs(instanceid text, userid text, perm text, filter_orgs text)
|
// eventstore.permitted_orgs(instanceid text, userid text, perm text, filter_orgs text)
|
||||||
wherePermittedOrgsClause = "%s = ANY(eventstore.permitted_orgs(?, ?, ?, ?))"
|
wherePermittedOrgsClause = "%s = ANY(eventstore.permitted_orgs(?, ?, ?, ?))"
|
||||||
|
wherePermittedOrgsOrCurrentUserClause = "(" + wherePermittedOrgsClause + " OR %s = ?" + ")"
|
||||||
)
|
)
|
||||||
|
|
||||||
// wherePermittedOrgs sets a `WHERE` clause to the query that filters the orgs
|
// wherePermittedOrgs sets a `WHERE` clause to the query that filters the orgs
|
||||||
@ -35,3 +36,17 @@ func wherePermittedOrgs(ctx context.Context, query sq.SelectBuilder, filterOrgId
|
|||||||
filterOrgIds,
|
filterOrgIds,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func wherePermittedOrgsOrCurrentUser(ctx context.Context, query sq.SelectBuilder, filterOrgIds, orgIDColumn, userIdColum, permission string) sq.SelectBuilder {
|
||||||
|
userID := authz.GetCtxData(ctx).UserID
|
||||||
|
logging.WithFields("permission_check_v2_flag", authz.GetFeatures(ctx).PermissionCheckV2, "org_id_column", orgIDColumn, "user_id_colum", userIdColum, "permission", permission, "user_id", userID).Debug("permitted orgs check used")
|
||||||
|
|
||||||
|
return query.Where(
|
||||||
|
fmt.Sprintf(wherePermittedOrgsOrCurrentUserClause, orgIDColumn, userIdColum),
|
||||||
|
authz.GetInstance(ctx).InstanceID(),
|
||||||
|
userID,
|
||||||
|
permission,
|
||||||
|
filterOrgIds,
|
||||||
|
userID,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
@ -655,7 +655,7 @@ func (q *Queries) searchUsers(ctx context.Context, queries *UserSearchQueries, f
|
|||||||
UserInstanceIDCol.identifier(): authz.GetInstance(ctx).InstanceID(),
|
UserInstanceIDCol.identifier(): authz.GetInstance(ctx).InstanceID(),
|
||||||
})
|
})
|
||||||
if permissionCheckV2 {
|
if permissionCheckV2 {
|
||||||
query = wherePermittedOrgs(ctx, query, filterOrgIds, UserResourceOwnerCol.identifier(), domain.PermissionUserRead)
|
query = wherePermittedOrgsOrCurrentUser(ctx, query, filterOrgIds, UserResourceOwnerCol.identifier(), UserIDCol.identifier(), domain.PermissionUserRead)
|
||||||
}
|
}
|
||||||
|
|
||||||
stmt, args, err := query.ToSql()
|
stmt, args, err := query.ToSql()
|
||||||
|
Loading…
x
Reference in New Issue
Block a user