Don't request HTTP Basic authenticaion when using a token

[himdel]

Only request fallback to HTTP Basic if no other authentication method is used.

Specifically don't fall back to HTTP Basic when using the token, because otherwise there is no way for the UI to prevent the browser log in dialog when attempting to log out with a stale token (or even trying to use one during a normal request, after suspend for example).

Such attempts will now return a proper HTTP 401 JSON response, but will not trigger the popup.

--

Closes ManageIQ/manageiq-ui-classic#4717
Introduced in #359

[himdel]

Testing the UI issue:

open the UI on the login screen,
in the browser console, do:

localStorage.miq_token = '123';
API.logout();

before: a browser login dialog will appear
after: the request will 401 (with an error in the console), but no dialog will appear in the browser

[himdel]

@miq-bot add_label hammer/yes

(#359 was never backported to gaprindashvili)

[himdel]

401 responses:

when not providing token and cancelling the dialog (same before and after):

{"error":{"kind":"unauthorized","message":"Api::AuthenticationError","klass":"Api::AuthenticationError"}}
(or {"error":{"kind":"unauthorized","message":"Authentication failed","klass":"Api::AuthenticationError"}})

when providing a token that is not valid (new now):

{"error":{"kind":"unauthorized","message":"Invalid Authentication Token 123 specified","klass":"Api::AuthenticationError"}}

[himdel]

@miq-bot remove_label wip

Specs are already passing, just added one that fails without this :).

[himdel]

@miq-bot add_label bug

[bdunne]

Shouldn't this be a method?

def using_http_basic_auth?
  !(request.headers[HttpHeaders::MIQ_TOKEN] || request.headers[HttpHeaders::AUTH_TOKEN])
end

[bdunne]

or def auth_mechanism or something

[himdel]

OK, makes sense :)

Will do, looks like there is a similar bit of logic in container deployments, I'll try to merge that into a helper.

app/controllers/api/container_deployments_controller.rb
20:      [HttpHeaders::MIQ_TOKEN, HttpHeaders::AUTH_TOKEN, "HTTP_AUTHORIZATION"].any? do |header|

[himdel]

@bdunne can you take another look please?

I added auth_mechanism returning one of :system, :token, :basic or nil (no headers),
made both conditions switch on auth_mechanism,
removed a similar condition from Logger, replacing it with auth_mechanism,
and changed ContainerDeploymentsController#authentication_requested? to also use it,
which means we're not touching those headers anywhere else (apart from logging) :).

I also extracted 4 lines from all 3 authentication methods and moved them to a shared auth_user method because it bugged me :).

[bdunne]

👍 Looks great! Thanks @himdel

[simaishi]

Hammer backport details:

$ git log -1
commit 4303f761089c8793b8474eb0f57d3d32b95e618a
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Thu Oct 11 10:00:17 2018 -0400

    Merge pull request #488 from himdel/fix-basic-401
    
    Don't request HTTP Basic authenticaion when using a token
    
    (cherry picked from commit c50b0aba41d5f2e7a6454983a6c59553e96e05a2)