Skip to content

refactor(api): extract shared SignedTokenCodec to dedup token services - #384

Merged
thomasluizon merged 1 commit into
mainfrom
chore/api-token-service-dedup
Jul 13, 2026
Merged

refactor(api): extract shared SignedTokenCodec to dedup token services#384
thomasluizon merged 1 commit into
mainfrom
chore/api-token-service-dedup

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

What

MarketingUnsubscribeTokenService and WaitlistConfirmationTokenService shared near-identical token logic — HMAC-SHA256 signing, base64url encode/decode, payload.signature composition, and constant-time signature verification. SonarCloud flagged ~40 duplicated lines in each file.

This extracts the genuinely-shared logic into a single internal static SignedTokenCodec (Encode(payloadBytes, signingKey) + TryDecode(token, signingKey, out payloadBytes)) in src/Orbit.Infrastructure/Services/. Each service keeps only its own concern:

  • Marketing — payload is the user id (N format), no expiry.
  • Waitlist — payload is email|language|expiresAtUnix, with expiry enforcement.

Behavior & wire-format preserving

The token string, the signing input (HMAC over the raw payload bytes), and the verification path are byte-identical to before, so a token minted before this change still validates after it. No public interface change, and no DI change (the codec is a static helper — the two services keep their existing AddSingleton registrations).

Tests

  • Both existing service test suites (MarketingUnsubscribeTokenServiceTests, WaitlistConfirmationTokenServiceTests) pass unchanged — they act as the round-trip / tamper / wrong-key / expiry / malformed regression guard.
  • Added SignedTokenCodecTests covering the shared helper directly (round-trip, wrong key, tampered signature, malformed-token branches incl. the invalid base64url-length path).
  • Full Orbit.Infrastructure.Tests: 1992 passed, 0 failed.

Refs thomasluizon/orbit-ui-mobile#243

#243)

MarketingUnsubscribeTokenService and WaitlistConfirmationTokenService
shared near-identical HMAC-SHA256 signing, base64url encode/decode, token
composition, and signature-verification logic (SonarCloud flagged ~40
duplicated lines each). Extract the genuinely-shared codec into a single
internal static SignedTokenCodec; each service keeps its own payload
format, key source, and expiry semantics.

Behavior- and wire-format-preserving: the token string, signing input, and
verification are byte-identical, so tokens minted before this change still
validate. No public interface or DI registration change (static helper).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test message with backtick code.

@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Code Review: PR #384 (thomasluizon/orbit-api)

Scope: PR #384 — refactor(api): extract shared SignedTokenCodec to dedup token services
Recommendation: APPROVE

Summary

This is a pure, behavior-preserving refactor. It extracts the HMAC-SHA256 signing, base64url encode/decode, and constant-time signature-verification logic that was 100%-duplicated between MarketingUnsubscribeTokenService and WaitlistConfirmationTokenService into one new internal static SignedTokenCodec (src/Orbit.Infrastructure/Services/SignedTokenCodec.cs). A line-by-line comparison of the extracted code against the removed private methods in both services confirms the signing input, encode/decode logic, and CryptographicOperations.FixedTimeEquals comparison are byte-identical — only parameterized by signingKey. No public interface, DI registration (AddSingleton unchanged in Orbit.Api/Extensions/ServiceCollectionExtensions.Infrastructure.cs:68-69), or wire format changed. New tests/Orbit.Infrastructure.Tests/Services/SignedTokenCodecTests.cs exercises the shared helper directly (round-trip, wrong-key, tampered-signature, and malformed-token branches including the invalid base64url-length path). Both pre-existing service test suites are unchanged and unaffected since they black-box test through the public service methods only.

Findings

Critical: None.
High: None.
Medium: None.
Low / Info: None (nothing met the signal-gate bar for concretely-actionable Medium; per the rubric, Low/Info is not posted on a PR review).

Subagents

  • security-reviewer: PASS — no injection, authn/authz, crypto, or data-exposure issues found. CryptographicOperations.FixedTimeEquals preserved for constant-time signature comparison; the new internal codec is scoped correctly via the pre-existing InternalsVisibleTo Orbit.Infrastructure.Tests (src/Orbit.Infrastructure/Orbit.Infrastructure.csproj:32) — no new public surface. Signing keys still sourced from IOptions/IOptions (env-var-backed, unchanged); no key material logged or hardcoded in appsettings.json/appsettings.Production.json (verified via grep).
  • contract-aligner: N/A — no DTO, Controller route, or packages/shared type touched; internal service-layer refactor only, not a wire-format/API-contract change.

Validation

  • Build (dotnet): Skipped per workflow — Build/Unit Tests/SonarCloud run as separate required CI checks on this PR.
  • Tests (dotnet): Skipped per workflow — same reason.

Deferred — N/A dimensions

  • Dimension 8 (DESIGN.md/AI-slop): N/A, no apps/* files in diff.
  • Dimension 9 (Parity web↔mobile): N/A, frontend-only dimension, no apps/* changed. Not verifiable in this CI job regardless (sibling orbit-ui-mobile repo not checked out).
  • Dimension 10 (i18n): N/A, no user-facing strings added.
  • Dimension 11 (Contract drift + backward-compat): N/A by gate (no DTO/Zod/route changed); separately verified under Correctness that the token wire format itself is unchanged, so previously-issued unsubscribe/waitlist links keep validating after this ships.
  • Dimension 14 (FEATURES.md parity): N/A, pure refactor with explicitly no behavior change (PR body: "Behavior & wire-format preserving").

All 4 changed files got a verdict: src/Orbit.Infrastructure/Services/MarketingUnsubscribeTokenService.cs, src/Orbit.Infrastructure/Services/SignedTokenCodec.cs, src/Orbit.Infrastructure/Services/WaitlistConfirmationTokenService.cs, tests/Orbit.Infrastructure.Tests/Services/SignedTokenCodecTests.cs — nothing in the diffs inventory was skipped.

Whats good

  • Textbook DRY extraction: the duplicated logic was already 100% identical in both services (SonarCloud-flagged ~40 duplicated lines each), not a speculative "third use" abstraction.
  • SignedTokenCodec.TryDecode keeps the out payloadBytes = [] default on every failure path, and the new tests exercise that contract directly.
  • No dead code left behind — the old private ComputeSignature/Base64UrlEncode/Base64UrlDecode methods were fully removed from both services, not just superseded.
  • Zero narration comments in the new file; code is self-documenting.

Recommendation: Safe to merge as-is. No action required.

Note: this comment carries the full review content. The formal decision above (APPROVE) was submitted as this PRs single review; its body field was accidentally left as placeholder test text during tooling setup and could not be edited after submission via the tools available in this session. This comment is the authoritative report body for that APPROVE decision.

@thomasluizon
thomasluizon merged commit c93fb50 into main Jul 13, 2026
19 checks passed
@thomasluizon
thomasluizon deleted the chore/api-token-service-dedup branch July 13, 2026 15:53
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.

1 participant