diff --git a/CHANGELOG b/CHANGELOG index 2aabdf2039..22f1040af4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,12 @@ +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) diff --git a/Gemfile b/Gemfile index fc30c9048a..e1b474c172 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index bf575c9100..2d373422c3 100644 --- a/Gemfile.lock +++ b/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) diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 30fb4c5552..987694260c 100644 --- a/app/controllers/snippets_controller.rb +++ b/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 diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index b29ab6cf40..4b0c69f2d2 100644 --- a/app/finders/snippets_finder.rb +++ b/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 diff --git a/app/views/devise/sessions/new.html.haml b/app/views/devise/sessions/new.html.haml index b983278744..ca7e9570b4 100644 --- a/app/views/devise/sessions/new.html.haml +++ b/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 diff --git a/features/snippets/public_snippets.feature b/features/snippets/public_snippets.feature new file mode 100644 index 0000000000..6964badc41 --- /dev/null +++ b/features/snippets/public_snippets.feature @@ -0,0 +1,5 @@ +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" diff --git a/features/snippets/snippets.feature b/features/snippets/snippets.feature index 4c4e3ee2cf..6e8019c326 100644 --- a/features/snippets/snippets.feature +++ b/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 diff --git a/features/steps/shared/snippet.rb b/features/steps/shared/snippet.rb index 432f32defc..bb596c1620 100644 --- a/features/steps/shared/snippet.rb +++ b/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 diff --git a/features/steps/snippets/public_snippets.rb b/features/steps/snippets/public_snippets.rb new file mode 100644 index 0000000000..956aa4a3e7 --- /dev/null +++ b/features/steps/snippets/public_snippets.rb @@ -0,0 +1,17 @@ +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 diff --git a/lib/gitlab/ldap/authentication.rb b/lib/gitlab/ldap/authentication.rb index a5944f9698..8af2c74e95 100644 --- a/lib/gitlab/ldap/authentication.rb +++ b/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 diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb index 3f31e9f4bf..256f579e83 100644 --- a/lib/gitlab/ldap/config.rb +++ b/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: { diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 5af7696818..c645cbc964 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/ldap/config_spec.rb b/spec/lib/gitlab/ldap/config_spec.rb index 76cc7f95c4..3ebb8aae24 100644 --- a/spec/lib/gitlab/ldap/config_spec.rb +++ b/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