diff --git a/app/Http/Controllers/Api/Client/TwoFactorController.php b/app/Http/Controllers/Api/Client/TwoFactorController.php index 40b5479cb..74857cd7a 100644 --- a/app/Http/Controllers/Api/Client/TwoFactorController.php +++ b/app/Http/Controllers/Api/Client/TwoFactorController.php @@ -8,7 +8,6 @@ use Illuminate\Http\Response; use Illuminate\Http\JsonResponse; use Pterodactyl\Facades\Activity; use Illuminate\Contracts\Validation\Factory; -use Illuminate\Validation\ValidationException; use Pterodactyl\Services\Users\TwoFactorSetupService; use Pterodactyl\Services\Users\ToggleTwoFactorService; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -73,22 +72,20 @@ class TwoFactorController extends ClientApiController * * @throws \Throwable * @throws \Illuminate\Validation\ValidationException - * @throws \PragmaRX\Google2FA\Exceptions\IncompatibleWithGoogleAuthenticatorException - * @throws \PragmaRX\Google2FA\Exceptions\InvalidCharactersException - * @throws \PragmaRX\Google2FA\Exceptions\SecretKeyTooShortException - * @throws \Pterodactyl\Exceptions\Service\User\TwoFactorAuthenticationTokenInvalid */ public function store(Request $request) { $validator = $this->validation->make($request->all(), [ - 'code' => 'required|string', + 'code' => ['required', 'string', 'size:6'], + 'password' => ['required', 'string'], ]); - if ($validator->fails()) { - throw new ValidationException($validator); + $data = $validator->validate(); + if (!password_verify($data['password'], $request->user()->password)) { + throw new BadRequestHttpException('The password provided was not valid.'); } - $tokens = $this->toggleTwoFactorService->handle($request->user(), $request->input('code'), true); + $tokens = $this->toggleTwoFactorService->handle($request->user(), $data['code'], true); Activity::event('user:two-factor.create')->log(); @@ -105,6 +102,7 @@ class TwoFactorController extends ClientApiController * is valid. * * @return \Illuminate\Http\JsonResponse + * @throws \Throwable */ public function delete(Request $request) { diff --git a/resources/scripts/api/account/enableAccountTwoFactor.ts b/resources/scripts/api/account/enableAccountTwoFactor.ts index e7a15f62d..25c63ad47 100644 --- a/resources/scripts/api/account/enableAccountTwoFactor.ts +++ b/resources/scripts/api/account/enableAccountTwoFactor.ts @@ -1,7 +1,7 @@ import http from '@/api/http'; -export default async (code: string): Promise => { - const { data } = await http.post('/api/client/account/two-factor', { code }); +export default async (code: string, password: string): Promise => { + const { data } = await http.post('/api/client/account/two-factor', { code, password }); return data.attributes.tokens; }; diff --git a/resources/scripts/components/dashboard/forms/ConfigureTwoFactorForm.tsx b/resources/scripts/components/dashboard/forms/ConfigureTwoFactorForm.tsx index 109eb182a..a59977f5f 100644 --- a/resources/scripts/components/dashboard/forms/ConfigureTwoFactorForm.tsx +++ b/resources/scripts/components/dashboard/forms/ConfigureTwoFactorForm.tsx @@ -1,16 +1,24 @@ -import React, { useState } from 'react'; +import React, { useEffect, useState } from 'react'; import { useStoreState } from 'easy-peasy'; import { ApplicationStore } from '@/state'; import tw from 'twin.macro'; import { Button } from '@/components/elements/button/index'; -import DisableTwoFactorModal from '@/components/dashboard/forms/DisableTwoFactorModal'; -import SetupTOTPModal from '@/components/dashboard/forms/SetupTOTPModal'; +import SetupTOTPDialog from '@/components/dashboard/forms/SetupTOTPDialog'; import RecoveryTokensDialog from '@/components/dashboard/forms/RecoveryTokensDialog'; +import DisableTOTPDialog from '@/components/dashboard/forms/DisableTOTPDialog'; +import { useFlashKey } from '@/plugins/useFlash'; export default () => { const [tokens, setTokens] = useState([]); const [visible, setVisible] = useState<'enable' | 'disable' | null>(null); const isEnabled = useStoreState((state: ApplicationStore) => state.user.data!.useTotp); + const { clearAndAddHttpError } = useFlashKey('account:two-step'); + + useEffect(() => { + return () => { + clearAndAddHttpError(); + }; + }, [visible]); const onTokens = (tokens: string[]) => { setTokens(tokens); @@ -19,9 +27,9 @@ export default () => { return (
- setVisible(null)} onTokens={onTokens} /> + setVisible(null)} onTokens={onTokens} /> 0} onClose={() => setTokens([])} /> - setVisible(null)} /> + setVisible(null)} />

{isEnabled ? 'Two-step verification is currently enabled on your account.' diff --git a/resources/scripts/components/dashboard/forms/DisableTOTPDialog.tsx b/resources/scripts/components/dashboard/forms/DisableTOTPDialog.tsx new file mode 100644 index 000000000..125376eb5 --- /dev/null +++ b/resources/scripts/components/dashboard/forms/DisableTOTPDialog.tsx @@ -0,0 +1,72 @@ +import React, { useContext, useEffect, useState } from 'react'; +import asDialog from '@/hoc/asDialog'; +import { Dialog, DialogWrapperContext } from '@/components/elements/dialog'; +import { Button } from '@/components/elements/button/index'; +import { Input } from '@/components/elements/inputs'; +import Tooltip from '@/components/elements/tooltip/Tooltip'; +import disableAccountTwoFactor from '@/api/account/disableAccountTwoFactor'; +import { useFlashKey } from '@/plugins/useFlash'; +import { useStoreActions } from '@/state/hooks'; +import FlashMessageRender from '@/components/FlashMessageRender'; + +const DisableTOTPDialog = () => { + const [submitting, setSubmitting] = useState(false); + const [password, setPassword] = useState(''); + const { clearAndAddHttpError } = useFlashKey('account:two-step'); + const { close, setProps } = useContext(DialogWrapperContext); + const updateUserData = useStoreActions((actions) => actions.user.updateUserData); + + useEffect(() => { + setProps((state) => ({ ...state, preventExternalClose: submitting })); + }, [submitting]); + + const submit = (e: React.FormEvent) => { + e.preventDefault(); + e.stopPropagation(); + + if (submitting) return; + + setSubmitting(true); + clearAndAddHttpError(); + disableAccountTwoFactor(password) + .then(() => { + updateUserData({ useTotp: false }); + close(); + }) + .catch(clearAndAddHttpError) + .then(() => setSubmitting(false)); + }; + + return ( +

+ + + setPassword(e.currentTarget.value)} + /> + + Cancel + 0} + content={'You must enter your account password to continue.'} + > + + Disable + + + + + ); +}; + +export default asDialog({ + title: 'Disable Two-Step Verification', + description: 'Disabling two-step verification will make your account less secure.', +})(DisableTOTPDialog); diff --git a/resources/scripts/components/dashboard/forms/DisableTwoFactorModal.tsx b/resources/scripts/components/dashboard/forms/DisableTwoFactorModal.tsx deleted file mode 100644 index 4f37ffbb4..000000000 --- a/resources/scripts/components/dashboard/forms/DisableTwoFactorModal.tsx +++ /dev/null @@ -1,73 +0,0 @@ -import React, { useContext } from 'react'; -import { Form, Formik, FormikHelpers } from 'formik'; -import FlashMessageRender from '@/components/FlashMessageRender'; -import Field from '@/components/elements/Field'; -import { object, string } from 'yup'; -import { Actions, useStoreActions } from 'easy-peasy'; -import { ApplicationStore } from '@/state'; -import disableAccountTwoFactor from '@/api/account/disableAccountTwoFactor'; -import tw from 'twin.macro'; -import Button from '@/components/elements/Button'; -import asModal from '@/hoc/asModal'; -import ModalContext from '@/context/ModalContext'; - -interface Values { - password: string; -} - -const DisableTwoFactorModal = () => { - const { dismiss, setPropOverrides } = useContext(ModalContext); - const { clearAndAddHttpError } = useStoreActions((actions: Actions) => actions.flashes); - const updateUserData = useStoreActions((actions: Actions) => actions.user.updateUserData); - - const submit = ({ password }: Values, { setSubmitting }: FormikHelpers) => { - setPropOverrides({ showSpinnerOverlay: true, dismissable: false }); - disableAccountTwoFactor(password) - .then(() => { - updateUserData({ useTotp: false }); - dismiss(); - }) - .catch((error) => { - console.error(error); - - clearAndAddHttpError({ error, key: 'account:two-factor' }); - setSubmitting(false); - setPropOverrides(null); - }); - }; - - return ( - - {({ isValid }) => ( -
- - -
- -
- - )} -
- ); -}; - -export default asModal()(DisableTwoFactorModal); diff --git a/resources/scripts/components/dashboard/forms/SetupTOTPModal.tsx b/resources/scripts/components/dashboard/forms/SetupTOTPDialog.tsx similarity index 67% rename from resources/scripts/components/dashboard/forms/SetupTOTPModal.tsx rename to resources/scripts/components/dashboard/forms/SetupTOTPDialog.tsx index b3d72a7a1..58f593b0f 100644 --- a/resources/scripts/components/dashboard/forms/SetupTOTPModal.tsx +++ b/resources/scripts/components/dashboard/forms/SetupTOTPDialog.tsx @@ -22,11 +22,12 @@ interface Props { const ConfigureTwoFactorForm = ({ onTokens }: Props) => { const [submitting, setSubmitting] = useState(false); const [value, setValue] = useState(''); + const [password, setPassword] = useState(''); const [token, setToken] = useState(null); const { clearAndAddHttpError } = useFlashKey('account:two-step'); const updateUserData = useStoreActions((actions: Actions) => actions.user.updateUserData); - const { close } = useContext(DialogWrapperContext); + const { close, setProps } = useContext(DialogWrapperContext); useEffect(() => { getTwoFactorTokenData() @@ -34,13 +35,19 @@ const ConfigureTwoFactorForm = ({ onTokens }: Props) => { .catch((error) => clearAndAddHttpError(error)); }, []); - const submit = () => { + useEffect(() => { + setProps((state) => ({ ...state, preventExternalClose: submitting })); + }, [submitting]); + + const submit = (e: React.FormEvent) => { + e.preventDefault(); + e.stopPropagation(); + if (submitting) return; setSubmitting(true); clearAndAddHttpError(); - - enableAccountTwoFactor(value) + enableAccountTwoFactor(value, password) .then((tokens) => { updateUserData({ useTotp: true }); onTokens(tokens); @@ -52,7 +59,7 @@ const ConfigureTwoFactorForm = ({ onTokens }: Props) => { }; return ( - <> +
{ {token?.secret.match(/.{1,4}/g)!.join(' ') || 'Loading...'}

-
-

- Scan the QR code above using the two-step authentication app of your choice. Then, enter the 6-digit - code generated into the field below. -

-
+

+ Scan the QR code above using the two-step authentication app of your choice. Then, enter the 6-digit + code generated into the field below. +

setValue(e.currentTarget.value)} - className={'mt-4'} + className={'mt-3'} placeholder={'000000'} type={'text'} inputMode={'numeric'} autoComplete={'one-time-code'} pattern={'\\d{6}'} /> + + setPassword(e.currentTarget.value)} + /> Cancel 0 && value.length === 6} + content={ + !token + ? 'Waiting for QR code to load...' + : 'You must enter the 6-digit code and your password to continue.' + } delay={100} > - - + ); }; diff --git a/resources/scripts/components/dashboard/forms/UpdateEmailAddressForm.tsx b/resources/scripts/components/dashboard/forms/UpdateEmailAddressForm.tsx index c13ed073a..462a37f1a 100644 --- a/resources/scripts/components/dashboard/forms/UpdateEmailAddressForm.tsx +++ b/resources/scripts/components/dashboard/forms/UpdateEmailAddressForm.tsx @@ -7,7 +7,7 @@ import Field from '@/components/elements/Field'; import { httpErrorToHuman } from '@/api/http'; import { ApplicationStore } from '@/state'; import tw from 'twin.macro'; -import Button from '@/components/elements/Button'; +import { Button } from '@/components/elements/button/index'; interface Values { email: string; @@ -66,9 +66,7 @@ export default () => { />
- +
diff --git a/resources/scripts/components/dashboard/forms/UpdatePasswordForm.tsx b/resources/scripts/components/dashboard/forms/UpdatePasswordForm.tsx index a82bfa1b7..707ee72d7 100644 --- a/resources/scripts/components/dashboard/forms/UpdatePasswordForm.tsx +++ b/resources/scripts/components/dashboard/forms/UpdatePasswordForm.tsx @@ -8,7 +8,7 @@ import updateAccountPassword from '@/api/account/updateAccountPassword'; import { httpErrorToHuman } from '@/api/http'; import { ApplicationStore } from '@/state'; import tw from 'twin.macro'; -import Button from '@/components/elements/Button'; +import { Button } from '@/components/elements/button/index'; interface Values { current: string; @@ -91,9 +91,7 @@ export default () => { />
- +
diff --git a/resources/scripts/components/elements/dialog/types.d.ts b/resources/scripts/components/elements/dialog/types.d.ts index f69a466d3..2701b1dda 100644 --- a/resources/scripts/components/elements/dialog/types.d.ts +++ b/resources/scripts/components/elements/dialog/types.d.ts @@ -27,7 +27,7 @@ export interface RenderDialogProps extends DialogProps { export type WrapperProps = Omit; export interface DialogWrapperContextType { props: Readonly; - setProps: Callback>; + setProps: React.Dispatch>; close: () => void; } diff --git a/tests/Integration/Api/Client/TwoFactorControllerTest.php b/tests/Integration/Api/Client/TwoFactorControllerTest.php index 903efb853..905d67d2b 100644 --- a/tests/Integration/Api/Client/TwoFactorControllerTest.php +++ b/tests/Integration/Api/Client/TwoFactorControllerTest.php @@ -59,13 +59,13 @@ class TwoFactorControllerTest extends ClientApiIntegrationTestCase /** @var \Pterodactyl\Models\User $user */ $user = User::factory()->create(['use_totp' => false]); - $response = $this->actingAs($user)->postJson('/api/client/account/two-factor', [ - 'code' => '', - ]); - - $response->assertStatus(Response::HTTP_UNPROCESSABLE_ENTITY); - $response->assertJsonPath('errors.0.code', 'ValidationException'); - $response->assertJsonPath('errors.0.meta.rule', 'required'); + $this->actingAs($user) + ->postJson('/api/client/account/two-factor', ['code' => '']) + ->assertUnprocessable() + ->assertJsonPath('errors.0.meta.rule', 'required') + ->assertJsonPath('errors.0.meta.source_field', 'code') + ->assertJsonPath('errors.1.meta.rule', 'required') + ->assertJsonPath('errors.1.meta.source_field', 'password'); } /** @@ -90,6 +90,7 @@ class TwoFactorControllerTest extends ClientApiIntegrationTestCase $response = $this->actingAs($user)->postJson('/api/client/account/two-factor', [ 'code' => $token, + 'password' => 'password', ]); $response->assertOk(); @@ -168,4 +169,39 @@ class TwoFactorControllerTest extends ClientApiIntegrationTestCase $response->assertStatus(Response::HTTP_NO_CONTENT); } + + /** + * Test that a valid account password is required when enabling two-factor. + */ + public function testEnablingTwoFactorRequiresValidPassword() + { + $user = User::factory()->create(['use_totp' => false]); + + $this->actingAs($user) + ->postJson('/api/client/account/two-factor', [ + 'code' => '123456', + 'password' => 'foo', + ]) + ->assertStatus(Response::HTTP_BAD_REQUEST) + ->assertJsonPath('errors.0.detail', 'The password provided was not valid.'); + + $this->assertFalse($user->refresh()->use_totp); + } + + /** + * Test that a valid account password is required when disabling two-factor. + */ + public function testDisablingTwoFactorRequiresValidPassword() + { + $user = User::factory()->create(['use_totp' => true]); + + $this->actingAs($user) + ->deleteJson('/api/client/account/two-factor', [ + 'password' => 'foo', + ]) + ->assertStatus(Response::HTTP_BAD_REQUEST) + ->assertJsonPath('errors.0.detail', 'The password provided was not valid.'); + + $this->assertTrue($user->refresh()->use_totp); + } }