Skip to content

Commit 66e1aa1

Browse files
cjcolvarcjcolvar
committed
Fix and test issues with disable inheritance: inherited non-admin roles and restricting visibility from non-private to private
1 parent 084b54c commit 66e1aa1

2 files changed

Lines changed: 156 additions & 8 deletions

File tree

‎app/models/ability.rb‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ def custom_permissions(user=nil, session=nil)
211211
# end
212212

213213
cannot :read, [MediaObject, SpeedyAF::Proxy::MediaObject] do |media_object|
214-
media_object.disable_inheritance? && is_exclusively_inherited_from_parent?(media_object)
214+
media_object.disable_inheritance? && is_exclusively_inherited_from_parent?(media_object) && !is_member_of?(media_object.collection)
215215
end
216216

217217
cannot :update, [MediaObject, SpeedyAF::Proxy::MediaObject] do |media_object|
@@ -381,8 +381,13 @@ def is_editor_of_unit?(unit)
381381
end
382382

383383
def is_exclusively_inherited_from_parent?(media_object)
384-
(!@user.in?(media_object.read_users) && @user.in?(media_object.inherited_read_users)) ||
385-
((@user_groups & media_object.read_groups).empty? && !(@user_groups & media_object.inherited_read_groups).empty?)
384+
# User isn't in item's read users or item's read groups (not explicitly granted access to item) AND
385+
# User isn't in item's read users and is inherited from parent (exclusively inherited user) OR
386+
# User isn't in item's read groups and is in a read group inherited from parent (exclusively inherited group)
387+
# Short form: NOT explicitly granted access to item AND (explicitly inherited user OR explicitly inherited group)
388+
!(@user.in?(media_object.read_users) || !(@user_groups & media_object.read_groups).empty?) &&
389+
((!@user.in?(media_object.read_users) && @user.in?(media_object.inherited_read_users)) ||
390+
((@user_groups & media_object.read_groups).empty? && !(@user_groups & media_object.inherited_read_groups).empty?))
386391
end
387392

388393
def is_member_of_any_collection?

‎spec/controllers/media_objects_controller_spec.rb‎

Lines changed: 148 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,32 @@
821821
end
822822
end
823823

824+
context 'inherited edit permissions' do
825+
let!(:unpublished_media_object) { FactoryBot.create(:media_object, visibility: 'private', collection: collection) }
826+
let!(:disabled_media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, disable_inheritance: true) }
827+
let!(:inherited_media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection) }
828+
let(:collection) { FactoryBot.create(:collection, managers: [user.username]) }
829+
830+
context 'for user' do
831+
context 'from collection level' do
832+
it "should return list of media_objects that the user is authorized to view" do
833+
get 'index', format: 'json'
834+
expect(json.count).to eq(4)
835+
end
836+
end
837+
838+
context 'from unit level' do
839+
let(:collection) { FactoryBot.create(:collection, unit: unit) }
840+
let(:unit) { FactoryBot.create(:unit, managers: [user.username]) }
841+
842+
it "should return list of media_objects that the user is authorized to view" do
843+
get 'index', format: 'json'
844+
expect(json.count).to eq(4)
845+
end
846+
end
847+
end
848+
end
849+
824850
context 'inherited read permissions' do
825851
let!(:unpublished_media_object) { FactoryBot.create(:media_object, visibility: 'private', collection: collection) }
826852
let!(:disabled_media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, disable_inheritance: true) }
@@ -1050,10 +1076,79 @@
10501076

10511077
context "inherited access" do
10521078
context "for user" do
1079+
context 'inherited edit permissions' do
1080+
context "from collection" do
1081+
let(:user) { FactoryBot.create(:user) }
1082+
let(:media_object) { FactoryBot.create(:published_media_object, collection: collection) }
1083+
let(:collection) { FactoryBot.create(:collection, managers: [user.username]) }
1084+
1085+
it "should be available to an edit user granted access at collection level" do
1086+
login_user user.username
1087+
get 'show', params: { id: media_object.id }
1088+
expect(response.response_code).to eq(200)
1089+
end
1090+
1091+
context "when inheritance disabled" do
1092+
let(:media_object) { FactoryBot.create(:published_media_object, collection: collection, disable_inheritance: true) }
1093+
1094+
it "should be available to an edit user granted access at collection level" do
1095+
login_user user.username
1096+
get 'show', params: { id: media_object.id }
1097+
expect(response.response_code).to eq(200)
1098+
end
1099+
1100+
context "and overridden visibility restricts from non-private to private" do
1101+
let(:collection) { FactoryBot.create(:collection, default_visibility: 'public', managers: [user.username]) }
1102+
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, disable_inheritance: true) }
1103+
1104+
it "should be available to an edit user granted access at collection level" do
1105+
login_user user.username
1106+
get 'show', params: { id: media_object.id }
1107+
expect(response.response_code).to eq(200)
1108+
end
1109+
end
1110+
end
1111+
end
1112+
1113+
context "from unit" do
1114+
let(:user) { FactoryBot.create(:user) }
1115+
let(:media_object) { FactoryBot.create(:published_media_object, collection: collection) }
1116+
let(:collection) { FactoryBot.create(:collection, unit: unit) }
1117+
let(:unit) { FactoryBot.create(:unit, managers: [user.username]) }
1118+
1119+
it "should be available to an edit user granted access at unit level" do
1120+
login_user user.username
1121+
get 'show', params: { id: media_object.id }
1122+
expect(response.response_code).to eq(200)
1123+
end
1124+
1125+
context "when inheritance disabled" do
1126+
let(:media_object) { FactoryBot.create(:published_media_object, :with_master_file, collection: collection, disable_inheritance: true) }
1127+
1128+
it "should be available to an edit user granted access at unit level" do
1129+
login_user user.username
1130+
get 'show', params: { id: media_object.id }
1131+
expect(response.response_code).to eq(200)
1132+
end
1133+
1134+
context "and overridden visibility restricts from non-private to private" do
1135+
let(:collection) { FactoryBot.create(:collection, default_visibility: 'public', unit: unit) }
1136+
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, disable_inheritance: true) }
1137+
1138+
it "should be available to an edit user granted access at collection level" do
1139+
login_user user.username
1140+
get 'show', params: { id: media_object.id }
1141+
expect(response.response_code).to eq(200)
1142+
end
1143+
end
1144+
end
1145+
end
1146+
end
1147+
10531148
context "from collection" do
10541149
let(:user) { FactoryBot.create(:user) }
10551150
let(:mo_member) { FactoryBot.create(:user) }
1056-
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_users: [mo_member.username]) }
1151+
let(:media_object) { FactoryBot.create(:published_media_object, collection: collection, read_users: [mo_member.username]) }
10571152
let(:collection) { FactoryBot.create(:collection, default_read_users: [user.username]) }
10581153

10591154
it "should be available to a user granted access at collection level" do
@@ -1064,7 +1159,7 @@
10641159

10651160
context "when inheritance disabled" do
10661161
let(:multi_user) { FactoryBot.create(:user) }
1067-
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_users: [mo_member.username, multi_user.username], disable_inheritance: true) }
1162+
let(:media_object) { FactoryBot.create(:published_media_object, collection: collection, read_users: [mo_member.username, multi_user.username], disable_inheritance: true) }
10681163
let(:collection) { FactoryBot.create(:collection, default_read_users: [user.username, multi_user.username]) }
10691164

10701165
it "should block access to inherited user" do
@@ -1084,13 +1179,24 @@
10841179
get 'show', params: { id: media_object.id }
10851180
expect(response.response_code).to eq(200)
10861181
end
1087-
end
1182+
1183+
context "and overridden visibility restricts from non-private to private" do
1184+
let(:collection) { FactoryBot.create(:collection, default_visibility: 'public') }
1185+
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_users: [mo_member.username], disable_inheritance: true) }
1186+
1187+
it "should not block access to non-inherited user" do
1188+
login_user mo_member.username
1189+
get 'show', params: { id: media_object.id }
1190+
expect(response.response_code).to eq(200)
1191+
end
1192+
end
1193+
end
10881194
end
10891195

10901196
context "from unit" do
10911197
let(:user) { FactoryBot.create(:user) }
10921198
let(:mo_member) { FactoryBot.create(:user) }
1093-
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_users: [mo_member.username]) }
1199+
let(:media_object) { FactoryBot.create(:published_media_object, collection: collection, read_users: [mo_member.username]) }
10941200
let(:collection) { FactoryBot.create(:collection, unit: unit) }
10951201
let(:unit) { FactoryBot.create(:unit, default_read_users: [user.username]) }
10961202

@@ -1102,7 +1208,7 @@
11021208

11031209
context "when inheritance disabled" do
11041210
let(:multi_user) { FactoryBot.create(:user) }
1105-
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_users: [mo_member.username, multi_user.username], disable_inheritance: true) }
1211+
let(:media_object) { FactoryBot.create(:published_media_object, collection: collection, read_users: [mo_member.username, multi_user.username], disable_inheritance: true) }
11061212
let(:collection) { FactoryBot.create(:collection, unit: unit) }
11071213
let(:unit) { FactoryBot.create(:unit, default_read_users: [user.username, multi_user.username]) }
11081214

@@ -1123,6 +1229,17 @@
11231229
get 'show', params: { id: media_object.id }
11241230
expect(response.response_code).to eq(200)
11251231
end
1232+
1233+
context "and overridden visibility restricts from non-private to private" do
1234+
let(:collection) { FactoryBot.create(:collection, default_visibility: 'public', unit: unit) }
1235+
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_users: [mo_member.username], disable_inheritance: true) }
1236+
1237+
it "should not block access to non-inherited user" do
1238+
login_user mo_member.username
1239+
get 'show', params: { id: media_object.id }
1240+
expect(response.response_code).to eq(200)
1241+
end
1242+
end
11261243
end
11271244
end
11281245
end
@@ -1175,6 +1292,19 @@
11751292
get 'show', params: { id: media_object.id }
11761293
expect(response.response_code).to eq(200)
11771294
end
1295+
1296+
context "and overridden visibility restricts from non-private to private" do
1297+
let(:collection) { FactoryBot.create(:collection, default_visibility: 'public') }
1298+
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_groups: [group.name], disable_inheritance: true) }
1299+
1300+
it "should not block access to non-inherited group" do
1301+
allow(controller).to receive(:current_user).and_return(group_member)
1302+
allow(group_member).to receive(:groups).and_return([group.name])
1303+
login_user group_member.username
1304+
get 'show', params: { id: media_object.id }
1305+
expect(response.response_code).to eq(200)
1306+
end
1307+
end
11781308
end
11791309
end
11801310

@@ -1221,6 +1351,19 @@
12211351
get 'show', params: { id: media_object.id }
12221352
expect(response.response_code).to eq(200)
12231353
end
1354+
1355+
context "and overridden visibility restricts from non-private to private" do
1356+
let(:collection) { FactoryBot.create(:collection, default_visibility: 'public', unit: unit) }
1357+
let(:media_object) { FactoryBot.create(:published_media_object, visibility: 'private', collection: collection, read_groups: [group.name], disable_inheritance: true) }
1358+
1359+
it "should not block access to non-inherited group" do
1360+
allow(controller).to receive(:current_user).and_return(group_member)
1361+
allow(group_member).to receive(:groups).and_return([group.name])
1362+
login_user group_member.username
1363+
get 'show', params: { id: media_object.id }
1364+
expect(response.response_code).to eq(200)
1365+
end
1366+
end
12241367
end
12251368
end
12261369
end

0 commit comments

Comments
 (0)