Skip to content

Commit

Permalink
misc: addressed comments 1
Browse files Browse the repository at this point in the history
  • Loading branch information
sheensantoscapadngan committed Nov 15, 2024
1 parent 6bae362 commit 433971a
Show file tree
Hide file tree
Showing 13 changed files with 249 additions and 90 deletions.
3 changes: 2 additions & 1 deletion backend/src/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,8 @@ export const registerRoutes = async (
tokenService,
smtpService,
authDAL,
userDAL
userDAL,
totpConfigDAL
});

const projectBotService = projectBotServiceFactory({ permissionService, projectBotDAL, projectDAL });
Expand Down
16 changes: 15 additions & 1 deletion backend/src/server/routes/v1/user-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export const registerUserRouter = async (server: FastifyZodProvider) => {
method: "DELETE",
url: "/me/totp",
config: {
rateLimit: readLimit
rateLimit: writeLimit
},
onRequest: verifyAuth([AuthMode.JWT]),
handler: async (req) => {
Expand Down Expand Up @@ -254,4 +254,18 @@ export const registerUserRouter = async (server: FastifyZodProvider) => {
});
}
});

server.route({
method: "POST",
url: "/me/totp/recovery-codes",
config: {
rateLimit: writeLimit
},
onRequest: verifyAuth([AuthMode.JWT]),
handler: async (req) => {
return server.services.totp.createUserTotpRecoveryCodes({
userId: req.permission.id
});
}
});
};
4 changes: 2 additions & 2 deletions backend/src/server/routes/v2/mfa-router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import jwt from "jsonwebtoken";
import { z } from "zod";

import { getConfig } from "@app/lib/config/env";
import { NotFoundError } from "@app/lib/errors";
import { BadRequestError, NotFoundError } from "@app/lib/errors";
import { mfaRateLimit } from "@app/server/config/rateLimiter";
import { AuthModeMfaJwtTokenPayload, AuthTokenType, MfaMethod } from "@app/services/auth/auth-type";

