-
Notifications
You must be signed in to change notification settings - Fork 19
New endpoint to allow admins to get all access requests #2888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…quests, with auth and flexibility to return only user relevant accessRequests
…lst working around express middleware
…nticationConnector
|
|
||
| // authorisation | ||
| const modelIds = results.map((result) => result.modelId) | ||
| let auths: any[] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth creating a return type for authorisation.accessRequests() so we don't have to use any here?
Although tracing through to accessRequests it looks a little more complicated than i thought
… new condition for viewing ARs
…hrough for filtering in case of no adminAccess, attempting to make the auth section cleaner?
…tion instead of multiple, tweaks to the query to work with new aggregation, and an updated auth loop
…et tests in a working state (to then update again)
…ed results nicer when adminAccess is true
…be same order as tests for findAccessRequests
| (await missingRequiredRole(user, model, ['owner'])) && | ||
| ([AccessRequestAction.Delete, AccessRequestAction.Update] as AccessRequestActionKeys[]).includes(action) | ||
| ) { | ||
| return { success: false, info: 'You cannot change an access request you do not own.', id: request.id } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This authorisation function doesn't seem to be getting run for the search for some reason...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
False alarm, a problem with my local instance
| } | ||
|
|
||
| if ( | ||
| !(await this.model(user, model, ModelAction.View)).success && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to update this to deny a user if they are viewing and access request and either they aren't able to view a model or they aren't named on the access request
TODO:- work out api routing issue with middleware- tests tbd