# Phase 2B-i — Reviewer Paper Access: Audit Trail + Watermarking **Status:** Approved (2026-04-12) **Builds on:** `phase-2a-core-design.md` (approved 2026-04-12), `adr-paper-detail-authorization.md` **Informs:** Phase 2B-ii (confidentiality agreement modal + annotated re-upload), Phase 3 implementation **Scope:** Server-side safety layer. Download audit trail + download watermarking. Documentation only — no source changes in this phase. --- ## 0. Problem statement Phase 2A gives an assigned reviewer a metadata-stripped preview and download of the manuscript and closes the `GET /papers/{id}` PII leak. It does **not** record who accessed which paper when, and it does **not** embed any leak-attribution signal in the downloaded bytes. Phase 2B-i adds both, layered onto 2A's `ReviewerPaperServiceImpl.getManuscriptForReview(...)` path without altering the endpoint surface. ### 0.1 Goals - Record every preview and every download that a reviewer performs against a paper, with reviewer id, paper id, review id, access type, timestamp, IP, and user agent. - Embed a visible but non-obstructive diagonal watermark on every downloaded PDF identifying the reviewer and date, so a leaked file points back to a specific person without requiring any database lookup. - Guarantee the audit write **cannot block** legitimate access (fail-open). - Guarantee watermarking **fails closed** — if we can't watermark, we don't serve the file. - Preview stays clean (no watermark) so reviewers have an unobstructed reading surface; download carries the watermark because that's the artifact that can leave the server. ### 0.2 Non-goals - No admin-facing query/search UI over the audit log in this phase (see §2B-i-3: deferred). - No rotation/archival policy for the audit table (see risk register, flagged for later). - No watermarking of the inline preview. - No new dependencies (backend or frontend). - The remaining 2B safety items — confidentiality agreement modal and annotated re-upload — belong to Phase 2B-ii. - **Phase 2B-i ships coupled to Phase 2B-ii in release, not merge.** Phase 2B-i is mergeable and testable in isolation, but **must not be enabled in production** before the 2B-ii confidentiality agreement modal is in production, because D10 embeds reviewer email in downloaded PDFs and that data flow needs explicit reviewer consent first (see §2.3.1). --- ## Part 2B-i-1 — Download Audit Trail ### 1.1 Why a dedicated table (not the existing `audit_log`) The existing `audit_log` table is a generic `(action, entity_type, entity_id, jsonb details)` store with a `BIGSERIAL` primary key. Shoving paper-access records into it is technically possible, but it has three concrete drawbacks: 1. **No FK integrity on `review_id`.** `audit_log.entity_id` is `VARCHAR(100)` and references nothing. A dedicated table gets `review_id UUID NOT NULL REFERENCES reviews(id)`, so we can join to the review row and the related paper/reviewer rows from one query. 2. **Query patterns differ.** Admin investigations need `(reviewer_id, paper_id, accessed_at)` range scans and `access_type` filters. On `audit_log`, those become JSON extraction queries on the `details` column, which are orders of magnitude slower than a composite btree and cannot be indexed without a generated-column hack. 3. **Retention scope differs.** The generic `audit_log` captures business actions like `PAPER_SUBMITTED`. Paper-access records are much higher cardinality (every preview click) and eventually need their own rotation policy. Mixing them defers a harder problem. Decision: new `paper_access_audit` table. The generic `audit_log` is left untouched. ### 1.2 Schema **Migration filename:** `shodh-sanchayan-api/src/main/resources/db/migration/V7__paper_access_audit.sql` ```sql -- Audit trail for reviewer access to manuscripts (preview + download). -- See docs/reviewer-paper-access/phase-2b-i-audit-watermark-design.md CREATE TABLE paper_access_audit ( id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), review_id UUID NOT NULL REFERENCES reviews(id), reviewer_id UUID NOT NULL REFERENCES users(id), paper_id UUID NOT NULL REFERENCES papers(id), access_type VARCHAR(16) NOT NULL CHECK (access_type IN ('PREVIEW', 'DOWNLOAD')), accessed_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), ip_address VARCHAR(45) NOT NULL, -- IPv6 max length user_agent VARCHAR(500) ); CREATE INDEX idx_paper_access_audit_review ON paper_access_audit(review_id); CREATE INDEX idx_paper_access_audit_reviewer ON paper_access_audit(reviewer_id); CREATE INDEX idx_paper_access_audit_paper ON paper_access_audit(paper_id); CREATE INDEX idx_paper_access_audit_time ON paper_access_audit(accessed_at DESC); -- Composite for admin query patterns: -- "show me every access a reviewer made to a paper, in order" CREATE INDEX idx_paper_access_audit_rev_pap_t ON paper_access_audit(reviewer_id, paper_id, accessed_at DESC); ``` Notes on conventions matched to V1: - UUID pk via `uuid_generate_v4()` (V1 uses this on every UUID table). - `TIMESTAMPTZ NOT NULL DEFAULT NOW()` matches `reviews.assigned_at`, `audit_log.created_at`. - `access_type` is a plain varchar with a check constraint (not a Postgres enum type). V1's enums (`paper_status`, `review_status`, etc.) are real Postgres enums, but they are shared across multiple columns and have been extended in later migrations. A two-value, table-local access type does not earn a global enum; a varchar+check matches the retention/flexibility posture we actually want. ### 1.3 Entity **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/entity/PaperAccessAudit.java` ```java @Entity @Table(name = "paper_access_audit") @Getter @Setter @NoArgsConstructor @AllArgsConstructor @Builder public class PaperAccessAudit { @Id @GeneratedValue(strategy = GenerationType.UUID) private UUID id; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "review_id", nullable = false) private Review review; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "reviewer_id", nullable = false) private User reviewer; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "paper_id", nullable = false) private Paper paper; @Enumerated(EnumType.STRING) @Column(name = "access_type", nullable = false, length = 16) private PaperAccessType accessType; @Column(name = "accessed_at", nullable = false) private Instant accessedAt; @Column(name = "ip_address", nullable = false, length = 45) private String ipAddress; @Column(name = "user_agent", length = 500) private String userAgent; @PrePersist void prePersist() { if (accessedAt == null) accessedAt = Instant.now(); } } ``` This entity deliberately does **not** extend `BaseEntity`. It's an immutable event log; `updatedAt` has no meaning and would be misleading. ### 1.4 Enum **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/enums/PaperAccessType.java` ```java public enum PaperAccessType { PREVIEW, DOWNLOAD } ``` Package matches existing enums (`UserRole`, `ReviewStatus`, etc.). ### 1.5 Repository **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/repository/PaperAccessAuditRepository.java` ```java @Repository public interface PaperAccessAuditRepository extends JpaRepository { // Read methods intentionally omitted in Phase 2B-i. // Admin query surface is deferred (see §2B-i-3). } ``` The repository exists for the insert path only. Query methods will be added in the phase that introduces admin endpoints. ### 1.6 Service **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/PaperAccessAuditService.java` ```java public interface PaperAccessAuditService { /** * Records a reviewer access event. This method is guaranteed not to throw. * Any failure (DB down, constraint violation, reference resolution failure) * is caught internally and logged at WARN with full context. The caller's * download/preview flow proceeds unaffected. * * Trade-off: an incomplete audit trail is preferable to blocking a * legitimate reviewer from accessing an assigned paper. */ void logAccess(UUID reviewId, UUID reviewerId, UUID paperId, PaperAccessType type, String ipAddress, String userAgent); } ``` **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/PaperAccessAuditServiceImpl.java` ```java @Service @RequiredArgsConstructor @Slf4j public class PaperAccessAuditServiceImpl implements PaperAccessAuditService { private final PaperAccessAuditRepository repository; private final EntityManager entityManager; // for getReference() — avoids extra SELECTs @Override @Transactional(propagation = Propagation.REQUIRES_NEW) public void logAccess(UUID reviewId, UUID reviewerId, UUID paperId, PaperAccessType type, String ipAddress, String userAgent) { try { PaperAccessAudit row = PaperAccessAudit.builder() .review(entityManager.getReference(Review.class, reviewId)) .reviewer(entityManager.getReference(User.class, reviewerId)) .paper(entityManager.getReference(Paper.class, paperId)) .accessType(type) .ipAddress(ipAddress) .userAgent(truncate(userAgent, 500)) .build(); repository.save(row); } catch (RuntimeException e) { log.warn( "Paper access audit insert failed — reviewerId={} paperId={} type={} : {}", reviewerId, paperId, type, e.getMessage() ); // Intentionally swallowed. See Javadoc. } } private static String truncate(String s, int max) { if (s == null) return null; return s.length() <= max ? s : s.substring(0, max); } } ``` Two important implementation choices: - **`Propagation.REQUIRES_NEW`**: the audit insert runs in its own transaction, so if it rolls back (constraint violation, connection drop), the caller's transaction — if any — is not poisoned. This also means a silent swallow can't leave the outer transaction in a rollback-only state. - **`EntityManager.getReference(...)`**: constructs proxies from the already-known UUIDs instead of issuing three SELECT queries to load the Review/User/Paper rows. We already know those ids are real because the authorization path just verified them. ### 1.7 Integration with 2A — the three committed decisions These decisions shape how `ReviewerPaperServiceImpl.getManuscriptForReview(...)` changes. They are consistent across audit-trail and watermarking scope. #### Decision D7 — HttpServletRequest is **not** injected into any service **Committed: option (a).** Extend `getManuscriptForReview` to accept `PaperAccessType accessType, String ipAddress, String userAgent` as primitive parameters. The controller extracts IP and User-Agent from `HttpServletRequest` in its own method body (where servlet types already live) and passes them in. **Justification:** No service or controller in the codebase currently injects `HttpServletRequest`. The only place that type appears at all is `JwtAuthenticationFilter`, which is a filter, not a service. Existing services consume primitives and call `SecurityUtils.getCurrentUserId()` when they need the caller's id. Breaking that pattern for one service would introduce a one-off convention that nobody else follows — exactly the shape 2A's D3 rejected for authorization (SpEL vs. service bean). Pass primitives. Keep service bodies servlet-agnostic so they remain testable without a mock request. #### Decision D8 — Active review id is returned by the authorization helper **Committed: option (b).** Change `ReviewAuthorizationService.assertReviewerCanAccess(...)` to return the `UUID reviewId` of the matched active assignment instead of returning `void`. On no-match, it still throws `ForbiddenException`. The underlying repository method changes from an `exists` query to a query returning `Optional`. **Justification:** - **Query count:** option (a) would require a separate `findActiveReviewId` call, doubling the authorization-side DB hit. Option (b) keeps it at one query and just changes its shape (`exists` → `select id`). Postgres treats both as equivalent-cost index lookups. - **Responsibility surface:** the helper already knows "does an active assignment exist"; returning the id it just proved existing is strictly less information loss, not a responsibility expansion. "Assert and return what you asserted" is a coherent shape. - **Backward compatible at call sites:** 2A's two existing callers (`ReviewerPaperServiceImpl`, `ReviewServiceImpl.submitReview`) can ignore the returned value. Only `ReviewerPaperServiceImpl` actually uses it — to pass to `paperAccessAuditService.logAccess(reviewId, ...)`. Concrete repository change: ```java // ReviewRepository.java @Query(""" select r.id from Review r where r.paper.id = :paperId and r.reviewer.id = :reviewerId and r.status in :statuses """) Optional findActiveReviewId( @Param("paperId") UUID paperId, @Param("reviewerId") UUID reviewerId, @Param("statuses") Collection statuses); ``` 2A's `existsByPaper_IdAndReviewer_IdAndStatusIn` is replaced by this. `ReviewAuthorizationServiceImpl.isAssignedReviewer(...)` becomes `return reviewRepository.findActiveReviewId(paperId, userId, ACTIVE_STATUSES).isPresent();` so the boolean surface is preserved for `PaperServiceImpl.findByIdForCaller`, which only needs a yes/no. New interface: ```java public interface ReviewAuthorizationService { boolean isAuthor(UUID userId, UUID paperId); boolean isAssignedReviewer(UUID userId, UUID paperId); /** * Asserts {@code reviewerId} has an active review assignment on * {@code paperId} (PENDING / IN_PROGRESS / COMPLETED) and returns the * id of that review row. Throws {@link ForbiddenException} otherwise. */ UUID assertReviewerCanAccess(UUID reviewerId, UUID paperId); } ``` 2A's `submitReview` refactor call site becomes `reviewAuthorizationService.assertReviewerCanAccess(reviewerId, review.getPaper().getId());` — the return is discarded. No logic change there. ### 1.8 Failure handling — fail-open contract Documented explicitly, because it's a deliberate trade-off: - Any exception inside `PaperAccessAuditServiceImpl.logAccess(...)` is caught at the method boundary and logged at WARN via the existing SLF4J setup with full context: `reviewerId`, `paperId`, `accessType`, `e.getMessage()`. - The download/preview response path proceeds as if the log call succeeded. - Consequence: an audit trail can be incomplete. A leak investigation using only the audit table can miss access events that happened while the DB was unreachable. - This is preferable to blocking a legitimate reviewer from an assigned paper because the audit subsystem is unhealthy. The access authorization decision has already been made by `ReviewAuthorizationService`; logging the fact is an observability concern, not a gating concern. - The `REQUIRES_NEW` transaction in §1.6 ensures a failed audit insert cannot contaminate the outer request context. ### 1.9 Admin query surface — deferred Phase 2B-i does **not** expose any admin-facing endpoint to query `paper_access_audit`. Admins with DB access can run SQL directly against the indexed columns. Adding controller/service surface for "list audit rows for paper X", "list audit rows for reviewer Y", etc., is flagged here as **explicitly deferred** so it is not forgotten. That work belongs in a follow-up phase that also includes UI. --- ## Part 2B-i-2 — Watermarking on Download ### 2.1 Scope - Applied on download only. The inline preview stays clean so reviewers have an unobstructed reading surface. - Applied to every page of the document. - Applied **after** metadata stripping, **before** serving bytes. The stored file is still never modified. ### 2.2 Watermark specification - **Text template:** `Confidential Review Copy — {reviewerIdentifier} — {YYYY-MM-DD}` - **Date format:** ISO-8601 local date, server time zone (`LocalDate.now()`). Reviewers across time zones see the same string because the date comes from the server, not the browser — it's an attribution marker, not a reviewer-local clock. - **Color:** light gray `#C8C8C8` = `new Color(200, 200, 200)`. - **Opacity:** set via a `PdfGState` with `setFillOpacity(0.35f)` so the watermark sits visibly beneath body text without swamping it. - **Rotation:** 45 degrees counter-clockwise (diagonal, bottom-left to top-right). - **Position:** geometric center of each page. Coordinates come from each page's `PdfReader.getPageSize(pageNum)`: ``` cx = (pageSize.getLeft() + pageSize.getRight()) / 2 cy = (pageSize.getBottom() + pageSize.getTop()) / 2 ``` - **Font size formula:** `max(24, min(72, pageWidth * 0.04f))`. On A4 portrait (595 pt wide) this gives ≈ 24 pt. On US Letter landscape (792 pt wide) it gives ≈ 32 pt. The floor/ceiling prevents the watermark from becoming invisible on small page sizes or obnoxiously large on posters. Font: `BaseFont.HELVETICA_BOLD`. - **Applied to every page** via `stamper.getOverContent(pageNum)` — over-content draws on top of the page, which is the correct layer for a watermark. ### 2.3 Reviewer identifier — email **Committed: option (a), email address.** **Justification based on Phase 1 discovery of `users` table columns:** The `users` table has these candidates: - `email VARCHAR(255) NOT NULL UNIQUE` ← always present, always unique - `phone VARCHAR(20) UNIQUE` ← nullable (`UNIQUE` but not `NOT NULL`) - `name_en`, `name_hi` ← not unique; two reviewers can share a name - `orcid_id VARCHAR(50)` ← nullable; only populated for academics who've registered one - `id UUID` ← rejected per prompt (opaque, requires DB access) Nothing like `username`, `handle`, `display_id`, or `staff_id` exists in the schema. Email is the **only** column that is simultaneously unique, mandatory, and human-meaningful without internal lookup. If a watermarked PDF leaks, the email on the watermark directly identifies the responsible reviewer and gives the investigator a communication channel to them. **Privacy consideration:** yes, this exposes the reviewer's email to anyone who opens the file. That is the intended leak-attribution signal — a watermark that requires a DB lookup is not a watermark, it's a nudge. A reviewer who is uncomfortable with that visibility is in the wrong role for confidential peer review; the confidentiality agreement modal in Phase 2B-ii will make that expectation explicit at assignment time (see §2.3.1). **No schema change recommended.** I considered recommending a new `display_id` column (a short opaque handle mapping back to the user), but (a) it introduces a population strategy problem for existing users, (b) it defeats the "no DB lookup" property unless we publish a lookup directory, and (c) the `email` field is already the right shape. If a future compliance requirement forces a pseudonymous watermark, that schema change belongs in its own phase. ### 2.3.1 Hard dependency on Phase 2B-ii (confidentiality agreement) D10 embeds the reviewer's email in every downloaded PDF. That is a defensible technical choice — it is the only unique, human-meaningful column on the `users` table — but it creates two concerns that a purely technical design cannot address on its own: 1. **Data-flow disclosure.** The reviewer's email leaving the server embedded in a PDF is personal data under GDPR/DPDP. It must be disclosed in the privacy policy as a purposeful data flow, with a stated purpose (leak attribution), retention bound (lifetime of the downloaded artifact, outside Shodh Sanchayan's control), and recipient category (any party in possession of the file). 2. **Retaliation surface.** Academic peer review has documented cases of authors using reviewer contact information to retaliate after unfavourable recommendations. Embedding the reviewer's email in a file that may ultimately reach the author — through desk-sharing, institutional channels, or a leak — gives a determined author a direct contact channel. Watermark attribution is still the right call (the deterrence and traceability benefits outweigh the risk, and every honest-use alternative failed the "no DB lookup" test), but reviewers must know about it before they opt in. **Dependency statement:** Decision D10 has a hard dependency on Phase 2B-ii. The confidentiality agreement modal designed there **MUST** include explicit language telling reviewers that their email address will appear on every downloaded copy of every assigned paper. The reviewer sees and accepts this disclosure before their first download. That converts an undisclosed data flow into informed consent, which is both the ethical floor and the legal floor. **Concretely, Phase 2B-ii must:** - State the disclosure verbatim in the modal body ("Every PDF you download will be watermarked with your email address and the download date. This is used to trace leaked copies back to their source."). - Block download actions until acceptance is recorded (the mechanism for this is 2B-ii's scope). - Not provide an opt-out — a reviewer who declines the agreement declines the assignment, not the watermark. **This section is a forward constraint on 2B-ii, not a blocker on 2B-i.** 2B-i as designed still ships correctly — the legal/ethical precondition is satisfied when 2B-ii ships alongside it, and the two phases are therefore a release-coupling, not a merge-coupling. Phase 3 implementation must not release reviewer downloads with watermarking enabled until the 2B-ii modal is in production. ### 2.4 Library OpenPDF 2.0.3 — already on the classpath from 2A. LGPL 2.1 / MPL 1.1 (same license analysis as 2A's `PdfMetadataStripper`). No new dependency. ### 2.5 New utility **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/util/PdfWatermarker.java` ```java @Component public class PdfWatermarker { /** * Returns a copy of {@code pdf} with a diagonal, centered, light-gray * "Confidential Review Copy — {reviewerIdentifier} — {yyyy-MM-dd}" * watermark on every page. * * The input bytes are never mutated. The returned byte[] is a new array. * The input PDF is expected to have been metadata-stripped already. * * @throws PdfSanitizationException on any parse / stamp failure — reuses * the 2A exception which maps to 503 MANUSCRIPT_PREVIEW_UNAVAILABLE. */ public byte[] watermark(byte[] pdf, String reviewerIdentifier, LocalDate date) { // Operations performed: // // 1. new PdfReader(pdf) // 2. new PdfStamper(reader, out) // 3. Build watermark string: // "Confidential Review Copy — {reviewerIdentifier} — " + date // 4. Create a PdfGState with fillOpacity = 0.35f // 5. Load BaseFont HELVETICA_BOLD // 6. For pageNum in 1..reader.getNumberOfPages(): // a. PdfContentByte over = stamper.getOverContent(pageNum) // b. Rectangle pageSize = reader.getPageSize(pageNum) // c. cx = (left + right) / 2; cy = (bottom + top) / 2 // d. fontSize = max(24, min(72, pageWidth * 0.04f)) // e. over.saveState() // f. over.setGState(gState) // g. over.setColorFill(new Color(200, 200, 200)) // h. ColumnText.showTextAligned( // over, Element.ALIGN_CENTER, // new Phrase(text, new Font(baseFont, fontSize)), // cx, cy, 45.0f /* rotation in degrees */) // i. over.restoreState() // 7. stamper.close() // 8. reader.close() // 9. return out.toByteArray() // // Any IOException / RuntimeException is caught and rethrown as // PdfSanitizationException("Failed to watermark PDF", e). } } ``` Implementation-body comments are the operation spec. ### 2.6 Integration with 2A — preview/download branching #### Decision D9 — Single method, accessType-driven branching **Committed: option (a).** Extend `getManuscriptForReview` to take a `PaperAccessType accessType` parameter and branch on it inside the service body. Do not split into two public methods. **Justification:** - **Principle of least change to 2A.** 2A's `ReviewerPaperService` interface has one method. Keeping it at one method, with a parameter that is already being plumbed through for audit logging, is the minimum delta. - **The branching parameter already has to exist** for audit logging (`type` in §1.6). Splitting into two public methods would still need that parameter flowing through to the audit call, which means we'd have the enum *and* method overloads doing the same thing. Pick one. The enum is the narrower change. - **The controller split is preserved.** 2A already has two controller endpoints (`/manuscript` inline, `/manuscript/download` attachment), matching the `MagazineController` pattern. Each endpoint passes a different `PaperAccessType` value. The URL-level split that the frontend sees is unchanged. - **Consistency with 2B-i-1's decision.** 2B-i-1 settled the shape of the service call as "primitives down from controller, including access type". Splitting for watermarking but not for audit would make the service interface inconsistent with itself. The audit-log branching and the watermark branching therefore collapse onto the **same** parameter. One branch point in one method. ### 2.7 Updated `ReviewerPaperServiceImpl` — integrated flow **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/ReviewerPaperServiceImpl.java` New interface: ```java public interface ReviewerPaperService { /** * Returns the manuscript bytes of {@code paperId}, provided {@code reviewerId} * has an active review assignment. Metadata is stripped on both paths; the * download path additionally carries a reviewer-attributed watermark built * from {@code reviewerEmail}. * * Side effect: records a {@code paper_access_audit} row (fail-open; see * {@link PaperAccessAuditService#logAccess}). * * Why reviewerEmail is a parameter: the existing SecurityUtils exposes * getCurrentUserEmail() from the JWT-populated principal, so the controller * can pass the email as a primitive without an extra DB lookup. This * preserves D7 (primitives down from the controller, service stays * servlet- and security-context-agnostic). */ ManuscriptContent getManuscriptForReview( UUID reviewerId, String reviewerEmail, UUID paperId, PaperAccessType accessType, String ipAddress, String userAgent); } ``` Integrated body (sketch): ```java @Override @Transactional(readOnly = true) public ManuscriptContent getManuscriptForReview( UUID reviewerId, String reviewerEmail, UUID paperId, PaperAccessType accessType, String ipAddress, String userAgent) { // 1) Authorization gate — returns the matched review id. UUID reviewId = reviewAuthorizationService.assertReviewerCanAccess(reviewerId, paperId); // 2) Load paper. Paper paper = paperRepository.findById(paperId) .orElseThrow(() -> new ManuscriptNotFoundException(paperId)); String key = paper.getManuscriptKey(); if (key == null || key.isBlank()) { throw new ManuscriptNotFoundException(paperId); } // 3) Read + strip (same as 2A). byte[] raw; try { raw = storageService.download(key); } catch (StorageException e) { throw new ManuscriptNotFoundException(paperId); } byte[] stripped = pdfMetadataStripper.strip(raw); // 4) Watermark only on DOWNLOAD. Fail closed on any error. // reviewerEmail comes from the JWT principal via SecurityUtils in the // controller; no DB lookup needed here. byte[] bytes = (accessType == PaperAccessType.DOWNLOAD) ? pdfWatermarker.watermark(stripped, reviewerEmail, LocalDate.now()) : stripped; // 5) Audit (fail-open — never throws). paperAccessAuditService.logAccess( reviewId, reviewerId, paperId, accessType, ipAddress, userAgent); return new ManuscriptContent(bytes, paper.getReferenceNo(), MediaType.APPLICATION_PDF_VALUE); } ``` Notes: - The audit log call is placed **after** both stripping and watermarking succeed, and **before** the return. Reason: we audit successful serves. A 503 from watermarking is not a reviewer access event; it is an operational failure and is covered by the WARN log inside the watermarker's catch block plus the WARN on `PdfSanitizationException` that 2A already wires. Logging a failed attempt to `paper_access_audit` would dirty the attribution table. - **No new repository dependency.** `ReviewerPaperServiceImpl` does not gain `UserRepository`. The reviewer email is supplied as a primitive by the controller (via `SecurityUtils.getCurrentUserEmail()`, which reads it from the JWT-populated principal) rather than looked up per download. Its final dependency list is: `ReviewAuthorizationService`, `PaperRepository`, `StorageService`, `PdfMetadataStripper` (all from 2A), plus `PdfWatermarker` and `PaperAccessAuditService` (new in 2B-i). That's it. ### 2.8 Failure handling — fail closed Documented explicitly: - If `PdfWatermarker.watermark(...)` throws, it propagates as `PdfSanitizationException`, which 2A's `GlobalExceptionHandler` already maps to **503 `MANUSCRIPT_PREVIEW_UNAVAILABLE`** ("Preview not available. Please contact the editor."). - There is **no fallback** to serving the unwatermarked-but-stripped bytes. Serving an unwatermarked download silently defeats the entire purpose of watermarking — a leak later would point at nothing. An outright 503 forces the reviewer to surface the problem to the editor, which is the correct escalation path. - The existing `PdfSanitizationException` (from 2A) is reused — no new exception type. The message on the thrown exception distinguishes the two call sites ("Failed to watermark PDF" vs "Failed to sanitize PDF metadata") so log triage remains specific. ### 2.9 Performance characteristics Metadata stripping is O(1) on the `/Info` dictionary plus the XMP packet — microseconds regardless of page count. Watermarking, by contrast, is O(pages × font-render-cost): - **Small papers (5–20 pages):** ≤ 50 ms. Indistinguishable from stripping latency. - **Typical research papers (20–60 pages):** 50–150 ms. - **Long documents (100–300 pages):** 200–600 ms. - **Pathological (500+ pages, e.g., thesis or edge-case survey paper):** up to ~1 s. All estimates assume warm JVM and no image re-rasterization — `ColumnText.showTextAligned` draws vector text on the over-content layer and never touches the underlying page resources, so it does not scale with embedded image count. **DB work on the download path:** exactly one `paper_access_audit` insert (in a `REQUIRES_NEW` transaction), plus the authorization query and paper load that 2A already does. There is **no extra user lookup** — the reviewer email is supplied as a primitive from the JWT principal via the controller, not resolved from the database. The earlier draft of this design added a `userRepository.findById` to extract the email; it was removed after verifying that `SecurityUtils.getCurrentUserEmail()` already exposes the value from `CustomUserPrincipal`. **Recommendation for 2B-i: no optimization.** The expected p95 is well within acceptable interactive download latency for a single-click action. If production measurement ever surfaces a long-tail 500+ page thesis as a repeat offender, the mitigation is **not** caching (see next point) — it is stamping the watermark text into a pre-computed `PdfTemplate` once per request and reusing it per page, which is a local optimization inside `PdfWatermarker`, not an architectural change. ### 2.10 Caching — rejected Confirmed: **do not cache watermarked output.** Reasons: 1. **The watermark contains `LocalDate.now()`.** A cache entry written yesterday serves the wrong date today. Invalidation would have to be time-bucketed, and time-bucketed invalidation across midnight boundaries is exactly the kind of maintenance burden that gets neglected. 2. **The watermark contains the reviewer's email.** A cache key would have to include both `paperId` and `reviewerId` — per-reviewer per-paper — which collapses the cache hit rate toward zero for the exact workload we'd be trying to optimize. 3. **The stored-file-is-never-modified invariant (2A D5)** means every request re-reads from `StorageService`. A watermark cache would have to sit on top of a strip cache to be useful, doubling the invariant surface area. **Safer alternative if latency ever becomes a real problem:** cache the **stripped** (non-watermarked) bytes per `paperId` in a bounded Caffeine LRU — that's 2A's own deferred R3 mitigation — and re-run watermarking per request. Stripping is the expensive, cache-friendly step; watermarking must always be fresh. This keeps the date-correctness and reviewer-attribution invariants intact while cutting the storage-read + strip cost on the hot path. It is still explicitly out of scope for 2B-i. --- ## Part 2B-i-3 — Cross-Cutting Concerns ### 3.1 Controller update **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/controller/api/ReviewerPaperController.java` Both endpoints gain `HttpServletRequest request` as a method parameter (Spring handles this without any configuration; servlets do not need to be injected into the bean). IP and User-Agent extraction happens in a small private helper. The reviewer email is extracted via `SecurityUtils.getCurrentUserEmail()`, which already reads it from the JWT-populated `CustomUserPrincipal`: ```java @GetMapping("/{paperId}/manuscript") public ResponseEntity previewManuscript( @PathVariable UUID paperId, HttpServletRequest request) { UUID reviewerId = SecurityUtils.getCurrentUserId(); String reviewerEmail = SecurityUtils.getCurrentUserEmail(); // from JWT principal ManuscriptContent content = reviewerPaperService.getManuscriptForReview( reviewerId, reviewerEmail, paperId, PaperAccessType.PREVIEW, clientIp(request), userAgent(request)); return ResponseEntity.ok() .contentType(MediaType.APPLICATION_PDF) .header(HttpHeaders.CONTENT_DISPOSITION, ContentDisposition.inline() .filename("review-" + content.referenceNo() + ".pdf") .build().toString()) .body(new ByteArrayResource(content.bytes())); } // /manuscript/download is identical except: // - PaperAccessType.DOWNLOAD // - ContentDisposition.attachment() private static String clientIp(HttpServletRequest r) { String xff = r.getHeader("X-Forwarded-For"); if (xff != null && !xff.isBlank()) { int comma = xff.indexOf(','); return (comma < 0 ? xff : xff.substring(0, comma)).trim(); } return r.getRemoteAddr(); } private static String userAgent(HttpServletRequest r) { String ua = r.getHeader("User-Agent"); return ua == null ? null : (ua.length() > 500 ? ua.substring(0, 500) : ua); } ``` `HttpServletRequest` as a **method parameter** is Spring-standard and does not constitute "injection into the service layer" — it stays at the boundary. The IP helper handles `X-Forwarded-For` because the deployment will sit behind a reverse proxy (TLS termination). First segment of XFF is the originating client. ### 3.2 Impact on 2A files — exhaustive list Every 2A file that Phase 2B-i touches: | File | Change | |--------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------| | `service/ReviewerPaperService.java` | Signature change: add `String reviewerEmail`, `PaperAccessType accessType`, `String ipAddress`, `String userAgent` parameters. Javadoc updated to mention audit + watermark side effects. | | `service/impl/ReviewerPaperServiceImpl.java` | Branch on `accessType`, call `PdfWatermarker` on DOWNLOAD using the passed-in email, call `PaperAccessAuditService.logAccess` on both paths. No new repository dependencies beyond 2A (adds `PdfWatermarker`, `PaperAccessAuditService` as new bean deps only). Uses returned reviewId from authorization helper. | | `controller/api/ReviewerPaperController.java` | Add `HttpServletRequest` method parameters to both handlers, extract IP + User-Agent, extract reviewer email via `SecurityUtils.getCurrentUserEmail()`, pass accessType + primitives to the service. | | `service/security/ReviewAuthorizationService.java` | `assertReviewerCanAccess` return type: `void` → `UUID`. Javadoc updated. | | `service/security/ReviewAuthorizationServiceImpl.java` | Implementation updated: calls `findActiveReviewId`, unwraps Optional, throws on empty. | | `repository/ReviewRepository.java` | Replace `existsByPaper_IdAndReviewer_IdAndStatusIn` (planned in 2A but not yet implemented) with `findActiveReviewId` `@Query`. `isAssignedReviewer` in the authorization service becomes `.isPresent()` on top of that query. | **Not touched by 2B-i** (explicit confirmation): `PaperService.java`, `PaperServiceImpl.java`, `PaperController.java`, `PaperRepository.java`, `ReviewServiceImpl.submitReview` (its call site discards the new return value — no logic change), `PaperMapper.java`, `ManuscriptValidator.java`, `PdfMetadataStripper.java`, `GlobalExceptionHandler.java`, all `exception/*` classes. The entire 2A single-gateway rule for `PaperDetailResponse` and the upload validation are unaffected. **Not touched by 2B-i — frontend:** zero files. The frontend sees the same two URLs with the same response shape. The watermark appears on bytes, not in JSON; audit logging is server-internal. ### 3.3 Schema conflicts with 2A 2A makes **zero schema changes** (explicitly stated in §0.2 of `phase-2a-core-design.md` under Non-goals and confirmed by the 2A file inventory which lists no migration files). `V7__paper_access_audit.sql` is the first migration in the reviewer-paper-access feature line, and there is no overlap or ordering hazard with any pending 2A work. ### 3.4 Dependencies - **Backend:** **zero new dependencies.** OpenPDF 2.0.3 is already in `pom.xml` from 2A. `PdfGState`, `PdfContentByte`, `ColumnText`, `BaseFont`, `Phrase`, `Element`, `Font` are all under `com.lowagie.text.*` / `com.lowagie.text.pdf.*` in that artifact. Spring Data JPA and `java.time.LocalDate` are already on the classpath. - **Frontend:** **zero new dependencies.** No frontend files are touched in 2B-i. ### 3.5 Risk register — Phase 2B-i scope only | # | Risk | Likelihood | Impact | Mitigation | |----|----------------------------------------------------------------------------------------------------|------------|--------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | B1 | Audit insert fails (DB unreachable, constraint violation, unique-id collision, proxy resolution miss) | Low | Low | Fail-open: `logAccess(...)` catches everything internally, logs WARN with `reviewerId / paperId / accessType / e.getMessage()`, returns normally. `Propagation.REQUIRES_NEW` isolates the insert from the outer request transaction so a rollback cannot poison the download. Trade-off documented: incomplete trail is preferable to blocked access. | | B2 | Watermarking fails on an edge-case PDF (unusual producer, malformed page resources) | Low | Medium | Fail closed: throw `PdfSanitizationException` (reused from 2A) → 503 `MANUSCRIPT_PREVIEW_UNAVAILABLE`. Do **not** serve unwatermarked bytes. The `ManuscriptValidator` from 2A already rejects encrypted / non-PDF / corrupt uploads at ingest time, so the risk surface is narrower than it would be without 2A's upload gate. Distinct log message at the catch site ("Failed to watermark PDF") makes triage specific. | | B3 | Watermarking latency on 500+ page documents | Low | Low | Expected p95 under 1 s even on pathological inputs. No optimization in 2B-i. If production measurement flags a long tail: cache stripped (not watermarked) bytes in a bounded Caffeine LRU keyed by paperId, re-watermark per request. Absolutely no caching of watermarked output (see §2.10). | | B4 | `paper_access_audit` table grows unbounded | Medium | Low | **Flagged for later.** No rotation policy is defined in 2B-i. Expected growth is bounded by reviewer activity, which is low relative to e.g. page hits (a reviewer previews a handful of papers per week). Rough order of magnitude: with 100 active reviewers each accessing 5 papers/week with ~10 preview+download events each, that is ~5000 rows/week = ~250k rows/year. At that rate the table remains healthy for multiple years on the indexes specified in §1.2. A rotation/archival policy (partition-by-month, drop-after-3-years, or move-to-cold-storage) belongs in a later phase and is explicitly out of scope here. Flagged so it is not forgotten. | | B5 | Schema / migration risk — `V7` must be the first migration in this feature line and must not collide with any concurrently-in-flight migration | Low | High | V6 is the current tip per Phase 1 discovery. 2A introduces no migrations. If any other feature branch lands a V7 first, this migration renames to V8 (or higher). The CI Flyway-validate step on merge will surface a number conflict immediately — no silent failure mode. No seed data required; the table starts empty, indexes build instantly. | --- ## 4. Summary of committed decisions | Decision | Question | Committed choice | |---------:|----------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------| | **D7** | How do IP and User-Agent reach the service? | Primitives from the controller. No `HttpServletRequest` injected into services. | | **D8** | How does `ReviewerPaperServiceImpl` learn the active `reviewId` for audit logging? | `ReviewAuthorizationService.assertReviewerCanAccess` returns the `UUID` directly. | | **D9** | How does `getManuscriptForReview` branch between preview (no watermark) and download (watermark)? | Single method with a `PaperAccessType` parameter. Same parameter also drives audit type. | | **D10** | Which column in `users` is the watermark reviewer identifier? | `email` — the only column that is mandatory, unique, and attribution-meaningful without a DB lookup. Obtained from the JWT principal via `SecurityUtils.getCurrentUserEmail()`; no DB query on the download path. **Requires Phase 2B-ii disclosure in the confidentiality agreement — see §2.3.1.** | | **D11** | Dedicated `paper_access_audit` table or reuse `audit_log`? | Dedicated table. Typed columns, FK integrity on `review_id`, indexable query patterns, independent retention. | | **D12** | Cache watermarked output? | No. Date-sensitive, per-reviewer keyed, and would require invalidation logic nobody will maintain. | | **D13** | Watermark on preview? | No. Preview stays clean. Downloads carry the attribution signal because downloads are the leavable artifact. | | **D14** | On watermark failure, serve unwatermarked bytes as a fallback? | No. Fail closed with 503 `MANUSCRIPT_PREVIEW_UNAVAILABLE`. Reuses 2A's existing `PdfSanitizationException`. | | **D15** | Admin endpoints to query the audit log in 2B-i? | No. Explicitly deferred. | --- ## 5. File inventory (for Phase 3 implementation) **Backend — new:** - `src/main/resources/db/migration/V7__paper_access_audit.sql` - `entity/PaperAccessAudit.java` - `enums/PaperAccessType.java` - `repository/PaperAccessAuditRepository.java` - `service/PaperAccessAuditService.java` - `service/impl/PaperAccessAuditServiceImpl.java` - `util/PdfWatermarker.java` **Backend — modified (all 2A files):** - `service/ReviewerPaperService.java` (signature change — adds `reviewerEmail`, `accessType`, `ipAddress`, `userAgent`) - `service/impl/ReviewerPaperServiceImpl.java` (branching, watermark call using passed-in email, audit call; no new repository dependencies) - `controller/api/ReviewerPaperController.java` (`HttpServletRequest` method param, IP/UA extraction, `SecurityUtils.getCurrentUserEmail()` extraction, accessType passthrough) - `service/security/ReviewAuthorizationService.java` (return-type change: `void` → `UUID`) - `service/security/ReviewAuthorizationServiceImpl.java` (Optional unwrap + throw) - `repository/ReviewRepository.java` (swap `existsBy...` for `findActiveReviewId` `@Query`) **Frontend — new:** none **Frontend — modified:** none **Dependencies:** zero new (backend or frontend). --- ## 6. Cross-references - **Phase 1 discovery:** `docs/reviewer-paper-access/phase-1-discovery.md` - **Phase 2A core design:** `docs/reviewer-paper-access/phase-2a-core-design.md` - **ADR (single-gateway rule):** `docs/reviewer-paper-access/adr-paper-detail-authorization.md` - **Follow-up:** Phase 2B-ii (confidentiality agreement modal + annotated re-upload) — carries the forward constraint from §2.3.1