leepeuker / movary

Self hosted web app to track and rate your watched movies
MIT License
391 stars 14 forks source link

Add two-factor authentication #473

Closed JVT038 closed 1 year ago

JVT038 commented 1 year ago

This PR adds 2FA and makes some changes surrounding the password and the login flow.

The password page is renamed to 'security' and so are all the routes related to the former password page.

To enable 2FA, the user can go to the security tab and they can click on a button which opens a modal. When clicking on the button, a post request will be sent to generate a TOTP URI and JS will process the URI to encode it in a QR code, which is shown in the modal. Additionally, the TOTP secret is also just shown in plain text below the QR code, if the user wants to manually copy the secret. (Useful for password managers such as Bitwarden).

The user has to import the TOTP URI into their verification app and enter the correct 6-digit code to enable 2FA.

The user must now first enter their email + password and click on the sign in button. (like usual)
If the user has a filled totp_uri column in the database, Movary will assume they have enabled 2FA. The user will now be redirected back to the login page, but now with the 2FA form instead. They will enter 6 digits and Movary will check if it's correct.

If it is correct, they will be authenticated. Additionally, if they have ticked the 'Remember me' box on the 2FA page, another cookie will be created to ensure they won't have to enter a 2FA code for the next 30 days. Question: Should this cookie be removed upon logout? It's currently not removed when the user logs out.

When the user has entered the code incorrectly, they will be redirected back to the 2FA page and see an error message, telling them it was wrong.

JVT038 commented 1 year ago

I've also been considering to make a CookieWrapper class that basically centralizes all the cookie-related methods (such as calculating the expiry date) into one class, because it's currently a bit of a mess with cookies IMO.

leepeuker commented 1 year ago

I have started looking into the PR a bit, I noticed that I cannot scan the QR code, I am using Aeagis on Android. Entering the code manually works.

JVT038 commented 1 year ago

Huh? I tried it (I'm also using Aegis on Android) and it works on my side.

Could you check the PHP logs and the browser console logs for any errors? Also, have you installed the new composer packages and ran the migrations?

Does the QR code appear at all? If so, could you send a screenshot of the QR code and could you run the following command in the browser console: document.getElementById('qrcode').getAttribute('title')? I want to know if the generated QR code is valid and what the contents of the QR code are.

The QR code should contain something like this: otpauth://totp/Movary%3AJVT038?issuer=Movary&secret=<mysecret>

leepeuker commented 1 year ago

The QR code is displayed, but when scanning it with Aegis simply nothing happens, as if Aegis does not recognize the QR code.

image

the title attribute value is: otpauth://totp/Movary%3Alee?issuer=Movary&secret=XAS2M34SVRCOQMTOPNNA2FOHEIHCKKRUU4B3OI4JUCC4INKXTEYA

JVT038 commented 1 year ago

The issue was that Aegis was unable to detect the QR code, because of the dark theme. I have added a 1px solid white border to the QR code div and it's barely noticeable in the dark theme, but noticeable enough so that Aegis can scan the code.