Skip to content

Add PDF streaming viewer and token caching #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
akashroy8210 wants to merge 3 commits into
base: dev
Choose a base branch
from
Open

Add PDF streaming viewer and token caching #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77ab302
Add PDF streaming viewer and token caching
annunuroyakash Jun 19, 2026
ea27ea3
Clean up PDF streaming token handling
annunuroyakash Jul 11, 2026
05517e9
Merge branch 'dev' into feature/pdf-viewer-streaming
RsbhThakur Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions client/src/screens/browse/components/file-display/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getThumbnail } from "../../../../api/File";
import clientRoot from "../../../../api/server";
import capitalise from "../../../../utils/capitalise.js";
import Share from "../../../share";
import API_BASE_URL from "../../../../api/server";
import { verifyFile, unverifyFile, renameFile } from "../../../../api/File";
import {
RemoveFileFromFolder,
Expand All @@ -19,12 +20,13 @@ import { getFileDownloadLink } from "../../../../api/File";
import { fetchFolder } from "../../../../api/Folder.js";

const FileDisplay = ({ file, path, code, isMobileView = false }) => {

const user = useSelector((state) => state.user?.user);
const fileSize = formatFileSize(file.size);
const fileType = formatFileType(file.name);
const [showDialog, setShowDialog] = useState(false);
const [dialogType, setDialogType] = useState("verify");
const [onConfirmAction, setOnConfirmAction] = useState(() => () => {});
const [onConfirmAction, setOnConfirmAction] = useState(() => () => { });
const [isProcessing, setIsProcessing] = useState(false);

let name = file.name;
Expand Down Expand Up @@ -139,7 +141,11 @@ const FileDisplay = ({ file, path, code, isMobileView = false }) => {
toast.error("Please login to preview file.");
return;
}
window.open(preview_url, "_blank");
window.open(
Comment thread
RsbhThakur marked this conversation as resolved.
`${API_BASE_URL}/api/contribution/view/${file._id}`,

@RsbhThakur RsbhThakur Jul 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Functional Regression (Office Documents): Opening the backend streaming URL works great for PDFs and images (which browsers can render inline natively). However, browsers cannot render Office files (like .docx, .xlsx, .pptx) natively.

Previously, clicking "View" on a .docx file opened Microsoft's web viewer (file.webUrl) so the user could read it online. Now, this change forces a raw binary download of the Office file. We should only use the backend stream proxy if the file type is strictly a PDF, falling back to the original webUrl for other formats.

"_blank"
);

};


Expand Down Expand Up @@ -187,9 +193,8 @@ const FileDisplay = ({ file, path, code, isMobileView = false }) => {

return (
<div
className={`file-display ${
user?.isBR ? (file.isVerified ? "verified" : "unverified") : ""
}`}
className={`file-display ${user?.isBR ? (file.isVerified ? "verified" : "unverified") : ""
}`}
>
<img
src={thumbnailUrl}
Expand Down
1 change: 1 addition & 0 deletions server/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ app.use((err, req, res, next) => {
logger.error(
{
message: err.message,
microsoftResponse: err.response?.data,
route: req.originalUrl,
method: req.method,
},
Expand Down
38 changes: 35 additions & 3 deletions server/modules/contribution/contribution.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import AppError from "../../utils/appError.js";
import validatePayload from "../../utils/validate.js";
import UploadFile from "../../services/UploadFile.js";
import fs from "fs";
import { FolderModel } from "../course/course.model.js";
import { FolderModel, FileModel } from "../course/course.model.js";
import logger from "../../utils/logger.js";
import { normalizeCourseCode, getCourseCodeCaseInsensitiveRegex } from "../../utils/course.js";
import { getAccessToken } from "../onedrive/onedrive.controller.js";
import axios from "axios";

async function ContributionCreation(contributionId, data) {
const existingContribution = await Contribution.findOne({ contributionId });
Expand Down Expand Up @@ -50,7 +52,6 @@ async function HandleFileUpload(req, res, next) {
logger.info("Handling File Upload");
const contributionId = req.headers["contribution-id"];
const files = req.files;

if (!files || files.length === 0) {
return res.status(400).json({ error: "No files were uploaded" });
}
Expand Down Expand Up @@ -157,12 +158,43 @@ async function GetBrContribution(req, res, next) {
}
}

async function viewFile(req, res, next) {
Comment thread
RsbhThakur marked this conversation as resolved.

@RsbhThakur RsbhThakur Jul 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing Authorization Check & Resource Leaks:

There are three issues with this controller:

  1. Access Bypass: There is no verification check. A regular user (non-BR) can access and download unverified files (file.isVerified === false) if they know the MongoDB ID, bypassing the BR curation block. Please check the file status and verify the user's role before streaming.
  2. Connection Leaks: If a user closes their tab or cancels the download mid-stream, the response connection terminates but the incoming Graph API stream remains open. We need to handle the request close event to destroy the incoming Axios stream and free up sockets.
  3. Missing Size Metadata: Only the Content-Type header is set on the response. Without the Content-Length header, the browser's PDF viewer won't display a loading progress bar or estimate total pages.

try {
const { id } = req.params;
const file = await FileModel.findById(id);
if (!file) {
return res.status(404).json({ message: "File not found" });
}
const accessToken = await getAccessToken();
if (!accessToken) {
return res.status(500).json({ message: "Access token not found" });
}

const response = await axios.get(`https://graph.microsoft.com/v1.0/me/drive/items/${file.fileId}/content`, {
headers: {
Authorization: `Bearer ${accessToken}`
},
responseType: "stream"
})
if (!response) {
return res.status(500).json({ message: "File not found" });
}
res.setHeader("Content-Type", response.headers["content-type"])

response.data.pipe(res)

} catch (error) {
next(error);
}
}

export default {
GetAllContributions,
CreateNewContribution,
HandleFileUpload,
GetMyContributions,
DeleteContribution,
GetContributionsUpdatedSince,
GetBrContribution
GetBrContribution,
viewFile
};
2 changes: 1 addition & 1 deletion server/modules/contribution/contribution.routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@ router.post("/upload", upload.array("file"), catchAsync(ContributionController.H
// router.get("/:id", catchAsync(ContributionController.CreateNewContribution));
router.post("/updated", catchAsync(ContributionController.GetContributionsUpdatedSince));
router.post("/br",isAuthenticated, ContributionController.GetBrContribution)

router.get('/view/:id',catchAsync(ContributionController.viewFile))

@RsbhThakur RsbhThakur Jul 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Security Vulnerability: This route is currently unprotected. There is no isAuthenticated middleware applied to it. Since the browser automatically sends cookie credentials for window.open tabs, this route should be protected. Otherwise, any unauthenticated user or external script can access and download files directly from the server.

export default router;
58 changes: 45 additions & 13 deletions server/modules/onedrive/onedrive.controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ export async function generateDeviceCode(req, res) {

if (!response.data) throw new AppError(500, "Something went wrong");

console.log(response.data.user_code);

fs.writeFileSync("./onedrive-device-code.token", response.data.device_code, "utf-8");
if (fs.existsSync("./onedrive-access-token.token")) {
fs.unlinkSync("./onedrive-access-token.token");
Expand Down Expand Up @@ -207,22 +205,19 @@ async function visitAllFiles() {
const searchDocument = await SearchResults.findOne({
code: getCourseCodeCaseInsensitiveRegex(courseCode),
});
console.log(searchDocument);
if (!searchDocument) {
await SearchResults.create({
name: courseName,
code: courseCode,
isAvailable: true,
});
console.log("Created", folder.name);
} else {
await SearchResults.updateOne(
{ code: getCourseCodeCaseInsensitiveRegex(courseCode) },
{
isAvailable: true,
}
);
console.log("Updated", folder.name);
}
}));
return "ok";
Expand Down Expand Up @@ -257,15 +252,13 @@ export async function visitCourseById(id) {
code: courseCode,
isAvailable: true,
});
console.log("Created", required_course.name);
} else {
await SearchResults.updateOne(
{ code: getCourseCodeCaseInsensitiveRegex(courseCode) },
{
isAvailable: true,
}
);
console.log("Updated", required_course.name);
}

return "ok";
Expand Down Expand Up @@ -322,14 +315,53 @@ async function visitFile(file, currCourse) {
return NewFile._id;
}

// export async function getAccessToken() {
// let data;
// if (fs.existsSync("./onedrive-refresh-token.token")) {
// data = await refreshAccessToken();
// } else {
// data = await generateAccessToken();
// }
// return data.access_token;
// }
let cachedAccessToken = null;
let tokenExpiry = 0;
let refreshPromise = null;
export async function getAccessToken() {

@RsbhThakur RsbhThakur Jul 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Robustness Issue (No Cache Eviction on 401):

Caching the token in memory to avoid duplicate Graph API authorization round-trips is a massive performance win.

However, if the cached token is invalidated or revoked on Microsoft's end before the local expiration (tokenExpiry) is reached, Microsoft will return a 401 Unauthorized. Since the cache never evicts the token on authorization failure, the server will continue to reuse the dead token and return errors to all users until the full hour expires. We need a way to invalidate the cache immediately upon hitting a 401 error.

let data;
if (fs.existsSync("./onedrive-refresh-token.token")) {
data = await refreshAccessToken();
} else {
data = await generateAccessToken();

if (
cachedAccessToken &&
Date.now() < tokenExpiry
) {
return cachedAccessToken;
}
if (refreshPromise) {
return refreshPromise;
}

refreshPromise = (async () => {
let data;

if (fs.existsSync("./onedrive-refresh-token.token")) {
data = await refreshAccessToken();
} else {
data = await generateAccessToken();
}

cachedAccessToken = data.access_token;

tokenExpiry =
Date.now() +
(data.expires_in - 60) * 1000;

return cachedAccessToken;
})();
try {
return await refreshPromise;
} finally {
refreshPromise = null;
}
return data.access_token;

}

async function refreshAccessToken() {
Expand Down