Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External auth lookup_by_identity should handle missing request parameter #16386

Merged
merged 2 commits into from
Nov 2, 2017

Conversation

jvlcek
Copy link
Member

@jvlcek jvlcek commented Nov 2, 2017

https://bugzilla.redhat.com/show_bug.cgi?id=1505922

Method Httpd#lookup_by_identity has been updated to take an optional parameter but
was not properly handling if the parameter was not provided.

This PR ensures Httpd#lookup_by_identity does the correct thing when the optional request parameter is and is not provided.

Steps for Testing/QA [Optional]

Setup MiQ for External Auth
Log into classic UI, with valid user.

Ensure the error pop-up: "400 /auth/api error message" (see screenshot in BZ) does not appear
and there are not tracebacks in the api.log

@jvlcek
Copy link
Member Author

jvlcek commented Nov 2, 2017

@miq-bot add_label authentication, bug

@jvlcek
Copy link
Member Author

jvlcek commented Nov 2, 2017

@abellotti and or @gtanzillo Please review

@miq-bot
Copy link
Member

miq-bot commented Nov 2, 2017

Checked commits jvlcek/manageiq@20dd38d~...33ddcc8 with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@abellotti
Copy link
Member

Looks good @jvlcek, I'll do a quick test in a Pod with ext-auth.

@abellotti
Copy link
Member

just tested fine with an AD enabled Pod. 👍

@abellotti abellotti added this to the Sprint 73 Ending Nov 13, 2017 milestone Nov 2, 2017
@abellotti abellotti merged commit a03cda1 into ManageIQ:master Nov 2, 2017
simaishi pushed a commit that referenced this pull request Nov 6, 2017
External auth lookup_by_identity should handle missing request parameter
(cherry picked from commit a03cda1)

https://bugzilla.redhat.com/show_bug.cgi?id=1509416
@simaishi
Copy link
Contributor

simaishi commented Nov 6, 2017

Gaprindashvili backport details:

$ git log -1
commit 3529133f5d33b0fe0927168a57728bc33646d066
Author: Alberto Bellotti <[email protected]>
Date:   Thu Nov 2 18:51:21 2017 -0400

    Merge pull request #16386 from jvlcek/BZ_1505922_api_pop_up
    
    External auth lookup_by_identity should handle missing request parameter
    (cherry picked from commit a03cda10907ac627fd0865fc34c9b6136197b8a1)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1509416

@jvlcek jvlcek deleted the BZ_1505922_api_pop_up branch November 10, 2017 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants