diff --git a/scripts/setup-mysql.sql b/scripts/setup-mysql.sql index f44586a..c7909d9 100644 --- a/scripts/setup-mysql.sql +++ b/scripts/setup-mysql.sql @@ -30,7 +30,7 @@ GRANT SELECT, UPDATE ON closet_lists TO impress2020; GRANT SELECT, DELETE ON item_outfit_relationships TO impress2020; GRANT SELECT ON neopets_connections TO impress2020; GRANT SELECT, INSERT, UPDATE, DELETE ON outfits TO impress2020; -GRANT SELECT, UPDATE ON users TO impress2020; +GRANT SELECT, INSERT, UPDATE ON users TO impress2020; GRANT SELECT, INSERT, UPDATE ON openneo_id.users TO impress2020; -- mysqldump diff --git a/src/app/components/LoginModal.js b/src/app/components/LoginModal.js index 028244c..ffa9197 100644 --- a/src/app/components/LoginModal.js +++ b/src/app/components/LoginModal.js @@ -21,7 +21,6 @@ import { } from "@chakra-ui/react"; import React from "react"; import { ErrorMessage, getGraphQLErrorMessage } from "../util"; -import WIPCallout from "./WIPCallout"; export default function LoginModal({ isOpen, onClose }) { return ( @@ -225,10 +224,12 @@ function CreateAccountForm() { return (
- - TODO: This form isn't wired up yet! - - + DTI Username - - This will be separate from your Neopets.com account. - {errorTypes.includes("USERNAME_IS_REQUIRED") && ( Username can't be blank )} + {errorTypes.includes("USERNAME_MUST_BE_UNIQUE") && ( + + This username is already taken! Try another? + + )} + + This will be separate from your Neopets.com account. + - - Careful, never use your Neopets password for another site! - {errorTypes.includes("PASSWORD_IS_REQUIRED") && ( Password can't be blank )} + + Careful, never use your Neopets password for another site! + @@ -284,7 +290,8 @@ function CreateAccountForm() { Email address @@ -296,17 +303,22 @@ function CreateAccountForm() { reset(); }} /> - - We'll use this in the future if you need to reset your password, or - for us to contact you about your account. We won't sell this address, - and we won't send marketing-y emails. - {errorTypes.includes("EMAIL_IS_REQUIRED") && ( Email can't be blank )} {errorTypes.includes("EMAIL_MUST_BE_VALID") && ( Email must be valid )} + {errorTypes.includes("EMAIL_MUST_BE_UNIQUE") && ( + + We already have an account with this address. Maybe it's yours? + + )} + + We'll use this in the future if you need to reset your password, or + for us to contact you about your account. We won't sell this address, + and we won't send marketing-y emails. + {error && ( diff --git a/src/server/auth-by-db.js b/src/server/auth-by-db.js index df588df..2e64d54 100644 --- a/src/server/auth-by-db.js +++ b/src/server/auth-by-db.js @@ -1,4 +1,4 @@ -import { createHmac } from "crypto"; +import { createHmac, randomBytes } from "crypto"; import { normalizeRow } from "./util"; // https://stackoverflow.com/a/201378/107415 @@ -153,8 +153,8 @@ function computeSignatureForAuthToken(unsignedAuthToken) { } export async function createAccount( - { username, password, email, _ /* ipAddress */ }, - __ /* db */ + { username, password, email, ipAddress }, + db ) { const errors = []; if (!username) { @@ -170,21 +170,76 @@ export async function createAccount( errors.push({ type: "EMAIL_MUST_BE_VALID" }); } - // TODO: Add an error for non-unique username. + const [nameRows] = await db.query( + ` + SELECT count(*) FROM openneo_id.users WHERE name = ?; + `, + [username] + ); + if (nameRows[0]["count(*)"] > 0) { + errors.push({ type: "USERNAME_MUST_BE_UNIQUE" }); + } + + // TODO: It'd be nice to not require email uniqueness, to avoid data leaks. + // I don't want to argue with this constraint from Classic yet though. + const [emailRows] = await db.query( + ` + SELECT count(*) FROM openneo_id.users WHERE email = ?; + `, + [email] + ); + if (emailRows[0]["count(*)"] > 0) { + errors.push({ type: "EMAIL_MUST_BE_UNIQUE" }); + } if (errors.length > 0) { return { errors, authToken: null }; } - throw new Error(`TODO: Actually create the account!`); + // We'll generate 16 cryptographically-secure random bytes, encoded as a + // length-32 hex string. This will be the salt for our encrypted password. (A + // unique salt per user prevents table-based lookup attacks, where the + // attacker acquires or generates a bunch of password hashes, then searches + // for them in our database; because each user's unique salt means that the + // same password from different services - or different users on DTI even - + // will have different hashes for each user. You'd need a lookup table for + // each user!) + const passwordSalt = randomBytes(16).toString("hex"); + const encryptedPassword = encryptPassword(password, passwordSalt); - // await db.query(` - // INSERT INTO openneo_id.users - // (name, encrypted_password, email, password_salt, sign_in_count, - // current_sign_in_at, current_sign_in_ip, created_at, updated_at) - // VALUES (?, ?, ?, ?, 1, CURRENT_TIMESTAMP(), ?, - // CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()); - // `, [username, encryptedPassword, email, passwordSalt, ipAddress]); + const connection = await db.getConnection(); + await connection.beginTransaction(); + let impressId; + try { + const [openneoIdResult] = await connection.query( + ` + INSERT INTO openneo_id.users + (name, encrypted_password, email, password_salt, sign_in_count, + current_sign_in_at, current_sign_in_ip, created_at, updated_at) + VALUES (?, ?, ?, ?, 1, CURRENT_TIMESTAMP(), ?, + CURRENT_TIMESTAMP(), CURRENT_TIMESTAMP()); + `, + [username, encryptedPassword, email, passwordSalt, ipAddress] + ); + const userIdInOpenneoId = openneoIdResult.insertId; + const [impressResult] = await connection.query( + ` + INSERT INTO openneo_impress.users + (name, remote_id, auth_server_id) VALUES (?, ?, 1); + `, + [username, userIdInOpenneoId] + ); + impressId = impressResult.insertId; + } catch (error) { + try { + await connection.rollback(); + } catch (error2) { + console.warn(`Error rolling back transaction:`, error2); + } + throw error; + } + await connection.commit(); - // return { errors: [], authToken: createAuthToken(6) }; + // Okay, user created! Return an auth token for the newly-created account. + return { errors: [], authToken: createAuthToken(impressId) }; } diff --git a/src/server/types/User.js b/src/server/types/User.js index 84009bf..cf1a7e8 100644 --- a/src/server/types/User.js +++ b/src/server/types/User.js @@ -77,10 +77,11 @@ const typeDefs = gql` } enum CreateAccountErrorType { USERNAME_IS_REQUIRED - USERNAME_ALREADY_TAKEN + USERNAME_MUST_BE_UNIQUE PASSWORD_IS_REQUIRED EMAIL_IS_REQUIRED EMAIL_MUST_BE_VALID + EMAIL_MUST_BE_UNIQUE } `;