update

lib/api/variables.rb

+module API
+  # Projects variables API
+  class Variables < Grape::API
+    before { authenticate! }
+    before { authorize_admin_project }
+
+    resource :projects do
+      # Get project variables
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   page (optional) - The page number for pagination
+      #   per_page (optional) - The value of items per page to show
+      # Example Request:
+      #   GET /projects/:id/variables
+      get ':id/variables' do
+        variables = user_project.variables
+        present paginate(variables), with: Entities::Variable
+      end
+
+      # Get specific variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (required) - The `key` of variable
+      # Example Request:
+      #   GET /projects/:id/variables/:key
+      get ':id/variables/:key' do
+        key = params[:key]
+        variable = user_project.variables.find_by(key: key.to_s)
+
+        return not_found!('Variable') unless variable
+
+        present variable, with: Entities::Variable
+      end
+
+      # Create a new variable in project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (required) - The key of variable
+      #   value (required) - The value of variable
+      # Example Request:
+      #   POST /projects/:id/variables
+      post ':id/variables' do
+        required_attributes! [:key, :value]
+
+        variable = user_project.variables.create(key: params[:key], value: params[:value])
+
+        if variable.valid?
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
+      end
+
+      # Update existing variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (optional) - The `key` of variable
+      #   value (optional) - New value for `value` field of variable
+      # Example Request:
+      #   PUT /projects/:id/variables/:key
+      put ':id/variables/:key' do
+        variable = user_project.variables.find_by(key: params[:key].to_s)
+
+        return not_found!('Variable') unless variable
+
+        attrs = attributes_for_keys [:value]
+        if variable.update(attrs)
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
+      end
+
+      # Delete existing variable of a project
+      #
+      # Parameters:
+      #   id (required) - The ID of a project
+      #   key (required) - The ID of a variable
+      # Example Request:
+      #   DELETE /projects/:id/variables/:key
+      delete ':id/variables/:key' do
+        variable = user_project.variables.find_by(key: params[:key].to_s)
+
+        return not_found!('Variable') unless variable
+        variable.destroy
+
+        present variable, with: Entities::Variable
+      end
+    end
+  end
+end

lib/ci/api/builds.rb

@@ -78,11 +78,13 @@ module Ci
         # Parameters:
         #   id (required) - The ID of a build
         #   token (required) - The build authorization token
-        #   file (required) - The uploaded file
+        #   file (required) - Artifacts file
         # Parameters (accelerated by GitLab Workhorse):
         #   file.path - path to locally stored body (generated by Workhorse)
         #   file.name - real filename as send in Content-Disposition
         #   file.type - real content type as send in Content-Type
+        #   metadata.path - path to locally stored body (generated by Workhorse)
+        #   metadata.name - filename (generated by Workhorse)
         # Headers:
         #   BUILD-TOKEN (required) - The build authorization token, the same as token
         # Body:
@@ -96,13 +98,20 @@ module Ci
           build = Ci::Build.find_by_id(params[:id])
           not_found! unless build
           authenticate_build_token!(build)
-          forbidden!('build is not running') unless build.running?
-          file = uploaded_file!(:file, ArtifactUploader.artifacts_upload_path)
-          file_to_large! unless file.size < max_artifacts_size
-          if build.update_attributes(artifacts_file: file)
-            present build, with: Entities::Build
+          forbidden!('Build is not running!') unless build.running?
+          artifacts_upload_path = ArtifactUploader.artifacts_upload_path
+          artifacts = uploaded_file(:file, artifacts_upload_path)
+          metadata = uploaded_file(:metadata, artifacts_upload_path)
+          bad_request!('Missing artifacts file!') unless artifacts
+          file_to_large! unless artifacts.size < max_artifacts_size
+
+          build.artifacts_file = artifacts
+          build.artifacts_metadata = metadata
+
+          if build.save
+            present(build, with: Entities::Build)
           else
             render_validation_error!(build)
           end
@@ -148,6 +157,7 @@ module Ci
           not_found! unless build
           authenticate_build_token!(build)
           build.remove_artifacts_file!
+          build.remove_artifacts_metadata!
         end
       end
     end

lib/gitlab/ci/build/artifacts/metadata.rb

