Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2FA auth flow does not utilize refresh tokens #298

Closed
slifty opened this issue Nov 3, 2023 · 2 comments · Fixed by #305
Closed

2FA auth flow does not utilize refresh tokens #298

slifty opened this issue Nov 3, 2023 · 2 comments · Fixed by #305
Assignees

Comments

@slifty
Copy link
Contributor

slifty commented Nov 3, 2023

It looks like #175 did not account for the processTwoFactorCodeResponse flow, which means if a user has 2FA enabled then they get an auth token with no refresh token.

We'll need to refactor to have both flows set up refresh tokens properly. This work may be done as part of #288, but I wanted to open the issue separately since they are technically separate issues.

@slifty slifty self-assigned this Nov 3, 2023
@slifty
Copy link
Contributor Author

slifty commented Nov 6, 2023

Looked into it more -- it appears that the final step of 2FA does not have a 202 response (that's the one that would signal the user isn't a client. I think we're going to need to make sure the user is registered for the application BEFORE begining 2FA flow (but after password).

This would mean a flow like the following:

  1. Attempt to login
  2. If the account has 2FA, then take the extra step of attempting to retrieve the user and confirm they are registered with the sftp application. If they are NOT, we would register before invoking the 2FA flow.
  3. invoke the 2FA flow.

One "downside" to this is that someone who knows the user's password could trigger that registration with the SFTP service. Now, I don't think that causes any actual harm (and indeed we were thinking about just automatically registering all users with SFTP as a potentially desired approach), but I wanted to raise it.


An alternative would be to simply reject any user with 2FA enabled, and avoid any complications at all. Given that the most likely long term solution to SFTP authentication is to avoid passwords and 2FA entirely / rely on SSH keys... that might not be a bad plan?

@slifty
Copy link
Contributor Author

slifty commented Nov 6, 2023

WAIT! I found the proper documentation: https://fusionauth.io/docs/apis/login#complete-multi-factor-authentication

And I do see a 202 response there 🎉

The question now is whether we can re-use the same MFA key to submit a second login request (e.g. "log in with MFA, get a 202, register the user, log in again with the same MFA).

slifty added a commit that referenced this issue Nov 7, 2023
When refactoring authentication to use refresh tokens we missed the 2FA
flow. It's not clear that we should be supporting 2FA to begin with
since rclone doesn't support it, and ultimately the real solution is to
use keys instead of passwords for sftp authentication.

That being said, while it's here we should make sure it isn't broken!

Issue #298 2FA auth flow does not utilize refresh tokens
slifty added a commit that referenced this issue Nov 7, 2023
When refactoring authentication to use refresh tokens we missed the 2FA
flow. It's not clear that we should be supporting 2FA to begin with
since rclone doesn't support it, and ultimately the real solution is to
use keys instead of passwords for sftp authentication.

That being said, while it's here we should make sure it isn't broken!

Issue #298 2FA auth flow does not utilize refresh tokens
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant