Skip to content

Commit

Permalink
Fix TOTP internal context after saving settings
Browse files Browse the repository at this point in the history
  • Loading branch information
kszafran committed Jan 10, 2023
1 parent ca35b45 commit acadd14
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 3 deletions.
1 change: 1 addition & 0 deletions selfservice/flow/settings/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ func (e *HookExecutor) PostSettingsHook(w http.ResponseWriter, r *http.Request,
ctxUpdate.Flow.UI = newFlow.UI
ctxUpdate.Flow.UI.ResetMessages()
ctxUpdate.Flow.UI.AddMessage(node.DefaultGroup, text.NewInfoSelfServiceSettingsUpdateSuccess())
ctxUpdate.Flow.InternalContext = newFlow.InternalContext
if err := e.d.SettingsFlowPersister().UpdateSettingsFlow(r.Context(), ctxUpdate.Flow); err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/lookup/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ func TestCompleteSettings(t *testing.T) {

actualFlow, err := reg.SettingsFlowPersister().GetSettingsFlow(context.Background(), uuid.FromStringOrNil(f.Id))
require.NoError(t, err)
assert.Equal(t, "{}", gjson.GetBytes(actualFlow.InternalContext, flow.PrefixInternalContextKey(identity.CredentialsTypeLookup, lookup.InternalContextKeyRegenerated)).Raw)
assert.Empty(t, gjson.GetBytes(actualFlow.InternalContext, flow.PrefixInternalContextKey(identity.CredentialsTypeLookup, lookup.InternalContextKeyRegenerated)).Raw)
}

t.Run("type=api", func(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion selfservice/strategy/webauthn/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ func TestCompleteSettings(t *testing.T) {

actualFlow, err := reg.SettingsFlowPersister().GetSettingsFlow(context.Background(), uuid.FromStringOrNil(f.Id))
require.NoError(t, err)
assert.Empty(t, gjson.GetBytes(actualFlow.InternalContext, flow.PrefixInternalContextKey(identity.CredentialsTypeWebAuthn, webauthn.InternalContextKeySessionData)))
// new session data has been generated
assert.NotEqual(t, settingsFixtureSuccessInternalContext, gjson.GetBytes(actualFlow.InternalContext, flow.PrefixInternalContextKey(identity.CredentialsTypeWebAuthn, webauthn.InternalContextKeySessionData)))

testhelpers.EnsureAAL(t, browserClient, publicTS, "aal2", string(identity.CredentialsTypeWebAuthn))
}
Expand Down
38 changes: 37 additions & 1 deletion test/e2e/cypress/integration/profiles/mfa/totp.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { authenticator } from "otplib"
import { routes as react } from "../../../helpers/react"
import { routes as express } from "../../../helpers/express"

context("2FA lookup secrets", () => {
context("2FA TOTP", () => {
;[
{
login: react.login,
Expand Down Expand Up @@ -243,6 +243,21 @@ context("2FA lookup secrets", () => {
expectAal: "aal2",
expectMethods: ["password", "totp", "totp", "totp", "totp"],
})

// The React app keeps using the same flow. The following scenario used to be broken,
// because the internal context wasn't populated properly in the flow after settings were saved.
cy.visit(settings)
cy.get('*[name="totp_unlink"]').click()
cy.expectSettingsSaved()

cy.get('[data-testid="node/text/totp_secret_key/text"]').then(($e) => {
secret = $e.text().trim()
})
cy.get('input[name="totp_code"]').then(($e) => {
cy.wrap($e).type(authenticator.generate(secret))
})
cy.get('*[name="method"][value="totp"]').click()
cy.expectSettingsSaved()
})

it("should not show totp as an option if not configured", () => {
Expand Down Expand Up @@ -274,6 +289,27 @@ context("2FA lookup secrets", () => {
"The provided authentication code is invalid, please try again.",
)
})

// The React app keeps using the same flow. The following scenario used to be broken,
// because the internal context wasn't populated properly in the flow after settings were saved.
it.only("should allow changing other settings and then setting up totp", () => {
cy.visit(settings)
cy.get('input[name="traits.website"]')
.clear()
.type("https://some-website.com")
cy.get('*[name="method"][value="profile"]').click()
cy.expectSettingsSaved()

let secret
cy.get('[data-testid="node/text/totp_secret_key/text"]').then(($e) => {
secret = $e.text().trim()
})
cy.get('input[name="totp_code"]').then(($e) => {
cy.wrap($e).type(authenticator.generate(secret))
})
cy.get('*[name="method"][value="totp"]').click()
cy.expectSettingsSaved()
})
})
})
})

0 comments on commit acadd14

Please sign in to comment.