Repository navigation
perf: role permissions in database - #9152
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9152 +/- ##
===========================================
+ Coverage 46.36% 62.52% +16.16%
===========================================
Files 1510 1597 +87
Lines 145674 149470 +3796
===========================================
+ Hits 67536 93456 +25920
+ Misses 74909 51423 -23486
- Partials 3229 4591 +1362
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
muhlemmer
commented
Jan 15, 2025
2 tasks done
muhlemmer
marked this pull request as ready for review
January 15, 2025 15:11
This was referenced Jan 15, 2025
adlerhurst
reviewed
Jan 16, 2025
adlerhurst
left a comment
Member
There was a problem hiding this comment.
nice work, seems to run very smoothly, i only have two questions about comments
muhlemmer
enabled auto-merge (squash)
January 16, 2025 08:48
Member
|
looks good, i would just add the permission feature here: https://github.com/zitadel/zitadel/blob/feat-permission-table/cmd/defaults.yaml#L1132 |
adlerhurst
approved these changes
Jan 16, 2025
muhlemmer
added a commit
that referenced
this pull request
Jan 17, 2025
# Which Problems Are Solved Memberships did not have a fields table fill migration. # How the Problems Are Solved Add filling of membership fields to the repeatable steps. # Additional Changes - Use the same repeatable step for multiple fill fields handlers. - Fix an error for PostgreSQL 15 where a subquery in a `FROM` clause needs an alias ing the `permitted_orgs` function. # Additional Context - Part of #9188 - Introduced in #9152
adlerhurst
pushed a commit
that referenced
this pull request
Jan 17, 2025
# Which Problems Are Solved Memberships did not have a fields table fill migration. # How the Problems Are Solved Add filling of membership fields to the repeatable steps. # Additional Changes - Use the same repeatable step for multiple fill fields handlers. - Fix an error for PostgreSQL 15 where a subquery in a `FROM` clause needs an alias ing the `permitted_orgs` function. # Additional Context - Part of #9188 - Introduced in #9152
2 tasks
This branch was successfully deployed
This file contains hidden or 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
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.
Which Problems Are Solved
Currently ZITADEL defines organization and instance member roles and permissions in defaults.yaml. The permission check is done on API call level. For example: "is this user allowed to make this call on this org". This makes sense on the V1 API where the API is permission-level shaped. For example, a search for users always happens in the context of the organization. (Either the organization the calling user belongs to, or through member ship and the x-zitadel-orgid header.
However, for resource based APIs we must be able to resolve permissions by object. For example, an IAM_OWNER listing users should be able to get all users in an instance based on the query filters. Alternatively a user may have user.read permissions on one or more orgs. They should be able to read just those users.
How the Problems Are Solved
Role permission mapping
The role permission mappings defined from
defaults.yamlor local config override are synchronized to the database on every run ofzitadel setup:addandremoveactions needed to reach the desired state or role permission mappings from the config.The migration is split in the following aggregates:
SYSTEMSYSTEM. This is in anticipation of instance level management over the API.Membership
Current instance / org / project membership events now have field table definitions. Like the role permissions this ensures strong consistency while still being able to use the indices of the fields table. A migration is provided to fill the membership fields.
Permission check
I aimed keeping the mental overhead to the developer to a minimal. The provided implementation only provides a permission check for list queries for org level resources, for example users. In the
querypackage there is a simple helper functionwherePermittedOrgswhich makes sure the underlying database function is called as part of theSELECTquery and the permitted organizations are part of theWHEREclause. This makes sure results from non-permitted organizations are omitted. Under the hood:WHEREclauses are set by the function.Additional Changes
Additional Context
Closes #9032
Closes #9014
#9188 defines follow-ups for the new permission framework based on this concept.