mirror of
https://github.com/hyperdxio/hyperdx
synced 2026-04-21 13:37:15 +00:00
feat: enhanced registration form validation (#81)
Close #44 - `/register/password` API validation with [zod](https://github.com/colinhacks/zod) and [zod-express-middleware](https://github.com/Aquila169/zod-express-middleware). - API will respond instead of redirect. Mainly for displaying validation and business logic errors without query params. - FE `AuthPage` will keep using original `/login/password` API with redirect, for `register` action a request will be made instead - Updated test to match implementation - Updated validation rules as discussed here - https://github.com/hyperdxio/hyperdx/issues/44#issuecomment-1741327421 Things I considered but decided to omit: - **Keep redirects and use query params for erros**: validation errors should not be in query params - **Update `/login/password` to drop redirect**: might be a good idea, but not in the context of this issue - **Break form into login and registration**: perhaps later - **Refactor and restructuring**: same above - more of a tech debt than feature related
This commit is contained in:
Mark Omarov
GitHub
parent
bf8af29d68
commit
7d636f24b3
5 changed files with 161 additions and 79 deletions
6
.changeset/warm-timers-whisper.md
Normal file
6
.changeset/warm-timers-whisper.md
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
'@hyperdx/api': minor
|
||||
'@hyperdx/app': minor
|
||||
---
|
||||
|
||||
feat: enhanced registration form validation
|
||||
|
|
@ -9,21 +9,20 @@ import {
|
|||
import { getTeam } from '../../../controllers/team';
|
||||
import { findUserByEmail } from '../../../controllers/user';
|
||||
|
||||
const MOCK_USER = {
|
||||
email: 'fake@deploysentinel.com',
|
||||
password: 'TacoCat!2#4X',
|
||||
};
|
||||
|
||||
describe('team router', () => {
|
||||
const server = getServer();
|
||||
|
||||
const login = async () => {
|
||||
const agent = getAgent(server);
|
||||
|
||||
await agent
|
||||
.post('/register/password')
|
||||
.send({
|
||||
email: 'fake@deploysentinel.com',
|
||||
password: 'tacocat1234',
|
||||
})
|
||||
.expect(302);
|
||||
await agent.post('/register/password').send(MOCK_USER).expect(200);
|
||||
|
||||
const user = await findUserByEmail('fake@deploysentinel.com');
|
||||
const user = await findUserByEmail(MOCK_USER.email);
|
||||
const team = await getTeam(user?.team as any);
|
||||
|
||||
if (team === null || user === null) {
|
||||
|
|
@ -33,13 +32,7 @@ describe('team router', () => {
|
|||
await user.save();
|
||||
|
||||
// login app
|
||||
await agent
|
||||
.post('/login/password')
|
||||
.send({
|
||||
email: 'fake@deploysentinel.com',
|
||||
password: 'tacocat1234',
|
||||
})
|
||||
.expect(302);
|
||||
await agent.post('/login/password').send(MOCK_USER).expect(302);
|
||||
|
||||
return {
|
||||
agent,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import express from 'express';
|
||||
import isemail from 'isemail';
|
||||
import { serializeError } from 'serialize-error';
|
||||
import { z } from 'zod';
|
||||
import { validateRequest } from 'zod-express-middleware';
|
||||
|
||||
import * as config from '../../config';
|
||||
import User from '../../models/user'; // TODO -> do not import model directly
|
||||
|
|
@ -8,13 +9,31 @@ import logger from '../../utils/logger';
|
|||
import passport from '../../utils/passport';
|
||||
import { Api404Error } from '../../utils/errors';
|
||||
import { isTeamExisting, createTeam, getTeam } from '../../controllers/team';
|
||||
import { validatePassword } from '../../utils/validators';
|
||||
import {
|
||||
isUserAuthenticated,
|
||||
redirectToDashboard,
|
||||
handleAuthError,
|
||||
} from '../../middleware/auth';
|
||||
|
||||
const registrationSchema = z.object({
|
||||
email: z.string().email(),
|
||||
password: z
|
||||
.string()
|
||||
.min(12, 'Password must have at least 12 characters')
|
||||
.refine(
|
||||
pass => /[a-z]/.test(pass) && /[A-Z]/.test(pass),
|
||||
'Password must include both lower and upper case characters',
|
||||
)
|
||||
.refine(
|
||||
pass => /\d/.test(pass),
|
||||
'Password must include at least one number',
|
||||
)
|
||||
.refine(
|
||||
pass => /[!@#$%^&*(),.?":{}|<>]/.test(pass),
|
||||
'Password must include at least one special character',
|
||||
),
|
||||
});
|
||||
|
||||
const router = express.Router();
|
||||
|
||||
router.get('/health', async (req, res) => {
|
||||
|
|
@ -64,49 +83,50 @@ router.post(
|
|||
handleAuthError,
|
||||
);
|
||||
|
||||
router.post('/register/password', async (req, res, next) => {
|
||||
try {
|
||||
const { email, password } = req.body;
|
||||
router.post(
|
||||
'/register/password',
|
||||
validateRequest({ body: registrationSchema }),
|
||||
async (req, res, next) => {
|
||||
try {
|
||||
const { email, password } = req.body;
|
||||
|
||||
if (!email || !password) {
|
||||
return res.redirect(`${config.FRONTEND_URL}/register?err=missing`);
|
||||
}
|
||||
if (await isTeamExisting()) {
|
||||
return res.status(409).json({ error: 'teamAlreadyExists' });
|
||||
}
|
||||
|
||||
if (!isemail.validate(email) || !validatePassword(password)) {
|
||||
return res.redirect(`${config.FRONTEND_URL}/register?err=invalid`);
|
||||
}
|
||||
(User as any).register(
|
||||
new User({ email }),
|
||||
password,
|
||||
async (err: Error, user: any) => {
|
||||
if (err) {
|
||||
logger.error(serializeError(err));
|
||||
return res.status(400).json({ error: 'invalid' });
|
||||
}
|
||||
|
||||
if (await isTeamExisting()) {
|
||||
return res.redirect(
|
||||
`${config.FRONTEND_URL}/register?err=teamAlreadyExists`,
|
||||
const team = await createTeam({
|
||||
name: `${email}'s Team`,
|
||||
});
|
||||
user.team = team._id;
|
||||
user.name = email;
|
||||
await user.save();
|
||||
|
||||
return passport.authenticate('local')(req, res, () => {
|
||||
if (req?.user?.team) {
|
||||
return res.status(200).json({ status: 'success' });
|
||||
}
|
||||
|
||||
logger.error(
|
||||
`Password login for user failed, user or team not found ${req?.user?._id}`,
|
||||
);
|
||||
return res.status(400).json({ error: 'invalid' });
|
||||
});
|
||||
},
|
||||
);
|
||||
} catch (e) {
|
||||
next(e);
|
||||
}
|
||||
|
||||
(User as any).register(
|
||||
new User({ email }),
|
||||
password,
|
||||
async (err: Error, user: any) => {
|
||||
if (err) {
|
||||
logger.error(serializeError(err));
|
||||
return res.redirect(`${config.FRONTEND_URL}/register?err=invalid`);
|
||||
}
|
||||
|
||||
const team = await createTeam({
|
||||
name: `${email}'s Team`,
|
||||
});
|
||||
user.team = team._id;
|
||||
user.name = email;
|
||||
await user.save();
|
||||
|
||||
return passport.authenticate('local')(req, res, () => {
|
||||
redirectToDashboard(req, res);
|
||||
});
|
||||
},
|
||||
);
|
||||
} catch (e) {
|
||||
next(e);
|
||||
}
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
router.get('/logout', (req, res) => {
|
||||
// @ts-ignore
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { useForm, SubmitHandler } from 'react-hook-form';
|
||||
import { Button, Form } from 'react-bootstrap';
|
||||
import { NextSeo } from 'next-seo';
|
||||
import { API_SERVER_URL } from './config';
|
||||
|
|
@ -9,27 +10,78 @@ import LandingHeader from './LandingHeader';
|
|||
import * as config from './config';
|
||||
import api from './api';
|
||||
|
||||
type FormData = {
|
||||
email: string;
|
||||
password: string;
|
||||
};
|
||||
|
||||
export default function AuthPage({ action }: { action: 'register' | 'login' }) {
|
||||
const isRegister = action === 'register';
|
||||
const {
|
||||
register,
|
||||
handleSubmit,
|
||||
formState: { errors, isSubmitting },
|
||||
setError,
|
||||
} = useForm<FormData>({
|
||||
reValidateMode: 'onSubmit',
|
||||
});
|
||||
const router = useRouter();
|
||||
const { err, msg } = router.query;
|
||||
|
||||
const { data: installation } = api.useInstallation();
|
||||
const registerPassword = api.useRegisterPassword();
|
||||
|
||||
const verificationSent = msg === 'verify';
|
||||
|
||||
const title = `HyperDX - ${action === 'register' ? 'Sign up' : 'Login'}`;
|
||||
const title = `HyperDX - ${isRegister ? 'Sign up' : 'Login'}`;
|
||||
|
||||
useEffect(() => {
|
||||
// If an OSS user accidentally lands on /register after already creating a team
|
||||
// redirect them to login instead
|
||||
if (
|
||||
config.IS_OSS &&
|
||||
installation?.isTeamExisting === true &&
|
||||
action === 'register'
|
||||
) {
|
||||
if (config.IS_OSS && installation?.isTeamExisting === true && isRegister) {
|
||||
router.push('/login');
|
||||
}
|
||||
}, [installation, action, router]);
|
||||
}, [installation, isRegister, router]);
|
||||
|
||||
const onSubmit: SubmitHandler<FormData> = data =>
|
||||
registerPassword.mutate(
|
||||
{ email: data.email, password: data.password },
|
||||
{
|
||||
onSuccess: () => router.push('/search'),
|
||||
onError: async error => {
|
||||
const jsonData = await error.response.json();
|
||||
|
||||
if (Array.isArray(jsonData) && jsonData[0]?.errors?.issues) {
|
||||
return jsonData[0].errors.issues.forEach((issue: any) => {
|
||||
setError(issue.path[0], {
|
||||
type: issue.code,
|
||||
message: issue.message,
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
setError('root', {
|
||||
type: 'manual',
|
||||
message: 'An unexpected error occurred, please try again later.',
|
||||
});
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
const form = isRegister
|
||||
? {
|
||||
controller: { onSubmit: handleSubmit(onSubmit) },
|
||||
email: register('email', { required: true }),
|
||||
password: register('password', { required: true }),
|
||||
}
|
||||
: {
|
||||
controller: {
|
||||
action: `${API_SERVER_URL}/login/password`,
|
||||
method: 'POST',
|
||||
},
|
||||
email: { name: 'email' },
|
||||
password: { name: 'password' },
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="AuthPage">
|
||||
|
|
@ -38,9 +90,9 @@ export default function AuthPage({ action }: { action: 'register' | 'login' }) {
|
|||
<div className="d-flex align-items-center justify-content-center vh-100 p-2">
|
||||
<div>
|
||||
<div className="text-center mb-4 fs-5">
|
||||
{config.IS_OSS && action === 'register'
|
||||
{config.IS_OSS && isRegister
|
||||
? 'Setup '
|
||||
: action === 'register'
|
||||
: isRegister
|
||||
? 'Register for '
|
||||
: 'Login to '}
|
||||
<span className="text-success fw-bold">HyperDX</span>
|
||||
|
|
@ -48,7 +100,7 @@ export default function AuthPage({ action }: { action: 'register' | 'login' }) {
|
|||
{action === 'login' && (
|
||||
<div className="text-center mb-4 text-muted">Welcome back!</div>
|
||||
)}
|
||||
{action === 'register' && config.IS_OSS === true && (
|
||||
{isRegister && config.IS_OSS === true && (
|
||||
<div className="text-center mb-4 text-muted">
|
||||
Let{"'"}s create your user account.
|
||||
</div>
|
||||
|
|
@ -58,15 +110,7 @@ export default function AuthPage({ action }: { action: 'register' | 'login' }) {
|
|||
style={{ maxWidth: 400, minWidth: 400, width: '100%' }}
|
||||
>
|
||||
<div className="text-center">
|
||||
<Form
|
||||
className="text-start"
|
||||
action={
|
||||
action === 'register'
|
||||
? `${API_SERVER_URL}/register/password`
|
||||
: `${API_SERVER_URL}/login/password`
|
||||
}
|
||||
method="POST"
|
||||
>
|
||||
<Form className="text-start" {...form.controller}>
|
||||
<Form.Label
|
||||
htmlFor="email"
|
||||
className="text-start text-muted fs-7.5 mb-1"
|
||||
|
|
@ -76,10 +120,10 @@ export default function AuthPage({ action }: { action: 'register' | 'login' }) {
|
|||
<Form.Control
|
||||
data-test-id="form-email"
|
||||
id="email"
|
||||
name="email"
|
||||
type="email"
|
||||
placeholder="you@company.com"
|
||||
className="border-0 mb-3"
|
||||
{...form.email}
|
||||
/>
|
||||
<Form.Label
|
||||
htmlFor="password"
|
||||
|
|
@ -90,10 +134,17 @@ export default function AuthPage({ action }: { action: 'register' | 'login' }) {
|
|||
<Form.Control
|
||||
data-test-id="form-password"
|
||||
id="password"
|
||||
name="password"
|
||||
type="password"
|
||||
className="border-0"
|
||||
{...form.password}
|
||||
/>
|
||||
{isRegister && Object.keys(errors).length > 0 && (
|
||||
<div className="text-danger mt-2">
|
||||
{Object.values(errors).map((error, index) => (
|
||||
<div key={index}>{error.message}</div>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
{err != null && (
|
||||
<div
|
||||
className="text-danger mt-2"
|
||||
|
|
@ -123,13 +174,13 @@ export default function AuthPage({ action }: { action: 'register' | 'login' }) {
|
|||
className="px-6"
|
||||
type="submit"
|
||||
data-test-id="submit"
|
||||
disabled={verificationSent}
|
||||
disabled={isSubmitting || verificationSent}
|
||||
>
|
||||
{action === 'register' ? 'Register' : 'Login'}
|
||||
{isRegister ? 'Register' : 'Login'}
|
||||
</Button>
|
||||
</div>
|
||||
</Form>
|
||||
{action === 'register' && config.IS_OSS === false && (
|
||||
{isRegister && config.IS_OSS === false && (
|
||||
<div data-test-id="login-link" className="mt-4 text-muted">
|
||||
Already have an account? <Link href="/login">Log in</Link>{' '}
|
||||
instead.
|
||||
|
|
|
|||
|
|
@ -544,5 +544,17 @@ const api = {
|
|||
}).json(),
|
||||
);
|
||||
},
|
||||
useRegisterPassword() {
|
||||
return useMutation<any, HTTPError, { email: string; password: string }>(
|
||||
async ({ email, password }) =>
|
||||
server(`register/password`, {
|
||||
method: 'POST',
|
||||
json: {
|
||||
email,
|
||||
password,
|
||||
},
|
||||
}).json(),
|
||||
);
|
||||
},
|
||||
};
|
||||
export default api;
|
||||
|
|
|
|||
Loading…
Reference in a new issue