-
Notifications
You must be signed in to change notification settings - Fork 6
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 allow passwords to be expired for non-local users #31
Conversation
f9e01b8
to
33538cc
Compare
Codecov Report
@@ Coverage Diff @@
## master #31 +/- ##
============================================
+ Coverage 64.37% 64.55% +0.18%
- Complexity 124 125 +1
============================================
Files 22 22
Lines 581 584 +3
============================================
+ Hits 374 377 +3
Misses 207 207
Continue to review full report at Codecov.
|
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.
👍 see optional comment
lib/Command/ExpirePassword.php
Outdated
@@ -88,6 +90,11 @@ protected function execute(InputInterface $input, OutputInterface $output) { | |||
return 67; | |||
} | |||
|
|||
if (!$user->canChangePassword()) { | |||
$output->writeln("<error>Passwords cannot be expired for user: $uid</error>"); |
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.
should we add a reason: because the user is in a backend that doesn't support password change ?
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.
Why not.
I mentioned this sort of thing in #29 (comment) (item 2 at the bottom of the post). In core, it would be nice to make some more friendly messages when password change is attempted for these type of users.
33538cc
to
6372ab3
Compare
Optional comment addressed. "cannot be expired" message expanded. |
Fixes issue #30