+require 'zlib'
+require 'json'
+
+module Gitlab
+  module Ci
+    module Build
+      module Artifacts
+        class Metadata
+          class ParserError < StandardError; end
+
+          VERSION_PATTERN = /^[\w\s]+(\d+\.\d+\.\d+)/
+          INVALID_PATH_PATTERN = %r{(^\.?\.?/)|(/\.?\.?/)}
+
+          attr_reader :file, :path, :full_version
+
+          def initialize(file, path)
+            @file, @path = file, path
+            @full_version = read_version
+          end
+
+          def version
+            @full_version.match(VERSION_PATTERN)[1]
+          end
+
+          def errors
+            gzip do |gz|
+              read_string(gz) # version
+              errors = read_string(gz)
+              raise ParserError, 'Errors field not found!' unless errors
+
+              begin
+                JSON.parse(errors)
+              rescue JSON::ParserError
+                raise ParserError, 'Invalid errors field!'
+              end
+            end
+          end
+
+          def find_entries!
+            gzip do |gz|
+              2.times { read_string(gz) } # version and errors fields
+              match_entries(gz)
+            end
+          end
+
+          def to_entry
+            entries = find_entries!
+            Entry.new(@path, entries)
+          end
+
+          private
+
+          def match_entries(gz)
+            entries = {}
+            match_pattern = %r{^#{Regexp.escape(@path)}[^/]*/?$}
+
+            until gz.eof? do
+              begin
+                path = read_string(gz).force_encoding('UTF-8')
+                meta = read_string(gz).force_encoding('UTF-8')
+               
+                next unless path.valid_encoding? && meta.valid_encoding?
+                next unless path =~ match_pattern
+                next if path =~ INVALID_PATH_PATTERN
+
+                entries[path] = JSON.parse(meta, symbolize_names: true)
+              rescue JSON::ParserError, Encoding::CompatibilityError
+                next
+              end
+            end
+
+            entries
+          end
+
+          def read_version
+            gzip do |gz|
+              version_string = read_string(gz)
+
+              unless version_string
+                raise ParserError, 'Artifacts metadata file empty!'
+              end
+
+              unless version_string =~ VERSION_PATTERN
+                raise ParserError, 'Invalid version!'
+              end
+
+              version_string.chomp
+            end
+          end
+
+          def read_uint32(gz)
+            binary = gz.read(4)
+            binary.unpack('L>')[0] if binary
+          end
+
+          def read_string(gz)
+            string_size = read_uint32(gz)
+            return nil unless string_size
+            gz.read(string_size)
+          end
+
+          def gzip(&block)
+            Zlib::GzipReader.open(@file, &block)
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/build/artifacts/metadata/entry.rb

+module Gitlab
+  module Ci::Build::Artifacts
+    class Metadata
+      ##
+      # Class that represents an entry (path and metadata) to a file or
+      # directory in GitLab CI Build Artifacts binary file / archive
+      #
+      # This is IO-operations safe class, that does similar job to
+      # Ruby's Pathname but without the risk of accessing filesystem.
+      #
+      # This class is working only with UTF-8 encoded paths.
+      #
+      class Entry
+        attr_reader :path, :entries
+        attr_accessor :name
+
+        def initialize(path, entries)
+          @path = path.dup.force_encoding('UTF-8')
+          @entries = entries
+
+          if path.include?("\0")
+            raise ArgumentError, 'Path contains zero byte character!'
+          end
+
+          unless path.valid_encoding?
+            raise ArgumentError, 'Path contains non-UTF-8 byte sequence!'
+          end
+        end
+
+        def directory?
+          blank_node? || @path.end_with?('/')
+        end
+
+        def file?
+          !directory?
+        end
+
+        def has_parent?
+          nodes > 0
+        end
+
+        def parent
+          return nil unless has_parent?
+          self.class.new(@path.chomp(basename), @entries)
+        end
+
+        def basename
+          (directory? && !blank_node?) ? name + '/' : name
+        end
+
+        def name
+          @name || @path.split('/').last.to_s
+        end
+
+        def children
+          return [] unless directory?
+          return @children if @children
+
+          child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$}
+          @children = select_entries { |path| path =~ child_pattern }
+        end
+
+        def directories(opts = {})
+          return [] unless directory?
+          dirs = children.select(&:directory?)
+          return dirs unless has_parent? && opts[:parent]
+
+          dotted_parent = parent
+          dotted_parent.name = '..'
+          dirs.prepend(dotted_parent)
+        end
+
+        def files
+          return [] unless directory?
+          children.select(&:file?)
+        end
+
+        def metadata
+          @entries[@path] || {}
+        end
+
+        def nodes
+          @path.count('/') + (file? ? 1 : 0)
+        end
+
+        def blank_node?
+          @path.empty? # "" is considered to be './'
+        end
+
+        def exists?
+          blank_node? || @entries.include?(@path)
+        end
+
+        def empty?
+          children.empty?
+        end
+
+        def to_s
+          @path
+        end
+
+        def ==(other)
+          @path == other.path && @entries == other.entries
+        end
+
+        def inspect
+          "#{self.class.name}: #{@path}"
+        end
+
+        private
+
+        def select_entries
+          selected = @entries.select { |path, _metadata| yield path }
+          selected.map { |path, _metadata| self.class.new(path, @entries) }
+        end
+      end
+    end
+  end
+end

