feat: improve share security

backend/src/auth/auth.controller.ts

@@ -27,6 +27,7 @@ export class AuthController {
   }
 
   @Post("signIn")
+  @HttpCode(200)
   signIn(@Body() dto: AuthSignInDTO) {
     return this.authService.signIn(dto);
   }

backend/src/file/file.service.ts

@@ -100,12 +100,12 @@ export class FileService {
     );
   }
 
-  verifyFileDownloadToken(shareId: string, fileId: string, token: string) {
+  verifyFileDownloadToken(shareId: string, token: string) {
     try {
       const claims = this.jwtService.verify(token, {
         secret: this.config.get("JWT_SECRET"),
       });
-      return claims.shareId == shareId && claims.fileId == fileId;
+      return claims.shareId == shareId;
     } catch {
       return false;
     }

backend/src/file/guard/fileDownload.guard.ts

@@ -1,23 +1,17 @@
 import { CanActivate, ExecutionContext, Injectable } from "@nestjs/common";
-import { Reflector } from "@nestjs/core";
 import { Request } from "express";
 import { FileService } from "src/file/file.service";
-import { PrismaService } from "src/prisma/prisma.service";
 
 @Injectable()
 export class FileDownloadGuard implements CanActivate {
-  constructor(
-    private reflector: Reflector,
-    private fileService: FileService,
-    private prisma: PrismaService
-  ) {}
+  constructor(private fileService: FileService) {}
 
   async canActivate(context: ExecutionContext) {
     const request: Request = context.switchToHttp().getRequest();
 
     const token = request.query.token as string;
-    const { shareId, fileId } = request.params;
+    const { shareId } = request.params;
 
-    return this.fileService.verifyFileDownloadToken(shareId, fileId, token);
+    return this.fileService.verifyFileDownloadToken(shareId, token);
   }
 }

backend/src/share/dto/createShare.dto.ts

@@ -1,5 +1,5 @@
 import { Type } from "class-transformer";
-import { IsString, Matches, ValidateNested } from "class-validator";
+import { IsString, Length, Matches, ValidateNested } from "class-validator";
 import { ShareSecurityDTO } from "./shareSecurity.dto";
 
 export class CreateShareDTO {
@@ -7,6 +7,7 @@ export class CreateShareDTO {
   @Matches("^[a-zA-Z0-9_-]*$", undefined, {
     message: "ID only can contain letters, numbers, underscores and hyphens",
   })
+  @Length(3, 50)
   id: string;
 
   @IsString()

backend/src/share/dto/sharePassword.dto.ts

@@ -1,6 +1,5 @@
 import { IsNotEmpty } from "class-validator";
 
 export class SharePasswordDto {
-  @IsNotEmpty()
   password: string;
 }

backend/src/share/guard/shareOwner.guard.ts

@@ -1,16 +1,16 @@
-import { CanActivate, ExecutionContext, Injectable } from "@nestjs/common";
-import { Reflector } from "@nestjs/core";
+import {
+  CanActivate,
+  ExecutionContext,
+  Injectable,
+  NotFoundException,
+} from "@nestjs/common";
 import { User } from "@prisma/client";
 import { Request } from "express";
-import { ExtractJwt } from "passport-jwt";
 import { PrismaService } from "src/prisma/prisma.service";
-import { ShareService } from "src/share/share.service";
 
 @Injectable()
 export class ShareOwnerGuard implements CanActivate {
-  constructor(
-    private prisma: PrismaService
-  ) {}
+  constructor(private prisma: PrismaService) {}
 
   async canActivate(context: ExecutionContext) {
     const request: Request = context.switchToHttp().getRequest();
@@ -26,7 +26,7 @@ export class ShareOwnerGuard implements CanActivate {
       include: { security: true },
     });
 
-
+    if (!share) throw new NotFoundException("Share not found");
 
     return share.creatorId == (request.user as User).id;
   }

backend/src/share/guard/shareSecurity.guard.ts

@@ -21,6 +21,7 @@ export class ShareSecurityGuard implements CanActivate {
 
   async canActivate(context: ExecutionContext) {
     const request: Request = context.switchToHttp().getRequest();
+    const shareToken = request.get("X-Share-Token");
     const shareId = Object.prototype.hasOwnProperty.call(
       request.params,
       "shareId"
@@ -36,19 +37,15 @@ export class ShareSecurityGuard implements CanActivate {
     if (!share || moment().isAfter(share.expiration))
       throw new NotFoundException("Share not found");
 
-    if (!share.security) return true;
-
-    if (share.security.maxViews && share.security.maxViews <= share.views)
-      throw new ForbiddenException(
-        "Maximum views exceeded",
-        "share_max_views_exceeded"
-      );
-
-    if (
-      !this.shareService.verifyShareToken(shareId, request.get("X-Share-Token"))
-    )
+    if (share.security?.password && !shareToken)
       throw new ForbiddenException(
         "This share is password protected",
+        "share_password_required"
+      );
+
+    if (!this.shareService.verifyShareToken(shareId, shareToken))
+      throw new ForbiddenException(
+        "Share token required",
         "share_token_required"
       );
 

backend/src/share/guard/shareTokenSecurity.guard.ts

@@ -0,0 +1,47 @@
+import {
+  CanActivate,
+  ExecutionContext,
+  ForbiddenException,
+  Injectable,
+  NotFoundException,
+} from "@nestjs/common";
+import { Reflector } from "@nestjs/core";
+import { Request } from "express";
+import * as moment from "moment";
+import { PrismaService } from "src/prisma/prisma.service";
+import { ShareService } from "src/share/share.service";
+
+@Injectable()
+export class ShareTokenSecurity implements CanActivate {
+  constructor(
+    private reflector: Reflector,
+    private shareService: ShareService,
+    private prisma: PrismaService
+  ) {}
+
+  async canActivate(context: ExecutionContext) {
+    const request: Request = context.switchToHttp().getRequest();
+    const shareId = Object.prototype.hasOwnProperty.call(
+      request.params,
+      "shareId"
+    )
+      ? request.params.shareId
+      : request.params.id;
+
+    const share = await this.prisma.share.findUnique({
+      where: { id: shareId },
+      include: { security: true },
+    });
+
+    if (!share || moment().isAfter(share.expiration))
+      throw new NotFoundException("Share not found");
+
+    if (share.security?.maxViews && share.security.maxViews <= share.views)
+      throw new ForbiddenException(
+        "Maximum views exceeded",
+        "share_max_views_exceeded"
+      );
+
+    return true;
+  }
+}

backend/src/share/share.controller.ts

@@ -18,6 +18,7 @@ import { ShareMetaDataDTO } from "./dto/shareMetaData.dto";
 import { SharePasswordDto } from "./dto/sharePassword.dto";
 import { ShareOwnerGuard } from "./guard/shareOwner.guard";
 import { ShareSecurityGuard } from "./guard/shareSecurity.guard";
+import { ShareTokenSecurity } from "./guard/shareTokenSecurity.guard";
 import { ShareService } from "./share.service";
 
 @Controller("shares")
@@ -68,11 +69,10 @@ export class ShareController {
     return this.shareService.isShareIdAvailable(id);
   }
 
-  @Post(":id/password")
-  async exchangeSharePasswordWithToken(
-    @Param("id") id: string,
-    @Body() body: SharePasswordDto
-  ) {
-    return this.shareService.exchangeSharePasswordWithToken(id, body.password);
+  @HttpCode(200)
+  @UseGuards(ShareTokenSecurity)
+  @Post(":id/token")
+  async getShareToken(@Param("id") id: string, @Body() body: SharePasswordDto) {
+    return this.shareService.getShareToken(id, body.password);
   }
 }

backend/src/share/share.service.ts

@@ -76,6 +76,9 @@ export class ShareService {
   }
 
   async complete(id: string) {
+    if (await this.isShareCompleted(id))
+      throw new BadRequestException("Share already completed");
+
     const moreThanOneFileInShare =
       (await this.prisma.file.findMany({ where: { shareId: id } })).length != 0;
 
@@ -117,8 +120,6 @@ export class ShareService {
       return file;
     });
 
-    await this.increaseViewCount(share);
-
     return share;
   }
 
@@ -160,27 +161,36 @@ export class ShareService {
     });
   }
 
-  async exchangeSharePasswordWithToken(shareId: string, password: string) {
-    const sharePassword = (
-      await this.prisma.shareSecurity.findFirst({
-        where: { share: { id: shareId } },
-      })
-    ).password;
+  async getShareToken(shareId: string, password: string) {
+    const share = await this.prisma.share.findFirst({
+      where: { id: shareId },
+      include: {
+        security: true,
+      },
+    });
 
-    if (!(await argon.verify(sharePassword, password)))
+    if (
+      share?.security?.password &&
+      !(await argon.verify(share.security.password, password))
+    )
       throw new ForbiddenException("Wrong password");
 
-    const token = this.generateShareToken(shareId);
+    const token = await this.generateShareToken(shareId);
+    await this.increaseViewCount(share);
     return { token };
   }
 
-  generateShareToken(shareId: string) {
+  async generateShareToken(shareId: string) {
+    const { expiration } = await this.prisma.share.findUnique({
+      where: { id: shareId },
+    });
+    console.log(moment(expiration).diff(new Date(), "seconds"));
     return this.jwtService.sign(
       {
         shareId,
       },
       {
-        expiresIn: "1h",
+        expiresIn: moment(expiration).diff(new Date(), "seconds") + "s",
         secret: this.config.get("JWT_SECRET"),
       }
     );

frontend/src/components/share/CreateUploadModalBody.tsx

@@ -31,7 +31,7 @@ const CreateUploadModalBody = ({
       .string()
       .required()
       .min(3)
-      .max(100)
+      .max(50)
       .matches(new RegExp("^[a-zA-Z0-9_-]*$"), {
         message: "Can only contain letters, numbers, underscores and hyphens",
       }),

frontend/src/components/share/DownloadAllButton.tsx

@@ -25,7 +25,7 @@ const DownloadAllButton = ({ shareId }: { shareId: string }) => {
           setIsZipReady(share.isZipReady);
           if (share.isZipReady) clearInterval(timer);
         })
-        .catch(() => {});
+        .catch(() => clearInterval(timer));
     }, 5000);
     return () => {
       clearInterval(timer);

frontend/src/pages/account/shares.tsx

@@ -14,9 +14,11 @@ import { useClipboard } from "@mantine/hooks";
 import { useModals } from "@mantine/modals";
 import { NextLink } from "@mantine/next";
 import moment from "moment";
-import { useEffect, useState } from "react";
+import { useRouter } from "next/router";
+import { useState } from "react";
 import { Link, Trash } from "tabler-icons-react";
 import Meta from "../../components/Meta";
+import useUser from "../../hooks/user.hook";
 import shareService from "../../services/share.service";
 import { MyShare } from "../../types/share.type";
 import toast from "../../utils/toast.util";
@@ -24,100 +26,108 @@ import toast from "../../utils/toast.util";
 const MyShares = () => {
   const modals = useModals();
   const clipboard = useClipboard();
+  const router = useRouter();
+  const user = useUser();
 
   const [shares, setShares] = useState<MyShare[]>();
 
-  useEffect(() => {
-    shareService.getMyShares().then((shares) => setShares(shares));
-  }, []);
+  // useEffect(() => {
+  //   shareService.getMyShares().then((shares) => setShares(shares));
+  // }, []);
 
-  if (!shares) return <LoadingOverlay visible />;
-  return (
-    <>
-      <Meta title="My shares" />
-      <Title mb={30} order={3}>
-        My shares
-      </Title>
-      {shares.length == 0 ? (
-        <Center style={{ height: "70vh" }}>
-          <Stack align="center" spacing={10}>
-            <Title order={3}>It's empty here 👀</Title>
-            <Text>You don't have any shares.</Text>
-            <Space h={5} />
-            <Button component={NextLink} href="/upload" variant="light">
-              Create one
-            </Button>
-          </Stack>
-        </Center>
-      ) : (
-        <Table>
-          <thead>
-            <tr>
-              <th>Name</th>
-              <th>Visitors</th>
-              <th>Expires at</th>
-              <th></th>
-            </tr>
-          </thead>
-          <tbody>
-            {shares.map((share) => (
-              <tr key={share.id}>
-                <td>{share.id}</td>
-                <td>{share.views}</td>
-                <td>
-                  {moment(share.expiration).format("MMMM DD YYYY, HH:mm")}
-                </td>
-                <td>
-                  <Group position="right">
-                    <ActionIcon
-                      color="victoria"
-                      variant="light"
-                      size={25}
-                      onClick={() => {
-                        clipboard.copy(
-                          `${window.location.origin}/share/${share.id}`
-                        );
-                        toast.success("Your link was copied to the keyboard.");
-                      }}
-                    >
-                      <Link />
-                    </ActionIcon>
-                    <ActionIcon
-                      color="red"
-                      variant="light"
-                      size={25}
-                      onClick={() => {
-                        modals.openConfirmModal({
-                          title: `Delete share ${share.id}`,
-                          children: (
-                            <Text size="sm">
-                              Do you really want to delete this share?
-                            </Text>
-                          ),
-                          confirmProps: {
-                            color: "red",
-                          },
-                          labels: { confirm: "Confirm", cancel: "Cancel" },
-                          onConfirm: () => {
-                            shareService.remove(share.id);
-                            setShares(
-                              shares.filter((item) => item.id !== share.id)
-                            );
-                          },
-                        });
-                      }}
-                    >
-                      <Trash />
-                    </ActionIcon>
-                  </Group>
-                </td>
+  if (!user) {
+    router.replace("/");
+  } else {
+    if (!shares) return <LoadingOverlay visible />;
+    return (
+      <>
+        <Meta title="My shares" />
+        <Title mb={30} order={3}>
+          My shares
+        </Title>
+        {shares.length == 0 ? (
+          <Center style={{ height: "70vh" }}>
+            <Stack align="center" spacing={10}>
+              <Title order={3}>It's empty here 👀</Title>
+              <Text>You don't have any shares.</Text>
+              <Space h={5} />
+              <Button component={NextLink} href="/upload" variant="light">
+                Create one
+              </Button>
+            </Stack>
+          </Center>
+        ) : (
+          <Table>
+            <thead>
+              <tr>
+                <th>Name</th>
+                <th>Visitors</th>
+                <th>Expires at</th>
+                <th></th>
               </tr>
-            ))}
-          </tbody>
-        </Table>
-      )}
-    </>
-  );
+            </thead>
+            <tbody>
+              {shares.map((share) => (
+                <tr key={share.id}>
+                  <td>{share.id}</td>
+                  <td>{share.views}</td>
+                  <td>
+                    {moment(share.expiration).format("MMMM DD YYYY, HH:mm")}
+                  </td>
+                  <td>
+                    <Group position="right">
+                      <ActionIcon
+                        color="victoria"
+                        variant="light"
+                        size={25}
+                        onClick={() => {
+                          clipboard.copy(
+                            `${window.location.origin}/share/${share.id}`
+                          );
+                          toast.success(
+                            "Your link was copied to the keyboard."
+                          );
+                        }}
+                      >
+                        <Link />
+                      </ActionIcon>
+                      <ActionIcon
+                        color="red"
+                        variant="light"
+                        size={25}
+                        onClick={() => {
+                          modals.openConfirmModal({
+                            title: `Delete share ${share.id}`,
+                            children: (
+                              <Text size="sm">
+                                Do you really want to delete this share?
+                              </Text>
+                            ),
+                            confirmProps: {
+                              color: "red",
+                            },
+                            labels: { confirm: "Confirm", cancel: "Cancel" },
+                            onConfirm: () => {
+                              shareService.remove(share.id);
+                              setShares(
+                                shares.filter((item) => item.id !== share.id)
+                              );
+                            },
+                          });
+                        }}
+                      >
+                        <Trash />
+                      </ActionIcon>
+                    </Group>
+                  </td>
+                </tr>
+              ))}
+            </tbody>
+          </Table>
+        )}
+      </>
+    );
+  }
 };
 
 export default MyShares;

frontend/src/pages/share/[shareId].tsx

@@ -15,12 +15,21 @@ const Share = () => {
   const shareId = router.query.shareId as string;
   const [fileList, setFileList] = useState<any[]>([]);
 
-  const submitPassword = async (password: string) => {
+  const getShareToken = async (password?: string) => {
     await shareService
-      .exchangeSharePasswordWithToken(shareId, password)
+      .getShareToken(shareId, password)
       .then(() => {
         modals.closeAll();
         getFiles();
+      })
+      .catch((e) => {
+        if (e.response.data.error == "share_max_views_exceeded") {
+          showErrorModal(
+            modals,
+            "Visitor limit exceeded",
+            "The visitor limit from this share has been exceeded."
+          );
+        }
       });
   };
 
@@ -38,14 +47,10 @@ const Share = () => {
             "Not found",
             "This share can't be found. Please check your link."
           );
+        } else if (error == "share_password_required") {
+          showEnterPasswordModal(modals, getShareToken);
         } else if (error == "share_token_required") {
-          showEnterPasswordModal(modals, submitPassword);
-        } else if (error == "share_max_views_exceeded") {
-          showErrorModal(
-            modals,
-            "Visitor limit exceeded",
-            "The visitor limit from this share has been exceeded."
-          );
+          getShareToken();
         } else if (error == "forbidden") {
           showErrorModal(
             modals,
@@ -69,9 +74,7 @@ const Share = () => {
         description="Look what I've shared with you."
       />
       <Group position="right" mb="lg">
-        <DownloadAllButton
-          shareId={shareId}
-        />
+        <DownloadAllButton shareId={shareId} />
       </Group>
       <FileList
         files={fileList}

frontend/src/services/share.service.ts

@@ -44,9 +44,8 @@ const getMyShares = async (): Promise<MyShare[]> => {
   return (await api.get("shares")).data;
 };
 
-const exchangeSharePasswordWithToken = async (id: string, password: string) => {
-  const { token } = (await api.post(`/shares/${id}/password`, { password }))
-    .data;
+const getShareToken = async (id: string, password?: string) => {
+  const { token } = (await api.post(`/shares/${id}/token`, { password })).data;
 
   localStorage.setItem(`share_${id}_token`, token);
 };
@@ -87,7 +86,7 @@ const uploadFile = async (
 export default {
   create,
   completeShare,
-  exchangeSharePasswordWithToken,
+  getShareToken,
   get,
   remove,
   getMetaData,