Skip to content

Conversation

@pyphilia
Copy link
Contributor

@pyphilia pyphilia commented Jul 26, 2024

This is a draft: it still looks messy. The goal would be to centralized the permission so it's not always "deduced" by the frontend.

We could add operations non related to items too (eg. prevent pseudo members to export their own analytics)

The idea would be to allow it to be used:

const shouldShow = can(COPY, item, member)

if(!shouldShow) { 
  return null
  // or
  return <You don't have the permission to perform this op>
}

return <div/>

Instead of having many ways to check the permissions

// create app data - only signed in member can create data
const shouldShow = Boolean(member)
// copy, create - only signed in member (and not pseudo) can copy
const shouldShow = Boolean(member && member !== PSEUDO)
// hide item - at least write, the front needs to always remember which permission is required
const shouldShow = PermissionLevelCompare.gte(permission, WRITE)

if(!shouldShow) { 
  return null
  // or
  return <You don't have the permission to perform this op>
}

return <div/>

Questions:

  • could this be used in the backend?

@pyphilia pyphilia self-assigned this Jul 26, 2024
@sonarqubecloud
Copy link

Copy link
Member

@spaenleh spaenleh left a comment

Choose a reason for hiding this comment

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

Thank you for this PR, I like the idea, and I see the need to have a better way to express the abilities in a central way, so the frontend can present a consistent UI and we are confident that users get the expected experience.

It reminds me of https://casl.js.org/v6/en/ I don't know if we should try to use it, as it may be overkill, so maybe we start with rolling out our own implementation. But maybe there are some things we can take from them.

Already it looks good, I did not fully review, let me know if you want to discuss this today.

Comment on lines +5 to +29
export enum Operation {
ReadItem = 'read-item',
MoveItem = 'move-item',
CopyItem = 'copy-item',
HideItem = 'hide-item',
DownloadItem = 'download-item',
}

// don't take into account hidden: suppose backend will prevent read from beginning
// pseudonymized users cannot create items and can only view a specific item
const SIGNED_IN_OPERATIONS_PERMISSIONS = {
[Operation.ReadItem]: {
admin: true,
write: true,
read: true,
pseudonymized: true,
public: true,
},
[Operation.CopyItem]: {
admin: true,
write: true,
read: true,
pseudonymized: false,
public: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This PR is 🔥, I love it !
What about using namespaces so we reduce the amount of operations ? We can also have an enumeration of roles so we can type check it !

Suggested change
export enum Operation {
ReadItem = 'read-item',
MoveItem = 'move-item',
CopyItem = 'copy-item',
HideItem = 'hide-item',
DownloadItem = 'download-item',
}
// don't take into account hidden: suppose backend will prevent read from beginning
// pseudonymized users cannot create items and can only view a specific item
const SIGNED_IN_OPERATIONS_PERMISSIONS = {
[Operation.ReadItem]: {
admin: true,
write: true,
read: true,
pseudonymized: true,
public: true,
},
[Operation.CopyItem]: {
admin: true,
write: true,
read: true,
pseudonymized: false,
public: true,
},
export enum Operation {
Read = 'read',
Move = 'move',
Copy = 'copy',
Hide = 'hide',
Download = 'download',
}
export enum Namespace {
Item = 'item',
Membership = 'Membership',
Analytics = 'Analytics',
Profile = 'Profile'
}
export enum Role {
Admin = 'admin',
Write = 'write',
Read = 'read',
Pseudonymized = 'pseudonymized',
Public = 'public'
}
// don't take into account hidden: suppose backend will prevent read from beginning
// pseudonymized users cannot create items and can only view a specific item
const SIGNED_IN_OPERATIONS_PERMISSIONS: { [key in Namespace]?: { [key in Operation]?: { [key in Role]?: boolean } } } = {
[Namespace.Item]: {
[Operation.Read]: {
[Role.Admin]: true,
[Role.Write]: true,
[Role.Read]: true,
[Role.Pseudonymized]: true,
[Role.Public]: true,
},
[Operation.Copy]: {
[Role.Admin]: true,
[Role.Write]: true,
[Role.Read]: true,
[Role.Pseudonymized]: false,
[Role.Public]: true,
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants