Karen, William, Jan,
Note: my earlier comment to William that long error
strings broke sysconfig was incorrect. This was due
to faulty testing on my part. I can confirm that long
error strings are correctly handled.
Karen,
I agree with your suggestion below and have moved
the user p/w confirmation check to be within the
"if login_name:" clause.
For completeness, I've updated the webrev to show
the latest code, which I am now pushing (having got
Dave's approval).
https://cr.opensolaris.org/action/browse/caiman/dermot/7056210/
- Dermot
On 08/15/11 19:04, Karen Tung wrote:
Hi Dermot,
Please see my response inline.
On 08/15/11 10:42 AM, Dermot McCluskey wrote:
karen,
Thanks for reviewing. Responses below.
On 08/15/11 18:28, Karen Tung wrote:
Hi Dermot,
I have 1 question and 1 comment on the changes.
1) What's the advantage of moving the checks in lines 296-301
of the OLD file down to after passwords are validated?
IMO, those are very simple mistakes to catch, and we shouldn't
waste the cycles to go through with validating the password
if we will eventually fail anyway.
If I did not move those lines you could have the following:
- user enters an invalid p/w (eg too short)
- user mis-types the confirmation
- user presses F2 and is told "password doesn't match"
- user corrects the confirmation to match the p/w and
presses F2 again: they are now told "password is too short"
So, they were needlessly forced to re-type the invalid p/w.
I think the password validation is a higher priority check
and should be preformed first.
OK, this make sense. Thanks for the explanation.
2) Line 321: I think you can change that to an "else". The "if"
in line 312 already checks for "if login_name".
There's another "if" clause at 317 that prevents this.
(although I could move 316:318 down a few lines, if you
wish, and then make 321 a simple "else"?)
ah, I see. I completely miss the fact that there's
another "if" in line 317. I think my mind is thinking that 316-318
is part of the if statement in line 312. Can we put 316-318
in the if clause of the if statement that start in 312? After all,
that check is only needed if they have a login name.
Thanks,
--Karen
- Dermot
--Karen
On 08/15/11 05:51 AM, Dermot McCluskey wrote:
Hi all (esp. Jan, William, Karen),
I'd like 2 or more reviewers for this fix for sysconfig:
http://monaco.sfbay.sun.com/detail.jsf?cr=7056210
restrictions for setting password in sysconfig does not match
"passwd" command
Webrev:
https://cr.opensolaris.org/action/browse/caiman/dermot/7056210/
The fix involves calling the validate_password()
function (already in use by GUI Install) from the
Sysconfig Users screen for both the user and root
password fields. The validation code code is slightly
restructured to ensure "higher priority" validation
errors are reported first.
This fix has been tested by running the new
sysconfig code in a VM and confirming that
validation of the root and user password
fields behaves as expected.
Thanks,
- Dermot
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
