born05 / craft-twofactorauthentication

Craft plugin for two-factor or two-step login using Time Based OTP.
MIT License
36 stars 26 forks source link

[BUG] can be bypassed by simply reclicking the admin link in plugin Two-Factor Authentication #67

Closed oxochenxa closed 1 year ago

oxochenxa commented 1 year ago

Hi Born05, I would like to report a bug that i've notice while using plugin Two-Factor Authentication version 2.10.0. I used Craft CMS Pro 3.7.55.3, PHP 7.4. After login, website show the verify 2FA form. But i can be bypassed by simply reclicking the admin link.

I hope you can fix this bug and release it or send me your solutions. Have a good day.

Coultsy commented 1 year ago

Yes we have also noticed that you can just click on the 'back' button on your browser when on the 2fa screen and it will take you to your craft dashboard without enforcing the 2fa. Kind of an urgent fix this..

roelvanhintum commented 1 year ago

@oxochenxa i can't reproduce this bug. Do you have a custom two-factor-authentication.php config file? What kind of session store do you use? What are the session durations?

@Coultsy after clicking back, can you refresh the page successfully? Just to make sure you're not looking at some browser cached state.

oxochenxa commented 1 year ago

Hi @roelvanhintum i don’t custom config file. I think session store i use cookie and session durations is 1 hour.

roelvanhintum commented 1 year ago

@oxochenxa Usually problems like these have something to do with the session config. Can you check Craft's userSessionDuration, which should be more than 0. And php's session.gc_maxlifetime? Just to be sure, do you use a non-default session component (like redis)? See: https://craftcms.com/docs/3.x/config/#session-component

And which other plugins are you using?

oxochenxa commented 1 year ago

Hi @roelvanhintum, I check config: userSessionDuration = 3600, session.gc_maxlifetime = 1440. When i uninstall all plugin except 2FA plugin, that error still occurs. I checked the config no problem. Any orther case? Where is save session before input code 2FA. I think session save to system before input code 2FA

roelvanhintum commented 1 year ago

I can't reproduce this. As soon as i try to load any other page, i'm logged out.

Coultsy commented 1 year ago

Just a recap on our experience. This problem is on 1 particular implementation. Other recent installs of this plugin on other sites have not experienced this issue. Config is the same across all these sites but something on this 1 site is causing this problem. So I don't think it is a fundamental issue with the plugin - just some sort of incompatibility with another plugin (maybe). Once we figure it out we will let you know.

oxochenxa commented 1 year ago

Hi @roelvanhintum @Coultsy, the reason is probably because we are not use CP_TRIGGER load the control panel. I use BASE_CP_URL load the control panel because i use different domain load control panel. If you can reproduce this, please talk to me. Thanks!

oxochenxa commented 1 year ago

when i check file vendor/born05/craft-twofactorauthentication/src/services/Request.php. $user in line 24 null. Can't get Identity. Do you know reason.

oxochenxa commented 1 year ago

Hi @roelvanhintum @Coultsy , I found the bug in vendor/born05/craft-twofactorauthentication/src/services/Request.php line 24, $user = NULL because Plugin run function validateRequest with EVENT_AFTER_LOAD_PLUGINS. When run event EVENT_AFTER_LOAD_PLUGINS, user can't found identity. I fix it bug by replace in vendor/born05/craft-twofactorauthentication/src/Plugin.php: Event::on(Plugins::class, Plugins::EVENT_AFTER_LOAD_PLUGINS, function () { $this->request->validateRequest(); });

to: Event::on(Application::class, Application::EVENT_INIT, function (Event $event) { $this->request->validateRequest(); } );

Please help me consider, if you think this code working with your plugin. Please help me update code to your plugin. Thanks!

roelvanhintum commented 1 year ago

You're right, it should be Application::EVENT_INIT.

roelvanhintum commented 1 year ago

@oxochenxa this is released in 2.10.1

oxochenxa commented 1 year ago

Hi @roelvanhintum @Coultsy When update plugin, i usually logged out after 5s login admin. Can you help me check it?