Merge branch '7-4-stable-ee-non-conflict' into 'master'

7-4-stable-ee to master. With resolved conflicts

See merge request !222

CHANGELOG

+v 7.4.2
+  - Fix internal snippet exposing for unauthenticated users
+
+v 7.4.1
+  - Fix LDAP authentication for Git HTTP access
+  - Fix LDAP config lookup for provider 'ldap'
+  - Fix public snippets
+  - Fix 500 error on projects with nested submodules
+
 v 7.4.0
   - Refactored membership logic
   - Improve error reporting on users API (Julien Bianchi)

Gemfile

@@ -31,7 +31,7 @@ gem 'omniauth-shibboleth'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem "gitlab_git", '7.0.0.rc9'
+gem "gitlab_git", '7.0.0.rc10'
 
 # Ruby/Rack Git Smart-HTTP Server Handler
 gem 'gitlab-grack', '~> 2.0.0.pre', require: 'grack'

Gemfile.lock

@@ -179,7 +179,7 @@ GEM
       mime-types (~> 1.19)
     gitlab_emoji (0.0.1.1)
       emoji (~> 1.0.1)
-    gitlab_git (7.0.0.rc9)
+    gitlab_git (7.0.0.rc10)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.6)
       gitlab-linguist (~> 3.0)
@@ -624,7 +624,7 @@ DEPENDENCIES
   gitlab-grack (~> 2.0.0.pre)
   gitlab-linguist (~> 3.0.0)
   gitlab_emoji (~> 0.0.1.1)
-  gitlab_git (= 7.0.0.rc9)
+  gitlab_git (= 7.0.0.rc10)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (= 1.1.0)
   gollum-lib (~> 3.0.0)

app/controllers/snippets_controller.rb

@@ -9,7 +9,7 @@ class SnippetsController < ApplicationController
 
   before_filter :set_title
 
-  skip_before_filter :authenticate_user!, only: [:index, :user_index]
+  skip_before_filter :authenticate_user!, only: [:index, :user_index, :show]
 
   respond_to :html
 

app/finders/snippets_finder.rb

@@ -29,6 +29,8 @@ class SnippetsFinder
   def by_user(current_user, user, scope)
     snippets = user.snippets.fresh.non_expired
 
+    return snippets.are_public unless current_user
+
     if user == current_user
       case scope
       when 'are_internal' then

app/views/devise/sessions/new.html.haml

@@ -2,22 +2,22 @@
   .login-heading
     %h3 Sign in
   .login-body
-    - if ldap_enabled? && gitlab_config.signin_enabled
+    - if ldap_enabled?
       %ul.nav.nav-tabs
         - @ldap_servers.each_with_index do |server, i|
-          %li{class: (:active if i==0)}
+          %li{class: (:active if i.zero?)}
             = link_to server['label'], "#tab-#{server['provider_name']}", 'data-toggle' => 'tab'
-        %li
-          = link_to 'Standard', '#tab-signin', 'data-toggle' => 'tab'
+        - if gitlab_config.signin_enabled
+          %li
+            = link_to 'Standard', '#tab-signin', 'data-toggle' => 'tab'
       .tab-content
-        - @ldap_servers.each_with_index do |server,i|
-          %div.tab-pane{id: "tab-#{server['provider_name']}", class: (:active if i==0)}
+        - @ldap_servers.each_with_index do |server, i|
+          %div.tab-pane{id: "tab-#{server['provider_name']}", class: (:active if i.zero?)}
             = render 'devise/sessions/new_ldap', provider: server['provider_name']
-        %div#tab-signin.tab-pane
-          = render 'devise/sessions/new_base'
+        - if gitlab_config.signin_enabled
+          %div#tab-signin.tab-pane
+            = render 'devise/sessions/new_base'
 
-    - elsif ldap_enabled?
-      = render 'devise/sessions/new_ldap', ldap_servers: @ldap_servers
     - elsif gitlab_config.signin_enabled
       = render 'devise/sessions/new_base'
     - else

features/snippets/public_snippets.feature

+Feature: Public snippets
+  Scenario: Unauthenticated user should see public snippets
+    Given There is public "Personal snippet one" snippet
+    And I visit snippet page "Personal snippet one"
+    Then I should see snippet "Personal snippet one"

features/snippets/snippets.feature

