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

feat(core): guard one-time token on consent request #7160

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

charIeszhao
Copy link
Member

@charIeszhao charIeszhao commented Mar 19, 2025

Summary

  • Guard one-time token on consent request. This makes sure even if the user has an active session, when they use magic sign-in link, the token is still checked before auto-consent.
  • Change the client side navigation mechanism when one_time_token param is presented in the URL. The logic is now handled separately in both /sign-in and /register routes.

Testing

Local dev test

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

@charIeszhao charIeszhao self-assigned this Mar 19, 2025
@github-actions github-actions bot added the feature Cool stuff label Mar 19, 2025
Copy link

github-actions bot commented Mar 19, 2025

COMPARE TO master

Total Size Diff 📈 +5.22 KB

Diff by File
Name Diff
packages/core/src/middleware/koa-consent-guard.test.ts 📈 +2.79 KB
packages/core/src/middleware/koa-consent-guard.ts 📈 +1.42 KB
packages/core/src/tenants/Tenant.ts 📈 +191 Bytes
packages/experience/src/App.tsx 📈 +198 Bytes
packages/experience/src/pages/OneTimeToken/index.tsx 📈 +55 Bytes
packages/experience/src/pages/Register/index.tsx 📈 +242 Bytes
packages/experience/src/pages/SignIn/index.tsx 📈 +280 Bytes
packages/experience/src/pages/SwitchAccount/index.module.scss 0 Bytes
packages/experience/src/pages/SwitchAccount/index.tsx 📈 +496 Bytes
packages/experience/src/utils/search-parameters.ts 📈 +428 Bytes

@charIeszhao charIeszhao force-pushed the charles-log-10974-guard-one-time-token-on-auto-consent branch from 90ba6bf to 7a6309d Compare March 19, 2025 10:05
@github-actions github-actions bot added size/m and removed size/s labels Mar 19, 2025
@charIeszhao charIeszhao changed the base branch from charles-log-10876-experience-build-landing-page-to-handle-token-parameter to master March 19, 2025 10:06
@github-actions github-actions bot added size/xl and removed size/m labels Mar 19, 2025
@charIeszhao charIeszhao changed the base branch from master to charles-log-10876-experience-build-landing-page-to-handle-token-parameter March 19, 2025 10:09
@github-actions github-actions bot added size/m and removed size/xl labels Mar 19, 2025
Comment on lines 24 to 27
const interactionDetails = await provider.interactionDetails(ctx.req, ctx.res);
const {
params: { token, login_hint: loginHint },
session,
} = interactionDetails;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: there is a koa-interaction-details middleware

session,
} = interactionDetails;

assertThat(session, new errors.SessionNotFound('session not found'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use RequestError('session.not_found') ?

import assertThat from '../utils/assert-that.js';

/**
* Guard before allowing auto-consent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need some comments here, this middleware should guard one-time-token sign-in only.

await libraries.oneTimeTokens.verifyOneTimeToken(token, loginHint);
} catch (error: unknown) {
if (error instanceof RequestError) {
if (error.code === 'one_time_token.email_mismatch') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should reject the sign-in request when login_hint does not match with the value stored in token

simeng-li
simeng-li previously approved these changes Mar 20, 2025
Copy link
Contributor

@simeng-li simeng-li left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updates are needed

@simeng-li simeng-li dismissed their stale review March 20, 2025 04:53

should fix the error handling logic

Copy link
Contributor

@simeng-li simeng-li left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to fix the email miss match error handling logic.

@charIeszhao charIeszhao force-pushed the charles-log-10876-experience-build-landing-page-to-handle-token-parameter branch 7 times, most recently from c5a27fc to 402e351 Compare March 25, 2025 09:32
@charIeszhao charIeszhao force-pushed the charles-log-10974-guard-one-time-token-on-auto-consent branch from 7a6309d to 4ae5524 Compare March 25, 2025 12:42
Base automatically changed from charles-log-10876-experience-build-landing-page-to-handle-token-parameter to master March 25, 2025 12:58
@charIeszhao charIeszhao force-pushed the charles-log-10974-guard-one-time-token-on-auto-consent branch 2 times, most recently from 1ccae5f to f5bbc6e Compare March 26, 2025 06:21
@github-actions github-actions bot added size/l and removed size/m labels Mar 26, 2025
@charIeszhao charIeszhao force-pushed the charles-log-10974-guard-one-time-token-on-auto-consent branch 2 times, most recently from e213767 to 7806877 Compare March 26, 2025 07:22
@charIeszhao charIeszhao force-pushed the charles-log-10974-guard-one-time-token-on-auto-consent branch from 7806877 to 1f073e0 Compare March 26, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants