[#8951] - [login forms] performance improvements in two factor authentication
- Fixed in Code Base
- 13 Aug 2016
- Medium
- Build: staging
- # 8951
- Diff
- andrepereiradasilva:twofactor-performance
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
Description
This PR improves performance and memory usage of the admin login page, the offline page, all the pages that have a login module in the frontend and the component users view login form.
Joomla is loading fof just to check if any two factor auth plugins are enabled to show the "Secret key" field. IMHO we can use Joomla (already loaded) libraries for that.
This way we can have performance improvements(around 4-6ms PHP 5.6.16) in every page that use this login modules.
Performance before PR (frontend mod_login as example)
Performance after PR (frontend mod_login as example)
How to test
- Install Joomla with latest staging with sample data
- Enable Google Authenticator (or Yubikey) two factor authentication
- Apply patch
- Create a dummy user and configure it to use with Google Authenticator (if you don't have use a chrome plugin for that, for instance https://chrome.google.com/webstore/detail/authenticator/bhghoamapcdpbohphigoooaddinpkbai)
- Check Two auth is working:
- Go to frontend and try to login with the new user without the two auth secret key. You can't
- Go to frontend and try to login with the new user with the two auth secret key. You can
Note: for test the performance go to any (or all this files):
- /modules/mod_login/mod_login.php
- /administrator/modules/mod_login/mod_login.php
- /templates/system/offline.php
- /components/com_users/views/login/view.html.php
And put a mark in the debug profiler before and after the call to get the two factor methods.
Example: /modules/mod_login/mod_login.php
JDEBUG ? JProfiler::getInstance('Application')->mark('- start getting twofactorauth methods') : null;
$twofactormethods = [....];
JDEBUG ? JProfiler::getInstance('Application')->mark('- <strong>Time to get</strong> twofactorauth methods') : null;Observations
Suggestions, improvements and code reviews are welcome.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Title |
|
||||||
Restarted Travis. FWIW it's not FOF that's the problem. You can remove the FOF code by replacing those two lines with the Joomla specific versions:
JPluginHelper::importPlugin('twofactorauth');
$identities = JEventDispatcher::getInstance()->trigger('onUserTwofactorIdentify', array());And I went from the method taking 84ms to 82ms :P (for the admin login page). Therefore I assume the time is actually gained by not running the plugin event (and it's a long know but impossible to fix without screwing b/c fact) that Joomla's plugin system is slow as hell. Therefore the question to me is that is it b/c doing what you are doing. Other 3rd party two factor plugins may have more logic in their onUserTwofactorIdentify methods than we do and now they won't be run in the future :/
Just as a sidenote we actually load FOF on every page :P https://github.com/joomla/joomla-cms/blob/staging/libraries/cms.php#L42-L46
| Category | ⇒ | Authentication |
@wilsonge didn't notice fof was loaded every time on every page. thanks for enlightening me. Don't know yet Joomla code so well. Is there a reason for fof being loaded in everypage and not only when needed?
And I went from the method taking 84ms to 82ms :P (for the admin login page). Therefore I assume the time is actually gained by not running the plugin event (and it's a long know but impossible to fix without screwing b/c fact) that Joomla's plugin system is slow as hell.
Yeah, you're right again.
I think it's triggered in https://github.com/joomla/joomla-cms/blob/staging/libraries/fof/integration/joomla/platform.php#L537-L558
I forgot about that...
Therefore the question to me is that is it b/c doing what you are doing. Other 3rd party two factor plugins may have more logic in their onUserTwofactorIdentify methods than we do and now they won't be run in the future :/
Yes, it seems it's only used in joomla plugins to get the section in the two auth plugins in joomla.
See https://github.com/joomla/joomla-cms/blob/staging/plugins/twofactorauth/totp/totp.php#L64-L97 and https://github.com/joomla/joomla-cms/blob/staging/plugins/twofactorauth/yubikey/yubikey.php#L64-L96
It that i mind i will see it after the changes needed this PR still makes sense later.
That file is just loading FOF's autoloader, not including the whole library into memory.
@mbabker, ok. thanks for explaining.
@wilsonge, after several testing i revised the PR to use the onUserTwofactorIdentify trigger, but with joomla.
We still get a performance/memory usage improvement in making the same thing so i think this PR still makes sense and this way, i think, there are no B/C problems.
Did some tests with PHP 5.6.16, without Joomla caching, with PHP OpCache disabled and without open_basedir.
Also test where made using the profiler before and after the xxxxxxxxxxx::getTwoFactorMethods(); call.
JDEBUG ? JProfiler::getInstance('Application')->mark('- START') : null;
$twofactormethods = xxxxxxxxxxxxxx::getTwoFactorMethods();
JDEBUG ? JProfiler::getInstance('Application')->mark('- <strong>DONE</strong>') : null;Before PR
| Page | Two Auth plugins enabled | Time Taken | Memory used |
|---|---|---|---|
| Frontend Mod login | 0 | ~5.5 ms | 0.605 MB |
| Frontend Mod login | 1 | ~6.6 ms | 0.659 MB |
| Frontend Mod login | 2 | ~7.4 ms | 0.714 MB |
After PR
| Page | Two Auth plugins enabled | Time Taken | Memory used |
|---|---|---|---|
| Frontend Mod login | 0 | ~1.4 ms | 0.125 MB |
| Frontend Mod login | 1 | ~2.4 ms | 0.179 MB |
| Frontend Mod login | 2 | ~3.4 ms | 0.235 MB |
So at least 4 ms and 0.5MB difference. The same could apply to all the other pages that have the login form (admin login, offline, com_users login).
@andrepereiradasilva Could you resolve the conflicts here please?
| Category | Authentication | ⇒ | Administration Modules Front End Components Libraries Templates (site) Authentication |
conflicts fixed. let's hope this additional work will make someone test this ...
@andrepereiradasilva I am going to get it tested tomorrow ;) Thanks.
Me too
thanks guys
Tested with patch and could login ok in all 4 places using the yubikey
Tested performance of the front-end login module with the supplied instructions
Before
21.05 ms / 573.97 ms Memory: 0.573 MB / 11.59 MB Application: - Time to get twofactorauth methods
After
Time: 9.10 ms / 551.29 ms Memory: 0.161 MB / 11.17 MB Application: - Time to get twofactorauth methods
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8951.
I have tested this item
tested @icampus:
- Two Factor Authentication - Google Authenticator (On Google Chrome,local server)
Before PR
Time to get twofactorauth methods : 0.18 ms
After PR:
Time to get twofactor methods: 0.15 ms
Although there is a small difference (probably because of my machine) there is a difference
I tested it several times
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8951.
| Status | Pending | ⇒ | Ready to Commit |
RTC as we have 2 successful tests
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8951.
| Labels |
Added:
?
|
||
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-08-13 17:02:39 |
| Closed_By | ⇒ | rdeutz |
| Labels |
Removed:
?
|
||
travis seems to be confused...