Skip to content

Conversation

@JRB66955
Copy link
Contributor

@JRB66955 JRB66955 commented Nov 7, 2025

TODO:

- work out api routing issue with middleware
- tests tbd

@JRB66955 JRB66955 marked this pull request as ready for review November 11, 2025 16:53
@JRB66955 JRB66955 requested a review from JR40159 November 18, 2025 09:23
ARADDCC002
ARADDCC002 previously approved these changes Nov 18, 2025
@JRB66955 JRB66955 added enhancement New feature or request javascript Pull requests that update Javascript code ready for review labels Nov 18, 2025

// authorisation
const modelIds = results.map((result) => result.modelId)
let auths: any[] = []
Copy link
Member

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

@JRB66955 JRB66955 requested a review from JR40159 November 24, 2025 16:59
…tion instead of multiple, tweaks to the query to work with new aggregation, and an updated auth loop
(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 }
Copy link
Member

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...?

Copy link
Member

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 &&
Copy link
Member

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

@JRB66955 JRB66955 merged commit c9f9af8 into main Nov 28, 2025
22 checks passed
@JRB66955 JRB66955 deleted the dev/all-access-requests-for-admin-endpoint branch November 28, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request javascript Pull requests that update Javascript code ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants