Skip to content

Commit

Permalink
Fix Service permitted_for(user) scope
Browse files Browse the repository at this point in the history
  • Loading branch information
mayorova committed Oct 22, 2024
1 parent 11f3b76 commit fb4f5f6
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 8 deletions.
6 changes: 4 additions & 2 deletions app/models/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,12 @@ def cinstance
scope :permitted_for, ->(user = nil) {
next all unless user

next none if user.no_services_allowed?

account = user.account
account_services = (account.provider? ? account : account.provider_account).services
self.merge(
account_services.merge(user.forbidden_some_services? ? where(id: user.member_permission_service_ids) : {})
merge(
account_services.merge(user.selected_services_allowed? ? where(id: user.member_permission_service_ids) : {})
)
}

Expand Down
48 changes: 43 additions & 5 deletions app/models/user/permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,18 @@ def existing_service_ids
# but then it is much harder to test MemberPermission#service_ids=
# returns [] if no services are enabled, and nil if all (current and future) services are enabled
def member_permission_service_ids
return nil if admin? || !services_member_permission
permitted_service_ids = services_member_permission.try(:service_ids) || []
permitted_service_ids & existing_service_ids
return @member_permission_service_ids if defined?(@member_permission_service_ids)

if admin? || !services_member_permission
@member_permission_service_ids = nil
else
permitted_service_ids = services_member_permission.try(:service_ids) || []
@member_permission_service_ids = permitted_service_ids & existing_service_ids
end
end

def services_member_permission
member_permissions.find { |permission| permission.admin_section == :services }
@services_member_permission ||= member_permissions.find { |permission| permission.admin_section == :services }
end

def has_access_to_service?(service)
Expand All @@ -95,16 +100,49 @@ def has_access_to_service?(service)

# Lack of the services section means it is the old permission system where everyone had access
# to every service. So to limit the scope only for new users, we start adding this permission.
# TODO: check if it can be replaced with other methods
def has_access_to_all_services?
!admin_sections.include?(:services) || admin?
end

# Returns:
# :none - if no services are allowed
# :all - if all services are allowed for the selected service-related permissions
# :selected - if a subset of services are allowed for the selected service-related permissions
def permitted_services_status
if admin? || (service_permissions_selected? && member_permission_service_ids.nil?)
:all
elsif service_permissions_selected? && member_permission_service_ids&.any?
:selected
else
:none
end
end

def no_services_allowed?
permitted_services_status == :none
end

def all_services_allowed?
permitted_services_status == :all
end

def selected_services_allowed?
permitted_services_status == :selected
end

# TODO: check if it can be replaced with other methods
def forbidden_some_services?
!has_access_to_all_services? && account.provider_can_use?(:service_permissions)
end

def service_permissions_selected?
@service_permissions_selected ||= (member_permission_ids & AdminSection::SERVICE_PERMISSIONS).any?
end

# TODO: check if it can be replaced with other methods
def access_to_service_admin_sections?
(member_permission_ids & AdminSection::SERVICE_PERMISSIONS).any? && accessible_services?
service_permissions_selected? && accessible_services?
end

def reload(*)
Expand Down
12 changes: 11 additions & 1 deletion test/integration/api/policies_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def setup
@provider = FactoryBot.create(:provider_account)
@service = @provider.default_service
login! @provider
Policies::PoliciesListService.expects(:call!).returns({})
Policies::PoliciesListService.stubs(:call!).returns({})
end


Expand Down Expand Up @@ -55,4 +55,14 @@ def setup
get edit_admin_service_policies_path(@service)
assert_response :service_unavailable
end

test 'policies edit for members with no permissions' do
member = FactoryBot.create(:member, account: @provider, state: 'active')
logout! && login!(@provider, user: member)

get edit_admin_service_policies_path(@service)

# TODO: maybe this should be be a :forbidden
assert_response :not_found
end
end
10 changes: 10 additions & 0 deletions test/integration/api/proxy_configs_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,14 @@ def setup
assert_response :success
assert_equal '{"foo":"bar"}', response.body
end

test 'proxy configs index for members with no permissions' do
member = FactoryBot.create(:member, account: @provider, state: 'active')
logout! && login!(@provider, user: member)

get admin_service_proxy_configs_path(service_id: service, environment: 'sandbox')

# TODO: maybe this should be be a :forbidden
assert_response :not_found
end
end

0 comments on commit fb4f5f6

Please sign in to comment.