lib/gitlab/ldap/access.rb

@@ -37,15 +37,15 @@ module Gitlab
           # Block user in GitLab if he/she was blocked in AD
           if Gitlab::LDAP::Person.disabled_via_active_directory?(user.ldap_identity.extern_uid, adapter)
-            user.block
+            user.ldap_block
             false
           else
-            user.activate if user.blocked? && !ldap_config.block_auto_created_users
+            user.activate if user.ldap_blocked?
             true
           end
         else
           # Block the user if they no longer exist in LDAP/AD
-          user.block 
+          user.ldap_block
           false
         end
       rescue

spec/controllers/admin/identities_controller_spec.rb

+require 'spec_helper'
+
+describe Admin::IdentitiesController do
+  let(:admin) { create(:admin) }
+  before { sign_in(admin) }
+
+  describe 'UPDATE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      put :update, user_id: user.username, id: user.ldap_identity.id, identity: { provider: 'twitter' }
+    end
+  end
+
+  describe 'DELETE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      delete :destroy, user_id: user.username, id: user.ldap_identity.id
+    end
+  end
+end

spec/controllers/admin/users_controller_spec.rb

@@ -34,17 +34,34 @@ describe Admin::UsersController do
   end
   describe 'PUT unblock/:id' do
-    let(:user) { create(:user) }
-
-    before do
-      user.block
+    context 'ldap blocked users' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain') }
+
+      before do
+        user.ldap_block
+      end
+
+      it 'will not unblock user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_truthy
+        expect(flash[:alert]).to eq 'This user cannot be unlocked manually from GitLab'
+      end
     end
-    it 'unblocks user' do
-      put :unblock, id: user.username
-      user.reload
-      expect(user.blocked?).to be_falsey
-      expect(flash[:notice]).to eq 'Successfully unblocked'
+    context 'manually blocked users' do
+      let(:user) { create(:user) }
+
+      before do
+        user.block
+      end
+
+      it 'unblocks user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_falsey
+        expect(flash[:notice]).to eq 'Successfully unblocked'
+      end
     end
   end

spec/controllers/sent_notification_controller_spec.rb

+require 'rails_helper'
+
+describe SentNotificationsController, type: :controller do
+  let(:user)              { create(:user) }
+  let(:issue)             { create(:issue, author: user) }
+  let(:sent_notification) { create(:sent_notification, noteable: issue) }
+
+  describe 'GET #unsubscribe' do
+    it 'returns a 404 when calling without existing id' do
+      get(:unsubscribe, id: '0' * 32)
+
+      expect(response.status).to be 404
+    end
+
+    context 'calling with id' do
+      it 'shows a flash message to the user' do
+        get(:unsubscribe, id: sent_notification.reply_key)
+
+        expect(response.status).to be 302
+
+        expect(response).to redirect_to new_user_session_path
+        expect(controller).to set_flash[:notice].to(/unsubscribed/).now
+      end
+    end
+  end
+end

spec/factories.rb

@@ -212,4 +212,11 @@ FactoryGirl.define do
     provider 'ldapmain'
     extern_uid 'my-ldap-id'
   end
+
+  factory :sent_notification do
+    project
+    recipient factory: :user
+    noteable factory: :issue
+    reply_key "0123456789abcdef" * 2
+  end
 end

spec/factories/ci/variables.rb

+# == Schema Information
+#
+# Table name: ci_variables
+#
+#  id                   :integer          not null, primary key
+#  project_id           :integer          not null
+#  key                  :string(255)
+#  value                :text
+#  encrypted_value      :text
+#  encrypted_value_salt :string(255)
+#  encrypted_value_iv   :string(255)
+#  gl_project_id        :integer
+#
+
+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :ci_variable, class: Ci::Variable do
+    sequence(:key) { |n| "VARIABLE_#{n}" }
+    value 'VARIABLE_VALUE'
+  end
+end

spec/features/builds_spec.rb

@@ -80,7 +80,11 @@ describe "Builds" do
         visit namespace_project_build_path(@project.namespace, @project, @build)
       end
-      it { expect(page).to have_content 'Download artifacts' }
+      it 'has button to download artifacts' do
+        page.within('.artifacts') do
+          expect(page).to have_content 'Download'
+        end
+      end
     end
   end
