feat(otp): replace inline OTPaaS calls with provider abstraction

[vlwk]

🚀 Summary

Migrate botp OTP provider logic into TW. Eliminates the cross-repo dependency for the email OTP auth flow by inlining the provider abstraction directly. Establishes an authenticated session after successful OTP verification using the existing session.Store abstraction global map.

Open Questions

• In config_test.go, should I add cfg.BundleDirectory = t.TempDir() in fixtureConfigWithProvider(t *testing.T, provider Provider) instead? Currently, it’s in the valid environment test.
• Old handler returned 504 for OTPaaS timeouts. New handler maps all unrecognised provider errors (including timeouts) to 500. Should we preserve the 504 behaviour?
• botp returns json for all errors (including 5xx). TW uses plain text via http.Error() for 5xx. Should we align with botp and return json for server errors too?
• botp returns 404 for expired flows. TW returns 422 with AUTHORIZATION_FAILED. Is 422 correct here?

✏️ Changes

• Add server/internal/otp/ package with both OTPaaS and mock provider implementation
• Add server/internal/otp/bimap/ package (note: decided to nest it within otp/ since its only being used by the mock provider)
• Rewire otp handlers to use provider.
• Update config to support provider selection
• Set up session after successful verify using session.Store global map

Commits

#	Commit	Description
1	feat(otp): add provider package with OTPaaS and mock implementations	Copy otp.Provider interface, OTPaaS provider, mock provider, and bimap from botp
2	feat(config): add OTP provider selection and mock configuration	Add TW_OTP_PROVIDER, TW_ALLOWED_EMAIL_DOMAINS, TW_MOCK_ALLOWED_EMAILS with conditional validation
3	feat(env): add otp related env vars	Update .env.example with new OTP-related environment variables
4	feat(handler): accept OTP provider in handler constructor	Add otpProvider field to Handler struct, change New() signature
5	feat(otp): make handler call providers	Rewrite RequestOTP/VerifyOTP to use provider, remove raw HTTP logic, rewrite tests
6	feat(session): integrate session store into OTP handlers	Replace global map with session.Store, establish authenticated session after verify
6	Revert feat(session): integrate session store into OTP handlers	Revert to using global map as session middleware is not finished yet.

Old TW vs New TW (handler behavior)

Aspect	Old	New
Provider communication	Raw http.Client with manual HMAC auth	otp.Provider interface
Auth token	buildAuthToken in handler	Handled internally by OTPaaS provider
Email validation	Hardcoded isAllowedEmail (env-based)	Config-driven AllowedEmailDomains list
Email normalization	None	strings.TrimSpace(strings.ToLower(...))
Content-Type validation	None	Rejects non-application/json with 415
PIN validation	Length check only	Trim + empty check + 6-digit numeric validation
Response headers	Content-Type: application/json	Adds X-Content-Type-Options: nosniff, charset=UTF-8
RequestOTP response	200 {id}	200 {id} (unchanged)
VerifyOTP response	204 No Content	200 {id, email}
Error mapping (request)	Non-200 from OTPaaS → generic 500	Rate limited → 429, domain/email not allowed → 422, default → 500
Error mapping (verify)	401 → 422, 404 → 422, other → 500	Invalid PIN → 422, flow expired → 422, default → 500
Timeout handling	context.DeadlineExceeded → 504	Wrapped in provider error → 500
Session store	Global map	session.Store with TTL (10min otp flow, 30min auth)

botp vs TW comparison

Aspect	botp	TW	Reason
Error response format	{message, errors: [{field, message}]}	{code, message, error: [{code, message}]}	TW's existing frontend contract
Response helper	renderJSON (generic)	writeClientErrorResponse + inline JSON	Minimal change to existing patterns
VerifyOTP flow ID source	URL path (/verify/{flow_id})	Session global map (cookie lookup)	TW manages session server-side
Flow expired status	404	422	TW treats it as auth failure, not missing resource
Server errors	JSON via renderJSON	Plain text via http.Error	TW's existing pattern

Test coverage (botp → TW mapping)

RequestOTP

botp test	TW test	Notes
"successful response"	"successful response"	TW adds session cookie + global map assertions
—	"creates new session when no cookie"	TW-specific (session behavior)
"unsupported media type"	"unsupported media type"	Same assertions
"invalid JSON"	"invalid JSON"	Same
"empty email"	"empty email"	Same
"domain not allowed"	"domain not allowed"	Same
"provider rate limited"	"provider rate limited"	Same
"provider domain not allowed"	"provider domain not allowed"	Same
—	"provider email not allowed"	Extra coverage in TW
"provider error"	"provider error"	Same

VerifyOTP

botp test	TW test	Notes
"successful response"	"successful response"	Same
"unsupported media type"	"unsupported media type"	Same
"invalid JSON"	"invalid JSON"	Same
"empty PIN"	"empty pin"	Same
"PIN must be 6 digits"	"invalid pin length"	Same
—	"missing cookie"	TW-specific (session)
—	"missing session"	TW-specific (session)
"provider invalid PIN"	"provider invalid pin"	Same
"provider flow expired"	"provider flow expired"	422 (tw) vs 404 (botp)
"provider error"	"provider error"	Same

🧪 Test Plan

• go test ./server/internal/otp/...
• go test ./server/internal/config/...
• go test ./server/internal/handler/...
• go test ./... (full suite)
• make lint
• Manual verification with mock provider:
  1. Start server: TW_OTP_PROVIDER=mock TW_ALLOWED_EMAIL_DOMAINS=@schools.gov.sg TW_MOCK_ALLOWED_EMAILS=test@schools.gov.sg go run ./server/cmd/tw
  2. Request OTP: curl -sv -X POST 'http://[::1]:3000/otp/request' -H 'Content-Type: application/json' -d '{"email":"test@schools.gov.sg"}' → 200
     with {id} and Set-Cookie: session_id=...
  3. Verify (correct PIN): curl -s -X POST 'http://[::1]:3000/otp/verify' -H 'Content-Type: application/json' -d '{"pin":"112233"}' --cookie 'session_id=<value>' → 200 with {id, email}
  4. Verify (wrong PIN): same with "pin":"999999" → 422 AUTHORIZATION_FAILED
  5. Verify (expired PIN): same with "pin":"223344" → 422 AUTHORIZATION_FAILED
  6. Request (disallowed domain): "email":"hacker@example.com" → 422 INVALID_FORM

[YimingIsCOLD]

Possible to continue to use the global map as the temporary session store as the session middleware is still work in progress?

[vlwk]

Possible to continue to use the global map as the temporary session store as the session middleware is still work in progress?

Reverted the last commit. Now back to using global map

[YimingIsCOLD]

Suggested change

	ProviderOTPaaS Provider = "otpaas"
	OTPProviderOTPaaS Provider = "otpaas"

I think we should prefix with OTP because there will be a lot of different provider services in the future.

[YimingIsCOLD]

BOTP service only focusing on OTP operations so the configurations do not need to be nested. TW will have a lot of different kind of configurations so I think we should nest OTP related configurations into a struct.

For example:

type Config struct {
    ...

    OTP OTPConfig `dotenv:",squash"`
}

type OTPConfig struct {
    Provider OTPProvider `dotenv:"TW_OTP_PROVIDER"`
    AllowedEmailDomains []string `dotenv:"TW_OTP_ALLOWED_EMAIL_DOMAINS"`

    OTPaaS OTPaaSConfig `dotenv:",squash"`
    Mock     MockConfig      `dotenv:",squash"`
}

Then we add comments to OTPaaSConfig and MockConfig:

// OTPaaSConfig is the configuration for the OTPaaS OTP provider.
type OTPaaSConfig struct {
    ...
}

// MockConfig is the configuration for the mock OTP provider.
type MockConfig struct {
    ...
}

WDYT?

[YimingIsCOLD]

Should we have a separate PR for adding config tests?

[YimingIsCOLD]

We added a random package to generate random alphanumeric. Can check if we can replace this helper?