66, 9, 44 role based access management & authorization #76
Conversation
dominic22
changed the title
| location: Type.Optional(Type.Union([LocationSchema, Type.Null()])), | ||
| organizationUrl: Type.Optional(Type.Union([Type.String(), Type.Null()])), | ||
| verifiableCredentials: Type.Optional(Type.Union([Type.Array(VerifiableCredentialSchema), Type.Null()])), | ||
| role: Type.Optional(Type.Union([Type.String(), Type.Null()])) |
There was a problem hiding this comment.
Is it possible that a user will have multiple roles? Maybe it is reasonable to change it into a list or a map with key:value pairs?
There was a problem hiding this comment.
for now it is only these roles, we don't have really requirements here I just needed some mechanism to be able to delete other user as an admin via api
|
|
||
| export interface UserPersistence extends OmittedUser { | ||
| role?: UserRoles; |
There was a problem hiding this comment.
role reads like a single role, while UserRoles is a plural
There was a problem hiding this comment.
yes because UserRoles is an enum which is mainly written in plural :/
| isAuthorizedToRevoke = async (kci: LinkedKeyCollectionIdentityPersistence, requestUser: User): Promise<AuthorizationCheck> => { | ||
| const isAuthorizedUser = this.authorizationService.isAuthorizedUser(requestUser.userId, kci.linkedIdentity); | ||
| const isAuthorizedInitiator = this.authorizationService.isAuthorizedUser(requestUser.userId, kci.initiatorId); | ||
| if (!isAuthorizedUser && !isAuthorizedInitiator) { |
There was a problem hiding this comment.
Should the condition resolve if both variables are false at the same time, or just if one of them is false?
Also, should it be && or || ?
There was a problem hiding this comment.
yes only if the requester is not the user itself or the initiator it shall go into the admin check. This avoids to query the db everytime an authorization check will be done. By thinking about that I could also add some package which does memorization for function calls to avoid unnecessary db calls... I will add an issue for that.
| return { isAuthorized: true, error: null }; | ||
| }; | ||
|
|
||
| isAuthorizedUser = (requestUserId: string, userId: string): boolean => { |
There was a problem hiding this comment.
This function doesn't really makes sense. On line 14 you can simply verify const isAuthorizedUser = requestUser.userId === userId
There was a problem hiding this comment.
oh yes, this were refactored and is now kind of obsolete I would keep it but make it less crowded
isAuthorizedUser = (requestUserId: string, userId: string): boolean => {
return requestUserId === userId;
};
Authorization for channels, users and verification/revocation and adding role based access management. Using role based access management admin users are able to delete other users or channels and also revoke/verify credentials of other organizations.
This PR is including the following PRs that's why they are abandoned:
#74
#68