feat(onedrive): implemented secure file download functionality #249
Conversation
- Added direct file download using Graph API and fetch - Implemented progress tracking for downloads - Added fallback mechanism for download URLs - Handled file type and thumbnail preservation - Added proper error handling and user feedback BREAKING CHANGE: Changes the way OneDrive files are downloaded. Uses direct download URLs instead of sharing links
amjedidiah
changed the title
amjedidiah
changed the title
src/hooks/useOneDriveAuth.ts
Outdated
| expiresOn: response.expiresOn!.getTime(), | ||
| } | ||
| setToken(newToken) | ||
| localStorage.setItem( |
There was a problem hiding this comment.
MSAL's built-in caching mechanisms are likely a better alternative to localStorage for token management, as they are designed to securely handle tokens and minimize risks like XSS attacks. I'm not sure about the specific case here that led to using localStorage, but if acquireTokenSilent (which I see you've used elsewhere) works in this context, it would definitely be a safer and more reliable approach.
There was a problem hiding this comment.
Not sure if we can do without either localStorage or sessionStorage
However, I will look this up.
There was a problem hiding this comment.
@amjedidiah , just to clarify, I wasn’t suggesting changing the entire cache location setting.
My thought was more about leveraging acquireTokenSilent to fetch the token directly without needing to manually set or retrieve it in localStorage during the flow. That said, I’m not entirely sure if it’s doable, as I haven’t worked with MSAL myself. I trust your judgment and appreciate you looking into it!
| ) => { | ||
| const response = await fetch(url, { | ||
| method: 'GET', | ||
| headers: { Accept: 'application/json' }, |
There was a problem hiding this comment.
downloadFile function sets the Accept header to 'application/json' when fetching file content
- removed unnecessary header in src/components/UpupUploader/FileBrowser/BrowserOD.tsx - build fixes
amjedidiah
changed the title
- Removed manual token storage in localStorage in favour of acquireTokenSilent for token management - Fixed logoutPopup type error - Moved `pako` package from devDependencies to dependencies as it should be
amjedidiah
changed the title
| const promises = files.map(async (file, index) => { | ||
| console.log('Processing file:', file) | ||
|
|
There was a problem hiding this comment.
Make sure to clean all the code from console.logs
|
|
||
| console.log('Got download URL:', downloadUrl) |
3 participants
BREAKING CHANGE: Changes the way OneDrive files are downloaded. Uses direct download URLs instead of sharing links
Demo.mp4