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
Fixes #20315 - Sessions mutually exclusive with basic auth #320
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one thing that I encountered during testing and seemed unexepected:
Having sessions enabled and username + password (or only username set), doing hammer auth login
always uses that account from config file. I'd expect to be able to specify user for new session. Would that make sense?
Another slightly confusing thing is when I have admin/changeme set in config file and I do
hammer -u ares user list
I got Invalid username or password
or successful login, since it uses the password from config file. Would it be possible to ask for password if user changed?
Otherwise it fixes the problem reported in the issue as well as destroying valid sessions on wrong credentials 👍
end | ||
|
||
def user_changed? | ||
@user_changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usually !!@user_changed
is used to ensure the result of method with ?
at the end returns boolean
@ares the issues you're mentioning should actually be the main thing this PR is supposed to fix. I couldn't reproduce them. Can you double-check you checked out correct branch and if yes, provide your config files? |
5d66e94
to
6caa032
Compare
Rebased |
thanks for helping to debug, the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
someone with deeper hammer knowledge should read the code whether it fits well, but except for the !!
nit it looks good to me
6caa032
to
e7725e0
Compare
e7725e0
to
4fe5d6e
Compare
@ares I added a comment to the default config file and fixed using incorrect password when sessions are off and a different username is passed. |
👍 works for me |
It fixes http://projects.theforeman.org/issues/20131 too.