Skip to content

feat(web): replace local OTP auth with @opengovsg/auth - #681

Open
karrui wants to merge 6 commits into
mainfrom
karrui/use-opengovsg-auth
Open

feat(web): replace local OTP auth with @opengovsg/auth#681
karrui wants to merge 6 commits into
mainfrom
karrui/use-opengovsg-auth

Conversation

@karrui

@karrui karrui commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Swap the in-repo OTP/PKCE auth implementation for @opengovsg/auth@0.0.0-snapshot-20260811091307
  • Add a Prisma VerificationTokenStore adapter and wire login/verify through createOtpAuth
  • Remove local auth.utils and lib/pkce now covered by the package

Test plan

  • pnpm -C apps/web typecheck
  • pnpm -C apps/web test:ci src/server/modules/auth/__tests__/auth.service.spec.ts src/validators/__tests__/auth.spec.ts
  • Manual sign-in: request OTP, verify with correct code, confirm session
  • Manual sign-in: wrong OTP / reused challenge surfaces generic failure without leaking stage

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
starter-kit Ready Ready Preview Aug 12, 2026 8:45am
starter-kit-web Error Error Aug 12, 2026 8:45am

Request Review

@datadog-opengovsg

datadog-opengovsg Bot commented Aug 11, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 76.19%
Overall Coverage: 56.50%

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: eacdea4 | Docs | Datadog PR Page | Give us feedback!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

Pull request overview

This PR migrates the web app’s OTP + PKCE sign-in flow from an in-repo implementation to @opengovsg/auth, wiring issuance/verification through createOtpAuth and removing the now-redundant local auth/PKCE utilities.

Changes:

  • Added @opengovsg/auth dependency and updated validators to use package-provided OTP defaults + code-challenge validation.
  • Reworked server auth flow to issue/verify OTPs via createOtpAuth backed by a Prisma VerificationTokenStore adapter.
  • Deleted local OTP hashing utilities and PKCE helpers; updated sign-in wizard + tests to use @opengovsg/auth/pkce.

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pnpm-lock.yaml Adds @opengovsg/auth snapshot and updates lock entries accordingly.
apps/web/package.json Adds @opengovsg/auth and removes direct nanoid dependency.
apps/web/src/validators/auth.ts Switches OTP/PKCE constants + code-challenge validation to @opengovsg/auth.
apps/web/src/validators/tests/auth.spec.ts Updates PKCE test to use @opengovsg/auth/pkce (now async).
apps/web/src/server/modules/auth/auth.service.ts Replaces custom OTP issuance/verification logic with createOtpAuth and error mapping.
apps/web/src/server/modules/auth/verification-token.store.ts Prisma-backed token store adapter for @opengovsg/auth (content excluded from review).
apps/web/src/server/modules/auth/tests/auth.service.spec.ts Refactors auth service tests to validate the new issue/verify behavior via email-parsed OTP.
apps/web/src/server/modules/auth/auth.utils.ts Removes in-repo OTP/token hashing + identifier helpers.
apps/web/src/server/modules/auth/tests/auth.utils.spec.ts Removes tests for deleted auth.utils helpers.
apps/web/src/lib/pkce/server-pkce.ts Removes server PKCE helper in favor of package implementation.
apps/web/src/lib/pkce/constants.ts Removes PKCE constants/generator in favor of package implementation.
apps/web/src/lib/pkce/browser-pkce.ts Removes browser PKCE helper in favor of package implementation.
apps/web/src/app/(public)/sign-in/_components/wizard/context.tsx Switches client-side PKCE generation to @opengovsg/auth/pkce.
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file
Files excluded by content exclusion policy (1)
  • apps/web/src/server/modules/auth/verification-token.store.ts
Suppressed comments (1)

apps/web/src/server/modules/auth/auth.service.ts:96

  • codeVerifier is a PKCE secret and should not be written to logs. Logging it as tokenId makes it retrievable from log storage and could weaken the OTP+PKCE binding if logs are compromised. Prefer logging a non-sensitive identifier (e.g., a redacted placeholder or the derived codeChallenge if you need correlation).
  if (result.error.code === 'token_reused') {
    logger.audit.authn.tokenReused({
      tokenId: codeVerifier,
      context: { email },
    })

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/web/src/server/modules/auth/auth.service.ts Outdated
Comment thread apps/web/src/validators/__tests__/auth.spec.ts Outdated
karrui added 6 commits August 12, 2026 16:27
Use the extracted package for PKCE and OTP issue/verify so starter-kit stops maintaining a forked copy of that flow.
Map package error codes to the previous user-facing copy so rate-limit and invalid/expired failures stay actionable.
Bump @opengovsg/auth so failed OTP verifies can log the real attempt count instead of a placeholder.
Collapse non-rate-limit OTP errors to one message, avoid logging the PKCE verifier, and shrink the code-challenge validation sample.
The importer still pinned 7.9.0 after the catalog bump, which broke frozen installs in CI.
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 this pull request may close these issues.

2 participants