Skip to content
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

[master < E129-MG] Add label-based authorization #484

Merged
merged 22 commits into from Sep 16, 2022

Conversation

BorisTasevski
Copy link
Contributor

@BorisTasevski BorisTasevski commented Aug 2, 2022

[master < Epic] PR

  • Check, and update documentation if necessary
  • Update changelog
  • Write E2E tests
  • Compare the benchmarking results between the master branch and the Epic branch
  • Provide the full content or a guide for the final git message

…s with bad checks on the old epic branch (#478)

* grammar expanded; (#462)

* T0954 mg expand user and role to hold permissions on labels (#465)

* added FineGrainedAccessPermissions class to model

* expanded user and role with fine grained access permissions

* fixed grammar

* [E129 < T0953-MG] GRANT, DENY, REVOKE added in interpreter and mainVisitor (#464)

* GRANT, DENY, REVOKE added in interpreter and mainVisitor

* Commented labelPermissons

* remove labelsPermission adding

* Removed extra lambda

* [E129<-T0955-MG] Expand ExecutionContext with label related information (#467)

* Added FineGrainedAccessChecker to Context

* fixed failing tests for label based authorization (#480)

* Marked FineGrainedAccessChecker ctor explicit; Introduced change to clang-tidy; (#483)

Co-authored-by: niko4299 <51059248+niko4299@users.noreply.github.com>
@BorisTasevski BorisTasevski changed the title [Master < E129-MG] Implement Label-based Authorization [master < E129-MG] Implement Label-based Authorization Aug 2, 2022
BorisTasevski and others added 11 commits August 4, 2022 19:20
* implemented scanall filtering

* minor code refactor

* FindNextNode -> FindNextVertex
…iterator over vertices (#494)

* implemented skipping vertices in Constructor and mgp_vertices_iterator_next

* Added utility function for moving iterator to next permitted vertex

* removed ifdef directive

* NextPermitted parameter type changed from mgp_vertices_iterator* to mgp_vertices_iterator&

* created support for lba-procedures e2e testing; Added test for vertex iterator skipping unauthorized vertices

* removed fixture from tests; converted generator to regular function;
* GRANT, REVOKE, DENY and access_checker DONE

* Added AccessChecker to ExecutionContext

* grammar expanded; (#462)

* current

* T0954 mg expand user and role to hold permissions on labels (#465)

* added FineGrainedAccessPermissions class to model

* expanded user and role with fine grained access permissions

* fixed grammar

* [E129 < T0953-MG] GRANT, DENY, REVOKE added in interpreter and mainVisitor (#464)

* GRANT, DENY, REVOKE added in interpreter and mainVisitor

* Commented labelPermissons

* remove labelsPermission adding

* Fixed

* Removed extra lambda

* fixed

* [E129<-T0955-MG] Expand ExecutionContext with label related information (#467)

* added

* Added FineGrainedAccessChecker to Context

* fixed

* Added filtering

* testing

* Added edge filtering to storage, need to add filtering in simple Expand in operator.cpp

* Removed storage changes

* MATCH filtering working

* EdgeTypeFiltering working, just need to test everything again

* Removed FineGrainedAccessChecker

* Removed Expand Path

* Fix

* Tested FineGrainedAccessHandler, need to test AuthChecker

* Added integration test for lba

* Fixed merge conflicts

* PR fix

* fixed

* PR fix

* Fix test

* removed .vscode, .cache, .githooks

* githooks

* added tests

* fixed build

* Changed ast.lcp and User pointer to value in context.hpp

* Fixed test

* Remove denies on grant all

* AuthChecker

* Pr fix, auth_checker still not fixed

* Create mg-glue and extract UserBasedAuthChecker from AuthChecker

* Build fixed, need to fix test

* e2e tests

* e2e test working

* Added unit test, e2e and FineGrainedChecker

* Mege E129, auth_checker tests

* Fixed test

* e2e fix

Co-authored-by: Boris Taševski <36607228+BorisTasevski@users.noreply.github.com>
Co-authored-by: josipmrden <josip.mrden@external-basf.com>
Co-authored-by: János Benjamin Antal <benjamin.antal@memgraph.io>
…nular permissions (#496)

* Added enum for more granular access control; Expanded functionality of fine grained access checker; Propagated changes to Edit, Deny and Revoke permissions methods in interpreter

* Introduced Merge method for merging two colle with permissions

* e2e tests implementation started

* FineGrainedAccessChecker Grant and Deny methods reworked

* removed faulty test addition

* Naming fixes; FineGrainedAccessChecker unit tests introduced

* unnecessary includes removed; minor code improvements

* Access checker reworked; denies and grant merged into single permission object; Created global_permission that applies to all non-created permissions. Grant, Deny and Revoke reworked; Merge method reworked

* Fixed wrong check;

* PR review changes; Naming and const fixed, replaced double tertiary with lambda

* unwrapping the iterator fix

* minor spelling fixes
…ns (#500)

* Added enum for more granular access control; Expanded functionality of fine grained access checker; Propagated changes to Edit, Deny and Revoke permissions methods in interpreter

* Introduced Merge method for merging two colle with permissions

* e2e tests implementation started

* Expanded cypher to support fine grained permissions

* ast.lcp::AuthQuery removed labels, added support for label permissions

* promoted label permissions to vector

* removed unnecesary enum value

* expanded glue/auth with LabelPrivilegeToLabelPermission

* added const

* extended Grant Deny and Revoke Privileges with new label privileges

* extended Edit Grant Deny and Revoke Privileges to properly use new model

* Fixed unit tests

* FineGrainedAccessChecker Grant and Deny methods reworked

* Revoke cypher slightly reworked; Revoke for labels works without label permissions

* EditPermission's label_permission lambda now takes two parameters

* constants naming enforced; replaced asterisks with string constant

* removed faulty test addition

* Naming fixes; FineGrainedAccessChecker unit tests introduced

* unnecessary includes removed; minor code improvements

* minor fix

* Access checker reworked; denies and grant merged into single permission object; Created global_permission that applies to all non-created permissions. Grant, Deny and Revoke reworked; Merge method reworked

* Fixed wrong check;

* Fix after merge; renamed constants; removed unused constant

* Fix after merge; workloads.yaml for lbaprocedures e2e tests updated with new grammar

* Fixes after merge

* Fixes after merge

* fixed Revoke that was not fixed after the merge

* updated cypher main visitor tests

* PR review changes; Naming and const fixed, replaced double tertiary with lambda

* unwrapping the iterator fix

* merge 1003 minor fix

* minor spelling fixes

* Introduced visitPrivilegesList because of the doubled code

* const added

* string const to enum

* redundant braces

* added const

* minor code improvement

* e2e tests expanded

* if -> switch

* enum class inherits uint8_t now

* LabelPrililege::EDIT -> LabelPrivilege::UPDATE

* LabelPermission -> EntityPermission; LabelPrivilege -> EntityPrivilege

* EntityPrivilege -> FineGrainedPrivilege; EntityPermission -> FineGrainedPermission
Added showing of label privileges functionality to fine grained access control.
* [T1007-MG < T0997-MG] Authorization on paths (#501)

* Added read authorization in paths operators

* [T1007-MG < T1016-MG] Added authorization in create and delete operators (#513)

* Added authorization in RemoveNodeCursor, RemoveExpandCursor, CreateNodeCursor, CreateExpandCursor,MergeCursor

* [T1007-MG < T1014-MG] Add authorization to read operators (#520)

Added label based access control to read operators (ScanAll).

* [T1007-MG < T1015-MG] Add authorization to update operators (SetProperty, SetProperties, RemoveProperty) (#521)

Added label based authorization to update operators

Co-authored-by: niko4299 <51059248+niko4299@users.noreply.github.com>
Co-authored-by: Josip Mrden <josip.mrden@memgraph.io>
* [T1006-MG < T1017-MG] Add LBA checks to all read procedures in C API (#515)

* Initial Impl

* NextPermittedEdge introduced

* revert moving constructor to cpp

* edge from and edge to methods expanded with lba check

* minor fix

* added check to path expand procedure

* Added integration tests for read query procedures

* additional check

* changed iterator type to reference

* comments from pr

Co-authored-by: Josip Mrden <josip.mrden@memgraph.io>

* [T1006-MG < T1018-MG] Add LBA checks to all update procedures in C API (#516)

* Initial Impl

* NextPermittedEdge introduced

* revert moving constructor to cpp

* edge from and edge to methods expanded with lba check

* minor fix

* extended update methods

* added check to path expand procedure

* Added integration tests for read query procedures

* Added integration tests for update query modules

* additional check

* changed iterator type to reference

* fixed bug in Update property for node; fixed 2 e2e tests

* replaced enum

Co-authored-by: Josip Mrden <josip.mrden@memgraph.io>

* [T1006-MG < T1019-MG] Add LBA checks to all Create and Delete procedures in C API (#517)

* Initial Impl

* NextPermittedEdge introduced

* revert moving constructor to cpp

* edge from and edge to methods expanded with lba check

* minor fix

* extended update methods

* initial implementation

* added check to path expand procedure

* Added integration tests for read query procedures

* Added integration tests for update query modules

* Added unit tests for creation of vertex, adding and removing vertex label

* additional check

* changed iterator type to reference

* Added unit tests for create edge

* Corrected query module in create edge

* fixed bug in Update property for node; fixed 2 e2e tests

* fixed merge errors

* Expanded FineGrainedAuthChecker with HasGlobalPermissionOnVertices and HasGlobalPermissionOnEdges

* Removed two wrong checks; Added two global checks

* return null added

* introduced new mgp_error value

* fixed endless loop

* replaced enum

* intermediate

* tests updated

* PermissionDeniedError -> AuthorizationError rename

* rename in enum permission_denied error -> authorization error

* mgp_vertex_remove_label check improved

* quotes changed; order of imports fixed

* string constant introduced

* import fixed

* yaml format

Co-authored-by: Josip Mrden <josip.mrden@memgraph.io>

Co-authored-by: Josip Mrden <josip.mrden@memgraph.io>
Exceptions added in update and create delete operators instead of logging
* renamed parameters (#539)

* added variable declarations in ifs; minor code improvements; (#541)

* dba parameter removed (#543)

* Accept -> Has rename; HasGlobalPermissionOnVertices/Edges -> HasGlobalPrivilegeOnVertices/Edges (#545)

* replaced passing dba from reference to pointer
@gitbuda gitbuda changed the title [master < E129-MG] Implement Label-based Authorization [master < E129-MG] Add label-based authorization Sep 13, 2022
@BorisTasevski BorisTasevski marked this pull request as ready for review September 15, 2022 06:35
Copy link
Member

@gitbuda gitbuda left a comment

Choose a reason for hiding this comment

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

It seems that we have a memory issue on some of the previous master commits but label-based RBAC seems fine, so let's merge this to master.

@gitbuda gitbuda merged commit 898c894 into master Sep 16, 2022
@gitbuda gitbuda deleted the E129-MG-label-based-authorization branch September 16, 2022 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants