Skip to content

APT-1935: Use SpiceDB for lease-related auth checks #42

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dannyeldridge
Copy link

@dannyeldridge dannyeldridge commented Mar 6, 2025

This PR integrates SpiceDB for lease-related authorization checks. Any action requiring a permission that starts with leases__ or lease_renewals__ will be affected.

For this proof of concept (POC), we are utilizing the rules array here.

Long-term, we aim to move away from using property data as the source of truth for determining which privilege symbols (role + permission) a UserRole has. Several potential approaches could address this, which we can discuss if we proceed further.

3/14/25

Latest commit for CR: ff3942a

Copy link

kermitapp bot commented Mar 6, 2025

@andrewdt23
Copy link

Does this gem support prereleases so that we don't actually have to merge this PR but can still use it in our property branch?

@dannyeldridge dannyeldridge force-pushed the apt-1935-spicedbBackendChecks-AuthGem-PrototypeOnly branch from d77ec46 to 7335a62 Compare March 7, 2025 19:32
@dannyeldridge dannyeldridge force-pushed the apt-1935-spicedbBackendChecks-AuthGem-PrototypeOnly branch from 758a57d to 4dac361 Compare March 10, 2025 15:19
@fargo-app fargo-app bot requested a review from a team March 13, 2025 21:38
@dannyeldridge dannyeldridge removed the request for review from a team March 13, 2025 21:38
@dannyeldridge dannyeldridge force-pushed the apt-1935-spicedbBackendChecks-AuthGem-PrototypeOnly branch 2 times, most recently from fee5ff2 to ff3942a Compare March 13, 2025 21:49
@dannyeldridge
Copy link
Author

The latest commit adds a check for the id_in_scope condition type.
It also cleans up some variable naming to more accurately reflect how the final approve/deny decision is made.

Each rule has zero or more attributes that get evaluated.
Each attribute has a single @conditions_hash which has one or more conditions that all must be met for the attribute to 'pass'.
If the rules join_operator is or, then the rule passes if any attribute passes.
If the rules join_operator is and, then the rule passes if all attributes pass.


if condition_type == :id_in_scope && condition_proc.is_a?(Proc)
puts " Checking Occupancy ID in scope condition"
user_has_access_to_occupancy = attribute.validate?(attr_validator)
Copy link
Author

Choose a reason for hiding this comment

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

When calling attribute.validate method, the condition_proc is executed, which returns a list of Occupancy ids the user has access to. This logic exists in the visible_by_conditions method here

Comment on lines 248 to 252
condition_matched = false
current_attribute_matched = true

Choose a reason for hiding this comment

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

Same note here about defaulting to the negative until the condition is met.

Copy link
Contributor

@mike-mike-mike-mike-mike mike-mike-mike-mike-mike left a comment

Choose a reason for hiding this comment

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

For posterity: Danny and I talked a little about this beyond the POC scope. I think we agreed that weaving spiceDB into this gem directly would not be the right track, but we will need a way to override certain behavior and logic - which might look like adding a plugin-type feature or configuration values for decl auth where you can supply classes that will be used in place of the default validators. Something like that.

@dannyeldridge dannyeldridge force-pushed the apt-1935-spicedbBackendChecks-AuthGem-PrototypeOnly branch from ff3942a to 777b4b9 Compare March 18, 2025 19:37
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.

4 participants