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

fix condition in update password #187

Closed
wants to merge 1 commit into from

Conversation

nrukavkov
Copy link

@nrukavkov nrukavkov commented Dec 13, 2024

SUMMARY

I've found a bug when update_password=always via api

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

rabbitmq_user

ADDITIONAL INFORMATION

There is incorrect condition. rabbitmq_user.check_password() returns true if everything is fine. But change password will be run only when rabbitmq_user.check_password() return false

@nrukavkov
Copy link
Author

@csmart look at here please

@csmart
Copy link
Collaborator

csmart commented Dec 27, 2024

@nrukavkov thanks for the report and patch! According to the docs, update_password: always will only change the password on an existing user if the one specified in the task differs to the one already set, so unless I'm missing something, I think the current behaviour might be correct?

            elif update_password == 'always':
                if not rabbitmq_user.check_password(): # if the passwords don't match
                    rabbitmq_user.change_password() # then change the password
                    result['changed'] = True

Basically, rabbitmq_user.check_password() returns True if the current user can authenticate with the current password, in which case there's no need to change the password. If the passwords do not match, then rabbitmq_user.check_password() will return False and the password will be changed.

I guess always might not necessarily be the best value name to use here, but it's not testing whether ansible can set passwords for users (that's done when users are created), but rather it's testing that the user's password will always be that value.

If I've missed something please let me know! 😄 Thanks

@nrukavkov nrukavkov closed this by deleting the head repository Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants