Skip to content

Commit

Permalink
fix user can access Cinstance
Browse files Browse the repository at this point in the history
The `#where_values_hash` method does not support joins and sub-queries.

Originally the `account.provided_cinstances` part was ignored because it
was JOINs. With the update to sub-queries, it turned into `plan_id: nil`
which is incorrect and broke the logic.

So now we keep logic as previously by resorting only to the
`Cinstance.permitted_for` part of the query.

This relies on the fact that `Cinstance.plan.issuer` is set as
`Cinstance.service` when that issuer is a service.

Also relies on the fact that `User.member_permission_service_ids` will
not set to ids of services that don't belong to the account.

Which may not be ideal but allows for permission checking without extra
database queries.
  • Loading branch information
akostadinov committed Aug 26, 2024
1 parent e93469f commit 2b95e7c
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
9 changes: 8 additions & 1 deletion config/abilities/provider_member.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@
can :create, Account
can :update, Account if account.provider_can_use?(:service_permissions)

can %i[read show edit update], Cinstance, user.accessible_cinstances.where_values_hash
# Using historical optimized way and leave canonical way commented out below
# The resulting hash presently is something like {"type"=>"Cinstance", "service_id"=>[ids..]}
# Seems like canonically though that we are moving towards accessing service through plan
can %i[read show edit update], Cinstance, Cinstance.permitted_for(user).where_values_hash
# can %i[read show edit update], Cinstance, user.accessible_cinstances do |cinstance|
# cinstance.plan&.issuer_type == "Service" && cinstance.plan.issuer.account == user.account &&
# (!user.forbidden_some_services? || user.member_permission_service_ids.include?(cinstance.plan.issuer.id))
# end

# abilities for buyer users
can %i[read update update_role destroy suspend unsuspend], User, account: { provider_account_id: user.account_id }
Expand Down
6 changes: 3 additions & 3 deletions test/unit/abilities/provider_member_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ def test_cinstances
service_2 = FactoryBot.create(:simple_service, account: @account)
service_3 = FactoryBot.create(:simple_service, account: @account)

plan_1 = FactoryBot.create(:simple_application_plan, service: service_1)
plan_2 = FactoryBot.create(:simple_application_plan, service: service_2)
plan_3 = FactoryBot.create(:simple_application_plan, service: service_3)
plan_1 = FactoryBot.create(:simple_application_plan, issuer: service_1)
plan_2 = FactoryBot.create(:simple_application_plan, issuer: service_2)
plan_3 = FactoryBot.create(:simple_application_plan, issuer: service_3)

app_1 = FactoryBot.create(:simple_cinstance, plan: plan_1)
app_2 = FactoryBot.create(:simple_cinstance, plan: plan_2)
Expand Down

0 comments on commit 2b95e7c

Please sign in to comment.