From d2bceff133ac152678a1407d45fea260a0fe8536 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 8 Oct 2019 12:10:43 +0100 Subject: [PATCH] FEATURE: Use full page redirection for all external auth methods (#8092) Using popups is becoming increasingly rare. Full page redirects are already used on mobile, and for some providers. This commit removes all logic related to popup authentication, leaving only the full page redirect method. For more info, see https://meta.discourse.org/t/do-we-need-popups-for-login/127988 --- .../discourse/controllers/login.js.es6 | 38 +++------- .../controllers/preferences/account.js.es6 | 2 +- .../initializers/auth-complete.js.es6 | 6 +- .../discourse/models/login-method.js.es6 | 70 +++++------------ .../discourse/routes/application.js.es6 | 4 +- .../templates/mobile/modal/login.hbs | 6 +- .../discourse/templates/modal/login.hbs | 5 -- .../omniauth-complete.js.no-module.es6 | 18 ----- app/controllers/application_controller.rb | 3 - .../users/omniauth_callbacks_controller.rb | 15 +--- app/serializers/auth_provider_serializer.rb | 8 +- .../omniauth_callbacks/complete.html.erb | 33 -------- .../confirm_request.html.erb | 1 - config/locales/client.en.yml | 6 -- lib/auth/auth_provider.rb | 12 ++- lib/discourse.rb | 2 +- spec/components/plugin/instance_spec.rb | 12 ++- spec/requests/application_controller_spec.rb | 5 -- .../omniauth_callbacks_controller_spec.rb | 76 +++++++++---------- .../complete.html.erb_spec.rb | 40 ---------- .../javascripts/fixtures/site-fixtures.js.es6 | 1 - 21 files changed, 92 insertions(+), 271 deletions(-) delete mode 100644 app/assets/javascripts/omniauth-complete.js.no-module.es6 delete mode 100644 app/views/users/omniauth_callbacks/complete.html.erb delete mode 100644 spec/views/omniauth_callbacks/complete.html.erb_spec.rb diff --git a/app/assets/javascripts/discourse/controllers/login.js.es6 b/app/assets/javascripts/discourse/controllers/login.js.es6 index 5dfc9227da7..7f0f5bdd33a 100644 --- a/app/assets/javascripts/discourse/controllers/login.js.es6 +++ b/app/assets/javascripts/discourse/controllers/login.js.es6 @@ -24,7 +24,6 @@ export default Ember.Controller.extend(ModalFunctionality, { forgotPassword: Ember.inject.controller(), application: Ember.inject.controller(), - authenticate: null, loggingIn: false, loggedIn: false, processingEmailLink: false, @@ -39,7 +38,6 @@ export default Ember.Controller.extend(ModalFunctionality, { resetForm() { this.setProperties({ - authenticate: null, loggingIn: false, loggedIn: false, secondFactorRequired: false, @@ -85,12 +83,12 @@ export default Ember.Controller.extend(ModalFunctionality, { loginDisabled: Ember.computed.or("loggingIn", "loggedIn"), - @computed("loggingIn", "authenticate", "application.canSignUp") - showSignupLink(loggingIn, authenticate, canSignUp) { - return canSignUp && !loggingIn && Ember.isEmpty(authenticate); + @computed("loggingIn", "application.canSignUp") + showSignupLink(loggingIn, canSignUp) { + return canSignUp && !loggingIn; }, - showSpinner: Ember.computed.or("loggingIn", "authenticate"), + showSpinner: Ember.computed.readOnly("loggingIn"), @computed("canLoginLocalWithEmail", "processingEmailLink") showLoginWithEmailLink(canLoginLocalWithEmail, processingEmailLink) { @@ -233,20 +231,13 @@ export default Ember.Controller.extend(ModalFunctionality, { return false; }, - externalLogin(loginMethod, { fullScreenLogin = false } = {}) { - const capabilities = this.capabilities; - // On Mobile, Android or iOS always go with full screen - if ( - this.isMobileDevice || - (capabilities && - (capabilities.isIOS || - capabilities.isAndroid || - capabilities.isSafari)) - ) { - fullScreenLogin = true; + externalLogin(loginMethod) { + if (this.loginDisabled) { + return; } - loginMethod.doLogin({ fullScreenLogin }); + this.set("loggingIn", true); + loginMethod.doLogin().catch(() => this.set("loggingIn", false)); }, createAccount() { @@ -324,16 +315,6 @@ export default Ember.Controller.extend(ModalFunctionality, { } }, - @computed("authenticate") - authMessage(authenticate) { - if (Ember.isEmpty(authenticate)) return ""; - - const method = findAll().findBy("name", authenticate); - if (method) { - return method.message; - } - }, - authenticationComplete(options) { const loginError = (errorMsg, className, callback) => { showModal("login"); @@ -341,7 +322,6 @@ export default Ember.Controller.extend(ModalFunctionality, { Ember.run.next(() => { if (callback) callback(); this.flash(errorMsg, className || "success"); - this.set("authenticate", null); }); }; diff --git a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 index 8bf5d24d27a..14fcc4b79fc 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 @@ -252,7 +252,7 @@ export default Ember.Controller.extend( }, connectAccount(method) { - method.doLogin({ reconnect: true, fullScreenLogin: false }); + method.doLogin({ reconnect: true }); } } } diff --git a/app/assets/javascripts/discourse/initializers/auth-complete.js.es6 b/app/assets/javascripts/discourse/initializers/auth-complete.js.es6 index cecf831405c..83e0183ff3b 100644 --- a/app/assets/javascripts/discourse/initializers/auth-complete.js.es6 +++ b/app/assets/javascripts/discourse/initializers/auth-complete.js.es6 @@ -4,11 +4,7 @@ export default { initialize(container) { let lastAuthResult; - if (window.location.search.indexOf("authComplete=true") !== -1) { - // Happens when a popup social login loses connection to the parent window - lastAuthResult = localStorage.getItem("lastAuthResult"); - localStorage.removeItem("lastAuthResult"); - } else if (document.getElementById("data-authentication")) { + if (document.getElementById("data-authentication")) { // Happens for full screen logins lastAuthResult = document.getElementById("data-authentication").dataset .authenticationData; diff --git a/app/assets/javascripts/discourse/models/login-method.js.es6 b/app/assets/javascripts/discourse/models/login-method.js.es6 index 0b1260447a1..ce59db24501 100644 --- a/app/assets/javascripts/discourse/models/login-method.js.es6 +++ b/app/assets/javascripts/discourse/models/login-method.js.es6 @@ -17,60 +17,24 @@ const LoginMethod = Ember.Object.extend({ return this.message_override || I18n.t(`login.${this.name}.message`); }, - doLogin({ reconnect = false, fullScreenLogin = true } = {}) { - const name = this.name; - const customLogin = this.customLogin; - - if (customLogin) { - customLogin(); - } else { - if (this.custom_url) { - window.location = this.custom_url; - return; - } - let authUrl = Discourse.getURL(`/auth/${name}`); - - if (reconnect) { - authUrl += "?reconnect=true"; - } - - if (reconnect || fullScreenLogin || this.full_screen_login) { - LoginMethod.buildPostForm(authUrl).then(form => { - document.cookie = "fsl=true"; - form.submit(); - }); - } else { - this.set("authenticate", name); - const left = this.lastX - 400; - const top = this.lastY - 200; - - const height = this.frame_height || 400; - const width = this.frame_width || 800; - - if (name === "facebook") { - authUrl += authUrl.includes("?") ? "&" : "?"; - authUrl += "display=popup"; - } - LoginMethod.buildPostForm(authUrl).then(form => { - const windowState = window.open( - "about:blank", - "auth_popup", - `menubar=no,status=no,height=${height},width=${width},left=${left},top=${top}` - ); - - form.target = "auth_popup"; - form.submit(); - - const timer = setInterval(() => { - // If the process is aborted, reset state in this window - if (!windowState || windowState.closed) { - clearInterval(timer); - this.set("authenticate", null); - } - }, 1000); - }); - } + doLogin({ reconnect = false } = {}) { + if (this.customLogin) { + this.customLogin(); + return Ember.RSVP.resolve(); } + + if (this.custom_url) { + window.location = this.custom_url; + return Ember.RSVP.resolve(); + } + + let authUrl = Discourse.getURL(`/auth/${this.name}`); + + if (reconnect) { + authUrl += "?reconnect=true"; + } + + return LoginMethod.buildPostForm(authUrl).then(form => form.submit()); } }); diff --git a/app/assets/javascripts/discourse/routes/application.js.es6 b/app/assets/javascripts/discourse/routes/application.js.es6 index f9cdcab186d..4c85a428625 100644 --- a/app/assets/javascripts/discourse/routes/application.js.es6 +++ b/app/assets/javascripts/discourse/routes/application.js.es6 @@ -265,9 +265,7 @@ const ApplicationRoute = Discourse.Route.extend(OpenComposer, { const methods = findAll(); if (!this.siteSettings.enable_local_logins && methods.length === 1) { - this.controllerFor("login").send("externalLogin", methods[0], { - fullScreenLogin: true - }); + this.controllerFor("login").send("externalLogin", methods[0]); } else { showModal(modal); this.controllerFor("modal").set("modalClass", modalClass); diff --git a/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs b/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs index 25fbf9bb0d6..b2d65fbe878 100644 --- a/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs +++ b/app/assets/javascripts/discourse/templates/mobile/modal/login.hbs @@ -62,10 +62,6 @@ {{/d-modal-body}}
- {{#if authenticate}} - {{i18n "login.authenticating"}} - {{/if}} - {{#if canLoginLocal}} {{#unless showSecurityKey }} {{d-button action=(action "login") @@ -82,6 +78,6 @@ {{/if}} {{/if}}
-
{{authMessage}}
+
{{alert}}
{{/login-modal}} diff --git a/app/assets/javascripts/discourse/templates/modal/login.hbs b/app/assets/javascripts/discourse/templates/modal/login.hbs index ff3b7f40eca..beaa5569cac 100644 --- a/app/assets/javascripts/discourse/templates/modal/login.hbs +++ b/app/assets/javascripts/discourse/templates/modal/login.hbs @@ -73,13 +73,8 @@ {{/if}} {{/if}} - {{#if authenticate}} -  {{i18n 'login.authenticating'}} - {{/if}} - {{conditional-loading-spinner condition=showSpinner size="small"}} -
{{authMessage}}
{{alert}}
{{/login-modal}} diff --git a/app/assets/javascripts/omniauth-complete.js.no-module.es6 b/app/assets/javascripts/omniauth-complete.js.no-module.es6 deleted file mode 100644 index 7be373d50f4..00000000000 --- a/app/assets/javascripts/omniauth-complete.js.no-module.es6 +++ /dev/null @@ -1,18 +0,0 @@ -(function() { - const { authResult, baseUrl } = document.getElementById( - "data-auth-result" - ).dataset; - const parsedAuthResult = JSON.parse(authResult); - - if ( - !window.opener || - !window.opener.Discourse || - !window.opener.Discourse.authenticationComplete - ) { - localStorage.setItem("lastAuthResult", authResult); - window.location.href = `${baseUrl}?authComplete=true`; - } else { - window.opener.Discourse.authenticationComplete(parsedAuthResult); - window.close(); - } -})(); diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 23467e68ce0..5d5db79982f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -724,9 +724,6 @@ class ApplicationController < ActionController::Base session[:destination_url] = destination_url redirect_to path('/session/sso') return - elsif params[:authComplete].present? - redirect_to path("/login?authComplete=true") - return else # save original URL in a cookie (javascript redirects after login in this case) cookies[:destination_url] = destination_url diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index e2cf35a0725..7e7e108a352 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -71,18 +71,9 @@ class Users::OmniauthCallbacksController < ApplicationController else @auth_result.authenticator_name = authenticator.name complete_response_data - - if provider&.full_screen_login || cookies['fsl'] - cookies.delete('fsl') - cookies['_bypass_cache'] = true - cookies[:authentication_data] = @auth_result.to_client_hash.to_json - redirect_to @origin - else - respond_to do |format| - format.html - format.json { render json: @auth_result.to_client_hash } - end - end + cookies['_bypass_cache'] = true + cookies[:authentication_data] = @auth_result.to_client_hash.to_json + redirect_to @origin end end diff --git a/app/serializers/auth_provider_serializer.rb b/app/serializers/auth_provider_serializer.rb index 404e3d9425b..320a2654cf9 100644 --- a/app/serializers/auth_provider_serializer.rb +++ b/app/serializers/auth_provider_serializer.rb @@ -3,7 +3,7 @@ class AuthProviderSerializer < ApplicationSerializer attributes :name, :custom_url, :pretty_name_override, :title_override, :message_override, - :frame_width, :frame_height, :full_screen_login, :can_connect, :can_revoke, + :frame_width, :frame_height, :can_connect, :can_revoke, :icon def title_override @@ -16,12 +16,6 @@ class AuthProviderSerializer < ApplicationSerializer object.pretty_name end - def full_screen_login - return SiteSetting.get(object.full_screen_login_setting) if object.full_screen_login_setting - return object.full_screen_login if object.full_screen_login - false - end - def message_override object.message end diff --git a/app/views/users/omniauth_callbacks/complete.html.erb b/app/views/users/omniauth_callbacks/complete.html.erb deleted file mode 100644 index dc6a0b0d4d0..00000000000 --- a/app/views/users/omniauth_callbacks/complete.html.erb +++ /dev/null @@ -1,33 +0,0 @@ - - - - <%= SiteSetting.title %> - - - - <%= tag.meta id: 'data-auth-result', data: { - auth_result: @auth_result.to_client_hash, - base_url: Discourse.base_url - } %> - <%= preload_script('omniauth-complete') %> - - - -
-

- <%=t "login.auth_complete" %> - <%= t("login.click_to_continue") %> -

-
- - diff --git a/app/views/users/omniauth_callbacks/confirm_request.html.erb b/app/views/users/omniauth_callbacks/confirm_request.html.erb index a25158dbb3f..9fa44acc4d9 100644 --- a/app/views/users/omniauth_callbacks/confirm_request.html.erb +++ b/app/views/users/omniauth_callbacks/confirm_request.html.erb @@ -3,7 +3,6 @@
<%= form_tag do %> - <%= button_tag(type: "submit", class: "btn btn-primary") do %> <%= SvgSprite.raw_svg('fa-plug') %><%= t 'login.omniauth_confirm_button' %> <% end %> diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 33736008fd8..91685d68ea2 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1497,27 +1497,21 @@ en: google_oauth2: name: "Google" title: "with Google" - message: "Authenticating with Google (make sure pop up blockers are not enabled)" twitter: name: "Twitter" title: "with Twitter" - message: "Authenticating with Twitter (make sure pop up blockers are not enabled)" instagram: name: "Instagram" title: "with Instagram" - message: "Authenticating with Instagram (make sure pop up blockers are not enabled)" facebook: name: "Facebook" title: "with Facebook" - message: "Authenticating with Facebook (make sure pop up blockers are not enabled)" github: name: "GitHub" title: "with GitHub" - message: "Authenticating with GitHub (make sure pop up blockers are not enabled)" discord: name: "Discord" title: "with Discord" - message: "Authenticating with Discord" second_factor_toggle: totp: "Use an authenticator app instead" backup_code: "Use a backup code instead" diff --git a/lib/auth/auth_provider.rb b/lib/auth/auth_provider.rb index 138bbea1934..d72748fc6a8 100644 --- a/lib/auth/auth_provider.rb +++ b/lib/auth/auth_provider.rb @@ -16,12 +16,20 @@ class Auth::AuthProvider attr_accessor(*auth_attributes) def enabled_setting=(val) - Discourse.deprecate("enabled_setting is deprecated. Please define authenticator.enabled? instead") + Discourse.deprecate("(#{authenticator.name}) enabled_setting is deprecated. Please define authenticator.enabled? instead") @enabled_setting = val end def background_color=(val) - Discourse.deprecate("background_color is no longer functional. Please use CSS instead") + Discourse.deprecate("(#{authenticator.name}) background_color is no longer functional. Please use CSS instead") + end + + def full_screen_login=(val) + Discourse.deprecate("(#{authenticator.name}) full_screen_login is now forced. The full_screen_login parameter can be removed from the auth_provider.") + end + + def full_screen_login_setting=(val) + Discourse.deprecate("(#{authenticator.name}) full_screen_login is now forced. The full_screen_login_setting parameter can be removed from the auth_provider.") end def name diff --git a/lib/discourse.rb b/lib/discourse.rb index 0295d92c560..53674172f0d 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -260,7 +260,7 @@ module Discourse Auth::AuthProvider.new(authenticator: Auth::GithubAuthenticator.new, icon: "fab-github"), Auth::AuthProvider.new(authenticator: Auth::TwitterAuthenticator.new, icon: "fab-twitter"), Auth::AuthProvider.new(authenticator: Auth::InstagramAuthenticator.new, icon: "fab-instagram"), - Auth::AuthProvider.new(authenticator: Auth::DiscordAuthenticator.new, icon: "fab-discord", full_screen_login: true) + Auth::AuthProvider.new(authenticator: Auth::DiscordAuthenticator.new, icon: "fab-discord") ] def self.auth_providers diff --git a/spec/components/plugin/instance_spec.rb b/spec/components/plugin/instance_spec.rb index 0ff78469865..7286f5cc0a9 100644 --- a/spec/components/plugin/instance_spec.rb +++ b/spec/components/plugin/instance_spec.rb @@ -161,20 +161,26 @@ describe Plugin::Instance do end it 'patches the enabled? function for auth_providers if not defined' do + SimpleAuthenticator = Class.new(Auth::Authenticator) do + def name + "my_authenticator" + end + end + plugin = Plugin::Instance.new # lets piggy back on another boolean setting, so we don't dirty our SiteSetting object SiteSetting.enable_badges = false # No enabled_site_setting - authenticator = Auth::Authenticator.new + authenticator = SimpleAuthenticator.new plugin.auth_provider(authenticator: authenticator) plugin.notify_before_auth expect(authenticator.enabled?).to eq(true) # With enabled site setting plugin = Plugin::Instance.new - authenticator = Auth::Authenticator.new + authenticator = SimpleAuthenticator.new plugin.auth_provider(enabled_setting: 'enable_badges', authenticator: authenticator) plugin.notify_before_auth expect(authenticator.enabled?).to eq(false) @@ -183,7 +189,7 @@ describe Plugin::Instance do plugin = Plugin::Instance.new SiteSetting.enable_badges = true - authenticator = Class.new(Auth::Authenticator) do + authenticator = Class.new(SimpleAuthenticator) do def enabled? false end diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index b44df1ba08f..fbac7915311 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -11,11 +11,6 @@ RSpec.describe ApplicationController do SiteSetting.login_required = true end - it "should carry-forward authComplete param to login page redirect" do - get "/?authComplete=true" - expect(response).to redirect_to('/login?authComplete=true') - end - it "should never cache a login redirect" do get "/" expect(response.headers["Cache-Control"]).to eq("no-cache, no-store") diff --git a/spec/requests/omniauth_callbacks_controller_spec.rb b/spec/requests/omniauth_callbacks_controller_spec.rb index d72f9f71dc8..a0c495c6969 100644 --- a/spec/requests/omniauth_callbacks_controller_spec.rb +++ b/spec/requests/omniauth_callbacks_controller_spec.rb @@ -197,17 +197,17 @@ RSpec.describe Users::OmniauthCallbacksController do get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) + expect(response.status).to eq(302) - response_body = JSON.parse(response.body) + data = JSON.parse(cookies[:authentication_data]) - expect(response_body["email"]).to eq(email) - expect(response_body["username"]).to eq("Some_Name") - expect(response_body["auth_provider"]).to eq("google_oauth2") - expect(response_body["email_valid"]).to eq(true) - expect(response_body["omit_username"]).to eq(false) - expect(response_body["name"]).to eq("Some Name") - expect(response_body["destination_url"]).to eq(destination_url) + expect(data["email"]).to eq(email) + expect(data["username"]).to eq("Some_Name") + expect(data["auth_provider"]).to eq("google_oauth2") + expect(data["email_valid"]).to eq(true) + expect(data["omit_username"]).to eq(false) + expect(data["name"]).to eq("Some Name") + expect(data["destination_url"]).to eq(destination_url) end it 'should include destination url in response' do @@ -216,8 +216,8 @@ RSpec.describe Users::OmniauthCallbacksController do get "/auth/google_oauth2/callback.json" - response_body = JSON.parse(response.body) - expect(response_body["destination_url"]).to eq(destination_url) + data = JSON.parse(cookies[:authentication_data]) + expect(data["destination_url"]).to eq(destination_url) end end @@ -254,15 +254,15 @@ RSpec.describe Users::OmniauthCallbacksController do expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in) - expect(response.status).to eq(200) + expect(response.status).to eq(302) - response_body = JSON.parse(response.body) + data = JSON.parse(cookies[:authentication_data]) - expect(response_body["authenticated"]).to eq(true) - expect(response_body["awaiting_activation"]).to eq(false) - expect(response_body["awaiting_approval"]).to eq(false) - expect(response_body["not_allowed_from_ip_address"]).to eq(false) - expect(response_body["admin_not_allowed_from_ip_address"]).to eq(false) + expect(data["authenticated"]).to eq(true) + expect(data["awaiting_activation"]).to eq(false) + expect(data["awaiting_approval"]).to eq(false) + expect(data["not_allowed_from_ip_address"]).to eq(false) + expect(data["admin_not_allowed_from_ip_address"]).to eq(false) user.reload expect(user.email_confirmed?).to eq(true) @@ -280,7 +280,7 @@ RSpec.describe Users::OmniauthCallbacksController do expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in) - expect(response.status).to eq(200) + expect(response.status).to eq(302) user.reload expect(user.email_confirmed?).to eq(true) @@ -299,7 +299,7 @@ RSpec.describe Users::OmniauthCallbacksController do expect(events.map { |event| event[:event_name] }).to include(:user_logged_in, :user_first_logged_in) - expect(response.status).to eq(200) + expect(response.status).to eq(302) user.reload expect(user.staged).to eq(false) @@ -330,18 +330,18 @@ RSpec.describe Users::OmniauthCallbacksController do it 'should return the right response' do get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) + expect(response.status).to eq(302) - response_body = JSON.parse(response.body) + data = JSON.parse(cookies[:authentication_data]) - expect(response_body["email"]).to eq(user.email) - expect(response_body["omniauth_disallow_totp"]).to eq(true) + expect(data["email"]).to eq(user.email) + expect(data["omniauth_disallow_totp"]).to eq(true) user.update!(email: 'different@user.email') get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) - expect(JSON.parse(response.body)["email"]).to eq(user.email) + expect(response.status).to eq(302) + expect(JSON.parse(cookies[:authentication_data])["email"]).to eq(user.email) end end @@ -383,11 +383,11 @@ RSpec.describe Users::OmniauthCallbacksController do it 'should return the right response' do get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) + expect(response.status).to eq(302) - response_body = JSON.parse(response.body) + data = JSON.parse(cookies[:authentication_data]) - expect(response_body["destination_url"]).to match(/\/session\/sso_provider\?sso\=.*\&sig\=.*/) + expect(data["destination_url"]).to match(/\/session\/sso_provider\?sso\=.*\&sig\=.*/) end end @@ -421,13 +421,13 @@ RSpec.describe Users::OmniauthCallbacksController do it 'should return the right response' do get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) + expect(response.status).to eq(302) - response_body = JSON.parse(response.body) + data = JSON.parse(cookies[:authentication_data]) expect(user.reload.active).to eq(false) - expect(response_body["authenticated"]).to eq(false) - expect(response_body["awaiting_activation"]).to eq(true) + expect(data["authenticated"]).to eq(false) + expect(data["awaiting_activation"]).to eq(true) end end @@ -534,7 +534,7 @@ RSpec.describe Users::OmniauthCallbacksController do expect(session[:auth_reconnect]).to eq(false) get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) + expect(response.status).to eq(302) expect(session[:current_user_id]).to eq(user.id) # Log into another user @@ -544,7 +544,7 @@ RSpec.describe Users::OmniauthCallbacksController do expect(session[:auth_reconnect]).to eq(false) get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) + expect(response.status).to eq(302) expect(session[:current_user_id]).to eq(user2.id) expect(UserAssociatedAccount.count).to eq(2) end @@ -556,7 +556,7 @@ RSpec.describe Users::OmniauthCallbacksController do expect(session[:auth_reconnect]).to eq(true) get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) + expect(response.status).to eq(302) expect(session[:current_user_id]).to eq(user.id) # Clear cookie after login @@ -605,8 +605,8 @@ RSpec.describe Users::OmniauthCallbacksController do Rails.application.env_config["omniauth.auth"] = OmniAuth.config.mock_auth[:google_oauth2] get "/auth/google_oauth2/callback.json" - expect(response.status).to eq(200) - JSON.parse(response.body) + expect(response.status).to eq(302) + JSON.parse(cookies[:authentication_data]) end it 'activates the correct email' do diff --git a/spec/views/omniauth_callbacks/complete.html.erb_spec.rb b/spec/views/omniauth_callbacks/complete.html.erb_spec.rb deleted file mode 100644 index 177c580533c..00000000000 --- a/spec/views/omniauth_callbacks/complete.html.erb_spec.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" - -require "auth/authenticator" - -describe "users/omniauth_callbacks/complete.html.erb" do - - let :rendered_data do - JSON.parse(rendered.match(/data-auth-result="([^"]*)"/)[1].gsub('"', '"')) - end - - it "renders auth info" do - result = Auth::Result.new - result.user = User.new - - assign(:auth_result, result) - - render - - expect(rendered_data["authenticated"]).to eq(false) - expect(rendered_data["awaiting_activation"]).to eq(false) - expect(rendered_data["awaiting_approval"]).to eq(false) - end - - it "renders cas data " do - result = Auth::Result.new - - result.email = "xxx@xxx.com" - result.authenticator_name = "CAS" - - assign(:auth_result, result) - - render - - expect(rendered_data["email"]).to eq(result.email) - expect(rendered_data["auth_provider"]).to eq("CAS") - end - -end diff --git a/test/javascripts/fixtures/site-fixtures.js.es6 b/test/javascripts/fixtures/site-fixtures.js.es6 index 5dbd4805d49..72712c317c1 100644 --- a/test/javascripts/fixtures/site-fixtures.js.es6 +++ b/test/javascripts/fixtures/site-fixtures.js.es6 @@ -601,7 +601,6 @@ export default { message_override: null, frame_width: 580, frame_height: 400, - full_screen_login: false, can_connect: true, can_revoke: true }