The potential problem
It's possible for the client side checkPermission
|
export const checkPermission = ( |
|
user: UserContextType['user'], |
|
requiredPermission: InstancePermission | ChapterPermission, |
|
variableValues?: Record<string, number>, |
|
) => { |
|
if (!user) return false; |
|
const context = { user, events: [], venues: [] }; |
|
return checker(context, requiredPermission, variableValues || {}); |
to fail when the server would allow a request. There are two ways it can fail when it should not
- It does not pass the correct variableValues
- It relies on
events or venues
The first case isn't fundamental. If we're careful, it can be avoided. However, there are situations where the second case cannot be avoided.
For example, if a chapter administrator checks their permission
checkPermission(user, Permissions.EventEdit, { eventId: 1 })
this will fail even if they're an administrator of the chapter this event belongs to. This is because the checker function needs the chapterId to see if the administrator has the permission and it needs the list of events to get the chapterId.
The potential solutions
- make
events + venues available to the client
- stop client side checking and, instead, query the server whenever the client needs to know what's allowed
- put
events and venues inside the user object. That way checker can find chapterId just by inspecting `user
Option 3 seems like the cleanest solution to me, but I'd welcome any opinions before I try to implement this.
The potential problem
It's possible for the client side checkPermission
chapter/client/src/util/check-permission.ts
Lines 8 to 15 in 730ac93
to fail when the server would allow a request. There are two ways it can fail when it should not
eventsorvenuesThe first case isn't fundamental. If we're careful, it can be avoided. However, there are situations where the second case cannot be avoided.
For example, if a chapter administrator checks their permission
this will fail even if they're an administrator of the chapter this event belongs to. This is because the
checkerfunction needs thechapterIdto see if the administrator has the permission and it needs the list of events to get thechapterId.The potential solutions
events+venuesavailable to the clienteventsandvenuesinside theuserobject. That waycheckercan findchapterIdjust by inspecting `userOption 3 seems like the cleanest solution to me, but I'd welcome any opinions before I try to implement this.