From 95e936c299d4cbf814bd31174cfd0c1650e3b9d3 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 28 Oct 2013 16:29:07 +1100
Subject: [PATCH] cleanup API for looking up a user by email or username, add
 specs, fix invalid auto association in open id provider

---
 app/models/user.rb                            | 16 ++---
 .../auth/open_id_authenticator_spec.rb        | 18 +++++
 spec/models/user_spec.rb                      | 67 ++++++-------------
 3 files changed, 42 insertions(+), 59 deletions(-)
 create mode 100644 spec/components/auth/open_id_authenticator_spec.rb

diff --git a/app/models/user.rb b/app/models/user.rb
index 7276677df..7473af76c 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -124,25 +124,19 @@ class User < ActiveRecord::Base
   end
 
   def self.find_by_username_or_email(username_or_email)
-    users = if username_or_email.include?('@')
-              find_by_email(username_or_email)
-            else
-              find_by_username(username_or_email)
-            end
-
-    if users.size > 1
-      raise Discourse::TooManyMatches
+    if username_or_email.include?('@')
+      find_by_email(username_or_email)
     else
-      users.first
+      find_by_username(username_or_email)
     end
   end
 
   def self.find_by_email(email)
-    where(email: Email.downcase(email))
+    where(email: Email.downcase(email)).first
   end
 
   def self.find_by_username(username)
-    where(username_lower: username.downcase)
+    where(username_lower: username.downcase).first
   end
 
   def enqueue_welcome_message(message_type)
diff --git a/spec/components/auth/open_id_authenticator_spec.rb b/spec/components/auth/open_id_authenticator_spec.rb
new file mode 100644
index 000000000..5c96cc6ca
--- /dev/null
+++ b/spec/components/auth/open_id_authenticator_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+# In the ghetto ... getting the spec to run in autospec
+#  thing is we need to load up all auth really early pre-fork
+#  it means that the require is not going to get a new copy
+Auth.send(:remove_const, :OpenIdAuthenticator)
+load 'auth/open_id_authenticator.rb'
+
+describe Auth::OpenIdAuthenticator do
+
+  it "can lookup pre-existing user if trusted" do
+    auth = Auth::OpenIdAuthenticator.new("test", "id", trusted: true)
+
+    user = Fabricate(:user)
+    result = auth.after_authenticate(info: {email: user.email}, extra: {identity_url: 'abc'})
+    result.user.should == user
+  end
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index aaf155332..a58f39700 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -782,59 +782,30 @@ describe User do
   end
 
   describe '.find_by_username_or_email' do
-    it 'finds user by username' do
-      bob = Fabricate(:user, username: 'bob')
+    it 'finds users' do
+      bob = Fabricate(:user, username: 'bob', email: 'bob@example.com')
+      found_user = User.find_by_username_or_email('Bob')
+      expect(found_user).to eq bob
 
-      found_user = User.find_by_username_or_email('bob')
+      found_user = User.find_by_username_or_email('bob@Example.com')
+      expect(found_user).to eq bob
 
+      found_user = User.find_by_username_or_email('Bob@Example.com')
+      expect(found_user).to be_nil
+
+      found_user = User.find_by_username_or_email('bob1')
+      expect(found_user).to be_nil
+
+      found_user = User.find_by_email('bob@Example.com')
+      expect(found_user).to eq bob
+
+      found_user = User.find_by_email('bob')
+      expect(found_user).to be_nil
+
+      found_user = User.find_by_username('bOb')
       expect(found_user).to eq bob
     end
 
-    it 'finds user by email' do
-      bob = Fabricate(:user, email: 'bob@example.com')
-
-      found_user = User.find_by_username_or_email('bob@example.com')
-
-      expect(found_user).to eq bob
-    end
-
-    context 'when user does not exist' do
-      it 'returns nil' do
-        found_user = User.find_by_username_or_email('doesnotexist@example.com') ||
-          User.find_by_username_or_email('doesnotexist')
-
-        expect(found_user).to be_nil
-      end
-    end
-
-    context 'when username case does not match' do
-      it 'finds user' do
-        bob = Fabricate(:user, username: 'bob')
-
-        found_user = User.find_by_username_or_email('Bob')
-
-        expect(found_user).to eq bob
-      end
-    end
-
-    context 'when email domain case does not match' do
-      it 'finds user' do
-        bob = Fabricate(:user, email: 'bob@example.com')
-
-        found_user = User.find_by_username_or_email('bob@Example.com')
-
-        expect(found_user).to eq bob
-      end
-    end
-
-    context 'when multiple users are found' do
-      it 'raises an exception' do
-        user_query = [stub, stub]
-        User.stubs(:find_by_username).with('bob').returns(user_query)
-
-        expect { User.find_by_username_or_email('bob') }.to raise_error(Discourse::TooManyMatches)
-      end
-    end
   end
 
   describe "#added_a_day_ago?" do