# Phase 2A — Reviewer Paper Access: Core Design **Status:** Approved (2026-04-12) **Supersedes:** n/a **Informs:** Phase 2B (tests + rollout), Phase 3 (implementation) **Phase 1 input:** `docs/reviewer-paper-access/phase-1-discovery.md` --- ## 0. Problem statement Phase 1 discovery surfaced two production-grade defects in the reviewer workflow: 1. **Reviewers cannot read the paper they are reviewing.** `ReviewController` exposes no endpoint to fetch the manuscript bytes, and `PaperController.getById` returns a `PaperDetailResponse` that includes author/coauthor PII, so it cannot be reused for reviewers without a redesign. 2. **`GET /papers/{id}` leaks coauthor PII** to any authenticated caller. There is no authorization check that narrows the response by role, and no centralized helper that knows whether a caller is author, assigned reviewer, admin, or none of the above. Separately, discovery confirmed that the upload pipeline accepts any `MultipartFile` regardless of content — encrypted PDFs, truncated PDFs, and non-PDF files all reach `StorageService.upload(...)`. Once persisted, those files would cause the reviewer preview endpoint (introduced in this phase) to fail with a permanent 503, because PDF sanitization is the last step before response streaming. Fixing reviewer access without fixing upload validation would ship a known-broken preview path for already-accepted submissions. Phase 2A designs the core fix to all three issues. Phase 2A is **documentation only**; no source files are modified. ## 0.1 Goals - Give an assigned reviewer a preview and download of the manuscript, with PDF metadata stripped so reviewer identity cannot be inferred from `/Info` or XMP. - Close the PII leak on `GET /papers/{id}` by role-shaping the response at the service layer. - Centralize reviewer access checks in one helper so the rule can only change in one place. - Block non-PDF / encrypted / corrupt manuscripts at upload time so the reviewer preview endpoint is never asked to sanitize something that cannot be sanitized. - Keep the wire-level `PaperDetailResponse` shape unchanged so existing author and admin UIs keep working. ## 0.2 Non-goals - No server-side annotation, highlighting, or commenting on the PDF. - No watermarking with reviewer identity (out of scope for Phase 2A). - No caching layer in this phase. A Caffeine LRU is in the risk register as a follow-up *if* p95 stripping latency exceeds 2s. - No changes to admin paper detail flows beyond the shared `findByIdForCaller` rule. - No new dependencies (backend or frontend). --- ## 1. Approved design The design has four parts: - **1A — Reviewer paper access endpoints + service + PDF metadata stripping** - **1B — Authorization helper + `PaperService` single-gateway refactor** - **1C — Upload-time PDF validation** - **1D — Frontend reviewer review page rework** Revisions applied during review rounds (all three captured below, in §4): - R1 — Authorization boundary tightening (`findByIdForCaller` is the only DTO gateway) - R2 — Upload-time PDF validation added to Phase 2A scope - R3 — `ReviewServiceImpl.submitReview` must be refactored to use `ReviewAuthorizationService` --- ## 1A. Reviewer paper access — backend ### 1A.1 New controller **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/controller/api/ReviewerPaperController.java` ```java @RestController @RequestMapping("/reviewer/papers") @PreAuthorize("hasAnyRole('REVIEWER','ADMIN')") @RequiredArgsConstructor public class ReviewerPaperController { private final ReviewerPaperService reviewerPaperService; private final AuthenticatedUserResolver authenticatedUserResolver; // existing /** * Inline preview of the (metadata-stripped) manuscript for an assigned reviewer. * Returns application/pdf with Content-Disposition: inline. */ @GetMapping("/{paperId}/manuscript") public ResponseEntity previewManuscript(@PathVariable UUID paperId) { UUID reviewerId = authenticatedUserResolver.currentUserId(); ManuscriptContent content = reviewerPaperService.getManuscriptForReview(reviewerId, paperId); 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())); } /** * Attachment download of the (metadata-stripped) manuscript for an assigned reviewer. * Delegates to the same service method; differs only in Content-Disposition. */ @GetMapping("/{paperId}/manuscript/download") public ResponseEntity downloadManuscript(@PathVariable UUID paperId) { UUID reviewerId = authenticatedUserResolver.currentUserId(); ManuscriptContent content = reviewerPaperService.getManuscriptForReview(reviewerId, paperId); return ResponseEntity.ok() .contentType(MediaType.APPLICATION_PDF) .header(HttpHeaders.CONTENT_DISPOSITION, ContentDisposition.attachment() .filename("review-" + content.referenceNo() + ".pdf") .build().toString()) .body(new ByteArrayResource(content.bytes())); } } ``` **HTTP contract:** | Method | Path | Auth | Success | Errors | |--------|-----------------------------------------|-----------------|------------|------------------------------------------------------------------------| | GET | `/reviewer/papers/{id}/manuscript` | REVIEWER, ADMIN | 200 PDF | 401, 403 `FORBIDDEN`, 404 `MANUSCRIPT_NOT_FOUND`, 503 `MANUSCRIPT_PREVIEW_UNAVAILABLE` | | GET | `/reviewer/papers/{id}/manuscript/download` | REVIEWER, ADMIN | 200 PDF | same as above | **Why two endpoints (not one with `?disposition=inline|attachment`):** The existing `MagazineController` already uses the two-endpoint pattern (`.../pdf` for inline, `.../pdf/download` for attachment). No controller in the codebase currently switches Content-Disposition on a query parameter. Matching the established pattern keeps routing rules and security config uniform. ### 1A.2 New service interface **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/ReviewerPaperService.java` ```java public interface ReviewerPaperService { /** * Returns the manuscript bytes of {@code paperId} with PDF metadata stripped, * provided {@code reviewerId} has an active review assignment on that paper * (PENDING, IN_PROGRESS, or COMPLETED; DECLINED and EXPIRED are not active). * * The stored file is never modified. Stripping happens on an in-memory copy. * * @throws ForbiddenException if the reviewer has no active assignment * @throws ManuscriptNotFoundException if the paper has no manuscript in storage * @throws PdfSanitizationException if OpenPDF cannot parse/stamp the file */ ManuscriptContent getManuscriptForReview(UUID reviewerId, UUID paperId); } ``` ### 1A.3 New service implementation **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/ReviewerPaperServiceImpl.java` ```java @Service @RequiredArgsConstructor @Slf4j public class ReviewerPaperServiceImpl implements ReviewerPaperService { private final ReviewAuthorizationService reviewAuthorizationService; private final PaperRepository paperRepository; private final StorageService storageService; private final PdfMetadataStripper pdfMetadataStripper; @Override public ManuscriptContent getManuscriptForReview(UUID reviewerId, UUID paperId) { // 1) Authorization gate — first line of the method, no early work above it. 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 bytes via existing StorageService — stored file is never touched. byte[] raw; try { raw = storageService.download(key); } catch (StorageException e) { throw new ManuscriptNotFoundException(paperId); } // 4) Strip PDF metadata on an in-memory copy. byte[] stripped; try { stripped = pdfMetadataStripper.strip(raw); } catch (PdfSanitizationException e) { log.warn("PDF sanitization failed for paper {}", paperId, e); throw e; } return new ManuscriptContent( stripped, paper.getReferenceNo(), MediaType.APPLICATION_PDF_VALUE ); } } ``` ### 1A.4 Service DTO **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/dto/ManuscriptContent.java` ```java public record ManuscriptContent(byte[] bytes, String referenceNo, String contentType) { } ``` ### 1A.5 PDF metadata stripper **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/util/PdfMetadataStripper.java` Uses OpenPDF 2.0.3 (already in `pom.xml` for magazine generation; no new dependency). License: **LGPL 2.1 / MPL 1.1** — not AGPL or GPL, so no copyleft concern for this project. ```java @Component public class PdfMetadataStripper { /** * Returns a copy of {@code raw} with PDF metadata stripped: * - /Info dictionary: Title, Author, Subject, Keywords, Creator, CreationDate, ModDate blanked * - Producer forced to "Shodh Sanchayan" * - XMP metadata packet removed * * The input bytes are never mutated. The returned byte[] is a new array. * * @throws PdfSanitizationException on any parse / stamp failure */ public byte[] strip(byte[] raw) { try (ByteArrayOutputStream out = new ByteArrayOutputStream(raw.length)) { PdfReader reader = new PdfReader(raw); PdfStamper stamper = new PdfStamper(reader, out); Map info = new HashMap<>(); info.put("Title", ""); info.put("Author", ""); info.put("Subject", ""); info.put("Keywords", ""); info.put("Creator", ""); info.put("Producer", "Shodh Sanchayan"); info.put("CreationDate", ""); info.put("ModDate", ""); stamper.setInfoDictionary(info); // Wipe XMP packet stamper.setXmpMetadata(new byte[0]); stamper.close(); reader.close(); return out.toByteArray(); } catch (IOException | RuntimeException e) { throw new PdfSanitizationException("Failed to sanitize PDF metadata", e); } } } ``` **Why OpenPDF (not PDFBox):** - Already on the classpath for magazine PDF generation (see `project_pdf_generation.md`). - LGPL 2.1 / MPL 1.1 — compatible with this codebase's licensing posture. - `PdfStamper.setInfoDictionary` + `setXmpMetadata(new byte[0])` is the minimal, well-documented path for stripping both the `/Info` dictionary and the XMP packet in one pass. **Note on OpenPDF 2.0.3 API reconciliation (Phase 3b-i, 2026-04-12):** The original draft of this section used `stamper.setMoreInfo(info)` (the iText 2.x name) and `null` values for `CreationDate` / `ModDate`. During Phase 3b-i implementation, `mvn clean compile` surfaced that OpenPDF 2.0.3's `PdfStamper` exposes the method as `setInfoDictionary(Map)` — `setMoreInfo` does not exist in the OpenPDF fork. The design was reconciled against the actual library: - Method name → `setInfoDictionary` - `CreationDate` / `ModDate` values → empty strings instead of `null`, to avoid any NPE risk inside OpenPDF's iteration of the map. Empty strings produce the same security outcome (date fields effectively cleared) while matching the `Map` signature strictly. The `Map` parameter type and the set of stripped keys are unchanged from the original design. --- ## 1B. Authorization helper + `PaperService` single-gateway refactor ### 1B.1 New authorization service **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/security/ReviewAuthorizationService.java` ```java public interface ReviewAuthorizationService { boolean isAuthor(UUID userId, UUID paperId); boolean isAssignedReviewer(UUID userId, UUID paperId); /** * Throws {@link ForbiddenException} if {@code reviewerId} is not an * assigned reviewer on {@code paperId} with an active assignment * (PENDING, IN_PROGRESS, or COMPLETED). */ void assertReviewerCanAccess(UUID reviewerId, UUID paperId); } ``` ```java @Service @RequiredArgsConstructor public class ReviewAuthorizationServiceImpl implements ReviewAuthorizationService { private static final Set ACTIVE_STATUSES = EnumSet.of(ReviewStatus.PENDING, ReviewStatus.IN_PROGRESS, ReviewStatus.COMPLETED); private final ReviewRepository reviewRepository; private final PaperRepository paperRepository; @Override public boolean isAuthor(UUID userId, UUID paperId) { return paperRepository.existsByIdAndSubmittedBy_Id(paperId, userId); } @Override public boolean isAssignedReviewer(UUID userId, UUID paperId) { return reviewRepository.existsByPaper_IdAndReviewer_IdAndStatusIn( paperId, userId, ACTIVE_STATUSES); } @Override public void assertReviewerCanAccess(UUID reviewerId, UUID paperId) { if (!isAssignedReviewer(reviewerId, paperId)) { throw new ForbiddenException("Reviewer is not assigned to this paper"); } } } ``` **Why service bean and not custom SpEL (`@PreAuthorize("@reviewAuth.canAccess(...)")`):** No controller in the codebase currently uses custom SpEL method security expressions. Adding one here would introduce a pattern nobody else follows. A plain Spring service called from the controller/service body keeps review and debugging uniform, and gives us an explicit call-site we can grep for. **Why `PENDING / IN_PROGRESS / COMPLETED` and not `EnumSet.complementOf(DECLINED, EXPIRED)`:** Positive enumeration. If a new `ReviewStatus` is introduced later, reviewers must not silently gain manuscript access on the new status — the developer adding the status must come here and decide. ### 1B.2 New repository methods **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/repository/ReviewRepository.java` ```java boolean existsByPaper_IdAndReviewer_IdAndStatusIn(UUID paperId, UUID reviewerId, Collection statuses); ``` **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/repository/PaperRepository.java` ```java boolean existsByIdAndSubmittedBy_Id(UUID id, UUID submittedById); ``` ### 1B.3 `PaperService` single-gateway refactor The central rule of this phase: > **Any `PaperDetailResponse` returned from a paper id MUST go through `PaperService.findByIdForCaller(UUID paperId, UUID callerId)`.** There is no other public path. Changes: **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/PaperService.java` - **Remove** `PaperDetailResponse findById(UUID paperId)`. - **Add:** ```java /** * Returns a {@link PaperDetailResponse} for {@code paperId}, shaped by the * caller's relationship to the paper: * * - Admin → full detail * - Author of paper → full detail * - Assigned reviewer → reviewer-safe detail (coauthors, filenames, author * names, internal admin fields, publication metadata * are nulled — see PaperMapper.toReviewerSafeDetail) * - Anyone else → ForbiddenException * * This is the ONLY public service method that resolves a paper id to a * PaperDetailResponse. Controllers MUST NOT call any other path (see * docs/reviewer-paper-access/adr-paper-detail-authorization.md). */ PaperDetailResponse findByIdForCaller(UUID paperId, UUID callerId); ``` **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/PaperServiceImpl.java` - Delete private `toDetailResponse(Paper)` helper. - `submit`, `uploadRevision`, `updatePaper` → return via `findByIdForCaller(paper.getId(), authorId)` instead of the old private helper. - Implementation sketch: ```java @Override @Transactional(readOnly = true) public PaperDetailResponse findByIdForCaller(UUID paperId, UUID callerId) { Paper paper = paperRepository.findById(paperId) .orElseThrow(() -> new PaperNotFoundException(paperId)); User caller = userRepository.findById(callerId) .orElseThrow(() -> new UnauthorizedException("Unknown caller")); boolean isAdmin = caller.hasRole(Role.ADMIN); boolean isAuthor = paper.getSubmittedBy().getId().equals(callerId); boolean isReviewer = !isAdmin && !isAuthor && reviewAuthorizationService.isAssignedReviewer(callerId, paperId); if (isAdmin || isAuthor) { return paperMapper.toDetail(paper); } if (isReviewer) { return paperMapper.toReviewerSafeDetail(paper); } throw new ForbiddenException("Not permitted to view this paper"); } ``` ### 1B.4 `PaperMapper` split **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/PaperMapper.java` (new `@Component`) ```java @Component @RequiredArgsConstructor public class PaperMapper { private final PaperStatusHistoryRepository paperStatusHistoryRepository; /** Full detail — for admin and author. */ public PaperDetailResponse toDetail(Paper paper) { ... } /** Reviewer-safe detail — sensitive fields nulled. */ public PaperDetailResponse toReviewerSafeDetail(Paper paper) { ... } } ``` **Reviewer-safe DTO — fields NULLED / zero / empty:** - `coauthors` (empty list) - `submittedByName` - `manuscriptName` (original uploaded filename — contains author surname in practice) - `doi`, `volume`, `issue`, `pageStart`, `pageEnd`, `acceptedAt`, `publishedAt` - `adminNotes` - `plagiarismScore`, `plagiarismReport` - `citationCount` (zero), `downloadCount` (zero) - `requiredReviewers` (zero), `statusHistory` (empty list) — workflow metadata; leaks editorial timeline **Reviewer-safe DTO — fields INCLUDED:** - `id`, `referenceNo` - `titleHi`, `titleEn` - `abstractHi`, `abstractEn` - `keywords` - `categoryNameHi`, `categoryNameEn` - `status` - `submittedAt` - `manuscriptSize` The wire shape (JSON keys) is identical to the admin/author response. Only the values differ. This keeps existing frontend TypeScript types and existing author/admin UIs working without changes. **Note on 3b-ii implementation reconciliation (2026-04-12):** When Phase 3b-ii implementation began, two design-reality gaps surfaced against the original sketch above and were resolved with explicit user approval before proceeding: 1. **PaperMapper did not previously exist.** The original sketch referenced `PaperMapper.toDetail` / `toReviewerSafeDetail` as if the class already existed. In reality, detail mapping lived inline as a private method `PaperServiceImpl.toDetailResponse(Paper)` (no `mapper/` package or `PaperMapper` class anywhere in the tree). Resolution: create new `service/PaperMapper.java` as a plain `@Component` (not an interface — the existing codebase uses hand-written `@Component` mappers, not MapStruct, and no other `mapper/` package exists), move the private helper's logic verbatim into `toDetail(Paper)`, add `toReviewerSafeDetail(Paper)`, inject into `PaperServiceImpl`, delete the private helper. Final path: `service/PaperMapper.java` (not `mapper/PaperMapper.java`). 2. **`PaperDetailResponse` was missing fields referenced by the nulled/included lists above.** The live DTO at the time of 3b-ii did not have `submittedByName`, `volume`, `issue`, `pageStart`, `pageEnd`, `acceptedAt`, `adminNotes`, `plagiarismReport`, `manuscriptSize`, `createdAt`, `updatedAt`, or `subcategory*` fields. Resolution: additively expand `PaperDetailResponse` with the 9 new fields needed for the nulled list + `manuscriptSize` (`submittedByName`, `volume`, `issue`, `pageStart`, `pageEnd`, `acceptedAt` as `Instant`, `adminNotes`, `plagiarismReport`, `manuscriptSize` as `Long`). This is additive and backwards-compatible — existing author/admin JSON responses gain 9 keys, which existing Lombok `@Data @Builder` consumers tolerate without change. **Fields deferred:** `createdAt`, `updatedAt`, and `subcategory` from the included list above were NOT added to the DTO. Rationale: no UI need has surfaced for them, the `BaseEntity` auditing timestamps are not read by any existing frontend consumer, and the subcategory enrichment would require deciding between name fields and id fields. These can land additively in a later phase if a UI need surfaces. The reviewer-safe policy still meets its security goal (PII stripped) without them. **Type note on `acceptedAt`:** added as `java.time.Instant` to match the `Paper.acceptedAt` entity field. The existing `submittedAt`/`publishedAt` fields on the DTO are instead `String` formatted via `DateTimeFormatter`, which creates a minor wire-format inconsistency (Jackson serializes `Instant` as ISO-8601 by default). This inconsistency is tolerated at 3b-ii scope; a later phase can unify to a single format if it becomes visible on the frontend. **Reconciliations applied to the `findByIdForCaller` implementation sketch at §1B.3** during the same phase: - `caller.hasRole(Role.ADMIN)` → `caller.getRole() == UserRole.ADMIN` (no `hasRole` method on `User`; no `Role` enum — the codebase uses `UserRole.ADMIN`). - `paperRepository.findById(...).orElseThrow(() -> new PaperNotFoundException(paperId))` → `ResourceNotFoundException("Paper not found with id: " + paperId)` (existing codebase idiom used by every other `findById` call in `PaperServiceImpl`; no `PaperNotFoundException` class exists). - `userRepository.findById(...).orElseThrow(() -> new UnauthorizedException("Unknown caller"))` → `ForbiddenException("Unknown caller")` (no `UnauthorizedException` class exists; the only way caller lookup can fail is a stale JWT or a deleted user, for which 403 is the appropriate HTTP category). ### 1B.5 `PaperController.getById` **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/controller/api/PaperController.java` ```java @GetMapping("/{id}") @PreAuthorize("isAuthenticated()") public PaperDetailResponse getById(@PathVariable UUID id) { UUID callerId = authenticatedUserResolver.currentUserId(); return paperService.findByIdForCaller(id, callerId); } ``` No other controller changes are needed for the single-gateway rule — `AdminPaperController` has its own admin-only flow and continues to use its existing admin-scoped service method. ### 1B.6 `ReviewServiceImpl.submitReview` refactor (R3 — required) **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/ReviewServiceImpl.java` The current implementation contains an inline check of the form "reviewer assigned to this review". That inline block is **replaced** by: ```java reviewAuthorizationService.assertReviewerCanAccess(reviewerId, review.getPaper().getId()); ``` This eliminates duplicate enforcement and makes `ReviewAuthorizationService` the single source of truth for "can this reviewer touch this paper". If we change the active-status set, we change it in one file. --- ## 1C. Upload-time PDF validation **Why this is in Phase 2A scope (R2):** If we ship reviewer preview without upload validation, a single encrypted PDF uploaded by an author will cause the preview endpoint to permanently 503 for the assigned reviewer, with no way to fix it short of admin intervention. Stripping cannot succeed on an encrypted PDF. The only sound design is "refuse bad PDFs at upload time", and that has to happen before Phase 3 ships the preview endpoint. ### 1C.1 New validator **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/util/ManuscriptValidator.java` ```java @Component public class ManuscriptValidator { private static final byte[] PDF_MAGIC = { '%', 'P', 'D', 'F', '-' }; @Value("${spring.servlet.multipart.max-file-size}") private DataSize maxFileSize; /** * Validates a manuscript upload. On any failure, throws * {@link InvalidManuscriptException} with one of the following codes: * * MANUSCRIPT_EMPTY – file is null or zero bytes * MANUSCRIPT_TOO_LARGE – exceeds spring.servlet.multipart.max-file-size * MANUSCRIPT_UNREADABLE – IOException reading the multipart bytes * MANUSCRIPT_NOT_PDF – first 5 bytes are not "%PDF-" * MANUSCRIPT_ENCRYPTED – PdfReader.isEncrypted() returned true * MANUSCRIPT_CORRUPT – PdfReader threw on parse */ public void validate(MultipartFile file) { if (file == null || file.isEmpty()) { throw new InvalidManuscriptException("MANUSCRIPT_EMPTY", "No manuscript file was provided."); } if (file.getSize() > maxFileSize.toBytes()) { throw new InvalidManuscriptException("MANUSCRIPT_TOO_LARGE", "Manuscript exceeds the maximum allowed size."); } byte[] bytes; try { bytes = file.getBytes(); } catch (IOException e) { throw new InvalidManuscriptException("MANUSCRIPT_UNREADABLE", "Could not read the uploaded file."); } // 1) Magic-byte check — do NOT trust extension or Content-Type header. if (bytes.length < PDF_MAGIC.length || !Arrays.equals(Arrays.copyOf(bytes, PDF_MAGIC.length), PDF_MAGIC)) { throw new InvalidManuscriptException("MANUSCRIPT_NOT_PDF", "File is not a PDF."); } // 2) Encryption + 3) parse-clean check via OpenPDF PdfReader reader = null; try { reader = new PdfReader(bytes); if (reader.isEncrypted()) { throw new InvalidManuscriptException("MANUSCRIPT_ENCRYPTED", "Encrypted PDFs are not accepted. Please remove the password and re-upload."); } } catch (InvalidManuscriptException e) { throw e; } catch (Exception e) { throw new InvalidManuscriptException("MANUSCRIPT_CORRUPT", "The PDF could not be parsed. Please re-export and re-upload."); } finally { if (reader != null) reader.close(); } } } ``` ### 1C.2 Wiring **File:** `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/service/impl/PaperServiceImpl.java` Both `submit(...)` and `uploadRevision(...)` call `manuscriptValidator.validate(file)` **before** `storageService.upload(...)`. Validation failures must prevent the storage write; there must be no path where an invalid file reaches `StorageService`. ### 1C.3 Error response Failures from `ManuscriptValidator` surface as HTTP 400 with the standard `ErrorResponse{code, message}` body produced by `GlobalExceptionHandler`. --- ## 1D. Frontend — reviewer review page rework ### 1D.1 Page restructure **File:** `shodh-sanchayan-ui/src/app/(dashboard)/reviewer/review/[id]/page.tsx` The page is restructured into a two-pane layout. The existing review form is extracted verbatim into its own component (§1D.2) and reused — no form logic changes in this phase. ```tsx // simplified sketch export default function ReviewerReviewPage({ params }: { params: { id: string } }) { const paperId = params.id; return (
); } ``` **Mobile:** same grid, collapsed to a single column (`grid-cols-1` only). No separate mobile route. ### 1D.2 New components All under `shodh-sanchayan-ui/src/components/reviewer/`: | Component | Responsibility | |-----------------------|--------------------------------------------------------------------------------------------------------| | `PaperPreviewPane.tsx` | react-pdf viewer. Prev/Next navigation. Page indicator (`3 / 24`). Zoom 100/125/150%. Fetches via `reviewerPaperApi.previewBlobUrl(paperId)`. Revokes the object URL on unmount. | | `PaperPreviewError.tsx`| Fallback when preview fetch / render fails. Shows a human-readable message, a Retry button, and a Download button (same API as the toolbar download). | | `DownloadPaperButton.tsx` | Calls `reviewerPaperApi.download(paperId, referenceNo)`. Triggers a browser download with filename `review-{referenceNo}.pdf`. | | `ReviewForm.tsx` | The existing review form body, extracted verbatim from the page file. No logic changes. | ### 1D.3 API client additions **File:** `shodh-sanchayan-ui/src/lib/api/client.ts` ```ts export const reviewerPaperApi = { async previewBlobUrl(paperId: string): Promise { const res = await apiClient.get(`/reviewer/papers/${paperId}/manuscript`, { responseType: 'blob', }); return URL.createObjectURL(res.data); }, async download(paperId: string, referenceNo: string): Promise { const res = await apiClient.get(`/reviewer/papers/${paperId}/manuscript/download`, { responseType: 'blob', }); const url = URL.createObjectURL(res.data); const a = document.createElement('a'); a.href = url; a.download = `review-${referenceNo}.pdf`; a.click(); URL.revokeObjectURL(url); }, }; ``` The existing axios interceptor already attaches `Authorization: Bearer ` based on the request config — no auth wiring changes are needed for these two calls. ### 1D.4 react-pdf worker ```ts import workerSrc from 'pdfjs-dist/build/pdf.worker.min.mjs?url'; pdfjs.GlobalWorkerOptions.workerSrc = workerSrc; ``` Next.js 14 handles the `?url` import natively. No new package. ### 1D.5 Component tree ``` reviewer/review/[id]/page.tsx ├── PaperPreviewPane │ ├── (react-pdf Document/Page) │ ├── toolbar: prev / next / indicator / zoom / DownloadPaperButton │ └── (on error) └── ReviewForm └── (existing form body, extracted verbatim) ``` --- ## 2. Backend error handling additions **Files under `shodh-sanchayan-api/src/main/java/com/shodh/sanchayan/exception/`:** | Exception | HTTP | Code | Message | |---------------------------------|------|----------------------------------|----------------------------------------------------------| | `ForbiddenException` | 403 | `FORBIDDEN` | "You are not permitted to perform this action." | | `ManuscriptNotFoundException` | 404 | `MANUSCRIPT_NOT_FOUND` | "Manuscript not found for this paper." | | `PdfSanitizationException` | 503 | `MANUSCRIPT_PREVIEW_UNAVAILABLE` | "Preview not available. Please contact the editor." | | `InvalidManuscriptException` | 400 | (see §1C.1) | (see §1C.1) | All handled via `GlobalExceptionHandler` returning the standard `ErrorResponse{code, message}` body. --- ## 3. Design decisions committed **D1 — Preview vs download: two endpoints, one service method.** Matches the existing `MagazineController` pattern. No query-param disposition switching anywhere in the codebase, so we don't introduce one here. **D2 — Role-based response shaping: service-layer branching, same DTO type.** `@JsonView` is not used anywhere in the codebase today. Keeping one endpoint, one DTO type, and branching in `PaperServiceImpl.findByIdForCaller` keeps the wire shape and TypeScript types stable. **D3 — Authorization helper: Spring service bean, not custom SpEL.** No custom SpEL method security expressions exist in the codebase. A plain service bean is greppable and matches the refactor pattern used for the inline `submitReview` check. **D4 — Active review statuses: positive enumeration `{PENDING, IN_PROGRESS, COMPLETED}`.** Not `complementOf(DECLINED, EXPIRED)`. Forces explicit consideration when a new status is introduced. **D5 — Stored file is never modified.** Stripping happens on an in-memory copy read from `StorageService.download(key)`. The author's original upload remains the canonical artifact. **D6 — Reference number is safe to expose.** `referenceNo` is used in the download filename because it is a system-assigned identifier with no author PII. The original upload filename is not. --- ## 4. Revisions applied during review ### R1 — Authorization boundary tightening Initial design kept `PaperService.findById(UUID)` and added a sibling `findByIdForCaller`. User pushed back: as long as `findById` exists, a future controller can call it and bypass role shaping. **Resolution:** `findById(UUID)` is **removed**. `findByIdForCaller(UUID, UUID)` becomes the only public service method returning a `PaperDetailResponse` from a paper id. All existing callers inside `PaperServiceImpl` (submit / uploadRevision / updatePaper) are refactored to go through the same gateway. This rule is captured in a separate ADR (see `adr-paper-detail-authorization.md`) so it survives future refactors. ### R2 — Upload-time PDF validation added to Phase 2A scope Initial design treated upload validation as "Phase 3, nice to have". User pointed out that without upload validation, the first encrypted PDF uploaded before Phase 3 would cause a permanent 503 at preview time, with no recovery path. **Resolution:** `ManuscriptValidator` is now part of Phase 2A's design and MUST ship in the same release as the reviewer preview endpoint. `PaperServiceImpl.submit` and `uploadRevision` call the validator before `StorageService.upload`. Six discrete error codes are defined so the frontend can surface actionable messages. ### R3 — Mandatory `submitReview` refactor Initial design left the inline "reviewer assigned to this review" check in `ReviewServiceImpl.submitReview` untouched, on the grounds that it wasn't strictly in scope. User pushed back: leaving two enforcement sites means the rule can drift. **Resolution:** `ReviewServiceImpl.submitReview` is refactored as part of Phase 2A to call `reviewAuthorizationService.assertReviewerCanAccess(...)`. Single source of truth. --- ## 5. Risk register | # | Risk | Likelihood | Impact | Mitigation | |----|---------------------------------------------------------------------|------------|--------|-------------------------------------------------------------------------------------------------------------| | R1 | OpenPDF fails to strip unusual PDFs | Low | Medium | `PdfSanitizationException` → 503 `MANUSCRIPT_PREVIEW_UNAVAILABLE` fallback. Encrypted PDFs blocked at upload time (§1C). | | R2 | Authorization bypass drift (future controller recreates leak) | Medium | High | Single `findByIdForCaller` gateway + ADR + Javadoc on the service interface. `submitReview` refactored to the same helper. | | R3 | Stripping latency on large PDFs | Medium | Low | Measure first. If p95 > 2s, add Caffeine LRU (10 entries, 15 min TTL, never persisted). Not in Phase 2A scope. | | R4 | Regression in author/admin detail flows | Low | High | `PaperDetailResponse` wire shape unchanged; integration tests cover all four access patterns (admin, author, assigned reviewer, stranger). | | R5 | Filename PII leak (upload filename contains author surname) | High | Medium | `manuscriptName` is nulled in reviewer-safe DTO. Download filename is `review-{referenceNo}.pdf`, not the upload filename. | --- ## 6. Out of scope (and why) - **Server-side PDF annotation.** Deliberately excluded. Helper text directs reviewers to a local PDF editor: "For annotations, download and edit in your preferred PDF editor." - **Reviewer-identity watermarking.** Would require per-request stamping with the reviewer's name/ID. Defer until we have a concrete leak-attribution requirement. - **Caching the stripped bytes.** Listed in the risk register (R3) as a conditional follow-up. Not in Phase 2A so we don't build a cache before we have a measured latency problem. - **Admin paper detail changes.** `AdminPaperController` uses its own admin-scoped service method and is unchanged by this phase, other than the transitive fact that the inline `toDetailResponse` helper is gone. --- ## 7. File inventory (to be created or modified in Phase 3) **Backend — new:** - `controller/api/ReviewerPaperController.java` - `service/ReviewerPaperService.java` - `service/impl/ReviewerPaperServiceImpl.java` - `service/dto/ManuscriptContent.java` - `service/security/ReviewAuthorizationService.java` - `service/security/ReviewAuthorizationServiceImpl.java` - `util/PdfMetadataStripper.java` - `util/ManuscriptValidator.java` - `mapper/PaperMapper.java` (if not already present; otherwise add two methods) - `exception/ForbiddenException.java` - `exception/ManuscriptNotFoundException.java` - `exception/PdfSanitizationException.java` - `exception/InvalidManuscriptException.java` **Backend — modified:** - `service/PaperService.java` — remove `findById`, add `findByIdForCaller` - `service/impl/PaperServiceImpl.java` — implement `findByIdForCaller`, delete private `toDetailResponse`, call `ManuscriptValidator` in `submit`/`uploadRevision`, route existing returns through `findByIdForCaller` - `service/impl/ReviewServiceImpl.java` — replace inline reviewer check with `assertReviewerCanAccess` - `controller/api/PaperController.java` — `getById` adds `@PreAuthorize("isAuthenticated()")` and calls `findByIdForCaller` - `repository/ReviewRepository.java` — add `existsByPaper_IdAndReviewer_IdAndStatusIn` - `repository/PaperRepository.java` — add `existsByIdAndSubmittedBy_Id` - `exception/GlobalExceptionHandler.java` — handlers for the four new exceptions **Frontend — new:** - `src/components/reviewer/PaperPreviewPane.tsx` - `src/components/reviewer/PaperPreviewError.tsx` - `src/components/reviewer/DownloadPaperButton.tsx` - `src/components/reviewer/ReviewForm.tsx` **Frontend — modified:** - `src/app/(dashboard)/reviewer/review/[id]/page.tsx` — two-pane layout, imports above components - `src/lib/api/client.ts` — add `reviewerPaperApi.previewBlobUrl` and `reviewerPaperApi.download` **Dependencies:** none new, backend or frontend. --- ## 8. Cross-references - **Phase 1 discovery:** `docs/reviewer-paper-access/phase-1-discovery.md` - **ADR (single-gateway rule):** `docs/reviewer-paper-access/adr-paper-detail-authorization.md` - **Follow-ups:** Phase 2B (test plan + rollout), Phase 3 (implementation)