@@ -25,4 +25,4 @@ Feature: Snippets
   Scenario: I destroy "Personal snippet one"
     Given I visit snippet page "Personal snippet one"
     And I click link "Destroy"
-    Then I should not see "Personal snippet one" in snippets
+    Then I should not see "Personal snippet one" in snippets
\ No newline at end of file

features/steps/shared/snippet.rb

@@ -51,4 +51,13 @@ module SharedSnippet
            visibility_level: Snippet::PUBLIC,
            author: current_user)
   end
+
+  step 'There is public "Personal snippet one" snippet' do
+    create(:personal_snippet,
+           title: "Personal snippet one",
+           content: "Test content",
+           file_name: "snippet.rb",
+           visibility_level: Snippet::PUBLIC,
+           author: create(:user))
+  end
 end

features/steps/snippets/public_snippets.rb

+class Spinach::Features::PublicSnippets < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedSnippet
+
+  step 'I should see snippet "Personal snippet one"' do
+    page.should have_no_xpath("//i[@class='public-snippet']")
+  end
+
+  step 'I visit snippet page "Personal snippet one"' do
+    visit snippet_path(snippet)
+  end
+
+  def snippet
+    @snippet ||= PersonalSnippet.find_by!(title: "Personal snippet one")
+  end
+end

lib/gitlab/ldap/authentication.rb

@@ -42,7 +42,7 @@ module Gitlab
       end
 
       def adapter
-        OmniAuth::LDAP::Adaptor.new(config.options)
+        OmniAuth::LDAP::Adaptor.new(config.options.symbolize_keys)
       end
 
       def config
@@ -68,4 +68,4 @@ module Gitlab
       end
     end
   end
-end
\ No newline at end of file
+end

lib/gitlab/ldap/config.rb

@@ -16,10 +16,23 @@ module Gitlab
         servers.map {|server| server['provider_name'] }
       end
 
+      def self.valid_provider?(provider)
+        providers.include?(provider)
+      end
+
+      def self.invalid_provider(provider)
+        raise "Unknown provider (#{provider}). Available providers: #{providers}"
+      end
+
       def initialize(provider)
-        @provider = provider
-        invalid_provider unless valid_provider?
-        @options = config_for(provider)
+        if self.class.valid_provider?(provider)
+          @provider = provider
+        elsif provider == 'ldap'
+          @provider = self.class.providers.first
+        else
+          self.class.invalid_provider(provider)
+        end
+        @options = config_for(@provider) # Use @provider, not provider
       end
 
       def enabled?
@@ -93,14 +106,6 @@ module Gitlab
         end
       end
 
-      def valid_provider?
-        self.class.providers.include?(provider)
-      end
-
-      def invalid_provider
-        raise "Unknown provider (#{provider}). Available providers: #{self.class.providers}"
-      end
-
       def auth_options
         {
           auth: {

spec/finders/snippets_finder_spec.rb

@@ -64,6 +64,13 @@ describe SnippetsFinder do
       snippets = SnippetsFinder.new.execute(user, filter: :by_user, user: user)
       snippets.should include(@snippet1, @snippet2, @snippet3)
     end
+
+    it "returns only public snippets if unauthenticated user" do
+      snippets = SnippetsFinder.new.execute(nil, filter: :by_user, user: user)
+      snippets.should include(@snippet3)
+      snippets.should_not include(@snippet2, @snippet1)
+    end
+
   end
 
   context 'by_project filter' do

spec/lib/gitlab/ldap/config_spec.rb

@@ -16,5 +16,19 @@ describe Gitlab::LDAP::Config do
     it "raises an error if a unknow provider is used" do
       expect{ Gitlab::LDAP::Config.new 'unknown' }.to raise_error
     end
+
+    context "if 'ldap' is the provider name" do
+      let(:provider) { 'ldap' }
+
+      context "and 'ldap' is not in defined as a provider" do
+        before { Gitlab::LDAP::Config.stub(providers: %w{ldapmain}) }
+
+        it "uses the first provider" do
+          # Fetch the provider_name attribute from 'options' so that we know
+          # that the 'options' Hash is not empty/nil.
+          expect(config.options['provider_name']).to eq('ldapmain')
+        end
+      end
+    end
   end
-end
\ No newline at end of file
+end