Expand Down Expand Up @@ -73,7 +73,7 @@ export const registerMfaRouter = async (server: FastifyZodProvider) => {
isVerified: Boolean(totpConfig)
};
} catch (error) {
if (error instanceof NotFoundError) {
if (error instanceof NotFoundError || error instanceof BadRequestError) {
return { isVerified: false };
}

Expand Down
6 changes: 3 additions & 3 deletions backend/src/services/auth/auth-login-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,9 +361,9 @@ export const authLoginServiceFactory = ({
}

const shouldCheckMfa = selectedOrg.enforceMfa || user.isMfaEnabled;
const orgMfaMethod = selectedOrg.enforceMfa ? selectedOrg.selectedMfaMethod : undefined;
const userMfaMethod = user.isMfaEnabled ? user.selectedMfaMethod : undefined;
const mfaMethod = orgMfaMethod ?? userMfaMethod ?? MfaMethod.EMAIL;
const orgMfaMethod = selectedOrg.enforceMfa ? selectedOrg.selectedMfaMethod ?? MfaMethod.EMAIL : undefined;
const userMfaMethod = user.isMfaEnabled ? user.selectedMfaMethod ?? MfaMethod.EMAIL : undefined;
const mfaMethod = orgMfaMethod ?? userMfaMethod;

if (shouldCheckMfa && (!decodedToken.isMfaVerified || decodedToken.mfaMethod !== mfaMethod)) {
enforceUserLockStatus(Boolean(user.isLocked), user.temporaryLockDateEnd);
Expand Down
11 changes: 10 additions & 1 deletion backend/src/services/auth/auth-password-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { generateSrpServerKey, srpCheckClientProof } from "@app/lib/crypto";
import { TAuthTokenServiceFactory } from "../auth-token/auth-token-service";
import { TokenType } from "../auth-token/auth-token-types";
import { SmtpTemplates, TSmtpService } from "../smtp/smtp-service";
import { TTotpConfigDALFactory } from "../totp/totp-config-dal";
import { TUserDALFactory } from "../user/user-dal";
import { TAuthDALFactory } from "./auth-dal";
import { TChangePasswordDTO, TCreateBackupPrivateKeyDTO, TResetPasswordViaBackupKeyDTO } from "./auth-password-type";
Expand All @@ -18,14 +19,16 @@ type TAuthPasswordServiceFactoryDep = {
userDAL: TUserDALFactory;
tokenService: TAuthTokenServiceFactory;
smtpService: TSmtpService;
totpConfigDAL: Pick<TTotpConfigDALFactory, "delete">;
};

export type TAuthPasswordFactory = ReturnType<typeof authPaswordServiceFactory>;
export const authPaswordServiceFactory = ({
authDAL,
userDAL,
tokenService,
smtpService
smtpService,
totpConfigDAL
}: TAuthPasswordServiceFactoryDep) => {
/*
* Pre setup for pass change with srp protocol
Expand Down Expand Up @@ -185,6 +188,12 @@ export const authPaswordServiceFactory = ({
temporaryLockDateEnd: null,
consecutiveFailedMfaAttempts: 0
});

/* we reset the mobile authenticator configs of the user
because we want this to be one of the recovery modes from account lockout */
await totpConfigDAL.delete({
userId
});
};

/*
Expand Down
81 changes: 70 additions & 11 deletions backend/src/services/totp/totp-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { TKmsServiceFactory } from "../kms/kms-service";
import { TUserDALFactory } from "../user/user-dal";
import { TTotpConfigDALFactory } from "./totp-config-dal";
import {
TCreateUserTotpRecoveryCodesDTO,
TDeleteUserTotpConfigDTO,
TGetUserTotpConfigDTO,
TRegisterUserTotpDTO,
Expand All @@ -27,8 +28,7 @@ export type TTotpServiceFactory = ReturnType<typeof totpServiceFactory>;
export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotpServiceFactoryDep) => {
const getUserTotpConfig = async ({ userId }: TGetUserTotpConfigDTO) => {
const totpConfig = await totpConfigDAL.findOne({
userId,
isVerified: true
userId
});

if (!totpConfig) {
Expand All @@ -37,6 +37,12 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp
});
}

if (!totpConfig.isVerified) {
throw new BadRequestError({
message: "TOTP configuration has not been verified"
});
}

const decryptWithRoot = kmsService.decryptWithRootKey();
const recoveryCodes = decryptWithRoot(totpConfig.encryptedRecoveryCodes).toString().split(",");

Expand Down Expand Up @@ -102,8 +108,7 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp

const verifyUserTotpConfig = async ({ userId, totp }: TVerifyUserTotpConfigDTO) => {
const totpConfig = await totpConfigDAL.findOne({
userId,
isVerified: false
userId
});

if (!totpConfig) {
Expand All @@ -112,6 +117,12 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp
});
}

if (totpConfig.isVerified) {
throw new BadRequestError({
message: "TOTP configuration has already been verified"
});
}

const decryptWithRoot = kmsService.decryptWithRootKey();
const secret = decryptWithRoot(totpConfig.encryptedSecret).toString();
const isValid = authenticator.verify({
Expand All @@ -132,8 +143,7 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp

const verifyUserTotp = async ({ userId, totp }: TVerifyUserTotpDTO) => {
const totpConfig = await totpConfigDAL.findOne({
userId,
isVerified: true
userId
});

if (!totpConfig) {
Expand All @@ -142,6 +152,12 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp
});
}

if (!totpConfig.isVerified) {
throw new BadRequestError({
message: "TOTP configuration has not been verified"
});
}

const decryptWithRoot = kmsService.decryptWithRootKey();
const secret = decryptWithRoot(totpConfig.encryptedSecret).toString();
const isValid = authenticator.verify({
Expand All @@ -158,8 +174,7 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp

const verifyWithUserRecoveryCode = async ({ userId, recoveryCode }: TVerifyWithUserRecoveryCodeDTO) => {
const totpConfig = await totpConfigDAL.findOne({
userId,
isVerified: true
userId
});

if (!totpConfig) {
Expand All @@ -168,6 +183,12 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp
});
}

if (!totpConfig.isVerified) {
throw new BadRequestError({
message: "TOTP configuration has not been verified"
});
}

const decryptWithRoot = kmsService.decryptWithRootKey();
const encryptWithRoot = kmsService.encryptWithRootKey();

Expand All @@ -188,8 +209,7 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp

const deleteUserTotpConfig = async ({ userId }: TDeleteUserTotpConfigDTO) => {
const totpConfig = await totpConfigDAL.findOne({
userId,
isVerified: true
userId
});

if (!totpConfig) {
Expand All @@ -201,12 +221,51 @@ export const totpServiceFactory = ({ totpConfigDAL, kmsService, userDAL }: TTotp
await totpConfigDAL.deleteById(totpConfig.id);
};

const createUserTotpRecoveryCodes = async ({ userId }: TCreateUserTotpRecoveryCodesDTO) => {
const decryptWithRoot = kmsService.decryptWithRootKey();
const encryptWithRoot = kmsService.encryptWithRootKey();

return totpConfigDAL.transaction(async (tx) => {
const totpConfig = await totpConfigDAL.findOne(
{
userId,
isVerified: true
},
tx
);

if (!totpConfig) {
throw new NotFoundError({
message: "Valid TOTP configuration not found"
});
}

const recoveryCodes = decryptWithRoot(totpConfig.encryptedRecoveryCodes).toString().split(",");
if (recoveryCodes.length >= 10) {
throw new BadRequestError({
message: "Cannot have more than 10 recovery codes at a time"
});
}

const toGenerateCount = 10 - recoveryCodes.length;
const newRecoveryCodes = Array.from({ length: toGenerateCount }).map(() =>
String(crypto.randomInt(10 ** 7, 10 ** 8 - 1))
);
const encryptedRecoveryCodes = encryptWithRoot(Buffer.from([...recoveryCodes, ...newRecoveryCodes].join(",")));

await totpConfigDAL.updateById(totpConfig.id, {
encryptedRecoveryCodes
});
});
};

return {
registerUserTotp,
verifyUserTotpConfig,
getUserTotpConfig,
verifyUserTotp,
verifyWithUserRecoveryCode,
deleteUserTotpConfig
deleteUserTotpConfig,
createUserTotpRecoveryCodes
};
};
4 changes: 4 additions & 0 deletions backend/src/services/totp/totp-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,7 @@ export type TVerifyWithUserRecoveryCodeDTO = {
export type TDeleteUserTotpConfigDTO = {
userId: string;
};

export type TCreateUserTotpRecoveryCodesDTO = {
userId: string;
};
28 changes: 17 additions & 11 deletions frontend/src/components/mfa/TotpRegistration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ type Props = {

const TotpRegistration = ({ onComplete }: Props) => {
const { data: registration, isLoading } = useGetUserTotpRegistration();
const { mutateAsync: verifyUserTotp } = useVerifyUserTotpRegistration();
const { mutateAsync: verifyUserTotp, isLoading: isVerifyLoading } =
useVerifyUserTotpRegistration();
const [qrCodeUrl, setQrCodeUrl] = useState("");
const [totp, setTotp] = useState("");

const handleTotpVerify = async () => {
const handleTotpVerify = async (event: React.FormEvent<HTMLFormElement>) => {
event.preventDefault();
await verifyUserTotp({
totp
});
Expand Down Expand Up @@ -54,15 +56,19 @@ const TotpRegistration = ({ onComplete }: Props) => {
<div className="mb-10 flex items-center justify-center">
<img src={qrCodeUrl} alt="registration-qr" />
</div>
<div className="mb-4 text-center">Enter the resulting verification code</div>
<div className="mb-4 flex flex-row gap-2">
<Input
onChange={(e) => setTotp(e.target.value)}
value={totp}
placeholder="Verification code"
/>
<Button onClick={handleTotpVerify}>Enable MFA</Button>
</div>
<form onSubmit={handleTotpVerify}>
<div className="mb-4 text-center">Enter the resulting verification code</div>
<div className="mb-4 flex flex-row gap-2">
<Input
onChange={(e) => setTotp(e.target.value)}
value={totp}
placeholder="Verification code"
/>
<Button isLoading={isVerifyLoading} type="submit">
Enable MFA
</Button>
</div>
</form>
</div>
);
};
Expand Down
14 changes: 14 additions & 0 deletions frontend/src/hooks/api/users/mutation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,17 @@ export const useDeleteUserTotpConfiguration = () => {
}
});
};

export const useCreateNewTotpRecoveryCodes = () => {
const queryClient = useQueryClient();
return useMutation({
mutationFn: async () => {
await apiRequest.post("/api/v1/user/me/totp/recovery-codes");

return {};
},
onSuccess: () => {
queryClient.invalidateQueries(userKeys.totpConfiguration);
}
});
};
2 changes: 1 addition & 1 deletion frontend/src/hooks/api/users/queries.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ export const useGetUserTotpConfiguration = () => {

return data;
} catch (error) {
if (error instanceof AxiosError && error.response?.data?.statusCode === 404) {
if (error instanceof AxiosError && [404, 400].includes(error.response?.data?.statusCode)) {
return {
isVerified: false,
recoveryCodes: []
Expand Down
Loading

0 comments on commit 433971a

Please sign in to comment.