From a549873e225583be071e05d20452c6ad2e43a73f Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Fri, 29 May 2015 16:05:04 +1200
Subject: [PATCH 01/11] Initial code for getting Squash working with Google
 Auth

---
 Gemfile.d/authentication.rb                   |   4 +
 .../google_authentication_helpers.rb          |  50 ++++++++
 app/models/additions/google_authentication.rb | 111 ++++++++++++++++++
 config/initializers/google_authentication.rb  |  23 ++++
 4 files changed, 188 insertions(+)
 create mode 100644 app/controllers/additions/google_authentication_helpers.rb
 create mode 100644 app/models/additions/google_authentication.rb
 create mode 100644 config/initializers/google_authentication.rb

diff --git a/Gemfile.d/authentication.rb b/Gemfile.d/authentication.rb
index 644fdd92..c8bf5245 100644
--- a/Gemfile.d/authentication.rb
+++ b/Gemfile.d/authentication.rb
@@ -15,3 +15,7 @@
 conditionally('authentication.strategy', 'ldap') do
   gem 'net-ldap', github: 'ruby-ldap/ruby-net-ldap', require: 'net/ldap'
 end
+
+conditionally('authentication.strategy', 'google') do
+  gem 'simple_google_auth'
+end
diff --git a/app/controllers/additions/google_authentication_helpers.rb b/app/controllers/additions/google_authentication_helpers.rb
new file mode 100644
index 00000000..f3128131
--- /dev/null
+++ b/app/controllers/additions/google_authentication_helpers.rb
@@ -0,0 +1,50 @@
+# Copyright 2015 Powershop Ltd.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License");
+#    you may not use this file except in compliance with the License.
+#    You may obtain a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS,
+#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#    See the License for the specific language governing permissions and
+#    limitations under the License.
+
+# Includes methods for authentication with LDAP. Information for the LDAP server
+# is stored in the `authentication.yml` Configoro file for the current
+# environment. A successful BIND yields an authenticated session.
+#
+# If this is the user's first time logging in, the User will be created for him
+# or her.
+
+module GoogleAuthenticationHelpers
+  def log_in
+    return false unless google_auth_data
+    # If you're authenticated by Google, if this fails, it should asplode:
+    log_in_user User.find_or_create_by_google_auth_data!(google_auth_data)
+  end
+
+  # We don't want to redirect to a the Squash login page with this
+  # authenticator
+  def login_required
+    logger.info "Current User = #{current_user.inspect}"
+    logger.info "Google Auth Data = #{google_auth_data.inspect}"
+
+    return true if logged_in?
+
+    respond_to do |format|
+      format.xml { head :unauthorized }
+      format.json { head :unauthorized }
+      format.atom { head :unauthorized }
+      format.html do
+        logger.info "Redirecting to Big G for Authentication"
+        # If we're Google authenticated, find/create user
+        # If we're not google-authenticated, then go get it!
+        redirect_if_not_google_authenticated unless log_in
+      end
+    end
+    return false
+  end
+end
diff --git a/app/models/additions/google_authentication.rb b/app/models/additions/google_authentication.rb
new file mode 100644
index 00000000..1bb94d55
--- /dev/null
+++ b/app/models/additions/google_authentication.rb
@@ -0,0 +1,111 @@
+# Copyright 2015 Powershop Ltd.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License");
+#    you may not use this file except in compliance with the License.
+#    You may obtain a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS,
+#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#    See the License for the specific language governing permissions and
+#    limitations under the License.
+
+# Adds GoogleAuth-based authentication to the {User} model. Mixed in if this
+# Squash install is configured to use "google" authentication.
+
+module GoogleAuthentication
+  extend ActiveSupport::Concern
+
+  class GoogleAuthError <RuntimeError; end
+  class InvalidGoogleUsernameError <GoogleAuthError; end
+
+  included do
+    attr_accessor :google_auth_data
+
+    # We choose the username:
+    before_validation(on: :create) {|obj| obj.username = unique_username }
+
+    validates :google_auth_data,
+      presence: true,
+      on:       :create
+    validates :google_email_address,
+      presence: true,
+      email:    true,
+      on:       :create
+    validate :validate_email_address_unique, on: :create
+
+    # Because we need an associated email object for the google_email_address
+    # validates_associated :emails  ?
+
+    def self.find_by_google_auth_data(auth_data)
+      return nil unless auth_data && auth_data["email"]
+      e = (Email.find_by(email: auth_data["email"]) or return nil)
+      logger.info "Email found = #{e}"
+      e.user.tap {|u| u.google_auth_data = auth_data }
+    end
+
+    #TODO(willjr): Remove?
+    # def self.create_by_google_auth_data!(auth_data)
+    #   User.create!(google_auth_data: auth_data)
+    # end
+
+    def self.find_or_create_by_google_auth_data!(auth_data)
+      # find_by_google_auth_data(auth_data) || create_by_google_auth_data(auth_data)
+      find_by_google_auth_data(auth_data) or
+        User.create!(google_auth_data: auth_data)
+    end
+  end
+
+  #####
+  # Instance methods
+
+  # @return [String] The unique Google ID for the authenticated account
+  def google_user_id
+    google_auth_data.try(:fetch, "sub", nil)
+  end
+
+  # @return [String] The Google email for the authenticated account
+  def google_email_address
+    google_auth_data.try(:fetch, "email", nil)
+  end
+
+  # @return [String] Calculate a username that's not already in-use for a new
+  #                  Google account / email-address
+  def unique_username
+    errors.add(:username, "google_auth_data is null") if google_auth_data.nil?
+    [sanitised_google_username, sanitised_google_username_id].each do |a_username|
+      logger.info "Searching for #{a_username.inspect}"
+      return a_username unless User.where(username: a_username).exists?
+    end
+  end
+
+  private
+
+  # @return [String] The username part of the Google email, sanitised for Squash's use
+  def sanitised_google_username
+    sanitised_username(google_email_address)
+  end
+
+  # @return [String] The username part of the Google email, sanitised for Squash's use, combined with the Unique Google ID
+  def sanitised_google_username_id
+    [sanitised_username(google_email_address), google_user_id].join("-")
+  end
+
+  def sanitised_username(an_email_address)
+    # Google also allows "." and "'" in a G.Apps email username
+    # but Squash doesn't:
+    m = an_email_address.match(/^(.+)@.+$/) or
+      raise InvalidGoogleUsernameError, "Can't extract username from #{an_email_address.inspect}"
+    m[1].gsub(/[\.\']/, "_")
+  end
+
+  def create_primary_email
+    emails.create!(email: google_email_address, primary: true)
+  end
+
+  def validate_email_address_unique
+    errors.add(:google_email_address, :taken) if Email.primary.where(email: google_email_address).exists?
+  end
+end
diff --git a/config/initializers/google_authentication.rb b/config/initializers/google_authentication.rb
new file mode 100644
index 00000000..3897ded5
--- /dev/null
+++ b/config/initializers/google_authentication.rb
@@ -0,0 +1,23 @@
+# encoding: utf-8
+
+#TODO: License
+# Copyright 2015 Powershop Ltd.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License");
+#    you may not use this file except in compliance with the License.
+#    You may obtain a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS,
+#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#    See the License for the specific language governing permissions and
+#    limitations under the License.
+
+SimpleGoogleAuth.configure do |config|
+  config.client_id     = Squash::Configuration.authentication.google.client_id
+  config.client_secret = Squash::Configuration.authentication.google.client_secret
+  config.redirect_uri  = Squash::Configuration.authentication.google.redirect_uri
+  config.authenticate  = lambda {|d| true }
+end if Squash::Configuration.authentication.strategy == 'google'

From c859c3279bed4adb5c3f16a9485abf20059afc35 Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Thu, 4 Jun 2015 16:34:52 +1200
Subject: [PATCH 02/11] Add Google Auth to setup.rb

---
 setup.rb | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/setup.rb b/setup.rb
index 860ca34a..46d7b6e9 100755
--- a/setup.rb
+++ b/setup.rb
@@ -332,8 +332,9 @@ def choose(question, choices, default=nil)
   say
   say "Now we'll cover authentication.".bold
 
-  auth = choose("How will users authenticate to Squash?", %w(password LDAP))
-  if auth == 'LDAP'
+  auth = choose("How will users authenticate to Squash?", %w(password LDAP GoogleAuth))
+  case auth
+  when 'LDAP'
     ldap_host  = query("What's the hostname of your LDAP server?")
     ldap_ssl   = prompt("Is your LDAP service using SSL?")
     ldap_port  = query("What port is your LDAP service running on?", ldap_ssl ? '636' : '389').to_i
@@ -356,7 +357,8 @@ def choose(question, choices, default=nil)
                  }
              }.to_yaml)
     end
