Skip to content

perf: role permissions in database - #9152

Merged
muhlemmer merged 21 commits into
mainfrom
feat-permission-table
Jan 16, 2025
Merged

muhlemmer merged 21 commits into
mainfrom
feat-permission-table

Conversation

@muhlemmer

@muhlemmer muhlemmer commented Jan 9, 2025 •

Copy link
Copy Markdown
Collaborator

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.yaml or local config override are synchronized to the database on every run of zitadel setup:

  • A single query per aggregate builds a list of add and remove actions needed to reach the desired state or role permission mappings from the config.
  • The required events based on the actions are pushed to the event store.
  • Events define search fields so that permission checking can use the indices and is strongly consistent for both query and command sides.

The migration is split in the following aggregates:

  • System aggregate for for roles prefixed with SYSTEM
  • Each instance for roles not prefixed with SYSTEM. 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 query package there is a simple helper function wherePermittedOrgs which makes sure the underlying database function is called as part of the SELECT query and the permitted organizations are part of the WHERE clause. This makes sure results from non-permitted organizations are omitted. Under the hood:

  • A Pg/PlSQL function searches for a list of organization IDs the passed user has the passed permission.
  • When the user has the permission on instance level, it returns early with all organizations.
  • The functions uses a number of views. The views help mapping the fields entries into relational data and simplify the code use for the function. The views provide some pre-filters which allow proper index usage once the final WHERE clauses 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.

@vercel

vercel Bot commented Jan 9, 2025 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 16, 2025 10:09am

@codecov

codecov Bot commented Jan 9, 2025 •

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 75.14793% with 84 lines in your changes missing coverage. Please review.

Project coverage is 62.52%. Comparing base (690147b) to head (47a0a70).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/setup/sync_role_permissions.go 64.55% 20 Missing and 8 partials ⚠️
internal/repository/permission/permission.go 64.40% 21 Missing ⚠️
internal/query/permission.go 0.00% 9 Missing ⚠️
cmd/setup/46.go 53.84% 4 Missing and 2 partials ⚠️
internal/repository/project/member.go 50.00% 4 Missing ⚠️
internal/command/instance_features_model.go 40.00% 3 Missing ⚠️
internal/command/system_features_model.go 40.00% 3 Missing ⚠️
internal/query/user.go 70.00% 2 Missing and 1 partial ⚠️
internal/query/instance_features_model.go 33.33% 2 Missing ⚠️
internal/query/system_features_model.go 33.33% 2 Missing ⚠️
... and 2 more
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     
Flag Coverage Δ
core-integration-tests-postgres 36.78% <75.14%> (?)
core-unit-tests 46.31% <14.52%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread cmd/setup/45/03-instance_members_view.sql
Comment thread cmd/defaults.yaml Outdated
Comment thread cmd/setup/45/06-permitted_orgs_function.sql Outdated
Comment thread cmd/setup/sync_role_permissions.sql
@muhlemmer muhlemmer changed the title [WIP] perf: role permissions in database perf: role permissions in database Jan 15, 2025
@muhlemmer
muhlemmer requested a review from adlerhurst January 15, 2025 15:11
@muhlemmer
muhlemmer marked this pull request as ready for review January 15, 2025 15:11

@adlerhurst adlerhurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nice work, seems to run very smoothly, i only have two questions about comments

Comment thread cmd/setup/sync_role_permissions.sql Outdated
Comment thread internal/query/user.go Outdated
Comment thread cmd/defaults.yaml
@adlerhurst

Copy link
Copy Markdown
Member

looks good, i would just add the permission feature here: https://github.com/zitadel/zitadel/blob/feat-permission-table/cmd/defaults.yaml#L1132

@muhlemmer
muhlemmer merged commit 3f6ea78 into main Jan 16, 2025
@muhlemmer
muhlemmer deleted the feat-permission-table branch January 16, 2025 10:09
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

This branch was successfully deployed

1 active deployment
Preview — 47a0a70b Deployed Jan 16, 2025 by vercel[bot]
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.

Zitadel permission function zitadel.user.v2.UserService/ListUsers permission check slow

2 participants