Skip to content

fix: removed user membership access filter based on current user memb… #29

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.

Sign up for GitHub

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

Closed
SeanBerrieHRI wants to merge 1 commit into from
Closed

fix: removed user membership access filter based on current user memb… #29

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/components/EntitySettings/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export default defineComponent({
this.buildGraph(entity.data)
this.members = _.orderBy(members.data, ['user.firstName', 'user.lastName'], ['asc'])
this.users = this.createUsers(users.data, this.members)
this.memberships = this.createMemberships(memberships.data)
this.memberships = memberships.data
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 question (security): Dropping the allowedEntities filter changes which memberships are available and may widen access scope.

createMemberships previously limited this.memberships to entries whose allowedEntities included this.config.uuid. Using memberships.data directly means the UI (and default inviteForm.membershipUuid) may now surface memberships for other entities. Unless the backend enforces the same entity constraint, this could allow invites with unintended memberships. If expanding scope is intentional, can you confirm the server is enforcing the necessary entity-level checks so the client-side filter is truly redundant?

this.inviteForm.membershipUuid = _.get(this.memberships, '0.uuid')
this.breadcrumbs = this.config.createBreadcrumbsWithSelf(meta.data.path, this.subject)
this.status.setDone()
Expand Down Expand Up @@ -183,9 +183,6 @@ export default defineComponent({
fullName: `${u.firstName} ${u.lastName}`,
})), ['firstName', 'lastName'], ['asc'])
},
Copy link
Copy Markdown

@hcvdwerf hcvdwerf Apr 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks too broad. The current code filters memberships by allowedEntities, and the backend still returns that field with entity-specific values. With the local API, Data Provider is only allowed for Catalog, but this PR would also show it for Dataset, Distribution, and Data Service. So this removes a real guard rather than fixing the underlying issue. Can we confirm whether allowedEntities is still authoritative before merging this?

Suggestion:
this.memberships = memberships.data.filter((m) => !Array.isArray(m.allowedEntities) || m.allowedEntities.length === 0 || m.allowedEntities.includes(this.config.uuid) )

createMemberships(memberships: Array<any>): Array<any> {
return memberships.filter((m) => _.includes(m.allowedEntities, this.config.uuid))
},
async submitInvite(): Promise<void> {
if (this.inviteForm.userUuid !== null && this.inviteForm.membershipUuid !== null) {
try {
Expand Down