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).
This commit is contained in:
parent
ef179bf67b
commit
213f2d153f
7 changed files with 46 additions and 1 deletions
|
@ -45,7 +45,9 @@ module ApplicationController::HandlesErrors
|
||||||
respond_to do |format|
|
respond_to do |format|
|
||||||
format.json { render json: humanize_error(e.message), status: status }
|
format.json { render json: humanize_error(e.message), status: status }
|
||||||
format.any do
|
format.any do
|
||||||
|
errors = humanize_error(e.message)
|
||||||
@exception = e
|
@exception = e
|
||||||
|
@message = errors[:error_human] || errors[:error] || param[:message]
|
||||||
@traceback = !Rails.env.production?
|
@traceback = !Rails.env.production?
|
||||||
file = File.open(Rails.root.join('public', "#{status_code}.html"), 'r')
|
file = File.open(Rails.root.join('public', "#{status_code}.html"), 'r')
|
||||||
render inline: file.read, status: status
|
render inline: file.read, status: status
|
||||||
|
|
|
@ -2,7 +2,7 @@
|
||||||
|
|
||||||
class SessionsController < ApplicationController
|
class SessionsController < ApplicationController
|
||||||
prepend_before_action :authentication_check, only: %i[switch_to_user list delete]
|
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"
|
# "Create" a login, aka "log the user in"
|
||||||
def create
|
def create
|
||||||
|
@ -165,6 +165,10 @@ class SessionsController < ApplicationController
|
||||||
redirect_to '/'
|
redirect_to '/'
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def failure_omniauth
|
||||||
|
raise Exceptions::UnprocessableEntity, "Message from #{params[:strategy]}: #{params[:message]}"
|
||||||
|
end
|
||||||
|
|
||||||
def create_sso
|
def create_sso
|
||||||
|
|
||||||
# in case, remove switched_from_user_id
|
# in case, remove switched_from_user_id
|
||||||
|
|
|
@ -3,6 +3,7 @@ Zammad::Application.routes.draw do
|
||||||
|
|
||||||
# omniauth
|
# omniauth
|
||||||
match '/auth/:provider/callback', to: 'sessions#create_omniauth', via: %i[post get puts delete]
|
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
|
# sso
|
||||||
match '/auth/sso', to: 'sessions#create_sso', via: %i[post get]
|
match '/auth/sso', to: 'sessions#create_sso', via: %i[post get]
|
||||||
|
|
|
@ -5,6 +5,9 @@
|
||||||
<link rel="stylesheet" href="/assets/error/style.css">
|
<link rel="stylesheet" href="/assets/error/style.css">
|
||||||
<body <% if @traceback %>class="error-message"<% end %>>
|
<body <% if @traceback %>class="error-message"<% end %>>
|
||||||
<h1>401: Unauthorized</h1>
|
<h1>401: Unauthorized</h1>
|
||||||
|
<% if @message.present? %>
|
||||||
|
<div><%= @message %></div>
|
||||||
|
<% end %>
|
||||||
<% if !@traceback %>
|
<% if !@traceback %>
|
||||||
<div class="error-image" style="background-image: url(/assets/error/error-1.svg)"></div>
|
<div class="error-image" style="background-image: url(/assets/error/error-1.svg)"></div>
|
||||||
<p>Sorry, but you're not allowed to access this page. If you're registered please log in and refresh this page.</p>
|
<p>Sorry, but you're not allowed to access this page. If you're registered please log in and refresh this page.</p>
|
||||||
|
|
|
@ -5,6 +5,9 @@
|
||||||
<link rel="stylesheet" href="/assets/error/style.css">
|
<link rel="stylesheet" href="/assets/error/style.css">
|
||||||
<body <% if @traceback %>class="error-message"<% end %>>
|
<body <% if @traceback %>class="error-message"<% end %>>
|
||||||
<h1>404: Requested Ressource was not found.</h1>
|
<h1>404: Requested Ressource was not found.</h1>
|
||||||
|
<% if @message.present? %>
|
||||||
|
<div><%= @message %></div>
|
||||||
|
<% end %>
|
||||||
<% if !@traceback %>
|
<% if !@traceback %>
|
||||||
<div class="error-image" style="background-image: url(/assets/error/error-2.svg)"></div>
|
<div class="error-image" style="background-image: url(/assets/error/error-2.svg)"></div>
|
||||||
<p>Sorry, but the Phoenix is not able to find your ressource. Try checking the URL for errors.</p>
|
<p>Sorry, but the Phoenix is not able to find your ressource. Try checking the URL for errors.</p>
|
||||||
|
|
|
@ -5,6 +5,9 @@
|
||||||
<link rel="stylesheet" href="/assets/error/style.css">
|
<link rel="stylesheet" href="/assets/error/style.css">
|
||||||
<body <% if @traceback %>class="error-message"<% end %>>
|
<body <% if @traceback %>class="error-message"<% end %>>
|
||||||
<h1>422: The change you wanted was rejected.</h1>
|
<h1>422: The change you wanted was rejected.</h1>
|
||||||
|
<% if @message.present? %>
|
||||||
|
<div><%= @message %></div>
|
||||||
|
<% end %>
|
||||||
<% if !@traceback %>
|
<% if !@traceback %>
|
||||||
<div class="error-image" style="background-image: url(/assets/error/error-1.svg)"></div>
|
<div class="error-image" style="background-image: url(/assets/error/error-1.svg)"></div>
|
||||||
<p>Maybe you tried to change something you didn't have access to.</p>
|
<p>Maybe you tried to change something you didn't have access to.</p>
|
||||||
|
|
29
test/controllers/o_auth_controller_test.rb
Normal file
29
test/controllers/o_auth_controller_test.rb
Normal file
|
@ -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('<title>422: Unprocessable Entity</title>', @response.body)
|
||||||
|
assert_match('<h1>422: The change you wanted was rejected.</h1>', @response.body)
|
||||||
|
assert_match('<div>Message from some_provider: 123</div>', @response.body)
|
||||||
|
end
|
||||||
|
|
||||||
|
end
|
Loading…
Reference in a new issue