Fixes #3064: Double-render error on HTTP Basic auth

This commit fixes a regression[0]
introduced in becbdb1ba (the Pundit migration).

Specifically, `CalendarSubscriptionsController` appears to be
the only controller that supports HTTP Basic authentication
(for calendar clients like Thunderbird Lightning or Calendar.app),
and the migration changed the control flow
for HTTP Basic authentication + authorization:

    # before
    authentication_check({ basic_auth_promt: true, permission: 'user_preferences.calendar' })

    # after
    authentication_check(basic_auth_promt: true) && authorize!

After this change, `#authentication_check` is expected
to communicate success or failure through its return value,
but prior to this bugfix, its return value was always truthy.
This led to a double-render error,
where a response code and message were set twice,
upon the failures of both authentication and authorization.

This fix adds a `return false` in the authorization failure case,
short-circuiting the `#authorize!` call and eliminating the error.

[0]: https://github.com/zammad/zammad/issues/3064
This commit is contained in:
Ryan Lue 2020-05-26 00:26:06 +08:00 committed by Thorsten Eckel
parent acd28480b0
commit 95de420a35
2 changed files with 13 additions and 1 deletions

View file

@ -27,7 +27,8 @@ module ApplicationController::Authenticates
# check if basic_auth fallback is possible
if auth_param[:basic_auth_promt] && !user
return request_http_basic_authentication
request_http_basic_authentication
return false
end
# return auth not ok

View file

@ -0,0 +1,11 @@
require 'rails_helper'
RSpec.describe 'iCal endpoints', type: :request do
context 'with no existing session' do
it 'gives HTTP Basic auth prompt (#3064)' do
get '/ical/tickets'
expect(response.body).to eq("HTTP Basic: Access denied.\n")
end
end
end