# Phase 2B-ii — Reviewer Paper Access: Confidentiality Agreement Modal **Status:** Approved (2026-04-12) **Builds on:** `phase-2a-core-design.md`, `phase-2b-i-audit-watermark-design.md`, `adr-paper-detail-authorization.md` **Satisfies:** forward constraint from `phase-2b-i-audit-watermark-design.md` §2.3.1 (informed consent for email-in-watermark data flow) **Scope:** Confidentiality agreement modal that gates download (not preview). Documentation only — no source changes. **Out of scope:** Annotated paper re-upload (deferred to a possible 2B-iii, not designed here). --- ## 0. Problem statement Phase 2B-i committed to embedding the reviewer's email address as a visible watermark on every downloaded PDF (D10). That is the right leak-attribution choice from the columns available in `users`, but it creates an undisclosed personal data flow: the reviewer has no way, today, to know that their email will leave the server in the bytes of every download they take. Under GDPR/DPDP that is not informed consent. 2B-i §2.3.1 explicitly flagged this as a hard dependency on 2B-ii: the confidentiality agreement modal designed here **must** disclose the watermark data flow before the reviewer's first download, and the download path must enforce acceptance server-side. Phase 2B-ii adds that gate. It does **not** touch the preview path, does **not** add any new dependency, and does **not** change any existing endpoint contract beyond adding a server-side check to the download path. ### 0.1 Goals - A reviewer sees the agreement modal on the first download attempt for a given paper. - The modal body contains verbatim disclosure of the email-watermark data flow. - Acceptance is recorded with timestamp, IP, and User-Agent. - Acceptance is one-per-(reviewer, paper), persistent, enforced by a database unique constraint. - The download endpoint rejects unaccepted attempts server-side, so the gate cannot be bypassed by direct API call. - Preview is unaffected — no agreement required. - Idempotent acceptance: repeat POSTs return the original `acceptedAt`, not a new one. ### 0.2 Non-goals - **Annotated paper re-upload** — explicitly out of scope for 2B-ii. Belongs to a possible 2B-iii. - **Agreement versioning** — if the text changes later, reviewers who accepted the old text are still bound by it. A future `agreement_version` column is flagged in the risk register but not built in this phase. - **Per-session re-acceptance** — the acceptance is permanent per (reviewer, paper). A reviewer who accepts once for paper X never sees the modal again for paper X. Per-session prompts would turn a consent record into a nag dialog and weaken its legal posture. - **Email-watermark opt-out** — the modal has no opt-out. A reviewer who cannot accept contacts the editor for reassignment (see §4.1 modal body). - **Auditing rejected downloads in `paper_access_audit`** — see §5.4. --- ## Part 2B-ii-1 — Database ### 1.1 Migration filename `shodh-sanchayan-api/src/main/resources/db/migration/V8__reviewer_paper_agreements.sql` V7 is 2B-i's `paper_access_audit`. V8 is the next available slot. If a concurrent branch lands V8 first, this migration renames to V9+ at merge time; Flyway-validate in CI catches the collision immediately. ### 1.2 Schema ```sql -- Per-reviewer-per-paper confidentiality agreement acceptance log. -- One row per acceptance. Immutable once written. -- Satisfies the informed-consent precondition for the email watermark -- introduced in docs/reviewer-paper-access/phase-2b-i-audit-watermark-design.md §2.3.1. CREATE TABLE reviewer_paper_agreements ( id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), reviewer_id UUID NOT NULL REFERENCES users(id), paper_id UUID NOT NULL REFERENCES papers(id), accepted_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), ip_address VARCHAR(45) NOT NULL, -- IPv6 max length; legal record of acceptance origin user_agent VARCHAR(500), CONSTRAINT uq_reviewer_paper_agreements_reviewer_paper UNIQUE (reviewer_id, paper_id) ); CREATE INDEX idx_reviewer_paper_agreements_reviewer ON reviewer_paper_agreements(reviewer_id); CREATE INDEX idx_reviewer_paper_agreements_paper ON reviewer_paper_agreements(paper_id); ``` Conventions matched: - UUID pk via `uuid_generate_v4()` (V1 pattern, also used by V7). - `TIMESTAMPTZ NOT NULL DEFAULT NOW()` for the acceptance moment. - IPv6 max-length `VARCHAR(45)` mirrors V7's `paper_access_audit.ip_address`. - User-Agent `VARCHAR(500)` mirrors V7. - Explicitly-named unique constraint `uq_reviewer_paper_agreements_reviewer_paper` so `DataIntegrityViolationException` can be interpreted without string-matching on the default generated name. ### 1.3 Why unique constraint, not just an index The idempotency pattern in §2.3 relies on the **database** rejecting the duplicate insert so the service layer can react to it. A non-unique index would let two parallel insertions succeed, producing two acceptance rows for the same (reviewer, paper) — which would then (a) contaminate admin queries that assume one row per acceptance, and (b) hand us two different `acceptedAt` values for the same consent event, which is exactly the shape a legal auditor will refuse to accept. The application layer **cannot** be the sole enforcement point because of the race condition covered in §2.3. DB is authoritative. --- ## Part 2B-ii-2 — Backend Components ### 2.1 Entity **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/entity/ReviewerPaperAgreement.java` ```java @Entity @Table( name = "reviewer_paper_agreements", uniqueConstraints = @UniqueConstraint( name = "uq_reviewer_paper_agreements_reviewer_paper", columnNames = {"reviewer_id", "paper_id"} ) ) @Getter @Setter @NoArgsConstructor @AllArgsConstructor @Builder public class ReviewerPaperAgreement { @Id @GeneratedValue(strategy = GenerationType.UUID) private UUID id; @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; @Column(name = "accepted_at", nullable = false) private Instant acceptedAt; @Column(name = "ip_address", nullable = false, length = 45) private String ipAddress; @Column(name = "user_agent", length = 500) private String userAgent; @PrePersist void prePersist() { if (acceptedAt == null) acceptedAt = Instant.now(); } } ``` Deliberately **does not** extend `BaseEntity` — this is an immutable consent record. `updatedAt` is meaningless and would be misleading: consent cannot be "edited", only revoked, and revocation is out of scope for 2B-ii. Matches 2B-i's choice to keep `PaperAccessAudit` off `BaseEntity` for the same reason. ### 2.2 Repository **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/repository/ReviewerPaperAgreementRepository.java` ```java @Repository public interface ReviewerPaperAgreementRepository extends JpaRepository { Optional findByReviewer_IdAndPaper_Id(UUID reviewerId, UUID paperId); boolean existsByReviewer_IdAndPaper_Id(UUID reviewerId, UUID paperId); } ``` `findByReviewer_IdAndPaper_Id` powers the status + idempotency paths. `existsByReviewer_IdAndPaper_Id` powers the fast gate check inside the download path; this stays as a separate method to preserve 2A/2B-i's pattern of "use `existsBy` when you only need a boolean". ### 2.3 Service **Interface** — `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/ReviewerPaperAgreementService.java` ```java public interface ReviewerPaperAgreementService { /** * Returns whether {@code reviewerId} has accepted the confidentiality * agreement for {@code paperId}. */ AgreementStatus getAgreementStatus(UUID reviewerId, UUID paperId); /** * Records the reviewer's acceptance. Idempotent: if an acceptance row * already exists for (reviewerId, paperId), returns the existing row's * acceptedAt unchanged. A repeat POST never produces a new timestamp * and never creates a duplicate row. * * The IP address and user agent of the FIRST successful acceptance are * the legally-meaningful record; subsequent calls do not overwrite them. */ AgreementStatus acceptAgreement(UUID reviewerId, UUID paperId, String ipAddress, String userAgent); /** * Throws {@link AgreementNotAcceptedException} if no acceptance row * exists for (reviewerId, paperId). Used to enforce the agreement gate * inside the download path. Fast path: backed by existsBy... — no * row materialization. */ void assertAgreementAccepted(UUID reviewerId, UUID paperId); } ``` **DTO** — `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/dto/AgreementStatus.java` ```java public record AgreementStatus(boolean accepted, Instant acceptedAt) { public static AgreementStatus notAccepted() { return new AgreementStatus(false, null); } public static AgreementStatus accepted(Instant acceptedAt) { return new AgreementStatus(true, acceptedAt); } } ``` Lives in `service/dto/` alongside 2A's `ManuscriptContent`. The same record is used internally and serialized directly as the HTTP response body — no separate response DTO because the record shape is already wire-safe (primitives + `Instant` which Jackson serializes as an ISO-8601 string by default in the existing configuration). **Implementation** — `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/ReviewerPaperAgreementServiceImpl.java` ```java @Service @RequiredArgsConstructor @Slf4j public class ReviewerPaperAgreementServiceImpl implements ReviewerPaperAgreementService { private final ReviewerPaperAgreementRepository repository; private final EntityManager entityManager; // for getReference() — avoids extra SELECTs @Override @Transactional(readOnly = true) public AgreementStatus getAgreementStatus(UUID reviewerId, UUID paperId) { return repository.findByReviewer_IdAndPaper_Id(reviewerId, paperId) .map(a -> AgreementStatus.accepted(a.getAcceptedAt())) .orElseGet(AgreementStatus::notAccepted); } @Override @Transactional public AgreementStatus acceptAgreement(UUID reviewerId, UUID paperId, String ipAddress, String userAgent) { // Idempotency pattern: attempt insert, let the DB be authoritative, // catch the uniqueness violation, re-read the winning row. // Explicitly NOT "select then insert" — that has a TOCTOU race. try { ReviewerPaperAgreement row = ReviewerPaperAgreement.builder() .reviewer(entityManager.getReference(User.class, reviewerId)) .paper(entityManager.getReference(Paper.class, paperId)) .ipAddress(ipAddress) .userAgent(truncate(userAgent, 500)) .build(); ReviewerPaperAgreement saved = repository.saveAndFlush(row); return AgreementStatus.accepted(saved.getAcceptedAt()); } catch (DataIntegrityViolationException e) { // Row already exists — race or genuine duplicate. Return the // winner's acceptedAt unchanged. The first acceptance wins; // its IP + UA are the legally-meaningful record. return repository.findByReviewer_IdAndPaper_Id(reviewerId, paperId) .map(a -> AgreementStatus.accepted(a.getAcceptedAt())) .orElseThrow(() -> // If we got a unique-violation but cannot find the row, // something is very wrong. Surface it. new IllegalStateException( "Unique violation on reviewer_paper_agreements but " + "no row found for reviewerId=" + reviewerId + " paperId=" + paperId, e)); } } @Override @Transactional(readOnly = true) public void assertAgreementAccepted(UUID reviewerId, UUID paperId) { if (!repository.existsByReviewer_IdAndPaper_Id(reviewerId, paperId)) { throw new AgreementNotAcceptedException( "Reviewer has not accepted the confidentiality agreement for this paper."); } } private static String truncate(String s, int max) { if (s == null) return null; return s.length() <= max ? s : s.substring(0, max); } } ``` **Why `saveAndFlush` instead of `save`:** `save` defers the actual INSERT until transaction commit or the next query. A deferred INSERT means `DataIntegrityViolationException` would be thrown later, potentially outside the `try` block, breaking the idempotency catch. `saveAndFlush` forces the INSERT to hit the DB inside the `try`, which is exactly what the race-safe pattern needs. **Why `getReference` instead of `findById`:** same reason as 2B-i's `PaperAccessAuditServiceImpl` — we already know the `reviewerId` and `paperId` are real (the controller ran `assertReviewerCanAccess` before calling us), so proxy-loading avoids two unnecessary SELECTs on the accept path. ### 2.4 Exception **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/exception/AgreementNotAcceptedException.java` ```java public class AgreementNotAcceptedException extends RuntimeException { public AgreementNotAcceptedException(String message) { super(message); } } ``` **GlobalExceptionHandler mapping:** | Exception | HTTP | Code | Message | |-----------------------------------|------|--------------------------|-------------------------------------------------------------------------| | `AgreementNotAcceptedException` | 403 | `AGREEMENT_NOT_ACCEPTED` | "You must accept the confidentiality agreement before downloading this paper." | **Why 403 and not 412:** 2A and 2B-i use HTTP status as a coarse category (403 = forbidden) and the `code` field on `ErrorResponse` as the precise discriminator. All reviewer-facing authorization rejections in the codebase today go through 403. 412 Precondition Failed would create a parallel pattern used by exactly one feature, which is the same anti-pattern 2A-D3 rejected for SpEL vs. service bean. Stay on 403. The frontend already has to branch on `code` for `FORBIDDEN` vs `AGREEMENT_NOT_ACCEPTED`, and branching on status + code is strictly easier than branching on status alone. ### 2.5 Idempotency pattern — committed, with race analysis **Committed pattern:** attempt-insert / catch / re-read, with `saveAndFlush` forcing the INSERT inside the try block and `existsByReviewer_IdAndPaper_Id` on the gate read. **Explicitly rejected:** "select then insert". **Race analysis, documented for the record:** - Thread A calls `acceptAgreement(r, p, ...)`. - Thread B calls `acceptAgreement(r, p, ...)` microseconds later — same reviewer, same paper, two browser tabs or a double-click. - Both threads run a `SELECT` that returns empty. - Both threads issue an `INSERT`. - First commit wins; second commit hits the unique constraint and rolls back. In the "select then insert" pattern both calls think they are the first and return their *own* freshly-generated timestamp, only one of which actually landed. The caller that lost the race returns a timestamp that does not correspond to any row in the database — an actively wrong result. The attempt-insert-catch-read pattern closes this hole because the loser's `INSERT` fails and the subsequent `SELECT` reads the actual winning row. This matters for audit/legal posture: the `acceptedAt` returned to the client must always be the real acceptedAt in the database, not a value generated client-side or transiently held. The DB is authoritative; the service just surfaces what it finds. --- ## Part 2B-ii-3 — API Endpoints ### 3.1 Endpoint 1 — GET agreement status **Handler:** added to existing `ReviewerPaperController` (same class that owns `/manuscript` and `/manuscript/download`). **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/controller/api/ReviewerPaperController.java` ```java @GetMapping("/{paperId}/agreement-status") public AgreementStatus getAgreementStatus(@PathVariable UUID paperId) { UUID reviewerId = SecurityUtils.getCurrentUserId(); // Same authorization gate as the download endpoints — unassigned // reviewers must not be able to probe agreement state for // arbitrary papers. reviewAuthorizationService.assertReviewerCanAccess(reviewerId, paperId); return reviewerPaperAgreementService.getAgreementStatus(reviewerId, paperId); } ``` **HTTP contract:** | Field | Value | |-----------|--------------------------------------------------------------------------------------------| | Method | `GET` | | Path | `/reviewer/papers/{paperId}/agreement-status` | | Auth | `hasAnyRole('REVIEWER','ADMIN')` (class-level from 2A) + `assertReviewerCanAccess` gate | | Response | `200 OK`, body `{ "accepted": boolean, "acceptedAt": "" \| null }` | | Errors | `401` unauthenticated · `403 FORBIDDEN` not an assigned reviewer · `404` paper not found (propagated from assertReviewerCanAccess if no review exists) | **Note:** a `GET` on agreement status intentionally does **not** bypass the authorization gate. A stranger probing `/reviewer/papers/{random-uuid}/agreement-status` must get `403 FORBIDDEN`, not `200 {accepted:false}`. Leaking "this paper exists but you have no agreement" is a minor enumeration vector and easy to avoid — just run the same `assertReviewerCanAccess` the download path uses. ### 3.2 Endpoint 2 — POST agreement accept ```java @PostMapping("/{paperId}/agreement-accept") public AgreementStatus acceptAgreement( @PathVariable UUID paperId, HttpServletRequest request) { UUID reviewerId = SecurityUtils.getCurrentUserId(); reviewAuthorizationService.assertReviewerCanAccess(reviewerId, paperId); return reviewerPaperAgreementService.acceptAgreement( reviewerId, paperId, clientIp(request), userAgent(request)); } ``` The `clientIp(request)` and `userAgent(request)` helpers are the ones introduced in 2B-i's controller (§3.1 of `phase-2b-i-audit-watermark-design.md`). They are already present on this controller by 2B-i; 2B-ii reuses them without modification. **HTTP contract:** | Field | Value | |-----------|-------------------------------------------------------------------------------------------------------------| | Method | `POST` | | Path | `/reviewer/papers/{paperId}/agreement-accept` | | Auth | `hasAnyRole('REVIEWER','ADMIN')` (class-level) + `assertReviewerCanAccess` gate | | Request body | None. The act of POSTing is the acceptance. | | Response | `200 OK`, body `{ "accepted": true, "acceptedAt": "" }` | | Idempotency | Repeat POSTs return the same `acceptedAt` — the first-acceptance timestamp. Subsequent POSTs are no-ops at the row level. | | Errors | `401` unauthenticated · `403 FORBIDDEN` not an assigned reviewer | **Why no request body:** there is nothing to parameterize. The agreement text is server-known; the reviewer is the JWT subject; the paper is the path variable; the IP and User-Agent are extracted from the request at the boundary. Adding a body would create the impression that reviewers can negotiate something, which is the exact opposite of what this endpoint is for. **Why `POST` and not `PUT`:** POST reflects the "create an acceptance event" semantics. PUT would imply the client sends the desired state, which invites "unaccept" semantics we don't want. No DELETE either — consent revocation is out of scope. The only verb that makes sense is "record that this happened right now", which is POST. ### 3.3 Server-side enforcement on download — the critical change The agreement gate is enforced in the service layer, not the controller, so the rule is centralized alongside the other access decisions and cannot be circumvented by a future controller that forgets a check. **File modified:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/ReviewerPaperServiceImpl.java` **New injected dependency:** `ReviewerPaperAgreementService`. **Placement:** inside `getManuscriptForReview(...)`, **after** `assertReviewerCanAccess` and **before** `paperRepository.findById`. Gate at the earliest point so a missing agreement costs zero storage reads, zero stripping cycles, and zero watermark CPU. The check runs **only** when `accessType == DOWNLOAD`. Integrated flow (only the delta from 2B-i §2.7 is shown in full; unchanged lines elided): ```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) Agreement gate — DOWNLOAD only. Preview is unaffected. // Earliest-possible placement: no storage read, no strip, no // watermark work happens unless the agreement is on file. if (accessType == PaperAccessType.DOWNLOAD) { reviewerPaperAgreementService.assertAgreementAccepted(reviewerId, paperId); } // 3) Load paper. (unchanged from 2B-i) Paper paper = paperRepository.findById(paperId) .orElseThrow(() -> new ManuscriptNotFoundException(paperId)); // ... rest unchanged from 2B-i §2.7 (read, strip, conditional watermark, audit, return) } ``` **Why preview is not gated:** - Preview bytes leave the server too, technically — but only as a streamed response that the reviewer's browser displays in place, not as a file the reviewer saves to disk. - The embedded email watermark (the thing the agreement discloses) applies only to downloads per 2B-i D13. - Gating preview would force the agreement modal on a user who is trying to *read* the paper, which is the whole point of having a reviewer. That is friction without safety benefit. - A sophisticated user could use browser devtools to save the preview bytes to disk, but those bytes are the stripped-but-unwatermarked version — i.e., they carry **no** attribution. That is a pre-existing property of the preview path (by 2A design), not a new hole. The agreement text reflects this accurately by disclosing specifically that downloads are watermarked. **Distinction documented for future maintainers:** preview is "server streams, browser renders, bytes do not persistently leave the browser unless the user deliberately extracts them". Download is "server writes a file into the user's filesystem, which then moves with the user". The agreement covers the second case because the second case is where bad outcomes (leaks, institutional re-upload, offline sharing) actually happen. ### 3.4 Interface change **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/ReviewerPaperService.java` No signature change. The agreement enforcement is a private concern of the implementation; callers of `getManuscriptForReview` do not need to know about it. They already pass `accessType`, which is the only input the gate needs. --- ## Part 2B-ii-4 — Frontend ### 4.1 New component — `ConfidentialityAgreementModal` **File:** `shodh-sanchayan-ui/src/components/reviewer/ConfidentialityAgreementModal.tsx` **Modal text (verbatim, from the prompt — this is the GDPR/DPDP disclosure and MUST NOT be paraphrased):** > **Confidentiality & Ethics Agreement** > > Before you can download this paper, please read and accept the following: > > By downloading this paper, you agree to: > > 1. **Use the content solely for the purpose of peer review.** You will not use any part of this paper — text, data, methodology, findings, or ideas — for personal benefit, third-party benefit, or any purpose beyond your assigned review. > 2. **Not share or distribute this paper.** You will not forward, upload, share, post, or republish this paper or any portion of it, in any form, to anyone, including colleagues, students, or institutional repositories. > 3. **Maintain confidentiality of the submission and its authors.** You will not discuss the existence, content, or status of this submission outside of the review process. > 4. **Delete all local copies after completing your review.** Once your review is submitted and final, you will delete every local copy of this paper from every device under your control. > > **Notice — Your email address is embedded in every download.** > Every PDF you download will be watermarked with your email address and the download date. This is used to trace leaked or shared copies back to their source. By proceeding, you understand that any copy of this paper that leaves your control will identify you as the source. > > If you do not accept these terms, click **Cancel** and the download will not proceed. If you cannot accept the email-watermark disclosure for personal, professional, or institutional reasons, please contact the editor to discuss alternative arrangements or reassignment of this review. > > \[Cancel] \[I Agree & Download] The closing paragraph deliberately routes a conscientious objector to **contact the editor**, not to "use the preview". Offering preview as the alternative would be coercive consent — the reviewer has a legitimate task that preview alone cannot support, so "use preview" is functionally "give up the part of the job that needed you in the first place". Reassignment is the real escape valve and keeps consent genuinely free. **Component contract:** ```tsx type Props = { isOpen: boolean; paperReferenceNo: string; // shown in the title bar: "Agreement — {refNo}" onAccept: () => Promise; // async; parent awaits and proceeds to download onCancel: () => void; // sync; no API call, modal just closes }; ``` **Internal state:** ```tsx const [submitting, setSubmitting] = React.useState(false); const [error, setError] = React.useState(null); ``` **Behavior:** - **On Accept click:** 1. `setSubmitting(true); setError(null);` 2. `try { await props.onAccept(); } catch (e) { setError(extractMessage(e)); setSubmitting(false); return; }` 3. Parent closes the modal on success via its own `isOpen` state transition. The modal does not close itself — the parent is the single owner of open/closed, which prevents the double-close-flash pattern where the modal unmounts mid-promise. - **On Cancel click:** `props.onCancel()` — no API call, no side effects. - **Accept button is disabled while `submitting === true`** — prevents the double-POST that would otherwise rely entirely on the service layer's idempotency as the only protection. Belt-and-suspenders. - **Error display:** if `onAccept` rejects, the modal shows an inline error message above the button row (``). The modal stays open so the reviewer can retry or cancel. - **No internal success state:** on success, the parent unmounts the modal by flipping `isOpen`. The modal never shows a "success!" screen — that would be friction between acceptance and the download the user actually wanted. - **Keyboard:** Esc triggers `onCancel` (Radix default). The Accept button is **not** auto-focused — the user should have to reach for it, matching the "deliberate act" shape the consent record is trying to capture. Focus lands on the Cancel button (or the close-X) on open, which is the safe default. ### 4.2 The shared modal primitive — commitment **Committed decision:** Create a thin shadcn-style wrapper `src/components/ui/dialog.tsx` on top of the already-installed `@radix-ui/react-dialog`. Use the wrapper in `ConfidentialityAgreementModal.tsx`. **Justification based on codebase discovery:** - `@radix-ui/react-dialog ^1.1.4` is already a direct dependency in `package.json`. Zero new packages. - `src/components/ui/README.md` explicitly lists `dialog.tsx` in the "to create" list. The intent to eventually have this wrapper is already recorded in the codebase; 2B-ii happens to be the first feature that needs it. - The codebase already uses multiple `@radix-ui/react-*` primitives directly (`react-dropdown-menu`, `react-avatar`, `react-tabs`, etc.), so the wrapper is not introducing a new category — it is finishing the scaffolding the repo was already set up for. - A shared wrapper means the next modal that lands (e.g., admin confirmation dialogs, reviewer "decline review" flows) reuses the same file and inherits consistent styling. **Wrapper scope** — standard shadcn `dialog.tsx` shape: `Dialog`, `DialogTrigger`, `DialogPortal`, `DialogOverlay`, `DialogContent`, `DialogHeader`, `DialogFooter`, `DialogTitle`, `DialogDescription`, `DialogClose`. Thin re-exports of the Radix primitives with `className` defaults applied via `cn(...)`. No logic beyond styling. **Rejected alternative:** use `@radix-ui/react-dialog` directly inside `ConfidentialityAgreementModal.tsx` with ad-hoc class names. This would work but creates a pattern no other modal in the codebase can follow, forcing a later feature to either copy the pattern or introduce the wrapper retroactively. Build the wrapper once, now, and have future modals pick it up for free. **Dependencies:** zero new. The primitive is already installed. The wrapper is code, not a package. ### 4.3 Modified component — `DownloadPaperButton` **File:** `shodh-sanchayan-ui/src/components/reviewer/DownloadPaperButton.tsx` (created by 2A, modified here) **Prop addition:** `paperReferenceNo: string` (needed for the modal title). 2A already passes it to the download API call, so the prop is already plumbed in from the parent `PaperPreviewPane` toolbar. **Click flow (replacing 2A's direct-to-download behavior):** ```tsx async function handleClick() { // Always re-check on click — the agreement status is authoritative // database state and a stale local cache could let a reviewer who // declined in one tab proceed in another. One round-trip per click // is an acceptable cost for an explicit consent gate. setChecking(true); try { const status = await reviewerPaperApi.getAgreementStatus(paperId); if (status.accepted) { await reviewerPaperApi.download(paperId, paperReferenceNo); return; } setModalOpen(true); // trigger agreement flow } catch (e) { toast.error(extractMessage(e)); // sonner, already in the codebase } finally { setChecking(false); } } async function handleModalAccept() { // Called by the modal's onAccept prop. Any error bubbles back to // the modal so it can surface inline without closing. await reviewerPaperApi.acceptAgreement(paperId); setModalOpen(false); await reviewerPaperApi.download(paperId, paperReferenceNo); } function handleModalCancel() { setModalOpen(false); } ``` **Component state:** - `checking: boolean` — disables the button while the pre-click status fetch is in flight. - `modalOpen: boolean` — controls the agreement modal visibility. Local to this component. **Deliberate non-decisions:** - **No caching of agreement status** — not in `sessionStorage`, not in `localStorage`, not in React Context, not in Zustand, not in TanStack Query. Every click re-reads. This is a product-level safety decision, not a performance oversight (see §4.4). - **No optimistic UI** for the Accept → Download handoff. The agreement must be server-confirmed before the download starts, because the server enforces the gate. An optimistic Download that fires before the accept response could race with the server and 403. ### 4.4 Pre-fetch optimization — recommendation: **defer** **Committed: do not pre-fetch in 2B-ii.** Ship with the one-round-trip-per-click behavior above. **Justification:** - The prompt allows pre-fetching on page load as an optimization, but the click handler would still have to re-check before showing the modal anyway (to catch cross-tab staleness), so pre-fetch only saves the *first* click per page load. Every subsequent click still round-trips. - The agreement status endpoint is a single indexed exists-check query — measured in single-digit milliseconds plus network RTT. On a warm keep-alive connection, the full click-to-modal latency is well under a frame. - Pre-fetching couples the review page's initial load to the agreement endpoint, introducing a new failure mode (agreement-status fetch fails on mount but the review form still needs to render). That failure mode has to be designed, tested, and maintained for a negligible UX win. - If measurement later shows that first-click latency is a real reviewer complaint, the pre-fetch is a five-line addition in Phase 3 or later — but there is no evidence it is needed now, and 2B-ii should not ship an optimization before the baseline is measured. ### 4.5 API client additions **File:** `shodh-sanchayan-ui/src/lib/api/client.ts` `reviewerPaperApi` is a 2A-planned namespace that does not yet exist in `client.ts`. 2A §1D.3 defines it with `previewBlobUrl(paperId)` and `download(paperId, referenceNo)`. 2B-ii extends the same namespace with two JSON methods: ```ts export type AgreementStatus = { accepted: boolean; acceptedAt: string | null; // ISO-8601 from the server, or null }; export const reviewerPaperApi = { // ...previewBlobUrl and download from 2A §1D.3... async getAgreementStatus(paperId: string): Promise { const res = await api.get( `/reviewer/papers/${paperId}/agreement-status`); return res.data; }, async acceptAgreement(paperId: string): Promise { const res = await api.post( `/reviewer/papers/${paperId}/agreement-accept`); return res.data; }, }; ``` Both methods use the existing `api` axios instance — the JWT interceptor at the top of `client.ts` already attaches `Authorization: Bearer` based on the request config (confirmed via reading `src/lib/api/client.ts`). No new client utilities. --- ## Part 2B-ii-5 — Cross-Cutting Concerns ### 5.1 Database migrations summary **New:** `V8__reviewer_paper_agreements.sql` — creates `reviewer_paper_agreements` table, unique constraint, and two btree indexes. Full SQL in §1.2. **Conflicts with V7 (2B-i):** none. V7 creates `paper_access_audit` with no dependencies on or from `reviewer_paper_agreements`. V8 creates a new table with FKs only to `users` and `papers`, both of which predate V7. Migration order is V1..V6 (pre-2A) → V7 (2B-i) → V8 (2B-ii). Flyway-validate in CI is the backstop against number collisions. **No schema impact on V7:** 2B-ii does not add or modify any column on `paper_access_audit`. ### 5.2 Dependency changes - **Backend:** zero new. Reuses JPA, Spring Transactional, the existing exception-handler pattern, and the IP/UA extraction helpers from 2B-i's controller. - **Frontend:** zero new packages. `@radix-ui/react-dialog` is already a direct dependency; the new `components/ui/dialog.tsx` wrapper is code, not a package. Sonner (toast) is already in use. No new client libraries. ### 5.3 Impact on files from 2A and 2B-i — exhaustive list **Backend — files modified by 2B-ii:** | File | Change | |-------------------------------------------------------------|----------------------------------------------------------------------------------------------------------| | `service/impl/ReviewerPaperServiceImpl.java` | Inject `ReviewerPaperAgreementService`. Add `assertAgreementAccepted` call inside `getManuscriptForReview` on the DOWNLOAD branch, placed between `assertReviewerCanAccess` and `paperRepository.findById`. No other change. | | `controller/api/ReviewerPaperController.java` | Add `getAgreementStatus` and `acceptAgreement` handlers. Inject `ReviewerPaperAgreementService` and `ReviewAuthorizationService` (the latter is already injected from 2B-i). Reuse the `clientIp`/`userAgent` helpers introduced by 2B-i. | | `exception/GlobalExceptionHandler.java` | Add `@ExceptionHandler(AgreementNotAcceptedException.class)` → 403 with code `AGREEMENT_NOT_ACCEPTED`. | **Backend — files NOT modified (explicit confirmation):** `service/ReviewerPaperService.java` (signature unchanged — gate is an impl concern), `PaperService.java`/`PaperServiceImpl.java`/`PaperController.java`, `ReviewAuthorizationService.java`/`Impl.java`, `ReviewRepository.java`, `PaperMapper.java`, `ManuscriptValidator.java`, `PdfMetadataStripper.java`, `PdfWatermarker.java`, `PaperAccessAuditService.java`/`Impl.java`, `PaperAccessAuditRepository.java`, `PaperAccessAudit.java`. None of these need changes. **Frontend — files modified:** | File | Change | |-----------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------| | `src/components/reviewer/DownloadPaperButton.tsx` (2A) | Replace direct-download click handler with the status-check / modal / accept / download flow in §4.3. Add `paperReferenceNo` prop wiring if not already present. | | `src/lib/api/client.ts` (2A extended) | Add `getAgreementStatus` and `acceptAgreement` to the `reviewerPaperApi` namespace. Add the `AgreementStatus` type. No changes to the axios instance or interceptors. | **Frontend — files created:** | File | Purpose | |-----------------------------------------------------------------------|-------------------------------------------------------------------------| | `src/components/ui/dialog.tsx` | Shadcn-style wrapper over `@radix-ui/react-dialog` — shared primitive. | | `src/components/reviewer/ConfidentialityAgreementModal.tsx` | The agreement modal component itself, using the wrapper. | **Frontend — files NOT modified (explicit):** `PaperPreviewPane.tsx` (2A — owns the preview, does not need to know about the agreement), `PaperPreviewError.tsx` (2A), `ReviewForm.tsx` (2A), `src/app/(dashboard)/reviewer/review/[id]/page.tsx` (2A — the click-flow change is localized inside `DownloadPaperButton`). ### 5.4 Audit log interaction — confirmed **Confirmed: do NOT write a `paper_access_audit` row when a download is rejected with `AGREEMENT_NOT_ACCEPTED`.** Reasoning, restated for the record: - `paper_access_audit` is 2B-i's attribution surface for successful file serves. Its purpose is "who saw which bytes". A 403 on the agreement gate is a pre-file event — no bytes were read, no stripping ran, no watermark was applied, no file left the server. - Mixing denied events into the same table dirties every admin query that counts `access_type = DOWNLOAD` as "a download happened". Investigators would have to distinguish "rejected" vs. "served" on every query, and that discrimination has no natural column — `paper_access_audit` has no "outcome" field and shouldn't, because its schema is tuned for "something actually happened". - The gate runs before the service method even loads the paper, so there is no `reviewId` resolution work to salvage the insert. - **If the product later wants "how many reviewers bailed at the agreement":** that is a separate metric, best captured as frontend analytics (the reviewer closed the modal via Cancel) or as a dedicated `agreement_interaction_log` table scoped to consent workflow. Not in 2B-ii. - **If the product later wants to detect abuse** (a reviewer who repeatedly tries the download endpoint without accepting): surface that from application logs or a dedicated WAF-style rate limit. `paper_access_audit` is the wrong tool. Pushback considered and rejected: "one could argue that recording the attempt is also legally useful evidence." Counter: the agreement record itself is the legally useful evidence, both when it exists (acceptance) and when it's absent (no acceptance). The 403 path produces no new information that isn't already implied by "no row in `reviewer_paper_agreements`". ### 5.5 Risk register — Phase 2B-ii scope only | # | Risk | Likelihood | Impact | Mitigation | |----|-------------------------------------------------------------------------------------------------|------------|--------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | C1 | Bypass via direct API call — a reviewer calls `GET /reviewer/papers/{id}/manuscript/download` directly without going through the modal | Medium | High | Enforcement is server-side in `ReviewerPaperServiceImpl.getManuscriptForReview` (§3.3), not in the controller or the frontend. The check runs before any file load. Controller-only enforcement would be one refactor away from a hole — the service is the single source of truth. | | C2 | Race condition on first acceptance — two parallel POSTs from the same reviewer for the same paper produce a duplicate row or return an inconsistent `acceptedAt` | Low | Medium | DB `UNIQUE(reviewer_id, paper_id)` is authoritative. `acceptAgreement` uses `saveAndFlush` inside a try block, catches `DataIntegrityViolationException`, re-reads the winning row, and returns that row's `acceptedAt`. "Select then insert" is explicitly rejected in §2.5. The concurrent-insert path is closed by the DB unique constraint, not by application-level locking. | | C3 | Stale agreement state across tabs — reviewer accepts in tab A, DB has acceptance; reviewer opens tab B where the React component's initial render thinks there is no acceptance, or vice versa for a decline | Medium | Low | Frontend rule: `DownloadPaperButton` re-fetches `getAgreementStatus` on every click (§4.3), regardless of any previously observed state. No caching in `sessionStorage`, `localStorage`, React context, Zustand, or TanStack Query — documented in §4.3 as a deliberate non-optimization. The cost is one round-trip per click; the safety benefit is that the DB is always the source of truth. | | C4 | Modal text drift — the agreement text changes in a later release (new regulation, new disclosure), but reviewers who accepted the old text are still recorded as having accepted | Medium | Medium | **Flagged as a known limitation of 2B-ii.** No `agreement_version` column is added in this phase because (a) it requires a text-versioning scheme we have not designed, (b) it forces a re-acceptance UX we have not designed, and (c) YAGNI applies until there is a concrete disclosure change pending. A future phase can add `agreement_version_id INT NOT NULL REFERENCES agreement_versions(id)` to `reviewer_paper_agreements` and a re-acceptance trigger ("text changed → agreement status for (reviewer, paper) returns accepted=false unless version matches"). Captured here so the gap is not forgotten. | --- ## 6. Summary of committed decisions | Decision | Question | Committed choice | |---------:|--------------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------| | **D16** | Where is the agreement gate enforced on the download path? | Service layer (`ReviewerPaperServiceImpl.getManuscriptForReview`), not the controller. Single source of truth for access rules. | | **D17** | At which point in the service method does the gate run? | After `assertReviewerCanAccess`, before `paperRepository.findById`. Earliest possible — saves storage read + strip cycle on rejection. | | **D18** | Does the agreement gate apply to preview? | No. Preview is clean, unwatermarked bytes; the agreement specifically discloses the download watermark, so there is nothing for the preview to disclose. | | **D19** | HTTP status for a missing agreement? | 403 with code `AGREEMENT_NOT_ACCEPTED`. Matches 2A convention (status = category, code = discriminator). Not 412. | | **D20** | Idempotency pattern for `acceptAgreement`? | `saveAndFlush` → catch `DataIntegrityViolationException` → re-read winning row. "Select then insert" explicitly rejected due to TOCTOU race (§2.5). | | **D21** | Does the GET agreement-status endpoint apply the same reviewer-access authorization as the download endpoint? | Yes. Unassigned reviewers probing `agreement-status` must get 403, not `{accepted:false}`. Prevents paper enumeration. | | **D22** | Is the frontend allowed to cache the agreement status? | No. Every click re-fetches. Rejects `sessionStorage`, `localStorage`, React context, Zustand, and TanStack Query as caches for this value. | | **D23** | Pre-fetch agreement status on review page load? | Deferred. The click handler would still need to re-check anyway (cross-tab staleness), so pre-fetch only saves the first click per page load. Ship without. | | **D24** | Shared modal primitive? | Create `src/components/ui/dialog.tsx` as a shadcn wrapper over the already-installed `@radix-ui/react-dialog`. Finishes the scaffolding the `components/ui/README.md` already declared. Zero new packages. | | **D25** | Does a rejected download (missing agreement) write a `paper_access_audit` row? | No. The audit table is for file-serve events, not gate rejections. Mixing would dirty `access_type = DOWNLOAD` counts. (§5.4) | | **D26** | Can a reviewer opt out of the email watermark? | No. The alternative path in the modal closing paragraph is "contact the editor for reassignment", which is a real escape valve and keeps consent genuinely free. No opt-out inside the modal. | | **D27** | Does repeat POST to `agreement-accept` update the `acceptedAt` or the IP/UA? | No. The FIRST acceptance is the legally-meaningful record; all subsequent POSTs return the original `acceptedAt` unchanged and do not overwrite IP or UA. (§2.3 Javadoc) | --- ## 7. File inventory (for Phase 3 implementation) **Backend — new:** - `src/main/resources/db/migration/V8__reviewer_paper_agreements.sql` - `entity/ReviewerPaperAgreement.java` - `repository/ReviewerPaperAgreementRepository.java` - `service/ReviewerPaperAgreementService.java` - `service/impl/ReviewerPaperAgreementServiceImpl.java` - `service/dto/AgreementStatus.java` - `exception/AgreementNotAcceptedException.java` **Backend — modified (across 2A + 2B-i files):** - `service/impl/ReviewerPaperServiceImpl.java` — inject `ReviewerPaperAgreementService`, add `assertAgreementAccepted` call on the DOWNLOAD branch. - `controller/api/ReviewerPaperController.java` — two new handlers (`getAgreementStatus`, `acceptAgreement`), inject `ReviewerPaperAgreementService`. Reuses existing `clientIp`/`userAgent` helpers. - `exception/GlobalExceptionHandler.java` — handler for `AgreementNotAcceptedException` (403 `AGREEMENT_NOT_ACCEPTED`). **Frontend — new:** - `src/components/ui/dialog.tsx` — shared shadcn wrapper. - `src/components/reviewer/ConfidentialityAgreementModal.tsx` **Frontend — modified:** - `src/components/reviewer/DownloadPaperButton.tsx` (2A) — click flow adds status check + modal + accept + download. - `src/lib/api/client.ts` — add `getAgreementStatus` and `acceptAgreement` to `reviewerPaperApi`, add `AgreementStatus` type. **Dependencies:** zero new (backend or frontend). --- ## 8. 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` - **Phase 2B-i audit + watermarking:** `docs/reviewer-paper-access/phase-2b-i-audit-watermark-design.md` (the §2.3.1 forward constraint this phase satisfies) - **ADR (single-gateway rule):** `docs/reviewer-paper-access/adr-paper-detail-authorization.md` - **Deferred / out of scope:** annotated paper re-upload (possible 2B-iii — not designed in 2B-ii).