diff options
39 files changed, 769 insertions, 103 deletions
diff --git a/DEVNOTES.md b/DEVNOTES.md new file mode 100644 index 000000000..01f58fa0f --- /dev/null +++ b/DEVNOTES.md @@ -0,0 +1,62 @@ + +# Authorization Logic in Policies + +## Base Rules + +### ApplicationPolicy + +Policies inheriting from the `ApplicationPolicy` authorize _Undestructive_ _Permissions_ whiche are `index?` and +`show?`. And forbid _Destructive_ _Permissions_ which are `create?`, `destroy?` & `update`. + +These _CRUD_ permissions are tied to to _Action_ permissions, `delete?`→ `destroy?`, `edit?` → `update? and `new?`→ `create?`. + +These three _Action_ permissions are not supposed to be overriden in `ApplicationPolicy` subclasses. + + +### Common Policy Types + +There are two common policy types. + +#### Read Only Type Policy + +This corresponds to inheriting from `ApplicationPolicy` without overriding one of the five aforementioned _CRUD_ permissions. + +The following Policies are of this type. + + - `Company` + - `GroupOfLine` + - `Line` + custom + - `Network` + - `StopArea` + +#### Standard Type Policy + +The standard type policy inherits from `ApplicationPolicy` does not override any _Undesructive_ _Pemission_ but overrides the _Destructive_ ones. + +Normally, but not always they are overriden as follows + +```ruby + def <destructive>? + !archived? && organisation_match? && user.has_permission('<resource in plural form>.<action>') + end +``` + +There are some variations (**TO BE CLARIFIED**) concerning `organisation_match?`. + +The following Policies are of this type. + + - `AccessLink` + - `AccessPoint` + - `Calendar` (*) + - `ConnectionLink` + - `JourneyPattern` + - `Referential` + custom + - `Route` + - `RoutingConstraintZone` + - `TimeTable` + custom + +`Calendar` is a strange exception where no user permission is checked for the _destructive_ _permissions_. + + + + diff --git a/app/controllers/journey_patterns_collections_controller.rb b/app/controllers/journey_patterns_collections_controller.rb index 7b97e1408..837ac65e7 100644 --- a/app/controllers/journey_patterns_collections_controller.rb +++ b/app/controllers/journey_patterns_collections_controller.rb @@ -49,11 +49,10 @@ class JourneyPatternsCollectionsController < ChouetteController end def user_permissions - @perms = {}.tap do |perm| - ['journey_patterns.create', 'journey_patterns.edit', 'journey_patterns.destroy'].each do |name| - perm[name] = policy(:journey_pattern).send("#{name.split('.').last}?") - end - end.to_json + @perms = + %w{create destroy edit}.inject({}) do | permissions, action | + permissions.merge( "journey_patterns.#{action}" => policy.authorizes_action?(action) ) + end.to_json end def update diff --git a/app/controllers/time_tables_controller.rb b/app/controllers/time_tables_controller.rb index 6d2639981..0e0cade56 100644 --- a/app/controllers/time_tables_controller.rb +++ b/app/controllers/time_tables_controller.rb @@ -17,7 +17,6 @@ class TimeTablesController < ChouetteController @time_table = @time_table.decorate(context: { referential: @referential }) - build_breadcrumb :show end end diff --git a/app/controllers/vehicle_journeys_controller.rb b/app/controllers/vehicle_journeys_controller.rb index fe2e2137f..f7e2fcdc1 100644 --- a/app/controllers/vehicle_journeys_controller.rb +++ b/app/controllers/vehicle_journeys_controller.rb @@ -159,11 +159,10 @@ class VehicleJourneysController < ChouetteController end def user_permissions - @perms = {}.tap do |perm| - ['vehicle_journeys.create', 'vehicle_journeys.edit', 'vehicle_journeys.destroy'].each do |name| - perm[name] = policy(:vehicle_journey).send("#{name.split('.').last}?") - end - end.to_json + @perms = + %w{create destroy update}.inject({}) do | permissions, action | + permissions.merge( "vehicle_journeys.#{action}" => policy.authorizes_action?(action) ) + end.to_json end private diff --git a/app/models/user.rb b/app/models/user.rb index 9cd4f64d6..1d9e435d5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,10 +28,21 @@ class User < ActiveRecord::Base end after_destroy :check_destroy_organisation - @@edit_offer_permissions = ['routes.create', 'routes.edit', 'routes.destroy', 'journey_patterns.create', 'journey_patterns.edit', 'journey_patterns.destroy', - 'vehicle_journeys.create', 'vehicle_journeys.edit', 'vehicle_journeys.destroy', 'time_tables.create', 'time_tables.edit', 'time_tables.destroy', - 'footnotes.edit', 'footnotes.create', 'footnotes.destroy', 'routing_constraint_zones.create', 'routing_constraint_zones.edit', - 'routing_constraint_zones.destroy', 'referentials.create', 'referentials.edit', 'referentials.destroy', 'boiv:edit-offer'] + def self.destructive_permissions_for(models) + models.product( %w{create destroy update} ).map{ |model_action| model_action.join('.') } + end + + @@edit_offer_permissions = + destructive_permissions_for( %w[ + footnotes + journey_patterns + referentials + routes + routing_constraint_zones + time_tables + vehicle_journeys + ]) << 'boiv:edit-offer' + mattr_reader :edit_offer_permissions def self.all_permissions diff --git a/app/policies/acces_point_policy.rb b/app/policies/acces_point_policy.rb deleted file mode 100644 index ce3a8a1ef..000000000 --- a/app/policies/acces_point_policy.rb +++ /dev/null @@ -1,25 +0,0 @@ -class AccessPointPolicy < ApplicationPolicy - class Scope < Scope - def resolve - scope - end - end - - def create? - !archived? && - organisation_match? && - user.has_permission?('access_points.create') - end - - def update? - !archived? && - organisation_match? && - user.has_permission?('access_points.edit') - end - - def destroy? - !archived? && - organisation_match? - && user.has_permission?('access_points.destroy') - end -end diff --git a/app/policies/access_link_policy.rb b/app/policies/access_link_policy.rb index a4f0e40e8..1f1147f60 100644 --- a/app/policies/access_link_policy.rb +++ b/app/policies/access_link_policy.rb @@ -6,11 +6,11 @@ class AccessLinkPolicy < ApplicationPolicy end def create? - !archived? && oragnisation_mathc? && user.has_permission?('access_links.create') + !archived? && organisation_match? && user.has_permission?('access_links.create') end def update? - !archived? && organisation_match? && user.has_permission?('access_links.edit') + !archived? && organisation_match? && user.has_permission?('access_links.update') end def destroy? diff --git a/app/policies/access_point_policy.rb b/app/policies/access_point_policy.rb index a1b57a3e5..41436e77c 100644 --- a/app/policies/access_point_policy.rb +++ b/app/policies/access_point_policy.rb @@ -10,7 +10,7 @@ class AccessPointPolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('access_points.edit') + !archived? && organisation_match? && user.has_permission?('access_points.update') end def destroy? diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index d5c1039fd..dbe4542e7 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -82,13 +82,12 @@ class ApplicationPolicy end def organisation_match? - user.organisation == organisation + user.organisation_id == organisation_id end - def organisation + def organisation_id # When sending permission to react UI, we don't have access to record object for edit & destroy.. actions - organisation = record.is_a?(Symbol) ? nil : record.try(:organisation) - organisation or referential.try :organisation + referential.try(:organisation_id) || record.try(:organisation_id) end diff --git a/app/policies/connection_link_policy.rb b/app/policies/connection_link_policy.rb index acadc807d..240c2a804 100644 --- a/app/policies/connection_link_policy.rb +++ b/app/policies/connection_link_policy.rb @@ -14,6 +14,6 @@ class ConnectionLinkPolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('connection_links.edit') + !archived? && organisation_match? && user.has_permission?('connection_links.update') end end diff --git a/app/policies/journey_pattern_policy.rb b/app/policies/journey_pattern_policy.rb index 810ead170..507a364b6 100644 --- a/app/policies/journey_pattern_policy.rb +++ b/app/policies/journey_pattern_policy.rb @@ -15,7 +15,7 @@ class JourneyPatternPolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('journey_patterns.edit') + !archived? && organisation_match? && user.has_permission?('journey_patterns.update') end end diff --git a/app/policies/line_policy.rb b/app/policies/line_policy.rb index 674ed9b8d..b84c7a69f 100644 --- a/app/policies/line_policy.rb +++ b/app/policies/line_policy.rb @@ -11,7 +11,7 @@ class LinePolicy < ApplicationPolicy end def edit_footnote? - !archived? && user.has_permission?('footnotes.edit') + !archived? && user.has_permission?('footnotes.update') end def destroy_footnote? diff --git a/app/policies/referential_policy.rb b/app/policies/referential_policy.rb index 7f8c9e939..bf970c2b8 100644 --- a/app/policies/referential_policy.rb +++ b/app/policies/referential_policy.rb @@ -14,7 +14,7 @@ class ReferentialPolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('referentials.edit') + !archived? && organisation_match? && user.has_permission?('referentials.update') end @@ -24,11 +24,11 @@ class ReferentialPolicy < ApplicationPolicy end def archive? - !archived? && update? + record.archived_at.nil? && user.has_permission?('referentials.update') end def unarchive? - archived? && update? + !record.archived_at.nil? && user.has_permission?('referentials.update') end def common_lines? diff --git a/app/policies/route_policy.rb b/app/policies/route_policy.rb index a33551737..be3ffce4d 100644 --- a/app/policies/route_policy.rb +++ b/app/policies/route_policy.rb @@ -14,6 +14,6 @@ class RoutePolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('routes.edit') + !archived? && organisation_match? && user.has_permission?('routes.update') end end diff --git a/app/policies/routing_constraint_zone_policy.rb b/app/policies/routing_constraint_zone_policy.rb index 3f2ad99a9..3cfcf46ff 100644 --- a/app/policies/routing_constraint_zone_policy.rb +++ b/app/policies/routing_constraint_zone_policy.rb @@ -14,6 +14,6 @@ class RoutingConstraintZonePolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('routing_constraint_zones.edit') + !archived? && organisation_match? && user.has_permission?('routing_constraint_zones.update') end end diff --git a/app/policies/time_table_policy.rb b/app/policies/time_table_policy.rb index acdc2d13c..c9f3a3ec6 100644 --- a/app/policies/time_table_policy.rb +++ b/app/policies/time_table_policy.rb @@ -15,7 +15,7 @@ class TimeTablePolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('time_tables.edit') + !archived? && organisation_match? && user.has_permission?('time_tables.update') end def actualize? diff --git a/app/policies/vehicle_journey_policy.rb b/app/policies/vehicle_journey_policy.rb index 27d96e43b..24040455f 100644 --- a/app/policies/vehicle_journey_policy.rb +++ b/app/policies/vehicle_journey_policy.rb @@ -14,6 +14,6 @@ class VehicleJourneyPolicy < ApplicationPolicy end def update? - !archived? && organisation_match? && user.has_permission?('vehicle_journeys.edit') + !archived? && organisation_match? && user.has_permission?('vehicle_journeys.update') end end diff --git a/app/views/time_tables/show.html.slim b/app/views/time_tables/show.html.slim index f596fd480..36b79cc25 100644 --- a/app/views/time_tables/show.html.slim +++ b/app/views/time_tables/show.html.slim @@ -1,6 +1,7 @@ - require 'calendar_helper' / PageHeader + = pageheader 'map-marker', @time_table.comment, '', diff --git a/spec/controllers/journey_patterns_collections_controller_spec.rb b/spec/controllers/journey_patterns_collections_controller_spec.rb index 888281036..442d73fb7 100644 --- a/spec/controllers/journey_patterns_collections_controller_spec.rb +++ b/spec/controllers/journey_patterns_collections_controller_spec.rb @@ -1,5 +1,3 @@ -require 'rails_helper' - RSpec.describe JourneyPatternsCollectionsController, :type => :controller do end diff --git a/spec/controllers/routes_controller_spec.rb b/spec/controllers/routes_controller_spec.rb index 18067dec7..000b799db 100644 --- a/spec/controllers/routes_controller_spec.rb +++ b/spec/controllers/routes_controller_spec.rb @@ -1,6 +1,4 @@ -require 'spec_helper' - -describe RoutesController, :type => :controller do +RSpec.describe RoutesController, :type => :controller do login_user let!(:route) { create(:route) } diff --git a/spec/features/time_tables_spec.rb b/spec/features/time_tables_spec.rb index 58a1dc98f..0fb4bb30d 100644 --- a/spec/features/time_tables_spec.rb +++ b/spec/features/time_tables_spec.rb @@ -1,7 +1,4 @@ -# -*- coding: utf-8 -*- -require 'spec_helper' - -describe "TimeTables", :type => :feature do +RSpec.describe "TimeTables", :type => :feature do login_user let!(:time_tables) { Array.new(2) { create(:time_table) } } diff --git a/spec/features/vehicle_journeys_spec.rb b/spec/features/vehicle_journeys_spec.rb index 5a3a9ad7d..16a79e2c5 100644 --- a/spec/features/vehicle_journeys_spec.rb +++ b/spec/features/vehicle_journeys_spec.rb @@ -43,7 +43,7 @@ describe 'VehicleJourneys', type: :feature do context 'user does not have permission to edit vehicle journeys' do it 'does not show an edit link for vehicle journeys' do - @user.tap { |u| u.permissions.delete('vehicle_journeys.edit') }.save + @user.tap { |u| u.permissions.delete('vehicle_journeys.update') }.save visit referential_line_route_vehicle_journey_path(referential, line, route, vehicle_journey) expect(page).not_to have_content(I18n.t('vehicle_journeys.actions.edit')) end diff --git a/spec/helpers/table_builder_helper_spec.rb b/spec/helpers/table_builder_helper_spec.rb index 4afd0774c..c2c287b99 100644 --- a/spec/helpers/table_builder_helper_spec.rb +++ b/spec/helpers/table_builder_helper_spec.rb @@ -16,7 +16,7 @@ describe TableBuilderHelper, type: :helper do organisation: referential.organisation, permissions: [ 'referentials.create', - 'referentials.edit', + 'referentials.update', 'referentials.destroy', ] ), diff --git a/spec/policies/access_link_policy_spec.rb b/spec/policies/access_link_policy_spec.rb new file mode 100644 index 000000000..6194ae55c --- /dev/null +++ b/spec/policies/access_link_policy_spec.rb @@ -0,0 +1,20 @@ +RSpec.describe AccessLinkPolicy, type: :policy do + + let( :record ){ build_stubbed :access_link } + + permissions :create? do + it_behaves_like 'permitted policy and same organisation', "access_links.create", archived: true + end + permissions :destroy? do + it_behaves_like 'permitted policy and same organisation', "access_links.destroy", archived: true + end + permissions :edit? do + it_behaves_like 'permitted policy and same organisation', "access_links.update", archived: true + end + permissions :new? do + it_behaves_like 'permitted policy and same organisation', "access_links.create", archived: true + end + permissions :update? do + it_behaves_like 'permitted policy and same organisation', "access_links.update", archived: true + end +end diff --git a/spec/policies/access_point_policy_spec.rb b/spec/policies/access_point_policy_spec.rb new file mode 100644 index 000000000..b6bc46eb4 --- /dev/null +++ b/spec/policies/access_point_policy_spec.rb @@ -0,0 +1,20 @@ +RSpec.describe AccessPointPolicy, type: :policy do + + let( :record ){ build_stubbed :access_point } + + permissions :create? do + it_behaves_like 'permitted policy and same organisation', "access_points.create", archived: true + end + permissions :destroy? do + it_behaves_like 'permitted policy and same organisation', "access_points.destroy", archived: true + end + permissions :edit? do + it_behaves_like 'permitted policy and same organisation', "access_points.update", archived: true + end + permissions :new? do + it_behaves_like 'permitted policy and same organisation', "access_points.create", archived: true + end + permissions :update? do + it_behaves_like 'permitted policy and same organisation', "access_points.update", archived: true + end +end diff --git a/spec/policies/calendar_policy_spec.rb b/spec/policies/calendar_policy_spec.rb new file mode 100644 index 000000000..f4423fb82 --- /dev/null +++ b/spec/policies/calendar_policy_spec.rb @@ -0,0 +1,47 @@ +RSpec.describe CalendarPolicy, type: :policy do + + let( :record ){ build_stubbed :calendar } + + shared_examples 'authorizes on archived and same organisation only' do + | permission, archived: false| + context 'same organisation →' do + before do + user.organisation_id = referential.organisation_id + end + it "allows a user with the same organisation" do + expect_it.to permit(user_context, record) + end + if archived + it 'removes permission for archived referentials' do + referential.archived_at = 42.seconds.ago + expect_it.not_to permit(user_context, record) + end + end + end + + context 'different organisations →' do + before do + add_permissions(permission, for_user: user) + end + it "denies a user with a different organisation" do + expect_it.not_to permit(user_context, record) + end + end + end + + permissions :create? do + it_behaves_like 'authorizes on archived and same organisation only', 'calendars.create', archived: true + end + permissions :destroy? do + it_behaves_like 'authorizes on archived and same organisation only', 'calendars.destroy', archived: true + end + permissions :edit? do + it_behaves_like 'authorizes on archived and same organisation only', 'calendars.update', archived: true + end + permissions :new? do + it_behaves_like 'authorizes on archived and same organisation only', 'calendars.create', archived: true + end + permissions :update? do + it_behaves_like 'authorizes on archived and same organisation only', 'calendars.update', archived: true + end +end diff --git a/spec/policies/company_policy_spec.rb b/spec/policies/company_policy_spec.rb new file mode 100644 index 000000000..2d249a2be --- /dev/null +++ b/spec/policies/company_policy_spec.rb @@ -0,0 +1,42 @@ +RSpec.describe CompanyPolicy, type: :policy do + + let( :record ){ build_stubbed :company } + before { stub_policy_scope(record) } + + + # + # Non Destructive + # --------------- + + context 'Non Destructive actions →' do + permissions :index? do + it_behaves_like 'always allowed', 'anything', archived: true + end + permissions :show? do + it_behaves_like 'always allowed', 'anything', archived: true + end + end + + + # + # Destructive + # ----------- + + context 'Destructive actions →' do + permissions :create? do + it_behaves_like 'always forbidden', 'companies.create', archived: true + end + permissions :destroy? do + it_behaves_like 'always forbidden', 'companies.destroy', archived: true + end + permissions :edit? do + it_behaves_like 'always forbidden', 'companies.update', archived: true + end + permissions :new? do + it_behaves_like 'always forbidden', 'companies.create', archived: true + end + permissions :update? do + it_behaves_like 'always forbidden', 'companies.update', archived: true + end + end +end diff --git a/spec/policies/connection_link_policy_spec.rb b/spec/policies/connection_link_policy_spec.rb new file mode 100644 index 000000000..23e40abe3 --- /dev/null +++ b/spec/policies/connection_link_policy_spec.rb @@ -0,0 +1,20 @@ +RSpec.describe ConnectionLinkPolicy, type: :policy do + + let( :record ){ build_stubbed :connection_link } + + permissions :create? do + it_behaves_like 'permitted policy and same organisation', "connection_links.create", archived: true + end + permissions :destroy? do + it_behaves_like 'permitted policy and same organisation', "connection_links.destroy", archived: true + end + permissions :edit? do + it_behaves_like 'permitted policy and same organisation', "connection_links.update", archived: true + end + permissions :new? do + it_behaves_like 'permitted policy and same organisation', "connection_links.create", archived: true + end + permissions :update? do + it_behaves_like 'permitted policy and same organisation', "connection_links.update", archived: true + end +end diff --git a/spec/policies/group_of_line_policy_spec.rb b/spec/policies/group_of_line_policy_spec.rb new file mode 100644 index 000000000..29fbb1bfb --- /dev/null +++ b/spec/policies/group_of_line_policy_spec.rb @@ -0,0 +1,42 @@ +RSpec.describe GroupOfLinePolicy, type: :policy do + + let( :record ){ build_stubbed :group_of_line } + before { stub_policy_scope(record) } + + + # + # Non Destructive + # --------------- + + context 'Non Destructive actions →' do + permissions :index? do + it_behaves_like 'always allowed', 'anything', archived: true + end + permissions :show? do + it_behaves_like 'always allowed', 'anything', archived: true + end + end + + + # + # Destructive + # ----------- + + context 'Destructive actions →' do + permissions :create? do + it_behaves_like 'always forbidden', 'group_of_lines.create', archived: true + end + permissions :destroy? do + it_behaves_like 'always forbidden', 'group_of_lines.destroy', archived: true + end + permissions :edit? do + it_behaves_like 'always forbidden', 'group_of_lines.update', archived: true + end + permissions :new? do + it_behaves_like 'always forbidden', 'group_of_lines.create', archived: true + end + permissions :update? do + it_behaves_like 'always forbidden', 'group_of_lines.update', archived: true + end + end +end diff --git a/spec/policies/journey_pattern_policy_spec.rb b/spec/policies/journey_pattern_policy_spec.rb new file mode 100644 index 000000000..39f849277 --- /dev/null +++ b/spec/policies/journey_pattern_policy_spec.rb @@ -0,0 +1,20 @@ +RSpec.describe JourneyPatternPolicy, type: :policy do + + let( :record ){ build_stubbed :journey_pattern } + + permissions :create? do + it_behaves_like 'permitted policy and same organisation', "journey_patterns.create", archived: true + end + permissions :destroy? do + it_behaves_like 'permitted policy and same organisation', "journey_patterns.destroy", archived: true + end + permissions :edit? do + it_behaves_like 'permitted policy and same organisation', "journey_patterns.update", archived: true + end + permissions :new? do + it_behaves_like 'permitted policy and same organisation', "journey_patterns.create", archived: true + end + permissions :update? do + it_behaves_like 'permitted policy and same organisation', "journey_patterns.update", archived: true + end +end diff --git a/spec/policies/line_policy_spec.rb b/spec/policies/line_policy_spec.rb index e720b2bc7..d9e684847 100644 --- a/spec/policies/line_policy_spec.rb +++ b/spec/policies/line_policy_spec.rb @@ -1,21 +1,163 @@ RSpec.describe LinePolicy, type: :policy do let( :record ){ build_stubbed :line } + before { stub_policy_scope(record) } - %w{create destroy edit}.each do | permission | - footnote_permission = "#{permission}_footnote" - permissions "#{footnote_permission}?".to_sym do - it_behaves_like 'permitted policy', "footnotes.#{permission}", archived: true + # + # Non Destructive + # --------------- + + context 'Non Destructive actions →' do + permissions :index? do + it_behaves_like 'always allowed', 'anything', archived: true + end + permissions :show? do + it_behaves_like 'always allowed', 'anything', archived: true + end + end + + + # + # Destructive + # ----------- + + context 'Destructive actions →' do + permissions :create? do + it_behaves_like 'always forbidden', 'lines.create', archived: true + end + permissions :destroy? do + it_behaves_like 'always forbidden', 'lines.destroy', archived: true + end + permissions :edit? do + it_behaves_like 'always forbidden', 'lines.update', archived: true + end + permissions :new? do + it_behaves_like 'always forbidden', 'lines.create', archived: true + end + permissions :update? do + it_behaves_like 'always forbidden', 'lines.update', archived: true + end + end + + + # + # Custom Footnote Permissions + # --------------------------- + + permissions :create_footnote? do + context 'permission present →' do + before do + add_permissions('footnotes.create', for_user: user) + end + + it 'authorized for unarchived referentials' do + expect_it.to permit(user_context, record) + end + + it 'forbidden for archived referentials' do + referential.archived_at = 1.second.ago + expect_it.not_to permit(user_context, record) + end + end + + context 'permission absent →' do + it 'is forbidden' do + expect_it.not_to permit(user_context, record) + end + end + end + + permissions :destroy_footnote? do + context 'permission present →' do + before do + add_permissions('footnotes.destroy', for_user: user) + end + + it 'authorized for unarchived referentials' do + expect_it.to permit(user_context, record) + end + + it 'forbidden for archived referentials' do + referential.archived_at = 1.second.ago + expect_it.not_to permit(user_context, record) + end + end + + context 'permission absent →' do + it 'is forbidden' do + expect_it.not_to permit(user_context, record) + end + end + end + + permissions :edit_footnote? do + context 'permission present →' do + before do + add_permissions('footnotes.update', for_user: user) + end + + it 'authorized for unarchived referentials' do + expect_it.to permit(user_context, record) + end + + it 'forbidden for archived referentials' do + referential.archived_at = 1.second.ago + expect_it.not_to permit(user_context, record) + end + end + + context 'permission absent →' do + it 'is forbidden' do + expect_it.not_to permit(user_context, record) + end end end permissions :new_footnote? do - it_behaves_like 'permitted policy', 'footnotes.create', archived: true + context 'permission present →' do + before do + add_permissions('footnotes.create', for_user: user) + end + + it 'authorized for unarchived referentials' do + expect_it.to permit(user_context, record) + end + + it 'forbidden for archived referentials' do + referential.archived_at = 1.second.ago + expect_it.not_to permit(user_context, record) + end + end + + context 'permission absent →' do + it 'is forbidden' do + expect_it.not_to permit(user_context, record) + end + end end permissions :update_footnote? do - it_behaves_like 'permitted policy', 'footnotes.edit', archived: true + context 'permission present →' do + before do + add_permissions('footnotes.update', for_user: user) + end + + it 'authorized for unarchived referentials' do + expect_it.to permit(user_context, record) + end + + it 'forbidden for archived referentials' do + referential.archived_at = 1.second.ago + expect_it.not_to permit(user_context, record) + end + end + + context 'permission absent →' do + it 'is forbidden' do + expect_it.not_to permit(user_context, record) + end + end end end diff --git a/spec/policies/network_policy_spec.rb b/spec/policies/network_policy_spec.rb new file mode 100644 index 000000000..ae4ffa03a --- /dev/null +++ b/spec/policies/network_policy_spec.rb @@ -0,0 +1,42 @@ +RSpec.describe NetworkPolicy, type: :policy do + + let( :record ){ build_stubbed :network } + before { stub_policy_scope(record) } + + + # + # Non Destructive + # --------------- + + context 'Non Destructive actions →' do + permissions :index? do + it_behaves_like 'always allowed', 'anything', archived: true + end + permissions :show? do + it_behaves_like 'always allowed', 'anything', archived: true + end + end + + + # + # Destructive + # ----------- + + context 'Destructive actions →' do + permissions :create? do + it_behaves_like 'always forbidden', 'networks.create', archived: true + end + permissions :destroy? do + it_behaves_like 'always forbidden', 'networks.destroy', archived: true + end + permissions :edit? do + it_behaves_like 'always forbidden', 'networks.update', archived: true + end + permissions :new? do + it_behaves_like 'always forbidden', 'networks.create', archived: true + end + permissions :update? do + it_behaves_like 'always forbidden', 'networks.update', archived: true + end + end +end diff --git a/spec/policies/referential_policy_spec.rb b/spec/policies/referential_policy_spec.rb new file mode 100644 index 000000000..d060317f9 --- /dev/null +++ b/spec/policies/referential_policy_spec.rb @@ -0,0 +1,102 @@ +RSpec.describe ReferentialPolicy, type: :policy do + + let( :record ){ build_stubbed :referential } + + + # + # Collection Based Permissions differ from standard as there is no referential yet + # -------------------------------------------------------------------------------- + + permissions :create? do + it 'permissions present → allowed' do + add_permissions('referentials.create', for_user: user) + expect_it.to permit(user_context, record) + end + it 'permissions absent → forbidden' do + expect_it.not_to permit(user_context, record) + end + end + + permissions :new? do + it 'permissions present → allowed' do + add_permissions('referentials.create', for_user: user) + expect_it.to permit(user_context, record) + end + it 'permissions absent → forbidden' do + expect_it.not_to permit(user_context, record) + end + end + + # + # Standard Destructive Action Permissions + # --------------------------------------- + + permissions :destroy? do + it_behaves_like 'permitted policy and same organisation', 'referentials.destroy', archived: true + end + permissions :edit? do + it_behaves_like 'permitted policy and same organisation', 'referentials.update', archived: true + end + permissions :update? do + it_behaves_like 'permitted policy and same organisation', 'referentials.update', archived: true + end + + # + # Custom Permissions + # ------------------ + + permissions :clone? do + it_behaves_like 'permitted policy and same organisation', 'referentials.create', archived: true + end + + permissions :archive? do + + context 'permission present →' do + before do + add_permissions('referentials.update', for_user: user) + end + + it 'allowed for unarchived referentials' do + expect_it.to permit(user_context, record) + end + + it 'forbidden for archived referentials' do + record.archived_at = 1.second.ago + expect_it.not_to permit(user_context, record) + end + end + + context 'permission absent →' do + it 'is forbidden' do + expect_it.not_to permit(user_context, record) + end + end + + end + + permissions :unarchive? do + + context 'permission present →' do + before do + add_permissions('referentials.update', for_user: user) + end + + it 'forbidden for unarchived referentials' do + expect_it.not_to permit(user_context, record) + end + + it 'allowed for archived referentials' do + record.archived_at = 1.second.ago + expect_it.to permit(user_context, record) + end + end + + context 'permission absent →' do + it 'is forbidden' do + record.archived_at = 1.second.ago + expect_it.not_to permit(user_context, record) + end + end + + end +end diff --git a/spec/policies/route_policy_spec.rb b/spec/policies/route_policy_spec.rb index cc949ff45..6be517048 100644 --- a/spec/policies/route_policy_spec.rb +++ b/spec/policies/route_policy_spec.rb @@ -11,7 +11,7 @@ RSpec.describe RoutePolicy, type: :policy do end permissions :edit? do - it_behaves_like 'permitted policy and same organisation', 'routes.edit', archived: true + it_behaves_like 'permitted policy and same organisation', 'routes.update', archived: true end permissions :new? do @@ -19,6 +19,6 @@ RSpec.describe RoutePolicy, type: :policy do end permissions :update? do - it_behaves_like 'permitted policy and same organisation', 'routes.edit', archived: true + it_behaves_like 'permitted policy and same organisation', 'routes.update', archived: true end end diff --git a/spec/policies/routing_constraint_zone_policy_spec.rb b/spec/policies/routing_constraint_zone_policy_spec.rb index f91313390..2ef15fa95 100644 --- a/spec/policies/routing_constraint_zone_policy_spec.rb +++ b/spec/policies/routing_constraint_zone_policy_spec.rb @@ -12,7 +12,7 @@ RSpec.describe RoutingConstraintZonePolicy, type: :policy do end permissions :edit? do - it_behaves_like 'permitted policy and same organisation', 'routing_constraint_zones.edit', archived: true + it_behaves_like 'permitted policy and same organisation', 'routing_constraint_zones.update', archived: true end permissions :new? do @@ -20,6 +20,6 @@ RSpec.describe RoutingConstraintZonePolicy, type: :policy do end permissions :update? do - it_behaves_like 'permitted policy and same organisation', 'routing_constraint_zones.edit', archived: true + it_behaves_like 'permitted policy and same organisation', 'routing_constraint_zones.update', archived: true end end diff --git a/spec/policies/stop_area_policy_spec.rb b/spec/policies/stop_area_policy_spec.rb new file mode 100644 index 000000000..8fe59c8e3 --- /dev/null +++ b/spec/policies/stop_area_policy_spec.rb @@ -0,0 +1,42 @@ +RSpec.describe StopAreaPolicy, type: :policy do + + let( :record ){ build_stubbed :stop_area } + before { stub_policy_scope(record) } + + + # + # Non Destructive + # --------------- + + context 'Non Destructive actions →' do + permissions :index? do + it_behaves_like 'always allowed', 'anything', archived: true + end + permissions :show? do + it_behaves_like 'always allowed', 'anything', archived: true + end + end + + + # + # Destructive + # ----------- + + context 'Destructive actions →' do + permissions :create? do + it_behaves_like 'always forbidden', 'stop_areas.create', archived: true + end + permissions :destroy? do + it_behaves_like 'always forbidden', 'stop_areas.destroy', archived: true + end + permissions :edit? do + it_behaves_like 'always forbidden', 'stop_areas.update', archived: true + end + permissions :new? do + it_behaves_like 'always forbidden', 'stop_areas.create', archived: true + end + permissions :update? do + it_behaves_like 'always forbidden', 'stop_areas.update', archived: true + end + end +end diff --git a/spec/policies/time_table_policy_spec.rb b/spec/policies/time_table_policy_spec.rb index 6c19362d2..dad3c13bc 100644 --- a/spec/policies/time_table_policy_spec.rb +++ b/spec/policies/time_table_policy_spec.rb @@ -2,17 +2,23 @@ RSpec.describe TimeTablePolicy, type: :policy do let( :record ){ build_stubbed :time_table } + permissions :create? do + it_behaves_like 'permitted policy and same organisation', 'time_tables.create', archived: true + end + + permissions :destroy? do + it_behaves_like 'permitted policy and same organisation', 'time_tables.destroy', archived: true + end - %w{create duplicate}.each do | permission | - permissions "#{permission}?".to_sym do - it_behaves_like 'permitted policy and same organisation', 'time_tables.create', archived: true - end + permissions :edit? do + it_behaves_like 'permitted policy and same organisation', 'time_tables.update', archived: true end - %w{destroy edit}.each do | permission | - permissions "#{permission}?".to_sym do - it_behaves_like 'permitted policy and same organisation', "time_tables.#{permission}", archived: true - end + permissions :new? do + it_behaves_like 'permitted policy and same organisation', 'time_tables.create', archived: true end + permissions :update? do + it_behaves_like 'permitted policy and same organisation', 'time_tables.update', archived: true + end end diff --git a/spec/support/devise.rb b/spec/support/devise.rb index d4a279a41..28703c072 100644 --- a/spec/support/devise.rb +++ b/spec/support/devise.rb @@ -4,12 +4,12 @@ module DeviseRequestHelper def login_user organisation = Organisation.where(:code => "first").first_or_create(attributes_for(:organisation)) @user ||= create(:user, :organisation => organisation, - :permissions => ['routes.create', 'routes.edit', 'routes.destroy', 'journey_patterns.create', 'journey_patterns.edit', 'journey_patterns.destroy', - 'vehicle_journeys.create', 'vehicle_journeys.edit', 'vehicle_journeys.destroy', 'time_tables.create', 'time_tables.edit', 'time_tables.destroy', - 'footnotes.edit', 'footnotes.create', 'footnotes.destroy', 'routing_constraint_zones.create', 'routing_constraint_zones.edit', 'routing_constraint_zones.destroy', - 'access_points.create', 'access_points.edit', 'access_points.destroy', 'access_links.create', 'access_links.edit', 'access_links.destroy', - 'connection_links.create', 'connection_links.edit', 'connection_links.destroy', 'route_sections.create', 'route_sections.edit', 'route_sections.destroy', - 'referentials.create', 'referentials.edit', 'referentials.destroy']) + :permissions => ['routes.create', 'routes.update', 'routes.destroy', 'journey_patterns.create', 'journey_patterns.update', 'journey_patterns.destroy', + 'vehicle_journeys.create', 'vehicle_journeys.update', 'vehicle_journeys.destroy', 'time_tables.create', 'time_tables.update', 'time_tables.destroy', + 'footnotes.update', 'footnotes.create', 'footnotes.destroy', 'routing_constraint_zones.create', 'routing_constraint_zones.update', 'routing_constraint_zones.destroy', + 'access_points.create', 'access_points.update', 'access_points.destroy', 'access_links.create', 'access_links.update', 'access_links.destroy', + 'connection_links.create', 'connection_links.update', 'connection_links.destroy', 'route_sections.create', 'route_sections.update', 'route_sections.destroy', + 'referentials.create', 'referentials.update', 'referentials.destroy']) login_as @user, :scope => :user # post_via_redirect user_session_path, 'user[email]' => @user.email, 'user[password]' => @user.password end @@ -36,25 +36,42 @@ module DeviseRequestHelper end module DeviseControllerHelper + def setup_user + _all_actions = %w{create destroy update} + _all_resources = %w{ access_links + access_points + connection_links + footnotes + journey_patterns + referentials + route_sections + routes + routing_constraint_zones + time_tables + vehicle_journeys } + join_with = -> (separator) do + -> (ary) { ary.join(separator) } + end + before do @request.env["devise.mapping"] = Devise.mappings[:user] organisation = Organisation.where(:code => "first").first_or_create(attributes_for(:organisation)) - @user = create(:user, :organisation => organisation, - :permissions => ['routes.create', 'routes.edit', 'routes.destroy', 'journey_patterns.create', 'journey_patterns.edit', 'journey_patterns.destroy', - 'vehicle_journeys.create', 'vehicle_journeys.edit', 'vehicle_journeys.destroy', 'time_tables.create', 'time_tables.edit', 'time_tables.destroy', - 'footnotes.edit', 'footnotes.create', 'footnotes.destroy', 'routing_constraint_zones.create', 'routing_constraint_zones.edit', 'routing_constraint_zones.destroy', - 'access_points.create', 'access_points.edit', 'access_points.destroy', 'access_links.create', 'access_links.edit', 'access_links.destroy', - 'connection_links.create', 'connection_links.edit', 'connection_links.destroy', 'route_sections.create', 'route_sections.edit', 'route_sections.destroy', - 'referentials.create', 'referentials.edit', 'referentials.destroy']) + @user = create(:user, + organisation: organisation, + permissions: _all_resources.product( _all_actions ).map(&join_with.('.'))) end end + def login_user() setup_user before do sign_in @user end end + + private + end RSpec.configure do |config| diff --git a/spec/support/pundit/shared_examples.rb b/spec/support/pundit/shared_examples.rb index 33ed1ffae..357004f4e 100644 --- a/spec/support/pundit/shared_examples.rb +++ b/spec/support/pundit/shared_examples.rb @@ -1,3 +1,69 @@ + +RSpec.shared_examples 'always allowed' do + | permission, archived: false| + context 'same organisation →' do + before do + user.organisation_id = referential.organisation_id + end + it "allows a user with the same organisation" do + expect_it.to permit(user_context, record) + end + if archived + it 'does not remove permission for archived referentials' do + referential.archived_at = 42.seconds.ago + expect_it.to permit(user_context, record) + end + end + end + + context 'different organisations →' do + before do + add_permissions(permission, for_user: user) + end + it "allows a user with a different organisation" do + expect_it.to permit(user_context, record) + end + if archived + it 'does not remove permission for archived referentials' do + referential.archived_at = 42.seconds.ago + expect_it.to permit(user_context, record) + end + end + end +end + +RSpec.shared_examples 'always forbidden' do + | permission, archived: false| + context 'same organisation →' do + before do + user.organisation_id = referential.organisation_id + end + it "allows a user with the same organisation" do + expect_it.not_to permit(user_context, record) + end + if archived + it 'still no permission for archived referentials' do + referential.archived_at = 42.seconds.ago + expect_it.not_to permit(user_context, record) + end + end + end + + context 'different organisations →' do + before do + add_permissions(permission, for_user: user) + end + it "denies a user with a different organisation" do + expect_it.not_to permit(user_context, record) + end + if archived + it 'still no permission for archived referentials' do + referential.archived_at = 42.seconds.ago + expect_it.not_to permit(user_context, record) + end + end + end +end RSpec.shared_examples 'permitted policy and same organisation' do | permission, archived: false| @@ -6,11 +72,11 @@ RSpec.shared_examples 'permitted policy and same organisation' do expect_it.not_to permit(user_context, record) end it 'and also a user with the same organisation' do - user.organisation = referential.organisation + user.organisation_id = referential.organisation_id expect_it.not_to permit(user_context, record) end end - + context 'permission present → ' do before do add_permissions(permission, for_user: user) @@ -21,13 +87,13 @@ RSpec.shared_examples 'permitted policy and same organisation' do end it 'but allows it for a user with the same organisation' do - user.organisation = referential.organisation + user.organisation_id = referential.organisation_id expect_it.to permit(user_context, record) end if archived it 'removes the permission for archived referentials' do - user.organisation = referential.organisation + user.organisation_id = referential.organisation_id referential.archived_at = 42.seconds.ago expect_it.not_to permit(user_context, record) end |