diff --git a/CHANGELOG b/CHANGELOG index b9399e4acc41f7466ca641f512ded30eb3bb0483..014c964256743614137f8d6123ffa34de64488bf 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -12,6 +12,7 @@ v 8.4.0 (unreleased) - Add user's last used IP addresses to admin page (Stan Hu) - Add housekeeping function to project settings page - The default GitLab logo now acts as a loading indicator + - LDAP group sync: Remove user from group when they are removed from LDAP - Fix caching issue where build status was not updating in project dashboard (Stan Hu) - Accept 2xx status codes for successful Web hook triggers (Stan Hu) - Fix missing date of month in network graph when commits span a month (Stan Hu) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 1d99cf07c14e3e7f2c4f22f73149dbba6484a0b4..37e5a7049ffe2ed690a25d1af4eb88169259d573 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -347,17 +347,17 @@ def artifacts_download_url end def artifacts_browse_url - if artifacts_metadata? + if artifacts_browser_supported? browse_namespace_project_build_artifacts_path(project.namespace, project, self) end end - def artifacts_metadata? + def artifacts_browser_supported? artifacts? && artifacts_metadata.exists? end - def artifacts_metadata_entry(path, **options) - Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path, **options).to_entry + def artifacts_metadata_entry(path) + Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path).to_entry end private diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index ceabd29fd52ad6d82562b3138b1281e2f52eacbf..e1bb4c92e405cac08507463c5db6bc546ae23646 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -2,7 +2,6 @@ module Projects class UpdatePagesService < BaseService BLOCK_SIZE = 32.kilobytes MAX_SIZE = 1.terabyte - SITE_PATH = 'public/' attr_reader :build @@ -61,42 +60,13 @@ def create_status end def extract_archive!(temp_path) - if artifacts.ends_with?('.tar.gz') || artifacts.ends_with?('.tgz') - extract_tar_archive!(temp_path) - elsif artifacts.ends_with?('.zip') - extract_zip_archive!(temp_path) - else - raise 'unsupported artifacts format' - end - end - - def extract_tar_archive!(temp_path) results = Open3.pipeline(%W(gunzip -c #{artifacts}), %W(dd bs=#{BLOCK_SIZE} count=#{blocks}), - %W(tar -x -C #{temp_path} #{SITE_PATH}), + %W(tar -x -C #{temp_path} public/), err: '/dev/null') raise 'pages failed to extract' unless results.compact.all?(&:success?) end - def extract_zip_archive!(temp_path) - raise 'missing artifacts metadata' unless build.artifacts_metadata? - - # Calculate page size after extract - public_entry = build.artifacts_metadata_entry(SITE_PATH, recursive: true) - - if public_entry.total_size > max_size - raise "artifacts for pages are too large: #{total_size}" - end - - # Requires UnZip at least 6.00 Info-ZIP. - # -n never overwrite existing files - # We add * to end of SITE_PATH, because we want to extract SITE_PATH and all subdirectories - site_path = File.join(SITE_PATH, '*') - unless system(*%W(unzip -n #{artifacts} #{site_path} -d #{temp_path})) - raise 'pages failed to extract' - end - end - def deploy_page!(archive_public_path) # Do atomic move of pages # Move and removal may not be atomic, but they are significantly faster then extracting and removal @@ -121,11 +91,10 @@ def latest? def blocks # Calculate dd parameters: we limit the size of pages - 1 + max_size / BLOCK_SIZE - end - - def max_size - current_application_settings.max_pages_size.megabytes || MAX_SIZE + max_size = current_application_settings.max_pages_size.megabytes + max_size ||= MAX_SIZE + blocks = 1 + max_size / BLOCK_SIZE + blocks end def tmp_path diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml index ba1fdc6f0e7416ea88419de2784facb9baf96e73..2be572d3b10edfe9f690066c7d6add3fca753f0a 100644 --- a/app/views/projects/builds/show.html.haml +++ b/app/views/projects/builds/show.html.haml @@ -96,7 +96,7 @@ .center .btn-group{ role: :group } = link_to "Download", @build.artifacts_download_url, class: 'btn btn-sm btn-primary' - - if @build.artifacts_metadata? + - if @build.artifacts_browser_supported? = link_to "Browse", @build.artifacts_browse_url, class: 'btn btn-sm btn-primary' .build-widget diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 14c822b0ce4b0fe7d49824e6aeaa74f5bc240526..ee8f71c22037e7477d45d7cd0c8ae31993b9115a 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -496,11 +496,6 @@ production: &base # If you use non-standard ssh port you need to specify it # ssh_port: 22 - # git-annex support (EE only) - # If this setting is set to true, the same setting in config.yml of - # gitlab-shell needs to be set to true - # git_annex_enabled: false - ## Git settings # CAUTION! # Use the default values unless you really know what you are doing diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index b9f0c3fd4ab92a03b266ac1d9a7513f1f3ae36f7..1f97862b79b210ee53db6751ef94742f729fe5e1 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -342,7 +342,6 @@ def on_standard_port?(config) Settings.gitlab_shell['ssh_user'] ||= Settings.gitlab.user Settings.gitlab_shell['owner_group'] ||= Settings.gitlab.user Settings.gitlab_shell['ssh_path_prefix'] ||= Settings.send(:build_gitlab_shell_ssh_path_prefix) -Settings.gitlab_shell['git_annex_enabled'] ||= false # # Backup diff --git a/doc/integration/ldap.md b/doc/integration/ldap.md index 026e327baa9504286b7bba1dcbf071efec2434da..fc9441d112f76c38bc618183038d025f057e116d 100644 --- a/doc/integration/ldap.md +++ b/doc/integration/ldap.md @@ -156,22 +156,18 @@ If multiple LDAP email attributes are present, e.g. `mail: foo@bar.com` and `ema ## LDAP group synchronization (GitLab Enterprise Edition) -LDAP group synchronization in GitLab Enterprise Edition allows you to -synchronize the members of a GitLab group with one or more LDAP groups. +LDAP group synchronization in GitLab Enterprise Edition allows you to synchronize the members of a GitLab group with one or more LDAP groups. ### Setting up LDAP group synchronization -Before enabling group synchronization, you need to make sure that the -`group_base` field is set in your LDAP settings on your `gitlab.rb` or -`gitlab.yml` file. This setting will tell GitLab where to look for groups -within your LDAP server. +Before enabling group synchronization, you need to make sure that the `group_base` field is set in your LDAP settings on +your `gitlab.rb` or `gitlab.yml` file. This setting will tell GitLab where to look for groups within your LDAP server. ``` group_base: 'OU=groups,DC=example,DC=com' ``` -Suppose we want to synchronize the GitLab group 'example group' with the LDAP -group 'Engineering'. +Suppose we want to synchronize the GitLab group 'example group' with the LDAP group 'Engineering'. 1. As an owner, go to the group settings page for 'example group'. @@ -183,25 +179,18 @@ As an admin you can also go to the group edit page in the admin area. 2. Enter 'Engineering' as the LDAP Common Name (CN) in the 'LDAP Group cn' field. -3. Enter a default group access level in the 'LDAP Access' field; let's say -Developer. +3. Enter a default group access level in the 'LDAP Access' field; let's say Developer. ![LDAP group settings filled in](ldap/select_group_cn_engineering.png) 4. Click 'Add synchronization' to add the new LDAP group link. -Now every time a member of the 'Engineering' LDAP group signs in, they -automatically become a Developer-level member of the 'example group' GitLab -group. Users who are already signed in will see the change in membership after -up to one hour. +Now every time a member of the 'Engineering' LDAP group signs in, they automatically become a Developer-level member of the 'example group' GitLab group. Users who are already signed in will see the change in membership after up to one hour. ### Synchronizing with more than one LDAP group (GitLab EE 7.3 and newer) -If you have two LDAP group links, and a user belongs to both LDAP groups, they -will receive the _highest_ access level of the two. For example, suppose you -have configured group sync for the 'Engineering' group at level 'Developer' and -'QA' group at level 'Reporter'. User 'Jane' belongs to both the 'Engineering' and -'QA' LDAP groups so she will receive the 'Developer' role. +If you want to add the members of LDAP group to your GitLab group you can add an additional LDAP group link. +If you have two LDAP group links, e.g. 'cn=Engineering' at level 'Developer' and 'cn=QA' at level 'Reporter', and user Jane belongs to both the 'Engineering' and 'QA' LDAP groups, she will get the _highest_ access level of the two, namely 'Developer'. ![Two linked LDAP groups](ldap/two_linked_ldap_groups.png) diff --git a/doc/workflow/groups.md b/doc/workflow/groups.md index ad795d72471038309d0a7f28ddaa4778ac7158b3..abf233c5a7a1779bd2a3ded5bfe900356c8cb2b2 100644 --- a/doc/workflow/groups.md +++ b/doc/workflow/groups.md @@ -70,28 +70,28 @@ gitlab_rails['gitlab_default_can_create_group'] = false # line in /home/git/gitlab/config/gitlab.yml ``` -## Lock project membership to members of the group +## Lock project membership to members of this group -In GitLab Enterprise Edition it is possible to lock membership in project to the level of members in group. +In GitLab Enterprise Edition it is possible to lock membership in project to the +level of members in group. -This allows group owner to lock down any new project membership to any of the projects within the group allowing tighter control over project membership. +This allows group owner to lock down any new project membership to any of the +projects within the group allowing tighter control over project membership. -To enable this feature, navigate to group settings page, select `Member lock` and `Save group`. +To enable this feature, navigate to group settings page, select `Member lock` +and `Save group`. ![Checkbox for membership lock](groups/membership_lock.png) -This will disable the option for all users who previously had permissions to operate project memberships so no new users can be added. Furthermore, any request to add new user to project through API will not be possible. +This will disable the option for all users who previously had permissions to +operate project memberships so no new users can be added. Furthermore, any +request to add new user to project through API will not be possible. -## Namespaces in groups +## Prevent projects in this group from sharing a project with another group -By default, groups only get 20 namespaces at a time because the API results are paginated. +In GitLab Enterprise it is possible to prevent projects in a group from sharing +a project with another group. This allows for tighter control over project +access. -To get more (up to 100), pass the following as an argument to the API call: -``` -/groups?per_page=100 -``` - -And to switch pages add: -``` -/groups?per_page=100&page=2 -``` +To enable this feature, navigate to the group settings page. Select `Share with +group lock` and save the group. diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index f2020c82d40543d1baf443a66fa0eb44c63dade6..1344f5d120b32f6f17d84dda408ddebb04e00cea 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -13,8 +13,8 @@ class ParserError < StandardError; end attr_reader :file, :path, :full_version - def initialize(file, path, **opts) - @file, @path, @opts = file, path, opts + def initialize(file, path) + @file, @path = file, path @full_version = read_version end @@ -52,9 +52,7 @@ def to_entry def match_entries(gz) entries = {} - - child_pattern = '[^/]*/?$' unless @opts[:recursive] - match_pattern = /^#{Regexp.escape(@path)}#{child_pattern}/ + match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$} until gz.eof? do begin diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb index 7f4c750b6fdaf8973296135c86be7a739764bc70..25b71fc3275e00cfd12c60d4bceae22d84e2303b 100644 --- a/lib/gitlab/ci/build/artifacts/metadata/entry.rb +++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb @@ -95,13 +95,6 @@ def empty? children.empty? end - def total_size - descendant_pattern = %r{^#{Regexp.escape(@path)}} - entries.sum do |path, entry| - (entry[:size] if path =~ descendant_pattern).to_i - end - end - def to_s @path end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index fc020be52773dca08b6bebb680b72a91e676933a..b578f0841b870d0307215d47626872e4b1480464 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -85,8 +85,6 @@ def download_access_check end def push_access_check(changes) - return build_status_object(true) if git_annex_branch_sync?(changes) - if user user_push_access_check(changes) elsif deploy_key @@ -327,23 +325,5 @@ def git_annex_access_check(project, changes) build_status_object(false, "You don't have permission") end end - - def git_annex_branch_sync?(changes) - return false unless Gitlab.config.gitlab_shell.git_annex_enabled - return false if changes.blank? - - changes = changes.lines if changes.kind_of?(String) - - # Iterate over all changes to find if user allowed all of them to be applied - # 0000000000000000000000000000000000000000 3073696294ddd52e9e6b6fc3f429109cac24626f refs/heads/synced/git-annex - # 0000000000000000000000000000000000000000 65be9df0e995d36977e6d76fc5801b7145ce19c9 refs/heads/synced/master - changes.map(&:strip).reject(&:blank?).each do |change| - unless change.end_with?("refs/heads/synced/git-annex") || change.include?("refs/heads/synced/") - return false - end - end - - true - end end end diff --git a/lib/gitlab/ldap/access.rb b/lib/gitlab/ldap/access.rb index 7b036a40853861e1798176ac8d0f7251ed719ef1..b719689a8d1650284ec87e957c07f5bb0dffdb8b 100644 --- a/lib/gitlab/ldap/access.rb +++ b/lib/gitlab/ldap/access.rb @@ -16,9 +16,11 @@ def self.open(user, &block) def self.allowed?(user) self.open(user) do |access| + # Whether user is allowed, or not, we should update + # permissions to keep things clean + access.update_permissions if access.allowed? - access.update_permissions - access.update_email + access.update_user user.last_credential_check_at = Time.now user.save true @@ -67,22 +69,15 @@ def ldap_user @ldap_user ||= Gitlab::LDAP::Person.find_by_dn(user.ldap_identity.extern_uid, adapter) end - def update_permissions - if sync_ssh_keys? - update_ssh_keys - end - + def update_user + update_email + update_ssh_keys if sync_ssh_keys? update_kerberos_identity if import_kerberos_identities? + end - # Skip updating group permissions - # if instance does not use group_base setting - return true unless group_base.present? - - update_ldap_group_links - - if admin_group.present? - update_admin_status - end + def update_permissions + update_ldap_group_links if group_base.present? + update_admin_status if admin_group.present? end # Update user ssh keys if they changed in LDAP @@ -164,7 +159,7 @@ def update_admin_status end end - # Loop through all ldap connected groups, and update the users link with it + # Loop throug all ldap conneted groups, and update the users link with it # # We documented what sort of queries an LDAP server can expect from # GitLab EE in doc/integration/ldap.md. Please remember to update that @@ -174,7 +169,7 @@ def update_ldap_group_links active_group_links = group.ldap_group_links.where(cn: cns_with_access) if active_group_links.any? - group.add_users([user.id], active_group_links.maximum(:group_access), skip_notification: true) + group.add_users([user.id], fetch_group_access(group, user, active_group_links), skip_notification: true) elsif group.last_owner?(user) logger.warn "#{self.class.name}: LDAP group sync cannot remove #{user.name} (#{user.id}) from group #{group.name} (#{group.id}) as this is the group's last owner" else @@ -191,6 +186,7 @@ def ldap_groups # returns a collection of cn strings to which the user has access def cns_with_access + return [] unless ldap_user.present? @ldap_groups_with_access ||= ldap_groups.select do |ldap_group| ldap_group.has_member?(ldap_user) end.map(&:cn) @@ -221,6 +217,16 @@ def gitlab_groups_with_ldap_link where(ldap_group_links: { provider: provider }) end + # Get the group_access for a give user. + # Always respect the current level, never downgrade it. + def fetch_group_access(group, user, active_group_links) + current_access_level = group.group_members.where(user_id: user).maximum(:access_level) + max_group_access_level = active_group_links.maximum(:group_access) + + # TODO: Test if nil value of current_access_level in handled properly + [current_access_level, max_group_access_level].compact.max + end + def logger Rails.logger end diff --git a/spec/fixtures/pages.zip b/spec/fixtures/pages.zip deleted file mode 100644 index 9558fcd4b9428385e15ae386f473434d56d3da3f..0000000000000000000000000000000000000000 Binary files a/spec/fixtures/pages.zip and /dev/null differ diff --git a/spec/fixtures/pages.zip.meta b/spec/fixtures/pages.zip.meta deleted file mode 100644 index 1e6198a15f07d743cb4f2fc0466761e4d336fb12..0000000000000000000000000000000000000000 Binary files a/spec/fixtures/pages.zip.meta and /dev/null differ diff --git a/spec/fixtures/pages_empty.zip b/spec/fixtures/pages_empty.zip deleted file mode 100644 index db3f0334c12d868859a979dc3f8fa3d8fa283813..0000000000000000000000000000000000000000 Binary files a/spec/fixtures/pages_empty.zip and /dev/null differ diff --git a/spec/fixtures/pages_empty.zip.meta b/spec/fixtures/pages_empty.zip.meta deleted file mode 100644 index d0b93b3b9c0fd3a1279532e9aa03b6f6915c2963..0000000000000000000000000000000000000000 Binary files a/spec/fixtures/pages_empty.zip.meta and /dev/null differ diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb index acca0b08bab3497622a089a26cd9ee614f59a682..41257103eadbae0377580ab449a1547b18923c52 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb @@ -4,13 +4,13 @@ let(:entries) do { 'path/' => {}, 'path/dir_1/' => {}, - 'path/dir_1/file_1' => { size: 10 }, - 'path/dir_1/file_b' => { size: 10 }, + 'path/dir_1/file_1' => {}, + 'path/dir_1/file_b' => {}, 'path/dir_1/subdir/' => {}, - 'path/dir_1/subdir/subfile' => { size: 10 }, + 'path/dir_1/subdir/subfile' => {}, 'path/second_dir' => {}, - 'path/second_dir/dir_3/file_2' => { size: 10 }, - 'path/second_dir/dir_3/file_3'=> { size: 10 }, + 'path/second_dir/dir_3/file_2' => {}, + 'path/second_dir/dir_3/file_3'=> {}, 'another_directory/'=> {}, 'another_file' => {}, '/file/with/absolute_path' => {} } @@ -112,11 +112,6 @@ def entry(path) subject { |example| path(example).empty? } it { is_expected.to be false } end - - describe '#total_size' do - subject { |example| path(example).total_size } - it { is_expected.to eq(30) } - end end end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index eea01f91879ec97d2ded8e56b17119ff4b74e6c5..828eedfa7b03c923c8faafd2656c4818424d2568 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Build::Artifacts::Metadata do - def metadata(path = '', **opts) - described_class.new(metadata_file_path, path, **opts) + def metadata(path = '') + described_class.new(metadata_file_path, path) end let(:metadata_file_path) do @@ -51,19 +51,6 @@ def metadata(path = '', **opts) end end - describe '#find_entries! recursively for other_artifacts_0.1.2/' do - subject { metadata('other_artifacts_0.1.2/', recursive: true).find_entries! } - - it 'matches correct paths' do - expect(subject.keys). - to contain_exactly 'other_artifacts_0.1.2/', - 'other_artifacts_0.1.2/doc_sample.txt', - 'other_artifacts_0.1.2/another-subdirectory/', - 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', - 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' - end - end - describe '#to_entry' do subject { metadata('').to_entry } it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) } diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index ff4a1c4ecebe1d98d350804d375f81d165c34be0..c462a70230ccb786954968a295e091ae9459b306 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -5,10 +5,6 @@ let(:project) { create(:project) } let(:user) { create(:user) } let(:actor) { user } - let(:git_annex_changes) do - ["6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/synced/git-annex", - "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/synced/named-branch"] - end describe 'can_push_to_branch?' do describe 'push to none protected branch' do @@ -248,65 +244,6 @@ def self.updated_permissions_matrix end end end - - context "when using git annex" do - before { project.team << [user, :master] } - - describe 'and git hooks unset' do - describe 'git annex enabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(true) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - - describe 'git annex disabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(false) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - end - - describe 'and git hooks set' do - before { project.create_git_hook } - - describe 'check commit author email' do - before do - project.git_hook.update(author_email_regex: "@only.com") - end - - describe 'git annex enabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(true) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - - describe 'git annex disabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(false) } - - it { expect(access.push_access_check(git_annex_changes)).to_not be_allowed } - end - end - - describe 'check commit author email' do - before do - allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(5.megabytes.to_i) - project.git_hook.update(max_file_size: 2) - end - - describe 'git annex enabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(true) } - - it { expect(access.push_access_check(git_annex_changes)).to be_allowed } - end - - describe 'git annex disabled' do - before { allow(Gitlab.config.gitlab_shell).to receive(:git_annex_enabled).and_return(false) } - - it { expect(access.push_access_check(git_annex_changes)).to_not be_allowed } - end - end - end - end end describe "git_hook_check" do diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index 8045d1107a02ca5f04a6dbede4d2e3c10983292d..ee6141fceebeaab5335d745b6291ead89eefc420 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -4,7 +4,7 @@ let(:access) { Gitlab::LDAP::Access.new user } let(:user) { create(:omniauth_user) } - describe :allowed? do + describe '#allowed?' do subject { access.allowed? } context 'when the user cannot be found' do @@ -40,7 +40,7 @@ end end - context 'and has no disabled flag in active diretory' do + context 'and has no disabled flag in active directory' do before do allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false) end @@ -71,7 +71,7 @@ end end - context 'withoud ActiveDirectory enabled' do + context 'without ActiveDirectory enabled' do before do allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false) @@ -82,41 +82,67 @@ end end - describe :update_permissions do - subject { access.update_permissions } + describe '#update_user' do + subject { access.update_user } + let(:entry) do + Net::LDAP::Entry.from_single_ldif_string("dn: cn=foo, dc=bar, dc=com") + end + before do + allow(access).to( + receive_messages( + ldap_user: Gitlab::LDAP::Person.new(entry, user.ldap_identity.provider) + ) + ) + end + + it 'updates email address' do + expect(access).to receive(:update_email).once + + subject + end - it "syncs ssh keys if enabled by configuration" do - allow(access).to receive_messages(group_base: '', sync_ssh_keys?: 'sshpublickey', import_kerberos_identities?: false) + it 'syncs ssh keys if enabled by configuration' do + allow(access).to receive_messages(group_base: '', sync_ssh_keys?: true) expect(access).to receive(:update_ssh_keys).once subject end - it "does update group permissions with a group base configured" do - allow(access).to receive_messages(group_base: 'my-group-base', sync_ssh_keys?: false, import_kerberos_identities?: false) + it 'update_kerberos_identity' do + allow(access).to receive_messages(import_kerberos_identities?: true) + expect(access).to receive(:update_kerberos_identity).once + + subject + end + end + + describe '#update_permissions' do + subject { access.update_permissions } + + it 'does update group permissions with a group base configured' do + allow(access).to receive_messages(group_base: 'my-group-base') expect(access).to receive(:update_ldap_group_links) subject end - it "does not update group permissions without a group base configured" do - allow(access).to receive_messages(group_base: '', sync_ssh_keys?: false, import_kerberos_identities?: false) + it 'does not update group permissions without a group base configured' do + allow(access).to receive_messages(group_base: '') expect(access).not_to receive(:update_ldap_group_links) subject end - it "does update admin group permissions if admin group is configured" do - allow(access).to receive_messages(admin_group: 'my-admin-group', update_ldap_group_links: nil, sync_ssh_keys?: false, import_kerberos_identities?: false) + it 'does update admin group permissions if admin group is configured' do + allow(access).to receive_messages(admin_group: 'my-admin-group') expect(access).to receive(:update_admin_status) subject end - it "does update Kerberos identities if Kerberos is enabled and the LDAP server is Active Directory" do - allow(access).to receive_messages(group_base: '', sync_ssh_keys?: false, import_kerberos_identities?: true) - - expect(access).to receive(:update_kerberos_identity) + it 'does not update admin status when admin group is not configured' do + allow(access).to receive_messages(admin_group: '') + expect(access).not_to receive(:update_admin_status) subject end @@ -328,19 +354,19 @@ end end - context 'existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER' do + context "existing access as MASTER for group-1, allowed via ldap-group1 as DEVELOPER" do before do gitlab_group_1.group_members.masters.create(user_id: user.id) gitlab_group_1.ldap_group_links.create({ cn: 'ldap-group1', group_access: Gitlab::Access::DEVELOPER, provider: 'ldapmain' }) end - it 'downgrades the users access' do - expect { access.update_ldap_group_links }.to \ - change{ gitlab_group_1.has_master?(user) }.from(true).to(false) + it "keeps the users master access for group 1" do + expect { access.update_ldap_group_links }.not_to \ + change{ gitlab_group_1.has_master?(user) } end - it 'does not send a notification email' do + it "doesn't send a notification email" do expect { access.update_ldap_group_links }.not_to \ change { ActionMailer::Base.deliveries } end @@ -451,17 +477,33 @@ ] end - let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, user.ldap_identity.provider) } - before do - allow(access).to receive_messages(ldap_user: ldap_user) - allow(ldap_user).to receive(:uid) { 'user1' } allow(ldap_user).to receive(:dn) { 'uid=user1,ou=People,dc=example' } + allow(access).to receive_messages(ldap_groups: ldap_groups) end - it "only returns ldap cns to which the user has access" do - allow(access).to receive_messages(ldap_groups: ldap_groups) - expect(access.cns_with_access).to eql ['group1'] + context 'when the LDAP user exists' do + let(:ldap_user) { Gitlab::LDAP::Person.new(Net::LDAP::Entry.new, user.ldap_identity.provider) } + + before do + allow(access).to receive_messages(ldap_user: ldap_user) + allow(ldap_user).to receive(:uid) { 'user1' } + end + + it 'only returns ldap cns to which the user has access' do + expect(access.cns_with_access).to eq(['group1']) + end + end + + context 'when the LADP user does not exist' do + let(:ldap_user) { nil } + before do + allow(access).to receive_messages(ldap_user: ldap_user) + end + + it 'returns an empty array' do + expect(access.cns_with_access).to eq([]) + end end end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index d30bc7d05541e02c9c3f39dbe48b9fe4bb0ba5a4..d12b9e65c8239a4ba910a425ba1ccc902f502d24 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -362,12 +362,12 @@ subject { build.artifacts_browse_url } it "should be nil if artifacts browser is unsupported" do - allow(build).to receive(:artifacts_metadata?).and_return(false) + allow(build).to receive(:artifacts_browser_supported?).and_return(false) is_expected.to be_nil end it 'should not be nil if artifacts browser is supported' do - allow(build).to receive(:artifacts_metadata?).and_return(true) + allow(build).to receive(:artifacts_browser_supported?).and_return(true) is_expected.to_not be_nil end end @@ -391,8 +391,8 @@ end - describe :artifacts_metadata? do - subject { build.artifacts_metadata? } + describe :artifacts_browser_supported? do + subject { build.artifacts_browser_supported? } context 'artifacts metadata does not exist' do it { is_expected.to be_falsy } end diff --git a/spec/services/projects/update_pages_worker_spec.rb b/spec/services/projects/update_pages_worker_spec.rb index 68e668663400079d5312bb10a0c4aec604bd7ab7..0607c025b9eefdb46b5df4f6922b3a21c60ed61f 100644 --- a/spec/services/projects/update_pages_worker_spec.rb +++ b/spec/services/projects/update_pages_worker_spec.rb @@ -4,7 +4,9 @@ let(:project) { create :project } let(:commit) { create :ci_commit, project: project, sha: project.commit('HEAD').sha } let(:build) { create :ci_build, commit: commit, ref: 'HEAD' } - let(:invalid_file) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png') } + let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/pages.tar.gz', 'application/octet-stream') } + let(:empty_file) { fixture_file_upload(Rails.root + 'spec/fixtures/pages_empty.tar.gz', 'application/octet-stream') } + let(:invalid_file) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'application/octet-stream') } subject { described_class.new(project, build) } @@ -12,50 +14,27 @@ project.remove_pages end - %w(tar.gz zip).each do |format| - context "for valid #{format}" do - let(:file) { fixture_file_upload(Rails.root + "spec/fixtures/pages.#{format}") } - let(:empty_file) { fixture_file_upload(Rails.root + "spec/fixtures/pages_empty.#{format}") } - let(:metadata) do - filename = Rails.root + "spec/fixtures/pages.#{format}.meta" - fixture_file_upload(filename) if File.exists?(filename) - end + context 'for valid file' do + before { build.update_attributes(artifacts_file: file) } - before do - build.update_attributes(artifacts_file: file) - build.update_attributes(artifacts_metadata: metadata) - end - - it 'succeeds' do - expect(project.pages_url).to be_nil - expect(execute).to eq(:success) - expect(project.pages_url).to_not be_nil - end - - it 'limits pages size' do - stub_application_setting(max_pages_size: 1) - expect(execute).to_not eq(:success) - end - - it 'removes pages after destroy' do - expect(PagesWorker).to receive(:perform_in) - expect(project.pages_url).to be_nil - expect(execute).to eq(:success) - expect(project.pages_url).to_not be_nil - project.destroy - expect(Dir.exist?(project.public_pages_path)).to be_falsey - end + it 'succeeds' do + expect(project.pages_url).to be_nil + expect(execute).to eq(:success) + expect(project.pages_url).to_not be_nil + end - it 'fails if sha on branch is not latest' do - commit.update_attributes(sha: 'old_sha') - build.update_attributes(artifacts_file: file) - expect(execute).to_not eq(:success) - end + it 'limits pages size' do + stub_application_setting(max_pages_size: 1) + expect(execute).to_not eq(:success) + end - it 'fails for empty file fails' do - build.update_attributes(artifacts_file: empty_file) - expect(execute).to_not eq(:success) - end + it 'removes pages after destroy' do + expect(PagesWorker).to receive(:perform_in) + expect(project.pages_url).to be_nil + expect(execute).to eq(:success) + expect(project.pages_url).to_not be_nil + project.destroy + expect(Dir.exist?(project.public_pages_path)).to be_falsey end end @@ -69,10 +48,21 @@ expect(execute).to_not eq(:success) end + it 'fails for empty file fails' do + build.update_attributes(artifacts_file: empty_file) + expect(execute).to_not eq(:success) + end + it 'fails for invalid archive' do build.update_attributes(artifacts_file: invalid_file) expect(execute).to_not eq(:success) end + + it 'fails if sha on branch is not latest' do + commit.update_attributes(sha: 'old_sha') + build.update_attributes(artifacts_file: file) + expect(execute).to_not eq(:success) + end def execute subject.execute[:status]