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

Fixes #4443 - Fix reporting for user without password on userManagementv... #444

Closed
wants to merge 1 commit into from
Closed

Fixes #4443 - Fix reporting for user without password on userManagementv... #444

wants to merge 1 commit into from

Conversation

nperron
Copy link
Contributor

@nperron nperron commented Jul 17, 2014

Fixes #4443 - Fix reporting for user without password on userManagementv2.0

cf http://www.rudder-project.org/redmine/issues/4443

@nperron
Copy link
Contributor Author

nperron commented Jul 17, 2014

The ifvarclass become difficult to read...
To understand the change, i've used another way of display.
Before:

(
  (!usermanagement_user_password_ok_${usergroup_user_index}.!usermanagement_user_password_repaired_${usergroup_user_index}.!usermanagement_user_password_failed_${usergroup_user_index})
  .usermanagement_user_pwoneshot_${usergroup_user_index}.usermanagement_user_exists_${usergroup_user_index}
)
|usermanagement_user_remove_${usergroup_user_index}

After:

(
  (!usermanagement_user_password_ok_${usergroup_user_index}.!usermanagement_user_password_repaired_${usergroup_user_index}.!usermanagement_user_password_failed_${usergroup_user_index})
  .(usermanagement_user_pwoneshot_${usergroup_user_index}.usermanagement_user_exists_${usergroup_user_index})
  |usermanagement_user_pwempty_${usergroup_user_index}|
  (usermanagement_user_update_${usergroup_user_index}.!usermanagement_user_exists_${usergroup_user_index})
)
|usermanagement_user_remove_${usergroup_user_index};

For empty password I've added usermanagement_user_pwempty_${usergroup_user_index}
For user who can't be updated since they don't exist, I've added (usermanagement_user_update_${usergroup_user_index}.!usermanagement_user_exists_${usergroup_user_index})

This is based from the PR #441

@peckpeck
Copy link
Member

I've check the logic of the ifvarclass, it matches your comment.
However I 'm not sure we should issue a success report if we can't update a user that don't exist.

@jooooooon
Copy link
Member

I'm not sure about the full logic (I didn't re-read the full technique, sorry) but the report that is changed here says "The user password change is not required".

I think this can in fact be two different cases:

  1. The password should be set but it is already OK - that is the definition of "success"
  2. We don't need to check the password for some reason (it's not configured, the user account doesn't exist, etc...) - that is the definition of the "result_na" report type.

Maybe we should be implementing two cases here?

@ncharles
Copy link
Member

Caution, result_na appeared only in Rudder 2.10 !

@peckpeck
Copy link
Member

Ticket already rejected

@peckpeck peckpeck closed this Jun 13, 2017
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.

4 participants