+
<%= format_text @document.description, attachments: @document.attachments %>
<%= list_attachments(api_v3_document_resource(@document)) %>
-
diff --git a/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb b/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb
index 698858902e5..0b0ded8222a 100644
--- a/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb
+++ b/modules/ldap_groups/app/services/ldap_groups/synchronize_filter_service.rb
@@ -69,7 +69,8 @@ module LdapGroups
##
# Create or update the synchronized group item
def create_or_update_sync_group(dn)
- LdapGroups::SynchronizedGroup.find_or_initialize_by(dn: dn).tap do |sync|
+ group = LdapGroups::SynchronizedGroup
+ .find_or_initialize_by(dn: dn).tap do |sync|
# Always set the filter and auth source, in case multiple filters match the same group
# they are simply being re-assigned to the latest one
sync.filter_id = filter.id
@@ -78,6 +79,8 @@ module LdapGroups
# Tell the group to synchronize users if the filter has requested it to
sync.sync_users = filter.sync_users
end
+
+ group.tap { group.save! if group.persisted? }
end
##
diff --git a/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb b/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb
index 0a9c817219a..8ae6642932d 100644
--- a/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb
+++ b/modules/ldap_groups/app/services/ldap_groups/synchronize_groups_service.rb
@@ -43,7 +43,7 @@ module LdapGroups
def map_to_users(sync_group, entries)
create_missing!(entries) if sync_group.sync_users
- User.where(login: entries.keys)
+ User.where('LOWER(login) IN (?)', entries.keys.map(&:downcase))
end
##
diff --git a/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb b/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb
index fe8d76ba70a..660cb0f9d02 100644
--- a/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb
+++ b/modules/ldap_groups/spec/factories/synchronized_filter_factory.rb
@@ -5,5 +5,6 @@ FactoryBot.define do
group_name_attribute { 'cn' }
base_dn { 'dc=example,dc=com' }
auth_source factory: :ldap_auth_source
+ sync_users { true }
end
end
diff --git a/modules/ldap_groups/spec/factories/synchronized_group_factory.rb b/modules/ldap_groups/spec/factories/synchronized_group_factory.rb
index cd773296f44..bab7846d7e7 100644
--- a/modules/ldap_groups/spec/factories/synchronized_group_factory.rb
+++ b/modules/ldap_groups/spec/factories/synchronized_group_factory.rb
@@ -3,5 +3,6 @@ FactoryBot.define do
dn { 'cn=foo,ou=groups,dc=example,dc=com' }
group factory: :group
auth_source factory: :ldap_auth_source
+ sync_users { true }
end
end
diff --git a/modules/ldap_groups/spec/services/synchronization_spec.rb b/modules/ldap_groups/spec/services/synchronization_spec.rb
index 9f27cb387aa..18533904d25 100644
--- a/modules/ldap_groups/spec/services/synchronization_spec.rb
+++ b/modules/ldap_groups/spec/services/synchronization_spec.rb
@@ -189,7 +189,6 @@ describe LdapGroups::SynchronizeGroupsService, with_ee: %i[ldap_groups] do
let(:user_aa729) { User.find_by login: 'aa729' }
let(:user_bb459) { User.find_by login: 'bb459' }
-
context 'and users sync in the groups enabled' do
let(:sync_users) { true }
@@ -222,7 +221,6 @@ describe LdapGroups::SynchronizeGroupsService, with_ee: %i[ldap_groups] do
let(:user_aa729) { User.find_by login: 'aa729' }
let(:user_bb459) { User.find_by login: 'bb459' }
-
context 'and users sync in the groups enabled' do
let(:sync_users) { true }
@@ -310,7 +308,7 @@ describe LdapGroups::SynchronizeGroupsService, with_ee: %i[ldap_groups] do
describe 'removing memberships' do
context 'with a user in a group thats not in ldap' do
let(:group_foo) { FactoryBot.create :group, lastname: 'foo_internal', members: [user_cc414, user_aa729] }
- let(:manager) { FactoryBot.create :role, name: 'Manager' }
+ let(:manager) { FactoryBot.create :role, name: 'Manager' }
let(:project) { FactoryBot.create :project, name: 'Project 1', identifier: 'project1', members: { group_foo => [manager] } }
before do
@@ -375,4 +373,18 @@ describe LdapGroups::SynchronizeGroupsService, with_ee: %i[ldap_groups] do
end
end
end
+
+ context 'when one user does not match case' do
+ before do
+ group_foo
+ synced_foo
+ user_aa729.update_attribute(:login, 'Aa729')
+ end
+
+ it 'synchronized the membership of aa729 to foo' do
+ subject
+ expect(synced_foo.users.count).to eq(1)
+ expect(group_foo.users).to eq([user_aa729])
+ end
+ end
end
diff --git a/modules/ldap_groups/spec/services/synchronize_filter_spec.rb b/modules/ldap_groups/spec/services/synchronize_filter_spec.rb
index 719f3440eb9..4c3176f2e78 100644
--- a/modules/ldap_groups/spec/services/synchronize_filter_spec.rb
+++ b/modules/ldap_groups/spec/services/synchronize_filter_spec.rb
@@ -29,11 +29,11 @@ describe LdapGroups::SynchronizeFilterService, with_ee: %i[ldap_groups] do
let(:synced_foo) do
FactoryBot.create :ldap_synchronized_group, dn: 'cn=foo,ou=groups,dc=example,dc=com', group: group_foo,
- auth_source: auth_source
+ auth_source: auth_source
end
let(:synced_bar) do
FactoryBot.create :ldap_synchronized_group, dn: 'cn=bar,ou=groups,dc=example,dc=com', group: group_bar,
- auth_source: auth_source
+ auth_source: auth_source
end
let(:filter_foo_bar) { FactoryBot.create :ldap_synchronized_filter, auth_source: auth_source }
@@ -83,6 +83,33 @@ describe LdapGroups::SynchronizeFilterService, with_ee: %i[ldap_groups] do
end
end
+ describe 'when one group already exists with different settings' do
+ let(:synced_foo) do
+ FactoryBot.create :ldap_synchronized_group,
+ dn: 'cn=foo,ou=groups,dc=example,dc=com',
+ group: group_foo,
+ sync_users: false,
+ auth_source: auth_source
+ end
+ let(:filter_foo_bar) do
+ FactoryBot.create :ldap_synchronized_filter,
+ sync_users: true,
+ auth_source: auth_source
+ end
+
+ before do
+ synced_foo
+ end
+
+ it 'the group receives the value of the filter' do
+ expect(synced_foo.sync_users).to eq false
+ expect { subject }.not_to raise_error
+
+ synced_foo.reload
+ expect(synced_foo.sync_users).to eq true
+ end
+ end
+
describe 'when it has a group that no longer exists in ldap' do
let!(:group_doesnotexist) { FactoryBot.create :group, lastname: 'doesnotexist' }
let!(:synced_doesnotexist) do
diff --git a/spec/contracts/groups/shared_contract_examples.rb b/spec/contracts/groups/shared_contract_examples.rb
index fba59aaf042..85af0dc25e1 100644
--- a/spec/contracts/groups/shared_contract_examples.rb
+++ b/spec/contracts/groups/shared_contract_examples.rb
@@ -32,18 +32,32 @@ require 'spec_helper'
shared_examples_for 'group contract' do
let(:group_name) { 'The group' }
+ let(:group_users_user_ids) { [42, 43] }
let(:group_users) do
- [FactoryBot.build_stubbed(:group_user, user_id: 1),
- FactoryBot.build_stubbed(:group_user, user_id: 2)]
+ group_users_user_ids.map { |id| FactoryBot.build_stubbed(:group_user, user_id: id) }
end
- it_behaves_like 'contract is valid for active admins and invalid for regular users'
+ shared_context 'with real group users' do
+ # make sure users actually exist (not just stubbed) in this case
+ # so GroupUser validations checking for the existance of group and user don't fail
+ before do
+ group_users_user_ids.each do |id|
+ FactoryBot.create :user, id: id
+ end
+ end
+ end
+
+ it_behaves_like 'contract is valid for active admins and invalid for regular users' do
+ include_context 'with real group users'
+ end
describe 'validations' do
let(:current_user) { FactoryBot.build_stubbed :admin }
context 'name' do
context 'is valid' do
+ include_context 'with real group users'
+
it_behaves_like 'contract is valid'
end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index a23ad9382f9..2c786338c29 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -73,6 +73,16 @@ describe Group, type: :model do
describe '#group_users' do
context 'when adding a user' do
+ context 'which does not exist' do
+ it 'does not create a group user' do
+ count = group.group_users.count
+ gu = group.group_users.create user_id: User.maximum(:id).to_i + 1
+
+ expect(gu).not_to be_valid
+ expect(group.group_users.count).to eq count
+ end
+ end
+
it 'updates the timestamp' do
updated_at = group.updated_at
group.group_users.create(user: user)
diff --git a/spec/requests/api/v3/attachments/attachment_resource_shared_examples.rb b/spec/requests/api/v3/attachments/attachment_resource_shared_examples.rb
index c98cacec997..832d72fd703 100644
--- a/spec/requests/api/v3/attachments/attachment_resource_shared_examples.rb
+++ b/spec/requests/api/v3/attachments/attachment_resource_shared_examples.rb
@@ -154,6 +154,34 @@ shared_examples 'it supports direct uploads' do
end
end
end
+
+ context 'with an attachment whitelist', with_settings: { attachment_whitelist: ['text/csv'] } do
+ context 'with an allowed content type' do
+ let(:metadata) { { fileName: 'cats.csv', fileSize: file.size, contentType: 'text/csv' } }
+
+ it 'should succeed' do
+ expect(subject.status).to eq 201
+ end
+ end
+
+ context 'with a forbidden content type' do
+ let(:metadata) { { fileName: 'cats.txt', fileSize: file.size, contentType: 'text/plain' } }
+
+ it 'should fail' do
+ expect(subject.status).to eq 422
+ expect(subject.body).to include "not whitelisted"
+ end
+ end
+
+ context 'with a non-specific content type not on the whitelist' do
+ let(:metadata) { { fileName: 'cats.bin', fileSize: file.size, contentType: 'application/binary' } }
+
+ # the actual whitelist check will be performed in the FinishDirectUpload job in this case
+ it 'should still succeed' do
+ expect(subject.status).to eq 201
+ end
+ end
+ end
end
end
end
diff --git a/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb b/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb
index 885e0136404..b6124d40012 100644
--- a/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb
+++ b/spec/workers/attachments/finish_direct_upload_job_integration_spec.rb
@@ -31,8 +31,11 @@
require 'spec_helper'
describe Attachments::FinishDirectUploadJob, 'integration', type: :job do
+ shared_let(:user) { FactoryBot.create :admin }
+
let!(:pending_attachment) do
FactoryBot.create(:attachment,
+ author: user,
downloads: -1,
digest: '',
container: container)
@@ -131,4 +134,41 @@ describe Attachments::FinishDirectUploadJob, 'integration', type: :job do
it_behaves_like 'turning pending attachment into a standard attachment'
it_behaves_like "adding a journal to the attachment in the name of the attachment's author"
end
+
+ context 'with an incompatible attachment whitelist',
+ with_settings: { attachment_whitelist: %w[image/png] } do
+ let!(:container) { FactoryBot.create(:work_package) }
+ let!(:container_timestamp) { container.updated_at }
+
+ it "Does not save the attachment" do
+ allow(pending_attachment).to receive(:save!)
+ allow(OpenProject.logger).to receive(:error)
+
+ job.perform(pending_attachment.id)
+
+ expect(pending_attachment).not_to have_received(:save!)
+ expect(OpenProject.logger).to have_received(:error)
+
+ container.reload
+
+ expect(container.attachments).to be_empty
+ expect { pending_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
+
+ context 'with the user not being allowed',
+ with_settings: { attachment_whitelist: %w[image/png] } do
+ shared_let(:user) { FactoryBot.create :user }
+ let!(:container) { FactoryBot.create(:work_package) }
+
+ it "Does not save the attachment" do
+ allow(pending_attachment).to receive(:save!)
+
+ job.perform(pending_attachment.id)
+
+ expect(pending_attachment).not_to have_received(:save!)
+
+ expect { pending_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+ end
end