Repository navigation
feat(web/server) Add more options to public shared link - #1348
Conversation
…e-options-to-public-sharing
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Something to consider with this is, if they deny showing exif, we should probably strip the exif from file downloads too, as otherwise that permission is pretty much pointless if they also allow downloading. Also, without this, the PR wouldn't really resolve #1302 |
| let allowOriginalFile = true; | ||
| if (authUser.isPublicUser && !authUser.isAllowDownload) { | ||
| allowOriginalFile = false; | ||
| } | ||
|
|
||
| await this.assetService.checkAssetsAccess(authUser, [assetId]); | ||
| return this.assetService.serveFile(assetId, query, res, headers); | ||
| return this.assetService.serveFile(assetId, query, res, headers, allowOriginalFile); |
There was a problem hiding this comment.
We're trying to move all the business logic into the service (out of the controller and repository). Would it make sense for the serveFile method to take in AuthUserDto, and then the allowOriginalFile logic could happen inside the service? In general I think it would be good for all the service methods to be refactored to take in authUser and dto, so this would be a good opportunity to do that here.
| async checkDownloadAccess(authUser: AuthUserDto) { | ||
| if (authUser.isPublicUser && !authUser.isAllowDownload) { | ||
| throw new ForbiddenException(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
It would be nice to keep all the access logic in one place. It would probably be better to refactor checkAssetAccess at some point to work for both these situations. You could do this by adding a permission so like checkAssetAccess(authUser, assets, AssetAction.READ) or checkAssetAccess(authUser, assets, AssetAction.DOWNLOAD)`, etc.
…e-options-to-public-sharing
|
Could 'show EXIF' be changed to 'show metadata'? |
…e-options-to-public-sharing
…e-options-to-public-sharing
| <AssetViewer | ||
| asset={selectedAsset} | ||
| publicSharedKey={key} | ||
| publicSharedKey={sharedLink?.key} |
There was a problem hiding this comment.
do we need to pass the whole shared link and the shared link key as two separate props?
This PR adds additional options to the public shared link. Resolves #1302
Default creation form