-  elsif auth == 'password'
+
+  when 'password'
     say "Updating config/environments/common/authentication.yml..."
     File.open('config/environments/common/authentication.yml', 'w') do |f|
       f.puts({
@@ -367,6 +369,25 @@ def choose(question, choices, default=nil)
                  }
              }.to_yaml)
     end
+
+  when 'GoogleAuth'
+    say "You will need your Google OAuth API Credentials from your project"
+    say "under https://console.developers.google.com/project to answer the following:"
+    google_client_id = query("What's your Google OAuth Client ID?")
+    google_client_secret = query("What's your Google OAuth Client Secret?")
+    google_redirect_uri = query("What Google OAuth Redirect URI do you plan to use for Squash?")
+
+    say "Updating config/environments/common/authentication.yml..."
+    File.open('config/environments/common/authentication.yml', 'w') do |f|
+      f.puts({
+                 'strategy'   => 'google',
+                 'google' => {
+                     'client_id'     => google_client_id,
+                     'client_secret' => google_client_secret,
+                     'redirect_uri'  => google_redirect_uri
+                 }
+             }.to_yaml)
+    end
   end
 
   File.open('/tmp/squash_install_progress', 'w') { |f| f.puts '2' }

From b094279ab719ef77ab06f65395af5e35083c19a5 Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Tue, 2 Jun 2015 10:05:44 +1200
Subject: [PATCH 03/11] Add third-party-login before-filter

---
 app/controllers/additions/authentication_helpers.rb |  8 ++++++++
 .../additions/google_authentication_helpers.rb      | 13 ++++---------
 app/controllers/application_controller.rb           |  1 +
 app/controllers/sessions_controller.rb              |  1 +
 app/models/additions/google_authentication.rb       | 10 +++++-----
 app/models/user.rb                                  |  7 +++++++
 6 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/app/controllers/additions/authentication_helpers.rb b/app/controllers/additions/authentication_helpers.rb
index d2999ab5..a3b947da 100644
--- a/app/controllers/additions/authentication_helpers.rb
+++ b/app/controllers/additions/authentication_helpers.rb
@@ -86,6 +86,14 @@ def login_required
     end
   end
 
+  # A `before_filter` that requires a session authenticated by a 3rd-party login
+  # service (e.g. Google Auth).
+  # By default (`true`) it assumes to 3P service, and should be overridden in a
+  # *_helper.rb
+  def third_party_login_required
+   true
+  end
+
   # A `before_filter` that requires an unauthenticated session to continue. If
   # the session is authenticated...
   #
diff --git a/app/controllers/additions/google_authentication_helpers.rb b/app/controllers/additions/google_authentication_helpers.rb
index f3128131..00bb07ed 100644
--- a/app/controllers/additions/google_authentication_helpers.rb
+++ b/app/controllers/additions/google_authentication_helpers.rb
@@ -26,23 +26,18 @@ def log_in
     log_in_user User.find_or_create_by_google_auth_data!(google_auth_data)
   end
 
-  # We don't want to redirect to a the Squash login page with this
-  # authenticator
-  def login_required
+  def third_party_login_required
+    #TODO(willjr): Safer logging data!!
     logger.info "Current User = #{current_user.inspect}"
     logger.info "Google Auth Data = #{google_auth_data.inspect}"
 
-    return true if logged_in?
-
     respond_to do |format|
-      format.xml { head :unauthorized }
-      format.json { head :unauthorized }
-      format.atom { head :unauthorized }
       format.html do
         logger.info "Redirecting to Big G for Authentication"
         # If we're Google authenticated, find/create user
         # If we're not google-authenticated, then go get it!
-        redirect_if_not_google_authenticated unless log_in
+        return true if log_in
+        redirect_if_not_google_authenticated
       end
     end
     return false
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index d1662aba..3dd82d93 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -152,6 +152,7 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  before_filter :third_party_login_required
   before_filter :login_required
 
   protected
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 895712ea..4b968d5c 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -17,6 +17,7 @@
 
 class SessionsController < ApplicationController
   skip_before_filter :login_required, only: [:new, :create]
+  # skip_before_filter :login_required, only: [:new, :create] unless Squash::Configuration.authentication.third_party_login
   before_filter :must_be_unauthenticated, except: :destroy
 
   respond_to :html
diff --git a/app/models/additions/google_authentication.rb b/app/models/additions/google_authentication.rb
index 1bb94d55..c462fac9 100644
--- a/app/models/additions/google_authentication.rb
+++ b/app/models/additions/google_authentication.rb
@@ -46,11 +46,6 @@ def self.find_by_google_auth_data(auth_data)
       e.user.tap {|u| u.google_auth_data = auth_data }
     end
 
-    #TODO(willjr): Remove?
-    # def self.create_by_google_auth_data!(auth_data)
-    #   User.create!(google_auth_data: auth_data)
-    # end
-
     def self.find_or_create_by_google_auth_data!(auth_data)
       # find_by_google_auth_data(auth_data) || create_by_google_auth_data(auth_data)
       find_by_google_auth_data(auth_data) or
@@ -61,6 +56,11 @@ def self.find_or_create_by_google_auth_data!(auth_data)
   #####
   # Instance methods
 
+  # @return [Boolean] Is this a third-party login service?
+  def third_party_login?
+    true
+  end
+
   # @return [String] The unique Google ID for the authenticated account
   def google_user_id
     google_auth_data.try(:fetch, "sub", nil)
diff --git a/app/models/user.rb b/app/models/user.rb
index fbba3747..b319f328 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -90,6 +90,13 @@ class User < ActiveRecord::Base
 
   include "#{Squash::Configuration.authentication.strategy}_authentication".camelize.constantize
 
+  # @return [Boolean] Is this a third-party login service?
+  # NB: by default, no
+  def third_party_login?
+    false
+  end
+
+
   # @return [String] The user's full name, or as much of it as available, or the
   #   `username`.
 

From 4aa0957059c3d876bb3b79c98997717cdefec003 Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Tue, 2 Jun 2015 19:50:22 +1200
Subject: [PATCH 04/11] Fix log-out button

