No validation for totp_code when unlinking TOTP 2FA method

KajsaEklof commented 2 years ago

Preflight checklist

[X] I could not find a solution in the existing issues, docs, nor discussions.
[X] I agree to follow this project's Code of Conduct.
[X] I have read and am following this repository's Contribution Guidelines.
[ ] This issue affects my Ory Cloud project.
[X] I have joined the Ory Community Slack.
[ ] I am signed up to the Ory Security Patch Newsletter.

Describe the bug

When unlinking the TOTP 2FA Method there is no validation on the 'totp_code' that is required to be passed in. We can pass in any value or an empty string and the response is still coming back as successful with status code 200.

We are using Vue and the SelfServiceFlows. Our UI has a button to disable 2FA and this submits a submitSelfServiceSettingsFlow with a TotpMethodBody.

  kratos
      .submitSelfServiceSettingsFlow(this.flowId, undefined, {
        csrf_token: this.csrfToken,
        method: 'totp',
        totp_unlink: true,
      })

We were getting an error with a message that the totp_code was missing so we added an input field in the UI to provide a verification code. So the submitSelfServiceSettingsFlow looks like this now:

  kratos
      .submitSelfServiceSettingsFlow(this.flowId, undefined, {
        csrf_token: this.csrfToken,
        method: 'totp',
        totp_unlink: true,
        totp_code: this.verificationCode,
      })

But we seem to be able to pass in any value for totp_code, the code doesn't seem to be validated?

Reproducing the bug

Using the Settings Flow for client-side browser clients initialise the settings flow with kratos.initializeSelfServiceSettingsFlowForBrowsers()

Set up 2FA with totp:

kratos
.submitSelfServiceSettingsFlow(this.flowId, undefined, {
    csrf_token: this.csrfToken,
    method: 'totp',
    totp_code: this.verificationCode,
});

Try to unlink 2FA totp method but provide an empty string for the totp_code:

kratos
.submitSelfServiceSettingsFlow(this.flowId, undefined, {
    csrf_token: this.csrfToken,
    method: 'totp',
    totp_unlink: true,
    totp_code: '',
});

Response is successful with status 200 when we expected this to fail with an invalid totp_code.

Relevant log output

No response

Relevant configuration

No response

Version

v0.9.0-alpha.3

On which operating system are you observing this issue?

Windows

In which environment are you deploying?

Docker Compose

Additional Context

No response

aeneasr commented 2 years ago

Thank you for the report. If we would require the TOTP code to remove the device, it would never be possible to remove a TOTP device that you lost. Removing the TOTP code requires a privileged session though, which will be requested by the user if the session is no longer considered privileged enough.

If you agree with these findings I think we can close this issue.

KajsaEklof commented 2 years ago

Thanks for looking at this! Yes I agree with you about that happy to close this, although I have another question now 😄

I've enable 2FA on my account and then wait for the privileged session to run out. When I try and disable 2FA and need to verify the action it only asks for one level of sign in, email + password for example, without my 2FA? Just wondering why I don't need to verify with 2FA at that stage, since it's not yet turned off?

jdudmesh commented 2 years ago

I'd like to understand "Removing the TOTP code requires a privileged session though". At the moment I can link/unlink a TOTP app at AAL1 - this isn't a privileged session (IMO). This feels wrong. It means that if an account is compromised then a hacker can simply unlink any established TOTP app and re-link their own. Surely this makes TOTP 2FA worthless? It is possible to hide the settings in a UI without AAL2 if the user has not set up TOTP but the public Kratos endpoint is still accessible.

I accept your point about removing a lost device but I would have thought that in the case of a lost device there should be an admin action which can force unlink TOTP credentials but for a user to do it they must be at AAL2 i.e. have supplied a TOTP code.

aeneasr commented 2 years ago

You'll need to set your AAL requirements to "highest_available" for the settings flow

jdudmesh commented 2 years ago

Ah, that fixed it, thanks. I wonder if it would be possible for Ory to throw an error if the user has linked a TOTP app but the AAL requirement is not set to highest available? Some sort of safety net would be useful here.

ory / kratos