First data filtering api route (supports RBAC policy) #162
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asafc
force-pushed
the
asaf/cto-314-data-filtering-poc
branch
from
dc95497 to
91a0520
Compare
asafc
changed the title
added 22 commits
August 12, 2024 16:15
…back to boolean expression
…e response encoding to the api wrapper
asafc
force-pushed
the
asaf/cto-314-data-filtering-poc
branch
from
05c4e93 to
1b4bcc8
Compare
omer9564
approved these changes
Aug 15, 2024
| @@ -0,0 +1,11 @@ | |||
| .PHONY: help | |||
There was a problem hiding this comment.
I'm in favor of using poetry instead of all of that, this is really hard to manage and poetry basically abstracts everything
Comment on lines
+1
to
+9
| # Permit.io Data Filtering SDK (EAP) | ||
|
|
||
| Initial SDK to enable data filtering scenarios based on compiling OPA policies from Rego AST into SQL-like expressions. | ||
|
|
||
| ## Installation | ||
|
|
||
| ```py | ||
| pip install permit-datafilter | ||
| ``` |
There was a problem hiding this comment.
add a detailed readme & reference our docs
Comment on lines
+65
to
+88
| TEMP_PARTIAL_EVAL_POLICY = """ | ||
| package permit.partial_eval | ||
|
|
||
| import future.keywords.contains | ||
| import future.keywords.if | ||
| import future.keywords.in | ||
|
|
||
| default allow := false | ||
|
|
||
| allow if { | ||
| checked_permission := sprintf("%s:%s", [input.resource.type, input.action]) | ||
|
|
||
| some granting_role, role_data in data.roles | ||
| some resource_type, actions in role_data.grants | ||
| granted_action := actions[_] | ||
| granted_permission := sprintf("%s:%s", [resource_type, granted_action]) | ||
|
|
||
| some tenant, roles in data.users[input.user.key].roleAssignments | ||
| role := roles[_] | ||
| role == granting_role | ||
| checked_permission == granted_permission | ||
| input.resource.tenant == tenant | ||
| } | ||
| """ |
| @@ -125,7 +125,7 @@ def _get_pdp_version(cls) -> Optional[str]: | |||
| if os.path.exists(PDP_VERSION_FILENAME): | |||
| with open(PDP_VERSION_FILENAME) as f: | |||
| return f.read().strip() | |||
| return None | |||
| return "0.0.0" | |||
Comment on lines
+740
to
+744
| return Response( | ||
| content=json.dumps(residual_policy.dict()), | ||
| status_code=status.HTTP_200_OK, | ||
| media_type="application/json", | ||
| ) |
There was a problem hiding this comment.
the response is schemaless ?
Comment on lines
+65
to
+75
| CompileResponse.update_forward_refs() | ||
| CompileResponseComponents.update_forward_refs() | ||
| CRQuerySet.update_forward_refs() | ||
| CRQuery.update_forward_refs() | ||
| CRExpression.update_forward_refs() | ||
| CRTerm.update_forward_refs() | ||
| CRSupportBlock.update_forward_refs() | ||
| CRSupportModule.update_forward_refs() | ||
| CRSupportModulePackage.update_forward_refs() | ||
| CRRegoRule.update_forward_refs() | ||
| CRRuleHead.update_forward_refs() |
Comment on lines
+30
to
+35
| "eq": lambda c, v: c == v, | ||
| "ne": lambda c, v: c != v, | ||
| "lt": lambda c, v: c < v, | ||
| "gt": lambda c, v: c > v, | ||
| "le": lambda c, v: c <= v, | ||
| "ge": lambda c, v: c >= v, |
There was a problem hiding this comment.
there are already builtin functions for that ( check out import operator )
Comment on lines
+57
to
+61
| def column_table_name(c: Column) -> str: | ||
| return cast(sa.Table, c.table).name | ||
|
|
||
| def is_main_table_column(c: Column) -> bool: | ||
| return column_table_name(c) == _get_table_name(table) |
There was a problem hiding this comment.
why are those functions declared inside the function ?
Comment on lines
+72
to
+75
| if len(join_conditions) == 0: | ||
| raise TypeError( | ||
| f"You must call QueryBuilder.join(table, condition) to map residual references to other SQL tables" | ||
| ) |
There was a problem hiding this comment.
this won't work with SQLAlchemy auto join techniques and "load options"
| @@ -14,3 +14,4 @@ httpx>=0.27.0,<1 | |||
| protobuf>=3.20.2 # not directly required, pinned by Snyk to avoid a vulnerability | |||
| opal-common==0.7.6 | |||
| opal-client==0.7.6 | |||
| permit-datafilter>=0.0.3,<1 | |||
There was a problem hiding this comment.
make this "optional" dependency, otherwise you require everyone that uses our SDK to have SQLAlchemy and many other unnecessary packages
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.