-
Notifications
You must be signed in to change notification settings - Fork 17
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
BC-5195 - poc permission context #4555
Conversation
THIS PR MUST NOT MERGED INTO MAIN |
Permission.BOARD_COLUMN_CREATE, | ||
Permission.BOARD_COLUMN_MOVE, | ||
Permission.BOARD_COLUMN_DELETE, | ||
Permission.BOARD_COLUMN_UPDATE_TITLE, |
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.
not sure if update title needs a permission by itself
edit column should cover that (also move / delete column)
export class PermissionContextEntity extends BaseEntityWithTimestamps { | ||
@Property() | ||
@Unique() | ||
contextReference: ObjectId; |
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.
should use EntityId instead of ObjectId
BOARD_ELEMENT_UPDATE = 'BOARD_ELEMENT_UPDATE', | ||
BOARD_ELEMENT_DELETE = 'BOARD_ELEMENT_DELETE', | ||
BOARD_ELEMENT_MOVE = 'BOARD_ELEMENT_MOVE', | ||
BOARD_ELEMENT_CAN_SUBMIT = 'BOARD_SUBMISSION_CAN_SUBMIT', |
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.
One idea would be to have only a limitted number of permitted actions (normally CRUD - but right now we tend to use only read and write only in the authorization / rules )
The "string" permissions are old, and should be avoided, as long as you have permission on the entity
|
||
@ManyToOne(() => PermissionContextEntity, { wrappedReference: true, fieldName: 'parentContext', nullable: true }) | ||
@Index() | ||
_parentContext: Reference<PermissionContextEntity> | null; |
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.
why the notation with _ ?
|
||
return permissionMatrix; | ||
} | ||
} |
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.
where would the user role come into play?
We might have something like implicit perrmissions frrom the role, and then, explicit permission here?
actor (group / role / user) | entity (collection or document) | action | allowed |
---|---|---|---|
student | column-node | read | true |
student | column-node | write | false |
teacher | column-node | write | true |
... | |||
userId | column-node-id | write | true |
userId | card-node-id | read | false |
also, the relation with parent will be difficult to manage... whenever the parent is updated
Description
This Proof of Concept (PoC) addresses an existing constraint in our role-based permission system.
The problem:
Consider a scenario where a user with the role "STUDENT" wishes to modify a card on the board. Simply identifying the user's role and associated permissions is insufficient to determine whether they have the authorization to modify the card.
For instance, let's say we have two courses, each with their own boards. A student from class A should not have the ability to edit a card on the board belonging to course B. This implies that a generic "STUDENT" role is inadequate. Instead, we would need distinct roles like "STUDENT OF COURSE A" and "STUDENT OF COURSE B".
Proposed solution:
This PR proposes the introduction of a permission_context that includes a list of users along with their permissions. It also supports nested contexts, simplifying permission computation by adjusting a base context. For example, the permissions for a card can be set to mirror those of the board it belongs to.
The PoC is still a work in progress. User grouping (via roles, teams, etc.) has not been implemented yet, but should be straightforward to add once the overall PoC receives positive feedback.
Currently, the permission_context cannot be added or edited. For testing on localhost, this needs to be done manually.
Here's an example to add to the permission-context collection to authorize a board:
Links to Tickets or other pull requests
https://ticketsystem.dbildungscloud.de/browse/BC-5195
Changes
A new collection permission-context has been added. Each document represents a context and includes a list of user IDs with their permissions and a pointer to a document signifying the object to be authorized. Duplicate contexts are prohibited (currently enforced by a unique constraint).
The introduced PermissionContextService offers the functionality to resolve permissions. All other entities are unaware of the permissionContext, maintaining a unidirectional flow, similar to an addon. The UC layer only needs to use the PermissionContextService.
Datasecurity
Deployment
New Repos, NPM pakages or vendor scripts
Approval for review
generate-client:server
was executed in vue frontend and changes were tested and put in a PR with the same branch name.