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

Do not downcase the amazon IAM username #296

Merged
merged 1 commit into from
Sep 8, 2017

Conversation

jvlcek
Copy link
Member

@jvlcek jvlcek commented Sep 8, 2017

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

Summary:

Fix authentication mode: amazon

Description:

This PR address a regression that was introduce by PR 15796 which
caused a regression in authentication preventing logins when the authentication
mode is configured for "Amazon"

Notes to QE on how to test this fix:

  1. Configure an appliance for authentication mode of "Amazon"
  2. Configure an valid group on the appliance for a valid Amazon user.
  3. Attempt to login with valid user credentials from the Amazon account.

@jvlcek
Copy link
Member Author

jvlcek commented Sep 8, 2017

@gtanzillo and @abellotti please review

@jvlcek
Copy link
Member Author

jvlcek commented Sep 8, 2017

@miq-bot add_labels authentication, bug

@miq-bot
Copy link
Member

miq-bot commented Sep 8, 2017

@jvlcek Cannot apply the following label because they are not recognized: authentication

@miq-bot miq-bot added the bug label Sep 8, 2017
@abellotti
Copy link
Member

Looks good @jvlcek, maybe just the .freeze to keep rubocop happy. Thanks !!

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me

@jvlcek
Copy link
Member Author

jvlcek commented Sep 8, 2017

Good point @abellotti. done.freeze :)

@jvlcek
Copy link
Member Author

jvlcek commented Sep 8, 2017

@gtanzillo and @abellotti Thank you. I'll squash the commits.

@miq-bot
Copy link
Member

miq-bot commented Sep 8, 2017

Checked commit jvlcek@3a2408b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@jvlcek
Copy link
Member Author

jvlcek commented Sep 8, 2017

@miq-bot add_labels fine/yes

@jvlcek
Copy link
Member Author

jvlcek commented Sep 8, 2017

@gtanzillo Can you please merge this?

@@ -141,5 +141,9 @@ def aws_connect(access_key_id, secret_access_key, service = :IAM, proxy_uri = ni
:log_formatter => Aws::Log::Formatter.new(Aws::Log::Formatter.default.pattern.chomp)
)
end

def normalize_username(username)
username
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems really strange. I've looked at the base code and I understand why this is here.

It's just strange that the subclass has the "reset" the default behavior (i.e., doesn't downcase the username) because the base implementation changes the default behavior (i.e., downcases the username).

Oh well, just something to keep in mind if this comes back to bite.

@blomquisg blomquisg merged commit ffa0832 into ManageIQ:master Sep 8, 2017
jvlcek added a commit to jvlcek/manageiq that referenced this pull request Sep 8, 2017
(manually picked from ManageIQ/manageiq-providers-amazon@3a2408b)

The code has been refactored on the master branch. It has been moved to
the https://github.com/ManageIQ/manageiq-providers-amazon repository.

The master PR for this change is:
ManageIQ/manageiq-providers-amazon#296

This commit addresses:
https://bugzilla.redhat.com/show_bug.cgi?id=1489596
@simaishi
Copy link
Contributor

Backported to Fine via ManageIQ/manageiq#15959

@bronaghs bronaghs added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 19, 2017
d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
(manually picked from ManageIQ/manageiq-providers-amazon@3a2408b)

The code has been refactored on the master branch. It has been moved to
the https://github.com/ManageIQ/manageiq-providers-amazon repository.

The master PR for this change is:
ManageIQ/manageiq-providers-amazon#296

This commit addresses:
https://bugzilla.redhat.com/show_bug.cgi?id=1489596
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.

7 participants