Merge branch 'ce_master' into 'master'

CE master to EE master

See merge request !203

app/controllers/omniauth_callbacks_controller.rb

@@ -49,22 +49,27 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
       redirect_to profile_path
     else
       @user = Gitlab::OAuth::User.new(oauth)
-
-      if Gitlab.config.omniauth['allow_single_sign_on'] && @user.new?
-        @user.save
-      end
+      @user.save
 
       # Only allow properly saved users to login.
       if @user.persisted? && @user.valid?
         sign_in_and_redirect(@user.gl_user)
-      elsif @user.gl_user.errors.any?
-        error_message = @user.gl_user.errors.map{ |attribute, message| "#{attribute} #{message}" }.join(", ")
-        redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return
       else
-        flash[:notice] = "There's no such user!"
-        redirect_to new_user_session_path
+        error_message =
+          if @user.gl_user.errors.any?
+            @user.gl_user.errors.map do |attribute, message|
+              "#{attribute} #{message}"
+            end.join(", ")
+          else
+            ''
+          end
+
+        redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return
       end
     end
+  rescue StandardError
+    flash[:notice] = "There's no such user!"
+    redirect_to new_user_session_path
   end
 
   def oauth

db/fixtures/development/12_snippets.rb

 Gitlab::Seeder.quiet do
-  contents = [
-    `curl https://gist.githubusercontent.com/randx/4275756/raw/da2f262920c96d1a970d48bf2e99147954b1f4bd/glus1204.sh`,
-    `curl https://gist.githubusercontent.com/randx/3754594/raw/11026a295e6ef3a151c635707a3e1e8e15fc4725/gitlab_setup.sh`,
-    `curl https://gist.githubusercontent.com/randx/3065552/raw/29fbd09f4605a5ea22a5a9095e35fd1938dea4d6/gistfile1.sh`,
-  ]
+  content =<<eos
+class Member < ActiveRecord::Base
+  include Notifiable
+  include Gitlab::Access
+
+  belongs_to :user
+  belongs_to :source, polymorphic: true
+
+  validates :user, presence: true
+  validates :source, presence: true
+  validates :user_id, uniqueness: { scope: [:source_type, :source_id], message: "already exists in source" }
+  validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true
+
+  scope :guests, -> { where(access_level: GUEST) }
+  scope :reporters, -> { where(access_level: REPORTER) }
+  scope :developers, -> { where(access_level: DEVELOPER) }
+  scope :masters,  -> { where(access_level: MASTER) }
+  scope :owners,  -> { where(access_level: OWNER) }
+
+  delegate :name, :username, :email, to: :user, prefix: true
+end
+eos
 
   (1..50).each  do |i|
     user = User.all.sample
@@ -12,10 +29,11 @@ Gitlab::Seeder.quiet do
       id: i,
       author_id: user.id,
       title: Faker::Lorem.sentence(3),
-      file_name:  Faker::Internet.domain_word + '.sh',
-      private: [true, false].sample,
-      content: contents.sample,
+      file_name:  Faker::Internet.domain_word + '.rb',
+      visibility_level: Gitlab::VisibilityLevel.values.sample,
+      content: content,
     }])
+
     print('.')
   end
 end

lib/gitlab/oauth/user.rb

@@ -13,22 +13,28 @@ module Gitlab
       end
 
       def persisted?
-        gl_user.persisted?
+        gl_user.try(:persisted?)
       end
 
       def new?
-        !gl_user.persisted?
+        !persisted?
       end
 
       def valid?
-        gl_user.valid?
+        gl_user.try(:valid?)
       end
 
       def save
-        gl_user.save!
-        log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
-        gl_user.block if needs_blocking?
+        unauthorized_to_create unless gl_user
+
+        if needs_blocking?
+          gl_user.save!
+          gl_user.block
+        else
+          gl_user.save!
+        end
 
+        log.info "(OAuth) saving user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
         gl_user
       rescue ActiveRecord::RecordInvalid => e
         log.info "(OAuth) Error saving user: #{gl_user.errors.full_messages}"
@@ -36,10 +42,29 @@ module Gitlab
       end
 
       def gl_user
-        @user ||= find_by_uid_and_provider || build_new_user
+        @user ||= find_by_uid_and_provider
+
+        if signup_enabled?
+          @user ||= build_new_user
+        end
+
+        @user
       end
 
       protected
+
+      def needs_blocking?
+        new? && block_after_signup?
+      end
+
+      def signup_enabled?
+        Gitlab.config.omniauth.allow_single_sign_on
+      end
+
+      def block_after_signup?
+        Gitlab.config.omniauth.block_auto_created_users
+      end
+
       def auth_hash=(auth_hash)
         @auth_hash = AuthHash.new(auth_hash)
       end
@@ -70,13 +95,13 @@ module Gitlab
         Gitlab::AppLogger
       end
 
-      def needs_blocking?
-        Gitlab.config.omniauth['block_auto_created_users']
-      end
-
       def model
         ::User
       end
+
+      def raise_unauthorized_to_create
+        raise StandardError.new("Unauthorized to create user, signup disabled for #{auth_hash.provider}")
+      end
     end
   end
 end

spec/lib/gitlab/oauth/user_spec.rb

@@ -29,26 +29,79 @@ describe Gitlab::OAuth::User do
   end
 
   describe :save do
-    context "LDAP" do
-      let(:provider) { 'ldap' }
-      it "creates a user from LDAP" do
-        oauth_user.save
-
-        expect(gl_user).to be_valid
-        expect(gl_user.extern_uid).to eql uid
-        expect(gl_user.provider).to eql 'ldap'
+    let(:provider) { 'twitter' }
+
+    describe 'signup' do
+      context "with allow_single_sign_on enabled" do
+        before { Gitlab.config.omniauth.stub allow_single_sign_on: true }
+
+        it "creates a user from Omniauth" do
+          oauth_user.save
+
+          expect(gl_user).to be_valid
+          expect(gl_user.extern_uid).to eql uid
+          expect(gl_user.provider).to eql 'twitter'
+        end
+      end
+
+      context "with allow_single_sign_on disabled (Default)" do
+        it "throws an error" do
+          expect{ oauth_user.save }.to raise_error StandardError
+        end
       end
     end
 
-    context "twitter" do
+    describe 'blocking' do
       let(:provider) { 'twitter' }
+      before { Gitlab.config.omniauth.stub allow_single_sign_on: true }
+
+      context 'signup' do
+        context 'dont block on create' do
+          before { Gitlab.config.omniauth.stub block_auto_created_users: false }
+
+          it do
+            oauth_user.save
+            gl_user.should be_valid
+            gl_user.should_not be_blocked
+          end
+        end
+
+        context 'block on create' do
+          before { Gitlab.config.omniauth.stub block_auto_created_users: true }
+
+          it do
+            oauth_user.save
+            gl_user.should be_valid
+            gl_user.should be_blocked
+          end
+        end
+      end
+
+      context 'sign-in' do
+        before do
+          oauth_user.save
+          oauth_user.gl_user.activate
+        end
+
+        context 'dont block on create' do
+          before { Gitlab.config.omniauth.stub block_auto_created_users: false }
+
+          it do
+            oauth_user.save
+            gl_user.should be_valid
+            gl_user.should_not be_blocked
+          end
+        end
 
-      it "creates a user from Omniauth" do
-        oauth_user.save
+        context 'block on create' do
+          before { Gitlab.config.omniauth.stub block_auto_created_users: true }
 
-        expect(gl_user).to be_valid
-        expect(gl_user.extern_uid).to eql uid
-        expect(gl_user.provider).to eql 'twitter'
+          it do
+            oauth_user.save
+            gl_user.should be_valid
+            gl_user.should_not be_blocked
+          end
+        end
       end
     end
   end