---
 app/controllers/additions/authentication_helpers.rb      | 6 +++++-
 .../additions/google_authentication_helpers.rb           | 6 ++++++
 app/controllers/sessions_controller.rb                   | 9 ++++++---
 app/models/additions/google_authentication.rb            | 5 -----
 app/models/user.rb                                       | 7 -------
 app/views/layouts/application.html.rb                    | 4 +++-
 config/locales/en.yml                                    | 1 +
 7 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/app/controllers/additions/authentication_helpers.rb b/app/controllers/additions/authentication_helpers.rb
index a3b947da..ecaaaa9e 100644
--- a/app/controllers/additions/authentication_helpers.rb
+++ b/app/controllers/additions/authentication_helpers.rb
@@ -28,7 +28,7 @@ module AuthenticationHelpers
   extend ActiveSupport::Concern
 
   included do
-    helper_method :current_user, :logged_in?, :logged_out?
+    helper_method :current_user, :logged_in?, :logged_out?, :third_party_login?
   end
 
   # Clears a user session.
@@ -94,6 +94,10 @@ def third_party_login_required
    true
   end
 
+  def third_party_login?
+    false
+  end
+
   # A `before_filter` that requires an unauthenticated session to continue. If
   # the session is authenticated...
   #
diff --git a/app/controllers/additions/google_authentication_helpers.rb b/app/controllers/additions/google_authentication_helpers.rb
index 00bb07ed..cdcfdd1c 100644
--- a/app/controllers/additions/google_authentication_helpers.rb
+++ b/app/controllers/additions/google_authentication_helpers.rb
@@ -31,6 +31,8 @@ def third_party_login_required
     logger.info "Current User = #{current_user.inspect}"
     logger.info "Google Auth Data = #{google_auth_data.inspect}"
 
+    return true if logged_in?
+
     respond_to do |format|
       format.html do
         logger.info "Redirecting to Big G for Authentication"
@@ -42,4 +44,8 @@ def third_party_login_required
     end
     return false
   end
+
+  def third_party_login?
+    true
+  end
 end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 4b968d5c..6e9a5bcb 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -17,7 +17,6 @@
 
 class SessionsController < ApplicationController
   skip_before_filter :login_required, only: [:new, :create]
-  # skip_before_filter :login_required, only: [:new, :create] unless Squash::Configuration.authentication.third_party_login
   before_filter :must_be_unauthenticated, except: :destroy
 
   respond_to :html
@@ -93,7 +92,11 @@ def create
   # * `GET /logout`
 
   def destroy
-    log_out
-    redirect_to login_url, notice: t('controllers.sessions.destroy.logged_out')
+    if third_party_login?
+      redirect_to root_url, notice: t('controllers.sessions.destroy.third_party_logout')
+    else
+      log_out
+      redirect_to login_url, notice: t('controllers.sessions.destroy.logged_out')
+    end
   end
 end
diff --git a/app/models/additions/google_authentication.rb b/app/models/additions/google_authentication.rb
index c462fac9..0c485e57 100644
--- a/app/models/additions/google_authentication.rb
+++ b/app/models/additions/google_authentication.rb
@@ -56,11 +56,6 @@ def self.find_or_create_by_google_auth_data!(auth_data)
   #####
   # Instance methods
 
-  # @return [Boolean] Is this a third-party login service?
-  def third_party_login?
-    true
-  end
-
   # @return [String] The unique Google ID for the authenticated account
   def google_user_id
     google_auth_data.try(:fetch, "sub", nil)
diff --git a/app/models/user.rb b/app/models/user.rb
index b319f328..fbba3747 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -90,13 +90,6 @@ class User < ActiveRecord::Base
 
   include "#{Squash::Configuration.authentication.strategy}_authentication".camelize.constantize
 
-  # @return [Boolean] Is this a third-party login service?
-  # NB: by default, no
-  def third_party_login?
-    false
-  end
-
-
   # @return [String] The user's full name, or as much of it as available, or the
   #   `username`.
 
diff --git a/app/views/layouts/application.html.rb b/app/views/layouts/application.html.rb
index a8d27527..6a5cb1b9 100644
--- a/app/views/layouts/application.html.rb
+++ b/app/views/layouts/application.html.rb
@@ -185,7 +185,9 @@ def navbar_large
               navbar_environments('large') if @project
               li(id: 'quicknav-container') { input type: 'search', id: 'quicknav', placeholder: 'Search' }
               li { link_to "My Account", account_url }
-              li { link_to(logout_url) { i class: 'icon-signout'} }
+              unless third_party_login?
+                li { link_to(logout_url) { i class: 'icon-signout'} }
+              end
             else
               li(id: 'quicknav-container') { text! '&nbsp;' }
               li { link_to "Log In", login_url }
diff --git a/config/locales/en.yml b/config/locales/en.yml
index dcd6a6a6..fbd5c771 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -323,6 +323,7 @@ en:
         missing_field: You must provide a username and a password.
       destroy:
         logged_out: You have been logged out.
+        third_party_logout: Logging-out doesn't make sense with a 3rd Party login service.
     users:
       create:
         success: "Welcome to Squash, %{name}!"

From 17bb1d6757ce6bf44b2a261117b65c12f937a686 Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Wed, 3 Jun 2015 11:22:22 +1200
Subject: [PATCH 05/11] Improve the doc-comments

---
 app/models/additions/google_authentication.rb | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/app/models/additions/google_authentication.rb b/app/models/additions/google_authentication.rb
index 0c485e57..69c3f7a2 100644
--- a/app/models/additions/google_authentication.rb
+++ b/app/models/additions/google_authentication.rb
@@ -39,6 +39,10 @@ class InvalidGoogleUsernameError <GoogleAuthError; end
     # Because we need an associated email object for the google_email_address
     # validates_associated :emails  ?
 
+    # @return [User] Searches the Users for an entry that matches appropriate Google Auth data
+    # @return [nil] If it can't find a user that matches the provided Google Auth data
+    #
+    # In this case, we search for a matching email address
     def self.find_by_google_auth_data(auth_data)
       return nil unless auth_data && auth_data["email"]
       e = (Email.find_by(email: auth_data["email"]) or return nil)
@@ -46,15 +50,14 @@ def self.find_by_google_auth_data(auth_data)
       e.user.tap {|u| u.google_auth_data = auth_data }
     end
 
+    # @return [User] Finds or Creates a User from Google Auth data
+    # @raise [RecordInvalid] If it fails to create the User (this should never happen!)
     def self.find_or_create_by_google_auth_data!(auth_data)
-      # find_by_google_auth_data(auth_data) || create_by_google_auth_data(auth_data)
       find_by_google_auth_data(auth_data) or
         User.create!(google_auth_data: auth_data)
     end
   end
 
-  #####
-  # Instance methods
 
   # @return [String] The unique Google ID for the authenticated account
   def google_user_id
@@ -66,8 +69,7 @@ def google_email_address
     google_auth_data.try(:fetch, "email", nil)
   end
 
-  # @return [String] Calculate a username that's not already in-use for a new
-  #                  Google account / email-address
+  # @return [String] Calculate a username that's not already in-use for a new Google account / email-address
   def unique_username
     errors.add(:username, "google_auth_data is null") if google_auth_data.nil?
     [sanitised_google_username, sanitised_google_username_id].each do |a_username|
@@ -88,9 +90,11 @@ def sanitised_google_username_id
     [sanitised_username(google_email_address), google_user_id].join("-")
   end
 
+  # @return [String] A username extracted from a Google email-address, and then sanitised.
+  #
+  # In this context, "sanitised" means certain disallowed char's are replaced with "_" as Google also
+  # allows `.` and `'` in a G.Apps email username but Squash doesn't.
   def sanitised_username(an_email_address)
-    # Google also allows "." and "'" in a G.Apps email username
-    # but Squash doesn't:
     m = an_email_address.match(/^(.+)@.+$/) or
       raise InvalidGoogleUsernameError, "Can't extract username from #{an_email_address.inspect}"
     m[1].gsub(/[\.\']/, "_")

From fa3d270f6b0d2f24e98718e0bd07571dc6840d62 Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Wed, 3 Jun 2015 15:21:56 +1200
Subject: [PATCH 06/11] Much better implementation for managing 3rd-party
 logins

---
 .../additions/authentication_helpers.rb       | 24 ++++----
 .../google_authentication_helpers.rb          | 45 +++++++-------
 app/controllers/application_controller.rb     |  1 -
 app/controllers/sessions_controller.rb        | 11 ++--
 app/models/additions/google_authentication.rb | 23 ++++----
 app/views/layouts/application.html.rb         |  4 +-
 config/locales/en.yml                         |  1 -
 .../google_authentication_helpers_spec.rb     | 58 +++++++++++++++++++
 8 files changed, 112 insertions(+), 55 deletions(-)
 create mode 100644 spec/controllers/additions/google_authentication_helpers_spec.rb

diff --git a/app/controllers/additions/authentication_helpers.rb b/app/controllers/additions/authentication_helpers.rb
index ecaaaa9e..5edd2c23 100644
--- a/app/controllers/additions/authentication_helpers.rb
+++ b/app/controllers/additions/authentication_helpers.rb
@@ -29,11 +29,18 @@ module AuthenticationHelpers
 
   included do
     helper_method :current_user, :logged_in?, :logged_out?, :third_party_login?
+
+    # An overridable method to let the base view disable certain elements that
+    # aren't appropriate when using a 3rd-party Login Service (e.g. Logout button)
+    def self.third_party_login?
+      false
+    end
   end
 
   # Clears a user session.
 
   def log_out
+    # Why doesn't this use #reset_session ?
     session[:user_id] = nil
     @current_user     = nil
   end
@@ -78,24 +85,21 @@ def login_required
         format.xml { head :unauthorized }
         format.json { head :unauthorized }
         format.atom { head :unauthorized }
-        format.html do
-          redirect_to login_url(next: request.fullpath), notice: t('controllers.authentication.login_required')
-        end
+        format.html { login_required_redirect }
       end
       return false
     end
   end
 
-  # A `before_filter` that requires a session authenticated by a 3rd-party login
-  # service (e.g. Google Auth).
-  # By default (`true`) it assumes to 3P service, and should be overridden in a
-  # *_helper.rb
-  def third_party_login_required
-   true
+  # An overridable method for selecting where a not-logged-in redirect goes
+  # Primarily used by 3rd-party Login Services
+
+  def login_required_redirect
+    redirect_to login_url(next: request.fullpath), notice: t('controllers.authentication.login_required')
   end
 
   def third_party_login?
-    false
+    self.class.third_party_login?
   end
 
   # A `before_filter` that requires an unauthenticated session to continue. If
diff --git a/app/controllers/additions/google_authentication_helpers.rb b/app/controllers/additions/google_authentication_helpers.rb
index cdcfdd1c..3b70db44 100644
--- a/app/controllers/additions/google_authentication_helpers.rb
+++ b/app/controllers/additions/google_authentication_helpers.rb
@@ -20,32 +20,33 @@
 # or her.
 
 module GoogleAuthenticationHelpers
-  def log_in
-    return false unless google_auth_data
-    # If you're authenticated by Google, if this fails, it should asplode:
-    log_in_user User.find_or_create_by_google_auth_data!(google_auth_data)
+  extend ActiveSupport::Concern
+
+  included do
+    def self.third_party_login?
+      true
+    end
   end
 
-  def third_party_login_required
-    #TODO(willjr): Safer logging data!!
-    logger.info "Current User = #{current_user.inspect}"
-    logger.info "Google Auth Data = #{google_auth_data.inspect}"
-
-    return true if logged_in?
-
-    respond_to do |format|
-      format.html do
-        logger.info "Redirecting to Big G for Authentication"
-        # If we're Google authenticated, find/create user
-        # If we're not google-authenticated, then go get it!
-        return true if log_in
-        redirect_if_not_google_authenticated
-      end
+  def log_in
+    unless google_auth_data
+      logger.tagged('AuthenticationHelpers') { logger.info "Denying login: not Google Auth data provided." }
+      return false
+    end
+
+    unless user = User.find_or_create_by_google_auth_data(google_auth_data)
+      logger.tagged('AuthenticationHelpers') { logger.info "Denying login to #{google_auth_data["email"]}: could not find or create." }
+      return false
     end
-    return false
+
+    log_in_user user
   end
 
-  def third_party_login?
-    true
+  def login_required_redirect
+    logger.info "Redirecting to Big G for Authentication"
+
+    # If we're Google authenticated, find/create user
+    # If we're not google-authenticated, then go get it!
+    redirect_if_not_google_authenticated unless log_in
   end
 end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 3dd82d93..d1662aba 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -152,7 +152,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  before_filter :third_party_login_required
   before_filter :login_required
 
   protected
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index 6e9a5bcb..001fdf89 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -16,7 +16,7 @@
 # {AuthenticationHelpers} for more information on how authentication works.
 
 class SessionsController < ApplicationController
-  skip_before_filter :login_required, only: [:new, :create]
+  skip_before_filter :login_required, only: [:new, :create] unless ApplicationController.third_party_login?
   before_filter :must_be_unauthenticated, except: :destroy
 
   respond_to :html
@@ -92,11 +92,8 @@ def create
   # * `GET /logout`
 
   def destroy
-    if third_party_login?
-      redirect_to root_url, notice: t('controllers.sessions.destroy.third_party_logout')
-    else
-      log_out
-      redirect_to login_url, notice: t('controllers.sessions.destroy.logged_out')
-    end
+    log_out
+    reset_session if third_party_login?
+    redirect_to login_url, notice: t('controllers.sessions.destroy.logged_out')
   end
 end
diff --git a/app/models/additions/google_authentication.rb b/app/models/additions/google_authentication.rb
index 69c3f7a2..7bc5614d 100644
--- a/app/models/additions/google_authentication.rb
+++ b/app/models/additions/google_authentication.rb
@@ -18,14 +18,14 @@
 module GoogleAuthentication
   extend ActiveSupport::Concern
 
-  class GoogleAuthError <RuntimeError; end
-  class InvalidGoogleUsernameError <GoogleAuthError; end
+  class InvalidGoogleAuthError <RuntimeError; end
+  class InvalidGoogleUsernameError <InvalidGoogleAuthError; end
 
   included do
     attr_accessor :google_auth_data
 
     # We choose the username:
-    before_validation(on: :create) {|obj| obj.username = unique_username }
+    before_validation(on: :create) {|obj| obj.username ||= unique_username.downcase }
 
     validates :google_auth_data,
       presence: true,
@@ -52,34 +52,34 @@ def self.find_by_google_auth_data(auth_data)
 
     # @return [User] Finds or Creates a User from Google Auth data
     # @raise [RecordInvalid] If it fails to create the User (this should never happen!)
-    def self.find_or_create_by_google_auth_data!(auth_data)
+    def self.find_or_create_by_google_auth_data(auth_data)
       find_by_google_auth_data(auth_data) or
-        User.create!(google_auth_data: auth_data)
+        User.create(google_auth_data: auth_data)
     end
   end
 
-
   # @return [String] The unique Google ID for the authenticated account
   def google_user_id
-    google_auth_data.try(:fetch, "sub", nil)
+    google_auth_data["sub"]
   end
 
   # @return [String] The Google email for the authenticated account
   def google_email_address
-    google_auth_data.try(:fetch, "email", nil)
+    google_auth_data["email"]
   end
 
+  private
+
   # @return [String] Calculate a username that's not already in-use for a new Google account / email-address
   def unique_username
-    errors.add(:username, "google_auth_data is null") if google_auth_data.nil?
+    raise InvalidGoogleAuthError, "google_auth_data is null" if google_auth_data.nil?
+
     [sanitised_google_username, sanitised_google_username_id].each do |a_username|
       logger.info "Searching for #{a_username.inspect}"
       return a_username unless User.where(username: a_username).exists?
     end
   end
 
-  private
-
   # @return [String] The username part of the Google email, sanitised for Squash's use
   def sanitised_google_username
     sanitised_username(google_email_address)
@@ -95,6 +95,7 @@ def sanitised_google_username_id
   # In this context, "sanitised" means certain disallowed char's are replaced with "_" as Google also
   # allows `.` and `'` in a G.Apps email username but Squash doesn't.
   def sanitised_username(an_email_address)
+    raise InvalidGoogleUsernameError, "Can't extract username from #{an_email_address.inspect}" if an_email_address.nil?
     m = an_email_address.match(/^(.+)@.+$/) or
       raise InvalidGoogleUsernameError, "Can't extract username from #{an_email_address.inspect}"
     m[1].gsub(/[\.\']/, "_")
diff --git a/app/views/layouts/application.html.rb b/app/views/layouts/application.html.rb
index 6a5cb1b9..a8d27527 100644
--- a/app/views/layouts/application.html.rb
+++ b/app/views/layouts/application.html.rb
@@ -185,9 +185,7 @@ def navbar_large
               navbar_environments('large') if @project
               li(id: 'quicknav-container') { input type: 'search', id: 'quicknav', placeholder: 'Search' }
               li { link_to "My Account", account_url }
-              unless third_party_login?
-                li { link_to(logout_url) { i class: 'icon-signout'} }
-              end
+              li { link_to(logout_url) { i class: 'icon-signout'} }
             else
               li(id: 'quicknav-container') { text! '&nbsp;' }
               li { link_to "Log In", login_url }
diff --git a/config/locales/en.yml b/config/locales/en.yml
index fbd5c771..dcd6a6a6 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -323,7 +323,6 @@ en:
         missing_field: You must provide a username and a password.
       destroy:
         logged_out: You have been logged out.
-        third_party_logout: Logging-out doesn't make sense with a 3rd Party login service.
     users:
       create:
         success: "Welcome to Squash, %{name}!"
diff --git a/spec/controllers/additions/google_authentication_helpers_spec.rb b/spec/controllers/additions/google_authentication_helpers_spec.rb
new file mode 100644
index 00000000..280d37e2
--- /dev/null
+++ b/spec/controllers/additions/google_authentication_helpers_spec.rb
@@ -0,0 +1,58 @@
+# Copyright 2014 Square Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License");
+#    you may not use this file except in compliance with the License.
+#    You may obtain a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS,
+#    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#    See the License for the specific language governing permissions and
+#    limitations under the License.
+
+require 'spec_helper'
+
+if Squash::Configuration.authentication.strategy == 'google'
+  class FakeController
+    attr_accessor :google_auth_data
+
+    def self.helper_method(*) end
+    def logger(*) Rails.logger end
+
+    include AuthenticationHelpers
+    include GoogleAuthenticationHelpers
+  end
+
+  describe GoogleAuthenticationHelpers do
+    before(:each) { @controller = FakeController.new }
+
+    describe "#log_in" do
+      before :all do
+        @auth_data = { "email" => "email.test@example.com", "sub" => "uid-123" }
+        @user = FactoryGirl.create(:user, google_auth_data: @auth_data)
+      end
+      before(:each) { @controller.google_auth_data = @auth_data }
+
+      it "should find a user from Google Auth data" do
+        expect(@controller).to receive(:google_auth_data).twice.and_return(@auth_data)
+        expect(@controller).to receive(:log_in_user).once.with(@user).and_return("USER")
+        expect(@controller.log_in).to be_truthy
+      end
+
+      it "should fail to find or create a user" do
+        expect(@controller).to receive(:google_auth_data).exactly(3).times.and_return(@auth_data)
+        expect(User).to receive(:find_or_create_by_google_auth_data).once.with(@auth_data).and_return(nil)
+        expect(@controller).not_to receive(:log_in_user)
+        expect(@controller.log_in).to be false
+      end
+    end
+
+    describe "#third_party_login?" do
+      it "should return false for being a 3rd-party login service" do
+        expect(@controller.send(:third_party_login?)).to be true
+      end
+    end
+  end
+end

From b6c667d084a46aeb95f878c645e4837411230a66 Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Wed, 3 Jun 2015 15:35:07 +1200
Subject: [PATCH 07/11] Fix session_controller specs

---
 .../additions/authentication_helpers_spec.rb  |   6 +
 spec/controllers/sessions_controller_spec.rb  | 133 ++++++++++--------
 spec/factories/users.rb                       |  16 ++-
 spec/models/user_spec.rb                      |  17 +++
 4 files changed, 112 insertions(+), 60 deletions(-)

diff --git a/spec/controllers/additions/authentication_helpers_spec.rb b/spec/controllers/additions/authentication_helpers_spec.rb
index 4e31a652..fb84c2eb 100644
--- a/spec/controllers/additions/authentication_helpers_spec.rb
+++ b/spec/controllers/additions/authentication_helpers_spec.rb
@@ -97,6 +97,12 @@ def _current_user=(u) @current_user = u end
     end
   end
 
+  describe "#third_party_login?" do
+    it "should return false for being a 3rd-party login service" do
+      expect(@controller.send(:third_party_login?)).to be_false
+    end unless Squash::Configuration.authentication.strategy == 'google'
+  end
+
   describe "#must_be_unauthenticated" do
     it "should return false and redirect if the user is logged in" do
       expect(@controller).to receive(:respond_to).once
diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb
index 1529dee3..d0058e68 100644
--- a/spec/controllers/sessions_controller_spec.rb
+++ b/spec/controllers/sessions_controller_spec.rb
@@ -21,76 +21,93 @@
   end
 
   describe "#create" do
-    before :each do
-      if defined?(Net::LDAP)
-        @ldap = double('Net::LDAP', :host= => nil, :port= => nil, :auth => nil)
-        allow(Net::LDAP).to receive(:new).and_return(@ldap)
+    context "[using internal login service]" do
+      before :each do
+        if defined?(Net::LDAP)
+          @ldap = double('Net::LDAP', :host= => nil, :port= => nil, :auth => nil)
+          allow(Net::LDAP).to receive(:new).and_return(@ldap)
+        end
       end
-    end
 
-    context '[valid credentials]' do
-      before :each do
-        allow(@ldap).to receive(:bind).and_return(true)
-        allow(@ldap).to receive :encryption
+      context '[valid credentials]' do
+        before :each do
+          allow(@ldap).to receive(:bind).and_return(true)
+          allow(@ldap).to receive :encryption
 
-        entry = OpenStruct.new(:givenname => %w(Sancho), :sn => %w(Sample), :dn => 'some dn')
-        allow(@ldap).to receive(:search).and_yield(entry)
-      end if defined?(Net::LDAP)
+          entry = {:givenname => %w(Sancho), :sn => %w(Sample)}
+          allow(entry).to receive(:dn).and_return('some dn')
+          allow(@ldap).to receive(:search).and_yield(entry)
+        end if defined?(Net::LDAP)
 
-      it "should log in a valid username and password" do
-        post :create, username: @user.username, password: 'password123'
-        expect(response).to redirect_to(root_url)
-        expect(session[:user_id]).to eql(@user.id)
-      end
+        it "should log in a valid username and password" do
+          post :create, username: @user.username, password: 'password123'
+          expect(response).to redirect_to(root_url)
+          expect(session[:user_id]).to eql(@user.id)
+        end
 
-      it "should create users that don't exist" do
-        post :create, username: 'new-user', password: 'password123'
-        expect(response).to redirect_to(root_url)
-        expect(User.find(session[:user_id]).username).to eql('new-user')
-      end if Squash::Configuration.authentication.strategy == 'ldap'
+        it "should create users that don't exist" do
+          post :create, username: 'new-user', password: 'password123'
+          expect(response).to redirect_to(root_url)
+          expect(User.find(session[:user_id]).username).to eql('new-user')
+        end if Squash::Configuration.authentication.strategy == 'ldap'
 
-      it "should redirect a user to :next if in the params" do
-        url = project_url(FactoryGirl.create(:project))
-        post :create, username: @user.username, password: 'password123', next: url
-        expect(response).to redirect_to(url)
-      end
+        it "should redirect a user to :next if in the params" do
+          url = project_url(FactoryGirl.create(:project))
+          post :create, username: @user.username, password: 'password123', next: url
+          expect(response).to redirect_to(url)
+        end
 
-      it "should use LDAP when creating a user" do
-        post :create, username: 'sancho', password: 'password123'
-        user = User.find(session[:user_id])
-        expect(user.first_name).to eql('Sancho')
-        expect(user.last_name).to eql('Sample')
-      end if Squash::Configuration.authentication.strategy == 'ldap'
-    end
+        it "should use LDAP when creating a user" do
+          post :create, username: 'sancho', password: 'password123'
+          user = User.find(session[:user_id])
+          expect(user.first_name).to eql('Sancho')
+          expect(user.last_name).to eql('Sample')
+        end if Squash::Configuration.authentication.strategy == 'ldap'
+      end
 
-    it "should not log in an invalid username and password" do
-      allow(@ldap).to receive(:bind).and_return(false) if defined?(Net::LDAP)
-      post :create, username: 'username', password: 'wrong'
-      expect(response).to render_template('new')
-      expect(session[:user_id]).to be_nil
-    end
+      it "should not log in an invalid username and password" do
+        allow(@ldap).to receive(:bind).and_return(false) if defined?(Net::LDAP)
+        post :create, username: 'username', password: 'wrong'
+        expect(response).to render_template('new')
+        expect(session[:user_id]).to be_nil
+      end
 
-    # these two are really applicable to LDAP moreso than password auth
-    it "should not allow a blank password" do
-      post :create, username: 'username'
-      expect(response).to render_template('new')
-      expect(session[:user_id]).to be_nil
-    end
+      # these two are really applicable to LDAP moreso than password auth
+      it "should not allow a blank password" do
+        post :create, username: 'username'
+        expect(response).to render_template('new')
+        expect(session[:user_id]).to be_nil
+      end
 
-    it "should not allow a blank username" do
-      post :create, password: 'password123'
-      expect(response).to render_template('new')
-      expect(session[:user_id]).to be_nil
-    end
+      it "should not allow a blank username" do
+        post :create, password: 'password123'
+        expect(response).to render_template('new')
+        expect(session[:user_id]).to be_nil
+      end
+    end unless Squash::Configuration.authentication.strategy == 'google'
+    context "[using 3rd-party login service]" do
+      it "should not log in a user" do
+        post :create
+        expect(response.location).to match(%r{^https://accounts.google.com})
+      end
+    end if Squash::Configuration.authentication.strategy == 'google'
   end
 
   describe "#destroy" do
-    before(:each) { login_as FactoryGirl.create(:user) }
-    it "should log out a user" do
-      delete :destroy
-      expect(session[:user_id]).to be_nil
-      expect(response).to redirect_to(login_url)
-      expect(flash[:notice]).to include('logged out')
-    end
+    context "[using internal login service]" do
+      before(:each) { login_as FactoryGirl.create(:user) }
+      it "should log out a user" do
+        delete :destroy
+        expect(session[:user_id]).to be_nil
+        expect(response).to redirect_to(login_url)
+        expect(flash[:notice]).to include('logged out')
+      end
+    end unless Squash::Configuration.authentication.strategy == 'google'
+    context "[using 3rd-party login service]" do 
+      it "should not reset the session var" do
+        delete :destroy
+        expect(session).to be {}
+      end
+    end if Squash::Configuration.authentication.strategy == 'google'
   end
 end
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 1b832b32..c65a797d 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -14,12 +14,24 @@
 
 FactoryGirl.define do
   factory :user do
-    sequence(:username) { |i| "user-#{i}" }
     first_name 'Sancho'
     last_name 'Sample'
-    if Squash::Configuration.authentication.strategy == 'password'
+
+    case Squash::Configuration.authentication.strategy
+    when 'password'
+      sequence(:username) { |i| "user-#{i}" }
       sequence(:email_address) { |i| "email-#{i}@example.com" }
       password 'correct horse battery staple'
+
+    when 'ldap'
+      sequence(:username) { |i| "user-#{i}" }
+
+    when 'google'
+      sequence(:google_auth_data) do |i|
+        { "email" => "email-#{i}@example.com",
+          "sub" => "uid-#{i}"
+        }
+      end
     end
   end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 7fa8affa..6f6b75e4 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -47,6 +47,13 @@
     end
   end
 
+  describe "#username" do
+    it "should return the sanitised user-name from a Google email address" do
+      auth_data = { "email" => "email.test@example.com", "sub" => "uid-123" }
+      expect(FactoryGirl.create(:user, google_auth_data: auth_data).username).to eql("email_test")
+    end if Squash::Configuration.authentication.strategy == 'google'
+  end
+
   describe "#email" do
     it "should return the user's corporate email address" do
       expect(FactoryGirl.create(:user, username: 'email-test').email).to eql("email-test@#{Squash::Configuration.mailer.domain}")
@@ -55,6 +62,11 @@
     it "should return the user's corporate email address" do
       expect(FactoryGirl.create(:user, email_address: 'email-test@example.com').email).to eql("email-test@example.com")
     end if Squash::Configuration.authentication.strategy == 'password'
+
+    it "should return the user's corporate email address" do
+      auth_data = { "email" => "email-test@example.com", "sub" => "uid-123" }
+      expect(FactoryGirl.create(:user, google_auth_data: auth_data).email).to eql("email-test@example.com")
+    end if Squash::Configuration.authentication.strategy == 'google'
   end
 
   describe "#distinguished_name" do
@@ -119,6 +131,11 @@
     it "should downcase the username" do
       expect(FactoryGirl.create(:user, username: 'TestCase').username).to eql('testcase')
     end
+
+    it "should downcase the Google Auth username" do
+      auth_data = { "email" => "Email.Test@example.com", "sub" => "uid-123" }
+      expect(FactoryGirl.create(:user, google_auth_data: auth_data).username).to eql("email_test")
+    end if Squash::Configuration.authentication.strategy == 'google'
   end
 
   describe "#watches?" do

From ff68be066e3ae72754c1a1660729bcaf23f7057d Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Wed, 3 Jun 2015 21:39:04 +1200
Subject: [PATCH 08/11] Fix the login-required tests

---
 .../account/bugs_controller_spec.rb           |  3 ++-
 spec/controllers/accounts_controller_spec.rb  |  3 ++-
 .../google_authentication_helpers_spec.rb     |  6 +++--
 spec/controllers/bugs_controller_spec.rb      | 12 ++++++----
 spec/controllers/comments_controller_spec.rb  |  6 +++--
 spec/controllers/events_controller_spec.rb    |  3 ++-
 ...notification_thresholds_controller_spec.rb |  3 ++-
 .../occurrences_controller_spec.rb            | 12 ++++++----
 .../project/membership_controller_spec.rb     |  9 +++++---
 .../project/memberships_controller_spec.rb    |  3 ++-
 spec/controllers/projects_controller_spec.rb  |  9 +++++---
 spec/rails_helper.rb                          | 22 +++++++++++++++++++
 12 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/spec/controllers/account/bugs_controller_spec.rb b/spec/controllers/account/bugs_controller_spec.rb
index 24ba11c1..1f580719 100644
--- a/spec/controllers/account/bugs_controller_spec.rb
+++ b/spec/controllers/account/bugs_controller_spec.rb
@@ -22,9 +22,10 @@ def sort(bugs, field, reverse=false)
       bugs
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :index
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb
index 7963922d..20422ea7 100644
--- a/spec/controllers/accounts_controller_spec.rb
+++ b/spec/controllers/accounts_controller_spec.rb
@@ -21,9 +21,10 @@
       @attrs = {password: 'newpass', password_confirmation: 'newpass', first_name: 'NewFN', last_name: 'NewLN'}
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       patch :update, user: @attrs
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect { @user.reload }.not_to change(@user, :first_name)
     end
 
diff --git a/spec/controllers/additions/google_authentication_helpers_spec.rb b/spec/controllers/additions/google_authentication_helpers_spec.rb
index 280d37e2..78143a30 100644
--- a/spec/controllers/additions/google_authentication_helpers_spec.rb
+++ b/spec/controllers/additions/google_authentication_helpers_spec.rb
@@ -12,7 +12,7 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
-require 'spec_helper'
+require 'rails_helper'
 
 if Squash::Configuration.authentication.strategy == 'google'
   class FakeController
@@ -23,9 +23,11 @@ def logger(*) Rails.logger end
 
     include AuthenticationHelpers
     include GoogleAuthenticationHelpers
+
+    def google_auth_data; end
   end
 
-  describe GoogleAuthenticationHelpers do
+  RSpec.describe GoogleAuthenticationHelpers, type: :model do
     before(:each) { @controller = FakeController.new }
 
     describe "#log_in" do
diff --git a/spec/controllers/bugs_controller_spec.rb b/spec/controllers/bugs_controller_spec.rb
index bd45fc59..0f1b5341 100644
--- a/spec/controllers/bugs_controller_spec.rb
+++ b/spec/controllers/bugs_controller_spec.rb
@@ -46,9 +46,10 @@ def sort(bugs, field, reverse=false)
       bugs
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :index, polymorphic_params(@env, true)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
@@ -175,9 +176,10 @@ def sort(bugs, field, reverse=false)
       repo = double('Git::Repo')
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :show, polymorphic_params(@bug, false)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
@@ -198,9 +200,10 @@ def sort(bugs, field, reverse=false)
   describe "#update" do
     before(:each) { @bug = FactoryGirl.create(:bug) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       patch :update, polymorphic_params(@bug, false, bug: {fixed: 'true'})
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect(@bug.reload).not_to be_fixed
     end
 
@@ -277,9 +280,10 @@ def sort(bugs, field, reverse=false)
   describe "#destroy" do
     before(:each) { @bug = FactoryGirl.create(:bug) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       delete :destroy, polymorphic_params(@bug, false)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect { @bug.reload }.not_to raise_error
     end
 
diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb
index a3195b95..f2243acb 100644
--- a/spec/controllers/comments_controller_spec.rb
+++ b/spec/controllers/comments_controller_spec.rb
@@ -26,9 +26,10 @@
       @user      = membership.user
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :index, polymorphic_params(@bug, true)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
@@ -71,9 +72,10 @@
       @user      = membership.user
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       post :create, polymorphic_params(@bug, true, comment: {body: 'Hello, world!'})
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect(@bug.comments(true)).to be_empty
     end
 
diff --git a/spec/controllers/events_controller_spec.rb b/spec/controllers/events_controller_spec.rb
index d554c46e..7c2acf0b 100644
--- a/spec/controllers/events_controller_spec.rb
+++ b/spec/controllers/events_controller_spec.rb
@@ -34,9 +34,10 @@
       @user      = membership.user
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :index, polymorphic_params(@bug, true)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
diff --git a/spec/controllers/notification_thresholds_controller_spec.rb b/spec/controllers/notification_thresholds_controller_spec.rb
index bebd5128..90f98afe 100644
--- a/spec/controllers/notification_thresholds_controller_spec.rb
+++ b/spec/controllers/notification_thresholds_controller_spec.rb
@@ -69,9 +69,10 @@
     before(:all) { @env = FactoryGirl.create(:environment) }
     before(:each) { @bug = FactoryGirl.create(:bug, environment: @env) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       delete :destroy, polymorphic_params(@bug, true)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect(@bug.reload).not_to be_fixed
     end
 
diff --git a/spec/controllers/occurrences_controller_spec.rb b/spec/controllers/occurrences_controller_spec.rb
index ff78805b..0d8f0454 100644
--- a/spec/controllers/occurrences_controller_spec.rb
+++ b/spec/controllers/occurrences_controller_spec.rb
@@ -27,9 +27,10 @@ def sort(occurrences, reverse=false)
       occurrences
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :index, polymorphic_params(@bug, true)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
@@ -66,9 +67,10 @@ def sort(occurrences, reverse=false)
       @bug = FactoryGirl.create(:bug, environment: @env)
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :histogram, polymorphic_params(@bug, true)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
@@ -108,9 +110,10 @@ def sort(occurrences, reverse=false)
   describe "#aggregate" do
     before(:all) { @bug = FactoryGirl.create(:bug) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :aggregate, polymorphic_params(@bug, true)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
@@ -187,9 +190,10 @@ def sort(occurrences, reverse=false)
   describe "#show" do
     before(:all) { @occurrence = FactoryGirl.create(:rails_occurrence) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :aggregate, polymorphic_params(@occurrence, false)
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
diff --git a/spec/controllers/project/membership_controller_spec.rb b/spec/controllers/project/membership_controller_spec.rb
index 2af41758..01d8d075 100644
--- a/spec/controllers/project/membership_controller_spec.rb
+++ b/spec/controllers/project/membership_controller_spec.rb
@@ -21,9 +21,10 @@
       @user    = FactoryGirl.create(:user)
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       post :join, project_id: @project.to_param
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect(@project.memberships.count).to eql(1)
     end
 
@@ -51,9 +52,10 @@
   describe "#update" do
     before(:each) { @membership = FactoryGirl.create(:membership) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       patch :update, project_id: @membership.project.to_param, membership: {send_comment_emails: '1'}
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect(@membership.reload.send_comment_emails).to eql(false)
     end
 
@@ -76,9 +78,10 @@
   describe "#destroy" do
     before(:each) { @membership = FactoryGirl.create(:membership) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       delete :destroy, project_id: @membership.project.to_param
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     it "should not allow the owner to delete his/her project" do
diff --git a/spec/controllers/project/memberships_controller_spec.rb b/spec/controllers/project/memberships_controller_spec.rb
index 745d51a3..5eba7571 100644
--- a/spec/controllers/project/memberships_controller_spec.rb
+++ b/spec/controllers/project/memberships_controller_spec.rb
@@ -26,9 +26,10 @@
       @memberships = @project.memberships.to_a # get the owner's membership too
     end
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :index, polymorphic_params(@project, true)
-      expect(response).to redirect_to login_url(next: request.fullpath)
+      expect(response).to redirect_to login_required_redirection_url(next: request.fullpath)
     end
 
     context '[authenticated]' do
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index a065e75b..cd969c43 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -20,9 +20,10 @@
   describe "#index" do
     before(:all) { @user = FactoryGirl.create(:user) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :index
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     context '[authenticated]' do
@@ -90,9 +91,10 @@
   describe "#edit" do
     before(:each) { @project = FactoryGirl.create(:project) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       get :edit, id: @project.to_param
-      expect(response).to redirect_to(login_url(next: request.fullpath))
+      expect(response).to redirect_to(login_required_redirection_url(next: request.fullpath))
     end
 
     it "should allow a project member" do
@@ -219,9 +221,10 @@
   describe "#destroy" do
     before(:each) { @project = FactoryGirl.create(:project) }
 
+    include_context "setup for required logged-in user"
     it "should require a logged-in user" do
       delete :destroy, id: @project.to_param
-      expect(response.status).to redirect_to(login_url(next: request.fullpath))
+      expect(response.status).to redirect_to(login_required_redirection_url(next: request.fullpath))
       expect { @project.reload }.not_to raise_error
     end
 
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 72e036cb..6c9014d6 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -82,6 +82,28 @@
 require Rails.root.join('config', 'initializers', 'active_record_observer_hooks')
 # see comments in this file for more info
 
+# Common login-testing strategies in the specs
+#
+if Squash::Configuration.authentication.strategy == 'google'
+  RSpec.shared_context "setup for required logged-in user" do
+    unless Squash::Configuration.authentication.strategy == 'google'
+      def login_required_redirection_url(*args)
+        login_url(*args)
+      end
+
+    else
+      before :each do
+        allow(SecureRandom).to receive(:hex).and_return('1234')
+      end
+
+      def login_required_redirection_url(*args)
+        controller.instance_eval { google_authentication_uri }
+      end
+    end
+
+  end
+end
+
 # Logs a user in for spec purposes.
 #
 # @params [User] user A user.

From 0964d9a2dc000b1197b6994baf6500e7ecce9c5b Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Mon, 8 Jun 2015 07:21:51 +1200
Subject: [PATCH 09/11] Docs

---
 README.md                                                  | 7 ++++---
 app/controllers/additions/authentication_helpers.rb        | 5 +++--
 app/controllers/additions/google_authentication_helpers.rb | 3 ++-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/README.md b/README.md
index 2ff5a340..b8b51877 100644
--- a/README.md
+++ b/README.md
@@ -240,9 +240,10 @@ delivered if all of the following conditions are met:
 
 ### Authentication and Authorization
 
-Authentication is done using either password verification or LDAP; see
-{AuthenticationHelpers} and related controller mixins, as well as the
-model mixins under `app/models/additions` for more information.
+Authentication is done using either password verification, LDAP or Google OAuth
+(via the `simple_google_auth` gem); see {AuthenticationHelpers} and related
+controller mixins, as well as the model mixins under `app/models/additions`
+for more information.
 
 There are four permissions levels that a User can have, specific to an
 individual Project:
diff --git a/app/controllers/additions/authentication_helpers.rb b/app/controllers/additions/authentication_helpers.rb
index 5edd2c23..8fee9429 100644
--- a/app/controllers/additions/authentication_helpers.rb
+++ b/app/controllers/additions/authentication_helpers.rb
@@ -14,7 +14,8 @@
 
 # Controller and view mixin with shared methods pertaining for authenticating
 # and authorizing {User Users}. For specifics on different authentication
-# methods, see {PasswordAuthenticationHelpers} and {LdapAuthenticationHelpers}.
+# methods, see {PasswordAuthenticationHelpers}, {LdapAuthenticationHelpers}
+# and {GoogleAuthenticationHelpers}.
 #
 # The ID of the authenticated user is stored in the session. The presence of a
 # valid user ID in `session[:user_id]` is indicative of an authenticated
@@ -40,7 +41,7 @@ def self.third_party_login?
   # Clears a user session.
 
   def log_out
-    # Why doesn't this use #reset_session ?
+    #TODO: Why doesn't this use #reset_session ?
     session[:user_id] = nil
     @current_user     = nil
   end
diff --git a/app/controllers/additions/google_authentication_helpers.rb b/app/controllers/additions/google_authentication_helpers.rb
index 3b70db44..2dda1401 100644
--- a/app/controllers/additions/google_authentication_helpers.rb
+++ b/app/controllers/additions/google_authentication_helpers.rb
@@ -12,7 +12,8 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
-# Includes methods for authentication with LDAP. Information for the LDAP server
+# Includes methods for authentication with Google OAuth.
+# NONO: Information for the LDAP server
 # is stored in the `authentication.yml` Configoro file for the current
 # environment. A successful BIND yields an authenticated session.
 #

From 6d7f27ad4b0d5f620fd0248858247dfa3126de2e Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Mon, 8 Jun 2015 23:00:38 +1200
Subject: [PATCH 10/11] Update Gemfile.lock dependencies

---
 Gemfile.lock | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Gemfile.lock b/Gemfile.lock
index 6d700294..59ee02e0 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -159,6 +159,8 @@ GEM
     serialbox (1.0.0)
       activesupport
       json
+    simple_google_auth (0.2.0)
+      rails (>= 3.2.0)
     slugalicious (2.1.0)
       rails (>= 4.0)
       stringex
@@ -250,6 +252,7 @@ DEPENDENCIES
   rspec-rails
   safe_yaml
   sass-rails
+  simple_google_auth
   slugalicious
   sourcemap
   sprockets (< 2.12.0)

From 0a5a1487abc3f59307d07277046f5b3e1189b034 Mon Sep 17 00:00:00 2001
From: mexisme <wildjim+dev@kiwinet.org>
Date: Tue, 9 Jun 2015 12:38:26 +1200
Subject: [PATCH 11/11] Default lambda checks for allowed Google domains

---
 config/initializers/google_authentication.rb | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/config/initializers/google_authentication.rb b/config/initializers/google_authentication.rb
index 3897ded5..fd368fb8 100644
--- a/config/initializers/google_authentication.rb
+++ b/config/initializers/google_authentication.rb
@@ -19,5 +19,10 @@
   config.client_id     = Squash::Configuration.authentication.google.client_id
   config.client_secret = Squash::Configuration.authentication.google.client_secret
   config.redirect_uri  = Squash::Configuration.authentication.google.redirect_uri
-  config.authenticate  = lambda {|d| true }
+  config.authenticate  = lambda do |data|
+    allowed_domains = Squash::Configuration.authentication.google.allowed_domains
+    fail "Must provide a list of accepted Google domains" if allowed_domains.nil? || allowed_domains.empty?
+
+    ! allowed_domains.select {|domain| data.email.ends_with? "@#{domain}" }.empty?
+  end
 end if Squash::Configuration.authentication.strategy == 'google'