From fad588f2bee102bf4ab090874d041e227d4e2ee4 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Thu, 16 Oct 2014 17:18:40 +0200
Subject: Remove LDAP save test

This is handled within the LDAP class
---
 spec/lib/gitlab/oauth/user_spec.rb | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/spec/lib/gitlab/oauth/user_spec.rb b/spec/lib/gitlab/oauth/user_spec.rb
index e4e96fd9f49..7c7d6babbfd 100644
--- a/spec/lib/gitlab/oauth/user_spec.rb
+++ b/spec/lib/gitlab/oauth/user_spec.rb
@@ -29,16 +29,16 @@ 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
+    let(:provider) { 'twitter' }
 
-        expect(gl_user).to be_valid
-        expect(gl_user.extern_uid).to eql uid
-        expect(gl_user.provider).to eql 'ldap'
-      end
+    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 "twitter" do
       let(:provider) { 'twitter' }
-- 
cgit v1.2.1


From d9bfebc0e87ef426aea7eb4fdd1338f04b106354 Mon Sep 17 00:00:00 2001
From: Jan-Willem van der Meer <mail@jewilmeer.nl>
Date: Thu, 16 Oct 2014 20:08:30 +0200
Subject: Add regressiontest to verify allow_single_sign_on setting

verification for #1677

Since testing omniauth_callback_controller.rb is very difficult, the logic
is moved to the models
---
 app/controllers/omniauth_callbacks_controller.rb | 13 +++++--------
 lib/gitlab/oauth/user.rb                         | 17 ++++++++++++++---
 spec/lib/gitlab/oauth/user_spec.rb               | 19 ++++++++-----------
 3 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 589f8387b03..58d1e37f655 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/app/controllers/omniauth_callbacks_controller.rb
@@ -49,22 +49,19 @@ 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?
+      else @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
       end
     end
+  rescue StandardError
+    flash[:notice] = "There's no such user!"
+    redirect_to new_user_session_path
   end
 
   def oauth
diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb
index 133445d3d05..18ec63a62a2 100644
--- a/lib/gitlab/oauth/user.rb
+++ b/lib/gitlab/oauth/user.rb
@@ -13,7 +13,7 @@ module Gitlab
       end
 
       def persisted?
-        gl_user.persisted?
+        gl_user.try(:persisted?)
       end
 
       def new?
@@ -21,10 +21,12 @@ module Gitlab
       end
 
       def valid?
-        gl_user.valid?
+        gl_user.try(:valid?)
       end
 
       def save
+        unauthorized_to_create unless gl_user
+
         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?
@@ -36,7 +38,12 @@ module Gitlab
       end
 
       def gl_user
-        @user ||= find_by_uid_and_provider || build_new_user
+        @user ||= find_by_uid_and_provider
+
+        if Gitlab.config.omniauth.allow_single_sign_on
+          @user ||= build_new_user
+        end
+        @user
       end
 
       protected
@@ -77,6 +84,10 @@ module Gitlab
       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
diff --git a/spec/lib/gitlab/oauth/user_spec.rb b/spec/lib/gitlab/oauth/user_spec.rb
index 7c7d6babbfd..e004d6edfab 100644
--- a/spec/lib/gitlab/oauth/user_spec.rb
+++ b/spec/lib/gitlab/oauth/user_spec.rb
@@ -31,17 +31,8 @@ describe Gitlab::OAuth::User do
   describe :save do
     let(:provider) { 'twitter' }
 
-    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 "twitter" do
-      let(:provider) { 'twitter' }
+    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
@@ -51,5 +42,11 @@ describe Gitlab::OAuth::User do
         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
 end
-- 
cgit v1.2.1