Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account creation #48

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
+442 −127
Draft

Account creation #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
75803a1
create new file for verification email
adic2023 Oct 1, 2023
a973e35
attemp at email verification model
adic2023 Oct 2, 2023
604b2a3
attempt at email verification model
adic2023 Oct 2, 2023
5754a79
create IDs for email verification
adic2023 Oct 2, 2023
b0aa3c6
import verification request
adic2023 Oct 2, 2023
2fdf5ed
edit verification email
adic2023 Oct 3, 2023
58df8dc
merge after fixed integration
adic2023 Oct 3, 2023
a8392c3
Merge branch 'master' into accountCreation
adic2023 Oct 3, 2023
bba4cf4
Don't lint webStatic
RobinsonZ Oct 6, 2023
4245278
push updates
adic2023 Oct 6, 2023
cf0766c
Merge branch 'master' into accountCreation
RobinsonZ Oct 6, 2023
adf1bd0
emailverificationmodel check
Oct 8, 2023
a3ca740
rename verifyEMail
adic2023 Oct 8, 2023
3e173aa
Merge branch 'accountCreation' of https://github.com/swat-sccs/sauce …
adic2023 Oct 8, 2023
f6a7416
account request expiration
adic2023 Oct 8, 2023
839a177
change some routes
Oct 8, 2023
192efe7
change back
Oct 8, 2023
760facc
create and init
Lstsk Nov 5, 2023
1c44672
init page pug
Nov 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions agent/src/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ const setRateLimitHeaders = (limiterRes: RateLimiterRes | null, res: Response) =
// key configured statically - we aren't locking out people for typos
const requestRateLimiter = new RateLimiterMemory({
keyPrefix: 'ratelimit_auth_failures',
points: 1, // 1 requests
duration: 5, // per 5 seconds
points: 10, // 1 requests
duration: 1, // per 5 seconds
});

export const denyRateLimited: RequestHandler = catchErrors(async (req, res, next) => {
Expand Down
22 changes: 22 additions & 0 deletions emailTemplates/createAccount.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<html>
<head> </head>
<body>
<p>Hi there!</p>
<p>Start creating your sccs account </strong>.</p>
<p>
To continue creating your sccs account:
<a href="${domain}/account/init?id=${resetId}&key=${resetKey}">this link</a>. Or, copy and
paste this link into your browser:
</p>
<p>${domain}/account/init?id=${resetId}&key=${resetKey}</p>
<p>
(This link will stay active for seven days; after that, you'll have to
<a href="${domain}/account/forgot">request a new link</a>.)
</p>
<p>
Enjoy your new SCCS account! We hope you'll find our services useful. If you have any
questions, comments, or suggestions, feel free to contact us at
<a href="mailto:[email protected]">[email protected]</a>
</p>
</body>
</html>
24 changes: 24 additions & 0 deletions emailTemplates/verification.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<html>
<head> </head>
<body>
<p>Hi there!</p>
<p>You are receiving this message to verify the email associated with your new SCCS account! Your username is: <strong>${username}</strong>.</p>
<p>
To access SCCS services, confirm your email by clicking:
<!-- insert link with proper id and key -->
<a href="${domain}/account/init?id=${verifyId}&key=${verifyKey}">this link</a>. Or, copy and
<!-- account/init -->
paste this link into your browser:
</p>
<p>${domain}/account/init?id=${resetId}&key=${resetKey}</p>
<p>
(This link will stay active for seven days; after that, you'll have to
<a href="${domain}/account/create">request a new link</a>.)
</p>
<p>
Enjoy your new SCCS account! We hope you'll find our services useful. If you have any
questions, comments, or suggestionsx, feel free to contact us at
<a href="mailto:[email protected]">[email protected]</a>
</p>
</body>
</html>
24 changes: 24 additions & 0 deletions emailTemplates/verifyEmail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<html>
<head> </head>
<body>
<p>Hi there!</p>
<p>You are recieving this email because you signed up for SCCS account</strong>.</p>
<p>
To finish setting up your account, confirm your email by clicking
<!-- insert link with proper id and key -->
<a href="${domain}/account/init?id=${verifyId}&key=${verifyKey}">this link</a>. Or, copy and
<!-- account/init -->
paste this link into your browser:
</p>
<p>${domain}/account/init?id=${verifyId}&key=${verifyKey}</p>
<p>
(This link will stay active for seven days; after that, you'll have to
<a href="${domain}/account/create">request a new link</a>.)
</p>
<p>
Enjoy your new SCCS account! We hope you'll find our services useful. If you have any
questions, comments, or suggestionsx, feel free to contact us at
<a href="mailto:[email protected]">[email protected]</a>
</p>
</body>
</html>
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@
"prettier --write"
],
"*.js": [
"eslint --fix",
"prettier --write"
],
"*.pug": [
Expand Down
107 changes: 77 additions & 30 deletions src/controllers/accountController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,21 @@ import { HttpException } from '../error/httpException';
import { mailTransporter } from '../integration/email';
import { ldapClient } from '../integration/ldap';
import { modifyForwardFile } from '../integration/localAgent';
import { PasswordResetRequest, PasswordResetRequestModel, TaskModel } from '../integration/models';
import {
PasswordResetRequest,
PasswordResetRequestModel,
TaskModel,
VerifyEmailRequest,
VerifyEmailRequestModel,
} from '../integration/models';
import { generateEmail } from '../util/emailTemplates';
import { sendTaskNotification } from '../util/emailUtils';
import { modifyLdap, searchAsync, searchAsyncUid } from '../util/ldapUtils';
import { logger } from '../util/logging';
import { createPasswordResetRequest } from '../util/passwordReset';
import { testPassword } from '../util/passwordStrength';
import { createVerifyAccountRequest } from '../util/accountVerify';
import { verify } from 'crypto';

export const VALID_CLASSES = ['24', '25', '26', '27', 'faculty', 'staff'];
// TODO: do we have accounts in the system that don't match this username pattern?
Expand All @@ -27,55 +35,70 @@ const USERNAME_REGEX = /^[a-z][-a-z0-9]*$/;
export class CreateAccountReq {
// usernames must comply with debian/ubuntu standards so we can give them
// Heron accounts
@jf
.string()
.regex(/^[a-z][-a-z0-9]*$/, 'POSIX username')
.required()
username: string;

@jf
.string()
.email()
.regex(/.+@swarthmore\.edu/, 'Swarthmore email address')
// .regex(/.+@swarthmore\.edu/, 'Swarthmore email address')
.required()
email: string;

@jf.string().required()
name: string;

// FIXME would be nice to not manually update this every year
// TODO for that matter, can't we pull this from Cygnet?
@jf.string().valid(VALID_CLASSES).required()
classYear: string;
}

export const submitCreateAccountRequest = async (req: CreateAccountReq) => {
// TODO how do we handle someone going to create an account if they're
// already logged in?

if (!(await isUsernameAvailable(req.username))) {
throw new HttpException(400, { message: `Username ${req.username} already exists` });
}

if (!(await isEmailAvailable(req.email))) {
throw new HttpException(400, {
message: `Email ${req.email} is already associated with an account`,
});
}

logger.info(`Submitting CreateAccountReq ${JSON.stringify(req)}`);
const operation = new TaskModel({
_id: uuidv4(),
operation: 'createAccount',
createdTimestamp: Date.now(),
data: req,
const email = req.email;

const [verifyId, verifyKey] = await createVerifyAccountRequest(email);

logger.info(`Creating email verification ${JSON.stringify(req)}`);

const [emailText, transporter] = await Promise.all([
generateEmail('verifyEmail.html', {
email: email,
domain: process.env.EXTERNAL_ADDRESS,
verifyKey: verifyKey,
verifyId: verifyId,
}),
mailTransporter,
]);

const info = await transporter.sendMail({
from: process.env.EMAIL_FROM,
to: email,
subject: 'Your SCCS Account',
html: emailText,
});

await operation.save();
const msgUrl = nodemailer.getTestMessageUrl(info);
if (msgUrl) {
logger.debug(`View message at ${msgUrl}`);
}

sendTaskNotification(operation);
// logger.info(`Submitting CreateAccountReq ${JSON.stringify(req)}`);

// const operation = new TaskModel({
// _id: uuidv4(),
// operation: 'createAccount',
// createdTimestamp: Date.now(),
// data: req,
// });
};

export class InitCredentials {
@jf.string().required()
id: string;
@jf.string().required()
key: string;
}

export const doPasswordResetRequest = async (identifier: string) => {
try {
let account: any = null;
Expand Down Expand Up @@ -125,7 +148,7 @@ export const doPasswordResetRequest = async (identifier: string) => {

/**
*/
export class PasswordResetCredentials {
export class ResetCredentials {
@jf.string().required()
id: string;
@jf.string().required()
Expand All @@ -134,7 +157,7 @@ export class PasswordResetCredentials {

/**
*/
export class PasswordResetRequestParams extends PasswordResetCredentials {
export class PasswordResetRequestParams extends ResetCredentials {
// we'll properly validate it later
@jf.string().required()
password: string;
Expand All @@ -146,7 +169,7 @@ export class PasswordResetRequestParams extends PasswordResetCredentials {
* @param {PasswordResetCredentials} creds The credentials to check
*/
export const verifyPasswordReset = async (
creds: PasswordResetCredentials,
creds: ResetCredentials,
): Promise<PasswordResetRequest> => {
const invalidProps = {
friendlyMessage:
Expand All @@ -170,6 +193,29 @@ export const verifyPasswordReset = async (
return resetRequest;
};

export const verifyEmail = async (creds: ResetCredentials): Promise<VerifyEmailRequest> => {
const invalidProps = {
friendlyMessage:
'This email verification link is invalid or expired. <a href="/account/create">Request a new one</a>.',
};

const verifyRequest = await VerifyEmailRequestModel.findById(creds.id);
if (!verifyRequest) {
throw new HttpException(400, {
...invalidProps,
message: `Email verification ID ${creds.id} did not match any request`,
});
}

if (!(await argon2.verify(verifyRequest.key, creds.key as string))) {
throw new HttpException(400, {
...invalidProps,
message: 'Email verification key did not match database',
});
}
return verifyRequest;
};

export const doPasswordReset = async (params: PasswordResetRequestParams) => {
const resetRequest = await verifyPasswordReset(params);

Expand Down Expand Up @@ -268,6 +314,7 @@ export const isEmailAvailable = async (email: string): Promise<boolean> => {
const [inDatabase, inPending] = await Promise.all([
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this code needs slightly more tweaking - needs to capture the results in three different variables

Suggested change
const [inDatabase, inPending] = await Promise.all([
const [inDatabase, inPending, inNeedsVerify] = await Promise.all([

searchAsync(ldapClient, ldapEscape.filter`(swatmail=${email})`),
TaskModel.exists({ 'data.email': email, status: 'pending' }),
VerifyEmailRequestModel.exists({ 'data.email': email }),
]);

if (inDatabase || inPending) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (inDatabase || inPending) {
if (inDatabase || inPending || inNeedsVerify) {

Expand Down
19 changes: 19 additions & 0 deletions src/functions/createAccount.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { generateEmail } from '../util/emailTemplates';
import { addLdap, searchAsyncMultiple } from '../util/ldapUtils';
import { logger } from '../util/logging';
import { createPasswordResetRequest } from '../util/passwordReset';
import { createVerifyAccountRequest } from '../util/accountVerify';

export interface CreateAccountData {
username: string;
Expand Down Expand Up @@ -96,4 +97,22 @@ export const createAccount = async (data: any) => {
} else {
throw new Error('CreateAccount data not properly formatted');
}

const [verifyId, verifyKey] = await createVerifyAccountRequest(data.email, 24 * 7, true);
const [emailText, transporter] = await Promise.all([
generateEmail('verification.html', {
email: data.email,
domain: process.env.EXTERNAL_ADDRESS,
verifyKey: verifyKey,
verifyId: verifyId,
}),
mailTransporter,
]);

const info = await transporter.sendMail({
from: process.env.EMAIL_FROM,
to: data.email,
subject: 'Verify your SCCS account',
html: emailText,
});
};
43 changes: 43 additions & 0 deletions src/integration/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,49 @@ export const PasswordResetRequestModel = mongoose.model<PasswordResetRequest>(
passwordResetRequestSchema,
);

// new interface for email verification
export interface VerifyEmailRequest {
// verification email links generate two keys: the ID, which is stored plain, and the key, which is
// hashed with argon2 before being stored.
// Both are provided to the user and they make a request;
// then we look up the request for the ID and compare hashed keys, basically exactly like a normal
// username/password login flow.
_id: string;
key: string;
email: string;
timestamp: Date;
suppressEmail?: boolean;
}

const verifyEmailRequestSchema = new mongoose.Schema<VerifyEmailRequest>({
_id: {
type: String,
required: true,
},
key: {
type: String,
required: true,
},
email: {
type: String,
required: true,
index: true,
},
timestamp: {
type: Date,
expires: 0,
},
suppressEmail: {
type: Boolean,
required: false,
},
Comment on lines +119 to +122
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't necessary for email verification requests, since there isn't really a "confirmation" of the email being verified like there is for password resets. We have this option in the password reset model because there's two scenarios where we do a password reset: 1) explicitly creating a new account and 2) a user-requested password reset. In scenario 2 we send a confirmation email after the change happens (see here), but not in scenario 1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, why was this marked as resolved? This hasn't been fixed.

});

export const VerifyEmailRequestModel = mongoose.model<VerifyEmailRequest>(
'VerifyEmail',
verifyEmailRequestSchema,
);

export interface MinecraftWhitelist {
_id: string;
mcUuid: string;
Expand Down
Loading