From e8c6f119cd7cf519db3ad1622786aad3156324d4 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 18 Jul 2016 15:14:30 -0600 Subject: [PATCH 1/2] Add an oauth provider path helper. The helper constructs the path for a given oauth provider since Devise 4.0 deprecated passing the provider to the omniauth authentication path. Fixes #18110. --- app/controllers/sessions_controller.rb | 2 +- app/helpers/auth_helper.rb | 6 ++++++ app/views/devise/sessions/_new_crowd.html.haml | 2 +- app/views/devise/shared/_omniauth_box.html.haml | 2 +- app/views/profiles/accounts/show.html.haml | 2 +- 5 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 17aed816cbd..25598e065b8 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -101,7 +101,7 @@ class SessionsController < Devise::SessionsController # Prevent alert from popping up on the first page shown after authentication. flash[:alert] = nil - redirect_to user_omniauth_authorize_path(provider.to_sym) + redirect_to provider_path(provider) end def valid_otp_attempt?(user) diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index cd4d778e508..dcc156311a1 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -60,6 +60,12 @@ module AuthHelper end end + # Constructs the OAuth provider path. + # For example: user_google_omniauth_authorize_path + def provider_path(provider) + send("user_#{provider.underscore}_omniauth_authorize_path") + end + def auth_active?(provider) current_user.identities.exists?(provider: provider.to_s) end diff --git a/app/views/devise/sessions/_new_crowd.html.haml b/app/views/devise/sessions/_new_crowd.html.haml index 8e81671b7e7..6c7c89700cd 100644 --- a/app/views/devise/sessions/_new_crowd.html.haml +++ b/app/views/devise/sessions/_new_crowd.html.haml @@ -1,4 +1,4 @@ -= form_tag(user_omniauth_authorize_path("crowd"), id: 'new_crowd_user' ) do += form_tag(provider_path("crowd"), id: 'new_crowd_user' ) do = text_field_tag :username, nil, {class: "form-control top", placeholder: "Username", autofocus: "autofocus"} = password_field_tag :password, nil, {class: "form-control bottom", placeholder: "Password"} - if devise_mapping.rememberable? diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index de18bc2d844..07d4caf6b94 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -5,4 +5,4 @@ - providers.each do |provider| %span.light - has_icon = provider_has_icon?(provider) - = link_to provider_image_tag(provider), user_omniauth_authorize_path(provider), method: :post, class: (has_icon ? 'oauth-image-link' : 'btn'), "data-no-turbolink" => "true" + = link_to provider_image_tag(provider), provider_path(provider), method: :post, class: (has_icon ? 'oauth-image-link' : 'btn'), "data-no-turbolink" => "true" diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 57d16d29158..7fadd7bf9f1 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -70,7 +70,7 @@ = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do Disconnect - else - = link_to user_omniauth_authorize_path(provider), method: :post, class: 'provider-btn not-active', "data-no-turbolink" => "true" do + = link_to provider_path(provider), method: :post, class: 'provider-btn not-active', "data-no-turbolink" => "true" do Connect %hr - if current_user.can_change_username? From 602fe111912bac119e752b0dfa3b4b3cd81585ff Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Mon, 25 Jul 2016 11:40:40 -0600 Subject: [PATCH 2/2] Remove provider path, replace with dynamic path. --- CHANGELOG | 1 + app/controllers/sessions_controller.rb | 2 +- app/helpers/auth_helper.rb | 6 ------ app/views/devise/sessions/_new_crowd.html.haml | 2 +- app/views/devise/shared/_omniauth_box.html.haml | 2 +- app/views/profiles/accounts/show.html.haml | 2 +- spec/features/login_spec.rb | 2 +- spec/views/devise/shared/_signin_box.html.haml_spec.rb | 2 +- 8 files changed, 7 insertions(+), 12 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 2bcbe501fb1..03dc62bfd1d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -20,6 +20,7 @@ v 8.11.0 (unreleased) - Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects - Retrieve rendered HTML from cache in one request - Fix renaming repository when name contains invalid chararacters under project settings + - Fix devise deprecation warnings. - Optimize checking if a user has read access to a list of issues !5370 - Nokogiri's various parsing methods are now instrumented - Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363 diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 25598e065b8..5d7ecfeacf4 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -101,7 +101,7 @@ class SessionsController < Devise::SessionsController # Prevent alert from popping up on the first page shown after authentication. flash[:alert] = nil - redirect_to provider_path(provider) + redirect_to omniauth_authorize_path(:user, provider) end def valid_otp_attempt?(user) diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb index dcc156311a1..cd4d778e508 100644 --- a/app/helpers/auth_helper.rb +++ b/app/helpers/auth_helper.rb @@ -60,12 +60,6 @@ module AuthHelper end end - # Constructs the OAuth provider path. - # For example: user_google_omniauth_authorize_path - def provider_path(provider) - send("user_#{provider.underscore}_omniauth_authorize_path") - end - def auth_active?(provider) current_user.identities.exists?(provider: provider.to_s) end diff --git a/app/views/devise/sessions/_new_crowd.html.haml b/app/views/devise/sessions/_new_crowd.html.haml index 6c7c89700cd..b7d3acac2b1 100644 --- a/app/views/devise/sessions/_new_crowd.html.haml +++ b/app/views/devise/sessions/_new_crowd.html.haml @@ -1,4 +1,4 @@ -= form_tag(provider_path("crowd"), id: 'new_crowd_user' ) do += form_tag(omniauth_authorize_path(:user, :crowd), id: 'new_crowd_user' ) do = text_field_tag :username, nil, {class: "form-control top", placeholder: "Username", autofocus: "autofocus"} = password_field_tag :password, nil, {class: "form-control bottom", placeholder: "Password"} - if devise_mapping.rememberable? diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index 07d4caf6b94..2e7da2747d0 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -5,4 +5,4 @@ - providers.each do |provider| %span.light - has_icon = provider_has_icon?(provider) - = link_to provider_image_tag(provider), provider_path(provider), method: :post, class: (has_icon ? 'oauth-image-link' : 'btn'), "data-no-turbolink" => "true" + = link_to provider_image_tag(provider), omniauth_authorize_path(:user, provider), method: :post, class: (has_icon ? 'oauth-image-link' : 'btn'), "data-no-turbolink" => "true" diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 7fadd7bf9f1..c80f22457b4 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -70,7 +70,7 @@ = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'provider-btn' do Disconnect - else - = link_to provider_path(provider), method: :post, class: 'provider-btn not-active', "data-no-turbolink" => "true" do + = link_to omniauth_authorize_path(:user, provider), method: :post, class: 'provider-btn not-active', "data-no-turbolink" => "true" do Connect %hr - if current_user.can_change_username? diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index 58753ff21f6..c4e8b1da531 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -128,7 +128,7 @@ feature 'Login', feature: true do end allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: saml_config) allow(Gitlab.config.omniauth).to receive_messages(messages) - allow_any_instance_of(Object).to receive(:user_omniauth_authorize_path).with('saml').and_return('/users/auth/saml') + expect_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml') end it 'should show 2FA prompt after OAuth login' do diff --git a/spec/views/devise/shared/_signin_box.html.haml_spec.rb b/spec/views/devise/shared/_signin_box.html.haml_spec.rb index 05a76ee4bdb..ee362e6fcb3 100644 --- a/spec/views/devise/shared/_signin_box.html.haml_spec.rb +++ b/spec/views/devise/shared/_signin_box.html.haml_spec.rb @@ -31,7 +31,7 @@ describe 'devise/shared/_signin_box' do def enable_crowd allow(view).to receive(:form_based_providers).and_return([:crowd]) allow(view).to receive(:crowd_enabled?).and_return(true) - allow(view).to receive(:user_omniauth_authorize_path).with('crowd'). + allow(view).to receive(:omniauth_authorize_path).with(:user, :crowd). and_return('/crowd') end end