From 213f2d153f1079018f49c7db4a47d7bb0ce6732c Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 17 Jul 2018 10:35:51 +0200 Subject: [PATCH] Fixed issue #2128 - Route for /auth/failure is missing to show login failure messages form oauth provider (if request was technical ok - only login was not possible by oauth provider). --- .../application_controller/handles_errors.rb | 2 ++ app/controllers/sessions_controller.rb | 6 +++- config/routes/auth.rb | 1 + public/401.html | 3 ++ public/404.html | 3 ++ public/422.html | 3 ++ test/controllers/o_auth_controller_test.rb | 29 +++++++++++++++++++ 7 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 test/controllers/o_auth_controller_test.rb diff --git a/app/controllers/application_controller/handles_errors.rb b/app/controllers/application_controller/handles_errors.rb index 98c1728f7..7d631e237 100644 --- a/app/controllers/application_controller/handles_errors.rb +++ b/app/controllers/application_controller/handles_errors.rb @@ -45,7 +45,9 @@ module ApplicationController::HandlesErrors respond_to do |format| format.json { render json: humanize_error(e.message), status: status } format.any do + errors = humanize_error(e.message) @exception = e + @message = errors[:error_human] || errors[:error] || param[:message] @traceback = !Rails.env.production? file = File.open(Rails.root.join('public', "#{status_code}.html"), 'r') render inline: file.read, status: status diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index fef102c5f..e7e34d000 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -2,7 +2,7 @@ class SessionsController < ApplicationController prepend_before_action :authentication_check, only: %i[switch_to_user list delete] - skip_before_action :verify_csrf_token, only: %i[create show destroy create_omniauth create_sso] + skip_before_action :verify_csrf_token, only: %i[create show destroy create_omniauth failure_omniauth create_sso] # "Create" a login, aka "log the user in" def create @@ -165,6 +165,10 @@ class SessionsController < ApplicationController redirect_to '/' end + def failure_omniauth + raise Exceptions::UnprocessableEntity, "Message from #{params[:strategy]}: #{params[:message]}" + end + def create_sso # in case, remove switched_from_user_id diff --git a/config/routes/auth.rb b/config/routes/auth.rb index b6fc7a6ea..92aa9fdd5 100644 --- a/config/routes/auth.rb +++ b/config/routes/auth.rb @@ -3,6 +3,7 @@ Zammad::Application.routes.draw do # omniauth match '/auth/:provider/callback', to: 'sessions#create_omniauth', via: %i[post get puts delete] + match '/auth/failure', to: 'sessions#failure_omniauth', via: %i[post get] # sso match '/auth/sso', to: 'sessions#create_sso', via: %i[post get] diff --git a/public/401.html b/public/401.html index 436a395b2..c70b0ca3f 100644 --- a/public/401.html +++ b/public/401.html @@ -5,6 +5,9 @@ class="error-message"<% end %>>

401: Unauthorized

+<% if @message.present? %> +
<%= @message %>
+<% end %> <% if !@traceback %>

Sorry, but you're not allowed to access this page. If you're registered please log in and refresh this page.

diff --git a/public/404.html b/public/404.html index a072faae7..a28343f5c 100644 --- a/public/404.html +++ b/public/404.html @@ -5,6 +5,9 @@ class="error-message"<% end %>>

404: Requested Ressource was not found.

+<% if @message.present? %> +
<%= @message %>
+<% end %> <% if !@traceback %>

Sorry, but the Phoenix is not able to find your ressource. Try checking the URL for errors.

diff --git a/public/422.html b/public/422.html index 2b0688b74..da495ac86 100644 --- a/public/422.html +++ b/public/422.html @@ -5,6 +5,9 @@ class="error-message"<% end %>>

422: The change you wanted was rejected.

+<% if @message.present? %> +
<%= @message %>
+<% end %> <% if !@traceback %>

Maybe you tried to change something you didn't have access to.

diff --git a/test/controllers/o_auth_controller_test.rb b/test/controllers/o_auth_controller_test.rb new file mode 100644 index 000000000..ac342c907 --- /dev/null +++ b/test/controllers/o_auth_controller_test.rb @@ -0,0 +1,29 @@ + +require 'test_helper' + +class OAuthControllerTest < ActionDispatch::IntegrationTest + + test 'o365 - start' do + get '/auth/microsoft_office365', params: {} + assert_response(302) + assert_match('https://login.microsoftonline.com/common/oauth2/v2.0/authorize', @response.body) + assert_match('redirect_uri=http%3A%2F%2Fzammad.example.com%2Fauth%2Fmicrosoft_office365%2Fcallback', @response.body) + assert_match('scope=openid+email+profile', @response.body) + assert_match('response_type=code', @response.body) + end + + test 'o365 - callback' do + get '/auth/microsoft_office365/callback?code=1234&state=1234', params: {} + assert_response(302) + assert_match('302 Moved', @response.body) + end + + test 'auth failure' do + get '/auth/failure?message=123&strategy=some_provider', params: {} + assert_response(422) + assert_match('422: Unprocessable Entity', @response.body) + assert_match('

422: The change you wanted was rejected.

', @response.body) + assert_match('
Message from some_provider: 123
', @response.body) + end + +end