Admin - Changing display of login expiry notices

With the introduced config options "PA-DSS Admin Session Timeout Enforced?" and "PA-DSS Strong Password Rules Enforced?" the login expiry notices in admin section is kind of confusing, when Non-Compliant is chosen.

Not sure though, that this is the best way to solve it, so feel free to comment and propose a better way/coding.

What it does:
- Displays the notices as today if in compliant mode. E.g. logout after 15 min etc.
- If "PA-DSS Admin Session Timeout Enforced? = Non-Compliant" it displays the same text, but with chosen session time (Admin Session Time Out in Seconds rounded to minutes).
- If "PA-DSS Strong Password Rules Enforced? = Non-Compliant" the notice for password change is hidden.

One ting I'm unsure about is what's happening if "PA-DSS Admin Session Timeout Enforced? = Yes", will the setting for "Admin Session Time Out in Seconds" still apply if it's set to 15 minutes or less, or is it 15 minutes anyway? Since it's now a "text field", and not a radio list.
<a href='#crh-start'></a><a href='#crh-data-%7B%22processed%22%3A%20%5B%22https%3A//github.com/zencart/zc-v1-series/pull/128%23issuecomment-52352885%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23issuecomment-52387219%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23issuecomment-52388677%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23discussion_r16324096%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23discussion_r16324132%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23issuecomment-52390149%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23discussion_r16324360%22%2C%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23issuecomment-52390555%22%5D%2C%20%22comments%22%3A%20%7B%22General%20Comment%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23issuecomment-52352885%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Nice%20clarification%21%20%3A%2B1%3A%20%22%2C%20%22created_at%22%3A%20%222014-08-15T20%3A18%3A45Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/1590605%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/texdc%22%7D%7D%2C%20%7B%22body%22%3A%20%22Screen%20captures%20from%20testing%20...%5Cr%5Cn%5Cr%5CnCompliant%20mode%3A%5Cr%5Cn%21%5Bzc_admin_login_compliant%5D%28https%3A//cloud.githubusercontent.com/assets/858132/3941273/3c5cd622-251d-11e4-9c60-1835def9ba62.png%29%5Cr%5CnNon-Compliant%20mode%3A%5Cr%5Cn%21%5Bzc_admin_login_non-compliant%5D%28https%3A//cloud.githubusercontent.com/assets/858132/3941274/3c5d7a50-251d-11e4-8297-d896e259341a.png%29%22%2C%20%22created_at%22%3A%20%222014-08-16T08%3A16%3A14Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/858132%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/syntaxerror%22%7D%7D%2C%20%7B%22body%22%3A%20%22After%20more%20testing%20it%20seems%20like%20if%20%5C%22PA-DSS%20Admin%20Session%20Timeout%20Enforced%3F%20%3D%20On%5C%22%2015%20minutes%20will%20be%20the%20maximum%2C%20whatever%20is%20defined%20in%20%5C%22Admin%20Session%20Time%20Out%20in%20Seconds%5C%22%2C%20if%20it%27s%20higher.%5Cr%5CnIf%20defined%20value%20is%20same%20or%20lower%20than%20900%20it%20will%20be%20used.%5Cr%5Cn%5Cr%5CnEg.%20if%20%5C%22PA-DSS%20Admin%20Session%20Timeout%20Enforced%3F%20%3D%20On%5C%22%20and%20%5C%22Admin%20Session%20Time%20Out%20in%20Seconds%20%3D%5C%22%5Cr%5Cn-%201800%20it%20will%20%5C%22fallback%5C%22%20to%2C%20and%20timeout%20after%2C%20900%20%2815%20min%29%5Cr%5Cn-%20300%20it%20will%20timeout%20after%205%20minutes%5Cr%5Cn%5Cr%5CnRight%3F%20Then%20we%20need%20to%20change%20a%20bit%20in%20the%20code%2C%20into%20something%20like%3A%5Cr%5Cn%5Cr%5Cn%20%20%20%20%3Cdiv%20id%3D%5C%22loginExpiryPolicy%5C%22%3E%5Cr%5Cn%20%20%20%20%20%20%3C%3Fphp%20if%20%28PADSS_ADMIN_SESSION_TIMEOUT_ENFORCED%20%3D%3D%201%20%26%26%20SESSION_TIMEOUT_ADMIN%20%3E%20900%29%20%7B%5Cr%5Cn%20%20%20%20%20%20%20%20%24sessTimeAdm%20%3D%2015%3B%5Cr%5Cn%20%20%20%20%20%20%7D%20else%20%7B%5Cr%5Cn%20%20%20%20%20%20%20%20%24sessTimeAdm%20%3D%20round%28SESSION_TIMEOUT_ADMIN/60%29%3B%5Cr%5Cn%20%20%20%20%20%20%7D%5Cr%5Cn%20%20%20%20%20%20%20%20echo%20LOGIN_EXPIRY_NOTICE_PART1%20.%20%27%20%27%20.%20%24sessTimeAdm%20.%20%27%20%27%20.%20LOGIN_EXPIRY_NOTICE_PART2%3B%20%20%5Cr%5Cn%20%20%20%20%20%20%3F%3E%5Cr%5Cn%20%20%20%20%3C/div%3E%22%2C%20%22created_at%22%3A%20%222014-08-16T09%3A34%3A22Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/858132%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/syntaxerror%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%20like%20the%20clarification%20this%20change%20would%20provide.%20Thank%20you%20for%20submitting%21%5Cr%5Cn%5Cr%5CnProbably%20a%20non%20issue%2C%20but%20while%20the%20login%20page%20is%20being%20updated%20/%20discussed...%20Should%20the%20login%20page%20advertise%20password%20policies%20to%20un-authenticated%20visitors%3F%20Or%20might%20it%20be%20better%20to%20display%20the%20information%20in%20an%20admin%20dashboard%20widget%3F%22%2C%20%22created_at%22%3A%20%222014-08-16T11%3A00%3A09Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/2963958%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/lhungil%22%7D%7D%2C%20%7B%22body%22%3A%20%22I%20agree%2C%20it%27s%20worth%20considering%20if%20this%20kind%20of%20info%20should%20be%20displayed%20for%20un-authenticated%20visitors%20even%20if%20it%27s%20a%20small%20issue.%20But%20instead%20of%20a%20widget%20it%27s%20possibly%20better%20to%20just%20display%20it%20as%20a%20notice%20at%20top%20of%20the%20screen%20/%20in%20header%3F%22%2C%20%22created_at%22%3A%20%222014-08-16T11%3A23%3A58Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/858132%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/syntaxerror%22%7D%7D%5D%2C%20%22title%22%3A%20%22General%20Comment%22%7D%2C%20%22Pull%2036e4bdd8a57140a101789e7705aa5fdd4a065187%20admin/includes/languages/english/login.php%2014%22%3A%20%7B%22html_url%22%3A%20%22https%3A//github.com/zencart/zc-v1-series/pull/128%23discussion_r16324096%22%2C%20%22comments%22%3A%20%5B%7B%22body%22%3A%20%22Thoughts%20on%20using%20sprintf%20instead%20of%20splitting%20this%20define%20into%20two%3F%5Cr%5Cn%5Cr%5CnI%27m%20thinking%20mostly%20of%20cases%20where%20people%20may%20be%20using%20%28outdated%29%20language%20packs...%20Keeping%20the%20same%20constant%20name%20may%20facilitate%20an%20easier%20upgrade...%22%2C%20%22created_at%22%3A%20%222014-08-16T10%3A28%3A10Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/2963958%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/lhungil%22%7D%7D%2C%20%7B%22body%22%3A%20%22Thoughts%20off%20yes%2C%20but%20haven%27t%20seen%20it%20in%20use%20in%20Zen%20Cart%2C%20so%20I%20dropped%20that%20thought.%20Anyhow%20that%20language%20string%20would%20be%20wrong%20anyway%20in%20old%20pack%2C%20by%20changing%20it%20it%20will%20be%20easier%20to%20spot%20while%20testing%20the%20new%20language%20pack%20-%20thus%20forcing%20an%20update%20of%20it.%22%2C%20%22created_at%22%3A%20%222014-08-16T10%3A40%3A53Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/858132%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/syntaxerror%22%7D%7D%2C%20%7B%22body%22%3A%20%22You%27re%20right.%20I%20thought%20about%20it%2C%20but%20could%20not%20remember%20it%20being%20used%20in%20ZC.%20But%20I%20was%20wrong%2C%20to%20long%20since%20I%20translated%20something%20with%20it%20I%20guess.%20Found%20quite%20many%20when%20searching%20now.%20%3A%29%5Cr%5Cn%5Cr%5CnFor%20the%20translation%20part%20it%27s%20nice%20to%20keep%20it%20clean%2C%20so%20I%20make%20the%20change.%20But%20the%20translators%20will%20need%20to%20add%20this%20update%20anyway%2C%20since%20some%20of%20that%20define%20is%20split%20into%20the%20new%20LOGIN_PASSWORD_EXPIRY_NOTICE%20define.%22%2C%20%22created_at%22%3A%20%222014-08-16T11%3A18%3A03Z%22%2C%20%22user%22%3A%20%7B%22avatar_url%22%3A%20%22https%3A//avatars.githubusercontent.com/u/858132%3Fv%3D2%22%2C%20%22html_url%22%3A%20%22https%3A//github.com/syntaxerror%22%7D%7D%5D%2C%20%22title%22%3A%20%22File%3A%20admin/includes/languages/english/login.php%3AL21-30%22%7D%7D%7D'></a>
<a href='https://www.codereviewhub.com/'><img src='http://www.codereviewhub.com/site/github-bar.png' height=40></a>
- [ ] <a href='#crh-comment-General Comment'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/128#issuecomment-52352885'>General Comment</a></b>
- <a href='https://github.com/texdc'><img border=0 src='https://avatars.githubusercontent.com/u/1590605?v=2' height=16 width=16'></a> Nice clarification! :+1:
- <a href='https://github.com/syntaxerror'><img border=0 src='https://avatars.githubusercontent.com/u/858132?v=2' height=16 width=16'></a> Screen captures from testing ...
Compliant mode:
![zc_admin_login_compliant](https://cloud.githubusercontent.com/assets/858132/3941273/3c5cd622-251d-11e4-9c60-1835def9ba62.png)
Non-Compliant mode:
![zc_admin_login_non-compliant](https://cloud.githubusercontent.com/assets/858132/3941274/3c5d7a50-251d-11e4-8297-d896e259341a.png)
- <a href='https://github.com/syntaxerror'><img border=0 src='https://avatars.githubusercontent.com/u/858132?v=2' height=16 width=16'></a> After more testing it seems like if "PA-DSS Admin Session Timeout Enforced? = On" 15 minutes will be the maximum, whatever is defined in "Admin Session Time Out in Seconds", if it's higher.
If defined value is same or lower than 900 it will be used.
Eg. if "PA-DSS Admin Session Timeout Enforced? = On" and "Admin Session Time Out in Seconds ="
- 1800 it will "fallback" to, and timeout after, 900 (15 min)
- 300 it will timeout after 5 minutes
Right? Then we need to change a bit in the code, into something like:
<div id="loginExpiryPolicy">
<?php if (PADSS_ADMIN_SESSION_TIMEOUT_ENFORCED == 1 && SESSION_TIMEOUT_ADMIN > 900) {
$sessTimeAdm = 15;
} else {
$sessTimeAdm = round(SESSION_TIMEOUT_ADMIN/60);
}
echo LOGIN_EXPIRY_NOTICE_PART1 . ' ' . $sessTimeAdm . ' ' . LOGIN_EXPIRY_NOTICE_PART2;
?>
</div>
- <a href='https://github.com/lhungil'><img border=0 src='https://avatars.githubusercontent.com/u/2963958?v=2' height=16 width=16'></a> I like the clarification this change would provide. Thank you for submitting!
Probably a non issue, but while the login page is being updated / discussed... Should the login page advertise password policies to un-authenticated visitors? Or might it be better to display the information in an admin dashboard widget?
- <a href='https://github.com/syntaxerror'><img border=0 src='https://avatars.githubusercontent.com/u/858132?v=2' height=16 width=16'></a> I agree, it's worth considering if this kind of info should be displayed for un-authenticated visitors even if it's a small issue. But instead of a widget it's possibly better to just display it as a notice at top of the screen / in header?
- [ ] <a href='#crh-comment-Pull 36e4bdd8a57140a101789e7705aa5fdd4a065187 admin/includes/languages/english/login.php 14'></a> <img src='http://www.codereviewhub.com/site/github-remaining.png' height=16 width=60>&nbsp;<b><a href='https://github.com/zencart/zc-v1-series/pull/128#discussion_r16324096'>File: admin/includes/languages/english/login.php:L21-30</a></b>
- <a href='https://github.com/lhungil'><img border=0 src='https://avatars.githubusercontent.com/u/2963958?v=2' height=16 width=16'></a> Thoughts on using sprintf instead of splitting this define into two?
I'm thinking mostly of cases where people may be using (outdated) language packs... Keeping the same constant name may facilitate an easier upgrade...
- <a href='https://github.com/syntaxerror'><img border=0 src='https://avatars.githubusercontent.com/u/858132?v=2' height=16 width=16'></a> Thoughts off yes, but haven't seen it in use in Zen Cart, so I dropped that thought. Anyhow that language string would be wrong anyway in old pack, by changing it it will be easier to spot while testing the new language pack - thus forcing an update of it.
- <a href='https://github.com/syntaxerror'><img border=0 src='https://avatars.githubusercontent.com/u/858132?v=2' height=16 width=16'></a> You're right. I thought about it, but could not remember it being used in ZC. But I was wrong, to long since I translated something with it I guess. Found quite many when searching now. :)
For the translation part it's nice to keep it clean, so I make the change. But the translators will need to add this update anyway, since some of that define is split into the new LOGIN_PASSWORD_EXPIRY_NOTICE define.


<a href='https://www.codereviewhub.com/zencart/zc-v1-series/pull/128?mark_as_completed=1'><img src='http://www.codereviewhub.com/site/github-mark-as-completed.png' height=26></a>&nbsp;<a href='https://www.codereviewhub.com/zencart/zc-v1-series/pull/128?approve=1'><img src='http://www.codereviewhub.com/site/github-approve.png' height=26></a>&nbsp;<a href='https://github.com/zencart/zc-v1-series/pull/128'><img src='http://www.codereviewhub.com/site/github-refresh.png' height=26></a>
<a href='#crh-end'></a>
True. :)