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/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) 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 d2999ab5..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 @@ -28,12 +29,19 @@ 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? + + # 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 + #TODO: Why doesn't this use #reset_session ? session[:user_id] = nil @current_user = nil end @@ -78,14 +86,23 @@ 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 + # 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? + self.class.third_party_login? + 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 new file mode 100644 index 00000000..2dda1401 --- /dev/null +++ b/app/controllers/additions/google_authentication_helpers.rb @@ -0,0 +1,53 @@ +# 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 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. +# +# If this is the user's first time logging in, the User will be created for him +# or her. + +module GoogleAuthenticationHelpers + extend ActiveSupport::Concern + + included do + def self.third_party_login? + true + end + 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 + + log_in_user user + end + + 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/sessions_controller.rb b/app/controllers/sessions_controller.rb index 895712ea..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 @@ -93,6 +93,7 @@ def create def destroy 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 new file mode 100644 index 00000000..7bc5614d --- /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 InvalidGoogleAuthError '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' } 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/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/additions/google_authentication_helpers_spec.rb b/spec/controllers/additions/google_authentication_helpers_spec.rb new file mode 100644 index 00000000..78143a30 --- /dev/null +++ b/spec/controllers/additions/google_authentication_helpers_spec.rb @@ -0,0 +1,60 @@ +# 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 'rails_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 + + def google_auth_data; end + end + + RSpec.describe GoogleAuthenticationHelpers, type: :model 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 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/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 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.