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

occ user:expire-password not working, when expiry is not explicitly enabled #66

Closed
patrickjahns opened this issue Jul 17, 2018 · 16 comments
Assignees
Labels
bug Something isn't working p3-medium
Milestone

Comments

@patrickjahns
Copy link
Contributor

During smoke testing the release tarball, I noticed a confusion.

After installing and enabling password_policy app - the occ command occ user:expire-password is available.
Withou further configuration, I have used the occ user:expire-password user1 - but confusingly user1 was still able to login.

Only when checking the top expiry box and using the occ command - the user was forced to change his password on login

image

@PVince81 PVince81 added the bug Something isn't working label Jul 17, 2018
@PVince81 PVince81 added this to the backlog milestone Jul 17, 2018
@PVince81
Copy link
Contributor

@pmaier1 something to mention in docs or release notes ? known issue ?

@PVince81
Copy link
Contributor

also to discuss: what is the expected behavior ?

  1. occ command fails "password expiration is not enabled": easy fix
    or
  2. expiration works anyway: more involved fix as we need to make expiration system work despite expiration being disabled

@pmaier1
Copy link
Contributor

pmaier1 commented Jul 18, 2018

  1. does not make much sense as the sysadmin could enable it via occ app:config and execute the occ command again, right?

The separation between oC admin and sysadmin is not perfect in this case but to avoid confusion I would go for 2.

@PVince81
Copy link
Contributor

Estimate: 0.5-3md depending on what we find

@PVince81 PVince81 modified the milestones: backlog, development Jul 20, 2018
@PVince81 PVince81 self-assigned this Jul 23, 2018
@PVince81
Copy link
Contributor

Assigning self to think about tech solution

@PVince81 PVince81 modified the milestones: development, backlog Aug 7, 2018
@PVince81 PVince81 removed their assignment Aug 7, 2018
@pmaier1
Copy link
Contributor

pmaier1 commented Aug 20, 2018

I still prefer 2. in #66 (comment). Still if it is more effort than 0,5 days, then go for 1.. The sysadmin can still activate it using occ app:config.

@PVince81 PVince81 self-assigned this Aug 20, 2018
@PVince81 PVince81 modified the milestones: backlog, development Aug 22, 2018
@PVince81
Copy link
Contributor

Looking into the code it looks like having password expiration disabled would be the equivalent of having a rule "every password must expire after 0 days" but would need to limit said rule to only virtual password entries created by the admin through the command.

It becomes more complicated if the admin runs the occ command not with the default date (expire now) but expiration in the future.

I'm not too keen on adding this additional layer of complexity in the already complex code paths we have now unless there is a big benefit from it.

Also note that with the approach with "0 days", if the admin decides to tick the checkbox "days until user password expired" the value of 0 days suddenly becomes 90 days (or whatever is in the field), which is equivalent to changing the "days" value, which, according to the docs, requires the admin to run the occ command again for all users.

@pmaier1 did you hope to have this feature mostly for immediate expiry or would future expiry still be interesting despite not having any rule configured ?

@PVince81
Copy link
Contributor

PVince81 commented Aug 23, 2018

A middle-ground approach would be to allow only immediate expiration (not deferred expiration) when no expiration rules were set. This would be easier as we could directly set the "password has expired" flag for said users and would need us to send the notification right away.

@PVince81
Copy link
Contributor

Ok, I decide to go the easy route for now: disable the command completely if no expiration rule was configured: #115

@pmaier1
Copy link
Contributor

pmaier1 commented Aug 28, 2018

@pmaier1 did you hope to have this feature mostly for immediate expiry or would future expiry still be interesting despite not having any rule configured ?

Actually I see this command as a (cumbersome) way to get the policy in place. Restricting to immediate expiry implies that, to put a rule in place, all users have to change their password immediately?

Ok, I decide to go the easy route for now: disable the command completely if no expiration rule was configured:

So you first need to enable it via webUI or appconfig, but then you can set a date for the initial expiry?

@PVince81
Copy link
Contributor

@pmaier1 you need to first enable it via web UI or occ app:config, yes. Then the occ command will accept to work.

@PVince81
Copy link
Contributor

and you should then use the command to set initial expiry, yes, as per release notes from the previous version.

@pmaier1
Copy link
Contributor

pmaier1 commented Aug 28, 2018

@pmaier1 you need to first enable it via web UI or occ app:config, yes. Then the occ command will accept to work.

Great. And expiration via occ user:expire-password is not limited to "immediate" with the solution you chose, right?

@PVince81
Copy link
Contributor

In my PR occ user:expire-password is completely disabled, neither immediate nor deferred expiry can be used if nothing was configured. It's the simplest approach.

@pmaier1
Copy link
Contributor

pmaier1 commented Aug 28, 2018

I mean when the policy is enabled and I want to set it up initially. Then I can specify a date for the first expiration. I am not limited to immediate expiration in this case, right?

@PVince81
Copy link
Contributor

PVince81 commented Aug 28, 2018

I mean when the policy is enabled and I want to set it up initially. Then I can specify a date for the first expiration. I am not limited to immediate expiration in this case, right?

yes, there is no limit to the occ command. it works as it used to in the previous version when expiration is configured

@PVince81 PVince81 modified the milestones: development, QA Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p3-medium
Projects
None yet
Development

No branches or pull requests

3 participants