@@ -111,7 +115,7 @@ describe "Builds" do
     before do
       @build.update_attributes(artifacts_file: artifacts_file)
       visit namespace_project_build_path(@project.namespace, @project, @build)
-      click_link 'Download artifacts'
+      page.within('.artifacts') { click_link 'Download' }
     end
     it { expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type) }

spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do
+  let(:entries) do
+    { 'path/' => {},
+      'path/dir_1/' => {},
+      'path/dir_1/file_1' => {},
+      'path/dir_1/file_b' => {},
+      'path/dir_1/subdir/' => {},
+      'path/dir_1/subdir/subfile' => {},
+      'path/second_dir' => {},
+      'path/second_dir/dir_3/file_2' => {},
+      'path/second_dir/dir_3/file_3'=> {},
+      'another_directory/'=> {},
+      'another_file' => {},
+      '/file/with/absolute_path' => {} }
+  end
+
+  def path(example)
+    entry(example.metadata[:path])
+  end
+
+  def entry(path)
+    described_class.new(path, entries)
+  end
+
+  describe '/file/with/absolute_path', path: '/file/with/absolute_path' do
+    subject { |example| path(example) }
+
+    it { is_expected.to be_file }
+    it { is_expected.to have_parent }
+
+    describe '#basename' do
+      subject { |example| path(example).basename }
+      it { is_expected.to eq 'absolute_path' }
+    end
+  end
+
+  describe 'path/dir_1/', path: 'path/dir_1/' do
+    subject { |example| path(example) }
+    it { is_expected.to have_parent }
+    it { is_expected.to be_directory }
+
+    describe '#basename' do
+      subject { |example| path(example).basename }
+      it { is_expected.to eq 'dir_1/' }
+    end
+
+    describe '#name' do
+      subject { |example| path(example).name }
+      it { is_expected.to eq 'dir_1' }
+    end
+
+    describe '#parent' do
+      subject { |example| path(example).parent }
+      it { is_expected.to eq entry('path/') }
+    end
+
+    describe '#children' do
+      subject { |example| path(example).children }
+
+      it { is_expected.to all(be_an_instance_of described_class) }
+      it do
+        is_expected.to contain_exactly entry('path/dir_1/file_1'),
+                                       entry('path/dir_1/file_b'),
+                                       entry('path/dir_1/subdir/')
+      end
+    end
+
+    describe '#files' do
+      subject { |example| path(example).files }
+
+      it { is_expected.to all(be_file) }
+      it { is_expected.to all(be_an_instance_of described_class) }
+      it do
+        is_expected.to contain_exactly entry('path/dir_1/file_1'),
+                                       entry('path/dir_1/file_b')
+      end
+    end
+
+    describe '#directories' do
+      context 'without options' do
+        subject { |example| path(example).directories }
+
+        it { is_expected.to all(be_directory) }
+        it { is_expected.to all(be_an_instance_of described_class) }
+        it { is_expected.to contain_exactly entry('path/dir_1/subdir/') }
+      end
+
+      context 'with option parent: true' do
+        subject { |example| path(example).directories(parent: true) }
+
+        it { is_expected.to all(be_directory) }
+        it { is_expected.to all(be_an_instance_of described_class) }
+        it do
+          is_expected.to contain_exactly entry('path/dir_1/subdir/'),
+                                         entry('path/')
+        end
+      end
+
+      describe '#nodes' do
+        subject { |example| path(example).nodes }
+        it { is_expected.to eq 2 }
+      end
+
+      describe '#exists?' do
+        subject { |example| path(example).exists? }
+        it { is_expected.to be true }
+      end
+
+      describe '#empty?' do
+        subject { |example| path(example).empty? }
+        it { is_expected.to be false }
+      end
+    end
+  end
+
+  describe 'empty path', path: '' do
+    subject { |example| path(example) }
+    it { is_expected.to_not have_parent }
+
+    describe '#children' do
+      subject { |example| path(example).children }
+      it { expect(subject.count).to eq 3 }
+    end
+
+  end
+
+  describe 'path/dir_1/subdir/subfile', path: 'path/dir_1/subdir/subfile' do
+    describe '#nodes' do
+      subject { |example| path(example).nodes }
+      it { is_expected.to eq 4 }
+    end
+  end
+
+  describe 'non-existent/', path: 'non-existent/' do
+    describe '#empty?' do
+      subject { |example| path(example).empty? }
+      it { is_expected.to be true }
+    end
+
+    describe '#exists?' do
+      subject { |example| path(example).exists? }
+      it { is_expected.to be false }
+    end
+  end
+
+  describe 'another_directory/', path: 'another_directory/' do
+    describe '#empty?' do
+      subject { |example| path(example).empty? }
+      it { is_expected.to be true }
+    end
+  end
+
+  describe '#metadata' do
+    let(:entries) do
+      { 'path/' => { name: '/path/' },
+        'path/file1' => { name: '/path/file1' },
+        'path/file2' => { name: '/path/file2' } }
+    end
+
+    subject do
+      described_class.new('path/file1', entries).metadata[:name]
+    end
+
+    it { is_expected.to eq '/path/file1' }
+  end
+end

spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Build::Artifacts::Metadata do
+  def metadata(path = '')
+    described_class.new(metadata_file_path, path)
+  end
+
+  let(:metadata_file_path) do
+    Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz'
+  end
+
+  context 'metadata file exists' do
+    describe '#find_entries! empty string' do
+      subject { metadata('').find_entries! }
+
+      it 'matches correct paths' do
+        expect(subject.keys).to contain_exactly 'ci_artifacts.txt',
+                                                'other_artifacts_0.1.2/',
+                                                'rails_sample.jpg',
+                                                'tests_encoding/'
+      end
+
+      it 'matches metadata for every path' do
+        expect(subject.keys.count).to eq 4
+      end
+
+      it 'return Hashes for each metadata' do
+        expect(subject.values).to all(be_kind_of(Hash))
+      end
+    end
+
+    describe '#find_entries! other_artifacts_0.1.2/' do
+      subject { metadata('other_artifacts_0.1.2/').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/'
+      end
+    end
+
+    describe '#find_entries! other_artifacts_0.1.2/another-subdirectory/' do
+      subject { metadata('other_artifacts_0.1.2/another-subdirectory/').find_entries! }
+
+      it 'matches correct paths' do
+        expect(subject.keys).
+          to contain_exactly '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) }
+    end
+
+    describe '#full_version' do
+      subject { metadata('').full_version }
+      it { is_expected.to eq 'GitLab Build Artifacts Metadata 0.0.1' }
+    end
+
+    describe '#version' do
+      subject { metadata('').version }
+      it { is_expected.to eq '0.0.1' }
+    end
+
+    describe '#errors' do
+      subject { metadata('').errors }
+      it { is_expected.to eq({}) }
+    end
+  end
+
+  context 'metadata file does not exist' do
+    let(:metadata_file_path) { '' }
+
+    describe '#find_entries!' do
+      it 'raises error' do
+        expect { metadata.find_entries! }.to raise_error(Errno::ENOENT)
+      end
+    end
+  end
+end

spec/lib/gitlab/ldap/access_spec.rb

@@ -13,64 +13,58 @@ describe Gitlab::LDAP::Access, lib: true do
       end
       it { is_expected.to be_falsey }
-      
+
       it 'should block user in GitLab' do
         access.allowed?
         expect(user).to be_blocked
+        expect(user).to be_ldap_blocked
       end
     end
     context 'when the user is found' do
       before do
-        allow(Gitlab::LDAP::Person).
-          to receive(:find_by_dn).and_return(:ldap_user)
+        allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user)
       end
       context 'and the user is disabled via active directory' do
         before do
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(true)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true)
         end
         it { is_expected.to be_falsey }
-        it "should block user in GitLab" do
+        it 'should block user in GitLab' do
           access.allowed?
           expect(user).to be_blocked
+          expect(user).to be_ldap_blocked
         end
       end
       context 'and has no disabled flag in active diretory' do
         before do
-          user.block
-
-          allow(Gitlab::LDAP::Person).
-            to receive(:disabled_via_active_directory?).and_return(false)
+          allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false)
         end
         it { is_expected.to be_truthy }
         context 'when auto-created users are blocked' do
-
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(true)
+            user.block
           end
-          it "does not unblock user in GitLab" do
+          it 'does not unblock user in GitLab' do
             access.allowed?
             expect(user).to be_blocked
+            expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic
           end
         end
-        context "when auto-created users are not blocked" do
-
+        context 'when auto-created users are not blocked' do
           before do
-            allow_any_instance_of(Gitlab::LDAP::Config).
-              to receive(:block_auto_created_users).and_return(false)
+            user.ldap_block
           end
-          it "should unblock user in GitLab" do
+          it 'should unblock user in GitLab' do
             access.allowed?
             expect(user).not_to be_blocked
           end
@@ -80,8 +74,7 @@ describe Gitlab::LDAP::Access, lib: true 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)
+          allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false)
         end
         it { is_expected.to be_truthy }

spec/mailers/notify_spec.rb

@@ -104,6 +104,14 @@ describe Notify do
     it { is_expected.to have_body_text /View Commit/ }
   end
+  shared_examples 'an unsubscribeable thread' do
+    it { is_expected.to have_body_text /unsubscribe/ }
+  end
+
+  shared_examples "a user cannot unsubscribe through footer link" do
+    it { is_expected.not_to have_body_text /unsubscribe/ }
+  end
+
   describe 'for new users, the email' do
     let(:example_site_path) { root_path }
     let(:new_user) { create(:user, email: new_user_address, created_by_id: 1) }
@@ -115,6 +123,7 @@ describe Notify do
     it_behaves_like 'an email sent from GitLab'
     it_behaves_like 'a new user email', new_user_address
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like 'a user cannot unsubscribe through footer link'
     it 'contains the password text' do
       is_expected.to have_body_text /Click here to set your password/
@@ -134,7 +143,6 @@ describe Notify do
     end
   end
-
   describe 'for users that signed up, the email' do
     let(:example_site_path) { root_path }
     let(:new_user) { create(:user, email: new_user_address, password: "securePassword") }
@@ -144,6 +152,7 @@ describe Notify do
     it_behaves_like 'an email sent from GitLab'
     it_behaves_like 'a new user email', new_user_address
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like 'a user cannot unsubscribe through footer link'
     it 'should not contain the new user\'s password' do
       is_expected.not_to have_body_text /password/
@@ -157,6 +166,7 @@ describe Notify do
     it_behaves_like 'an email sent from GitLab'
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like 'a user cannot unsubscribe through footer link'
     it 'is sent to the new user' do
       is_expected.to deliver_to key.user.email
@@ -181,6 +191,7 @@ describe Notify do
     subject { Notify.new_email_email(email.id) }
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like 'a user cannot unsubscribe through footer link'
     it 'is sent to the new user' do
       is_expected.to deliver_to email.user.email
@@ -227,6 +238,7 @@ describe Notify do
           it_behaves_like 'an assignee email'
           it_behaves_like 'an email starting a new thread', 'issue'
           it_behaves_like 'it should show Gmail Actions View Issue link'
+          it_behaves_like 'an unsubscribeable thread'
           it 'has the correct subject' do
             is_expected.to have_subject /#{project.name} \| #{issue.title} \(##{issue.iid}\)/
@@ -253,6 +265,7 @@ describe Notify do
           it_behaves_like 'a multiple recipients email'
           it_behaves_like 'an answer to an existing thread', 'issue'
           it_behaves_like 'it should show Gmail Actions View Issue link'
+          it_behaves_like "an unsubscribeable thread"
           it 'is sent as the author' do
             sender = subject.header[:from].addrs[0]
@@ -283,6 +296,7 @@ describe Notify do
           it_behaves_like 'an answer to an existing thread', 'issue'
           it_behaves_like 'it should show Gmail Actions View Issue link'
+          it_behaves_like 'an unsubscribeable thread'
           it 'is sent as the author' do
             sender = subject.header[:from].addrs[0]
@@ -319,6 +333,7 @@ describe Notify do
           it_behaves_like 'an assignee email'
           it_behaves_like 'an email starting a new thread', 'merge_request'
           it_behaves_like 'it should show Gmail Actions View Merge request link'
+          it_behaves_like "an unsubscribeable thread"
           it 'has the correct subject' do
             is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
@@ -345,6 +360,7 @@ describe Notify do
           subject { Notify.new_merge_request_email(merge_request_with_description.assignee_id, merge_request_with_description.id) }
           it_behaves_like 'it should show Gmail Actions View Merge request link'
+          it_behaves_like "an unsubscribeable thread"
           it 'contains the description' do
             is_expected.to have_body_text /#{merge_request_with_description.description}/
@@ -357,6 +373,7 @@ describe Notify do
           it_behaves_like 'a multiple recipients email'
           it_behaves_like 'an answer to an existing thread', 'merge_request'
           it_behaves_like 'it should show Gmail Actions View Merge request link'
+          it_behaves_like "an unsubscribeable thread"
           it 'is sent as the author' do
             sender = subject.header[:from].addrs[0]
@@ -387,6 +404,7 @@ describe Notify do
           it_behaves_like 'an answer to an existing thread', 'merge_request'
           it_behaves_like 'it should show Gmail Actions View Merge request link'
+          it_behaves_like "an unsubscribeable thread"
           it 'is sent as the author' do
             sender = subject.header[:from].addrs[0]
@@ -417,6 +435,7 @@ describe Notify do
           it_behaves_like 'a multiple recipients email'
           it_behaves_like 'an answer to an existing thread', 'merge_request'
           it_behaves_like 'it should show Gmail Actions View Merge request link'
+          it_behaves_like "an unsubscribeable thread"
           it 'is sent as the merge author' do
             sender = subject.header[:from].addrs[0]
@@ -446,6 +465,7 @@ describe Notify do
       it_behaves_like 'an email sent from GitLab'
       it_behaves_like 'it should not have Gmail Actions links'
+      it_behaves_like "a user cannot unsubscribe through footer link"
       it 'has the correct subject' do
         is_expected.to have_subject /Project was moved/
@@ -468,6 +488,7 @@ describe Notify do
       it_behaves_like 'an email sent from GitLab'
       it_behaves_like 'it should not have Gmail Actions links'
+      it_behaves_like "a user cannot unsubscribe through footer link"
       it 'has the correct subject' do
         is_expected.to have_subject /Access to project was granted/
@@ -518,6 +539,7 @@ describe Notify do
         it_behaves_like 'a note email'
         it_behaves_like 'an answer to an existing thread', 'commit'
         it_behaves_like 'it should show Gmail Actions View Commit link'
+        it_behaves_like "a user cannot unsubscribe through footer link"
         it 'has the correct subject' do
           is_expected.to have_subject /#{commit.title} \(#{commit.short_id}\)/
@@ -538,6 +560,7 @@ describe Notify do
         it_behaves_like 'a note email'
         it_behaves_like 'an answer to an existing thread', 'merge_request'
         it_behaves_like 'it should show Gmail Actions View Merge request link'
+        it_behaves_like 'an unsubscribeable thread'
         it 'has the correct subject' do
           is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
@@ -558,6 +581,7 @@ describe Notify do
         it_behaves_like 'a note email'
         it_behaves_like 'an answer to an existing thread', 'issue'
         it_behaves_like 'it should show Gmail Actions View Issue link'
+        it_behaves_like 'an unsubscribeable thread'
         it 'has the correct subject' do
           is_expected.to have_subject /#{issue.title} \(##{issue.iid}\)/
@@ -579,6 +603,7 @@ describe Notify do
     it_behaves_like 'an email sent from GitLab'
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'has the correct subject' do
       is_expected.to have_subject /Access to group was granted/
@@ -607,6 +632,7 @@ describe Notify do
     subject { ActionMailer::Base.deliveries.last }
     it_behaves_like 'an email sent from GitLab'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'is sent to the new user' do
       is_expected.to deliver_to 'new-email@mail.com'
@@ -629,6 +655,7 @@ describe Notify do
     subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :create) }
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]
@@ -657,6 +684,7 @@ describe Notify do
     subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :create) }
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]
@@ -684,6 +712,7 @@ describe Notify do
     subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :delete) }
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]
@@ -707,6 +736,7 @@ describe Notify do
     subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) }
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]
@@ -734,6 +764,7 @@ describe Notify do
     subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: send_from_committer_email) }
     it_behaves_like 'it should not have Gmail Actions links'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]
@@ -839,6 +870,7 @@ describe Notify do
     subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) }
     it_behaves_like 'it should show Gmail Actions View Commit link'
+    it_behaves_like "a user cannot unsubscribe through footer link"
     it 'is sent as the author' do
       sender = subject.header[:from].addrs[0]

spec/models/build_spec.rb

-# == Schema Information
-#
-# Table name: builds
-#
-#  id                 :integer          not null, primary key
-#  project_id         :integer
-#  status             :string(255)
-#  finished_at        :datetime
-#  trace              :text
-#  created_at         :datetime
-#  updated_at         :datetime
-#  started_at         :datetime
-#  runner_id          :integer
-#  commit_id          :integer
-#  coverage           :float
-#  commands           :text
-#  job_id             :integer
-#  name               :string(255)
-#  deploy             :boolean          default(FALSE)
-#  options            :text
-#  allow_failure      :boolean          default(FALSE), not null
-#  stage              :string(255)
-#  trigger_request_id :integer
-#
-
 require 'spec_helper'
 describe Ci::Build, models: true do
@@ -368,21 +343,75 @@ describe Ci::Build, models: true do
     end
   end
-  describe :download_url do
-    subject { build.download_url }
+  describe :artifacts_download_url do
+    subject { build.artifacts_download_url }
     it "should be nil if artifact doesn't exist" do
       build.update_attributes(artifacts_file: nil)
       is_expected.to be_nil
     end
-    it 'should be nil if artifact exist' do
+    it 'should not be nil if artifact exist' do
       gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
       build.update_attributes(artifacts_file: gif)
       is_expected.to_not be_nil
     end
   end
+  describe :artifacts_browse_url do
+    subject { build.artifacts_browse_url }
+
+    it "should be nil if artifacts browser is unsupported" do
+      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_browser_supported?).and_return(true)
+      is_expected.to_not be_nil
+    end
+  end
+
+  describe :artifacts? do
+    subject { build.artifacts? }
+
+    context 'artifacts archive does not exist' do
+      before { build.update_attributes(artifacts_file: nil) }
+      it { is_expected.to be_falsy }
+    end
+
+    context 'artifacts archive exists' do
+      before do
+        gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
+        build.update_attributes(artifacts_file: gif)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
+
+  describe :artifacts_browser_supported? do
+    subject { build.artifacts_browser_supported? }
+    context 'artifacts metadata does not exist' do
+      it { is_expected.to be_falsy }
+    end
+
+    context 'artifacts archive is a zip file and metadata exists' do
+      before do
+        fixture_dir = Rails.root + 'spec/fixtures/'
+        archive = fixture_file_upload(fixture_dir + 'ci_build_artifacts.zip',
+                                      'application/zip')
+        metadata = fixture_file_upload(fixture_dir + 'ci_build_artifacts_metadata.gz',
+                                       'application/x-gzip')
+        build.update_attributes(artifacts_file: archive)
+        build.update_attributes(artifacts_metadata: metadata)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
   describe :repo_url do
     let(:build) { FactoryGirl.create :ci_build }
     let(:project) { build.project }

spec/models/identity_spec.rb

+# == Schema Information
+#
+# Table name: identities
+#
+#  id         :integer          not null, primary key
+#  extern_uid :string(255)
+#  provider   :string(255)
+#  user_id    :integer
+#  created_at :datetime
+#  updated_at :datetime
+#
+
+require 'spec_helper'
+
+RSpec.describe Identity, models: true do
+
+  describe 'relations' do
+    it { is_expected.to belong_to(:user) }
+  end
+
+  describe 'fields' do
+    it { is_expected.to respond_to(:provider) }
+    it { is_expected.to respond_to(:extern_uid) }
+  end
+
+  describe '#is_ldap?' do
+    let(:ldap_identity) { create(:identity, provider: 'ldapmain') }
+    let(:other_identity) { create(:identity, provider: 'twitter') }
+
+    it 'returns true if it is a ldap identity' do
+      expect(ldap_identity.ldap?).to be_truthy
+    end
+
+    it 'returns false if it is not a ldap identity' do
+      expect(other_identity.ldap?).to be_falsey
+    end
+  end
+end

spec/models/user_spec.rb

@@ -569,27 +569,39 @@ describe User, models: true do
     end
   end
-  describe :ldap_user? do
-    it "is true if provider name starts with ldap" do
-      user = create(:omniauth_user, provider: 'ldapmain')
-      expect( user.ldap_user? ).to be_truthy
-    end
-    it "is false for other providers" do
-      user = create(:omniauth_user, provider: 'other-provider')
-      expect( user.ldap_user? ).to be_falsey
+  context 'ldap synchronized user' do
+    describe :ldap_user? do
+      it 'is true if provider name starts with ldap' do
+        user = create(:omniauth_user, provider: 'ldapmain')
+        expect(user.ldap_user?).to be_truthy
+      end
+      it 'is false for other providers' do
+        user = create(:omniauth_user, provider: 'other-provider')
+        expect(user.ldap_user?).to be_falsey
+      end
+
+      it 'is false if no extern_uid is provided' do
+        user = create(:omniauth_user, extern_uid: nil)
+        expect(user.ldap_user?).to be_falsey
+      end
     end
-    it "is false if no extern_uid is provided" do
-      user = create(:omniauth_user, extern_uid: nil)
-      expect( user.ldap_user? ).to be_falsey
+    describe :ldap_identity do
+      it 'returns ldap identity' do
+        user = create :omniauth_user
+        expect(user.ldap_identity.provider).not_to be_empty
+      end
     end
-  end
-  describe :ldap_identity do
-    it "returns ldap identity" do
-      user = create :omniauth_user
-      expect(user.ldap_identity.provider).not_to be_empty
+    describe '#ldap_block' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain', name: 'John Smith') }
+
+      it 'blocks user flaging the action caming from ldap' do
+        user.ldap_block
+        expect(user.blocked?).to be_truthy
+        expect(user.ldap_blocked?).to be_truthy
+      end
     end
   end