From 34de608351edaa9e13eef2cbe660c4f378f6654b Mon Sep 17 00:00:00 2001 From: joostdecock Date: Sun, 19 Mar 2023 16:59:26 +0100 Subject: [PATCH] chore(backend): Use signin instead of login --- sites/backend/openapi/lib.mjs | 6 +- sites/backend/openapi/users.mjs | 64 ++++++++++++---- sites/backend/prisma/schema.prisma | 2 +- sites/backend/src/controllers/users.mjs | 21 ++++- sites/backend/src/models/user.mjs | 76 +++++++++++++++---- sites/backend/src/routes/users.mjs | 9 ++- sites/backend/src/templates/email/index.mjs | 6 +- .../email/loginlink/loginlink.en.yaml | 5 -- .../email/{loginlink => signinlink}/index.mjs | 12 +-- .../email/signinlink/signinlink.en.yaml | 6 ++ .../src/templates/email/signup/signup.en.yaml | 2 +- sites/backend/tests/account.mjs | 10 +-- sites/backend/tests/mfa.mjs | 14 ++-- sites/backend/tests/user.mjs | 26 +++---- 14 files changed, 178 insertions(+), 81 deletions(-) delete mode 100644 sites/backend/src/templates/email/loginlink/loginlink.en.yaml rename sites/backend/src/templates/email/{loginlink => signinlink}/index.mjs (60%) create mode 100644 sites/backend/src/templates/email/signinlink/signinlink.en.yaml diff --git a/sites/backend/openapi/lib.mjs b/sites/backend/openapi/lib.mjs index 8b655f850c6..9ccaeaa303a 100644 --- a/sites/backend/openapi/lib.mjs +++ b/sites/backend/openapi/lib.mjs @@ -391,8 +391,8 @@ Also: Introvert 🙊 example: 'en', enum: ['en', 'es', 'de', 'fr', 'nl'], }, - lastLogin: { - description: 'Timestamp of when the User last authenticated, in ISO 8601 format.', + lastSignIn: { + description: 'Timestamp of when the User last signed in, in ISO 8601 format.', type: 'string', example: '2022-12-18T18:14:30.460Z', }, @@ -462,7 +462,7 @@ for (const remove of [ 'email', 'github', 'initial', - 'lastLogin', + 'lastSignIn', 'lusername', 'mfaEnabled', 'newsletter', diff --git a/sites/backend/openapi/users.mjs b/sites/backend/openapi/users.mjs index 60dc5fcf18a..3942249c068 100644 --- a/sites/backend/openapi/users.mjs +++ b/sites/backend/openapi/users.mjs @@ -24,7 +24,7 @@ import { ) */ const common = { - tags: ['Signup & Login'], + tags: ['Sign Up & Sign In'], security: [jwt, key], } @@ -66,10 +66,10 @@ const local = { // Paths export const paths = {} -// Create account (signup) +// Create account (sign up) paths['/signup'] = { post: { - tags: ['Signup & Login'], + tags: ['Sign Up & Sign In'], summary: 'Sign up for a FreeSewing account', description: 'Creates a new inactive account. The account will require confirmation via a link sent to the email address that the user submitted.', @@ -114,7 +114,7 @@ paths['/signup'] = { // Confirm account paths['/confirm/signup/{id}'] = { post: { - tags: ['Signup & Login'], + tags: ['Sign Up & Sign In'], parameters: [local.params.id], summary: 'Confirm a FreeSewing account', description: 'Confirmes a new inactive account.', @@ -152,13 +152,13 @@ paths['/confirm/signup/{id}'] = { }, } -// Login -paths['/login'] = { +// Sign In +paths['/signin'] = { post: { - tags: ['Signup & Login'], - summary: 'Log in to a FreeSewing account', + tags: ['Sign Up & Sign In'], + summary: 'Sign in to a FreeSewing account', description: - "Logs in to an existing and active account. If MFA is enabled, you must also send the `token`.
The `username` field used for the login can contain one the User's `username`, `email`, or `id`.", + "Signs in to an existing and active account. If MFA is enabled, you must also send the `token`.
The `username` field used for the sign in can contain one the User's `username`, `email`, or `id`.", requestBody: { required: true, content: { @@ -195,6 +195,45 @@ paths['/login'] = { }, } +// Send sign In Link +paths['/signinlink'] = { + post: { + tags: ['Sign Up & Sign In'], + summary: 'Send a sign in link via email (aka magic link)', + description: + "Sends an email containing a sign in link that will sign in the user without the need for a password (also known as a 'magic link').
The `username` field used for the sign in can contain one the User's `username`, `email`, or `id`.", + requestBody: { + required: true, + content: { + 'application/json': { + schema: { + type: 'object', + properties: { + username: response.body.userAccount.properties.email, + }, + }, + }, + }, + }, + responses: { + 200: { + ...response.status['200'], + ...jsonResponse({ + result: { ...fields.result, example: 'sent' }, + }), + }, + 400: { + ...response.status['400'], + description: + response.status['400'].description + + errorExamples(['postBodyMissing', 'usernameMissing']), + }, + 401: response.status['401'], + 500: response.status['500'], + }, + }, +} + // Load user account paths['/account/{auth}'] = { get: { @@ -209,10 +248,7 @@ paths['/account/{auth}'] = { '**Success - Account data returned**\n\n' + 'Status code `200` indicates that the resource was returned successfully.', ...jsonResponse({ - result: { - ...fields.result, - example: 'success', - }, + result: fields.result, account: response.body.userAccount, }), }, @@ -522,7 +558,7 @@ paths['/available/username'] = { tags: ['Users'], summary: `Checks whether a username is available`, description: - 'This allows a background check to see whether a username is available during signup', + 'This allows a background check to see whether a username is available during sign up', requestBody: { required: true, content: { diff --git a/sites/backend/prisma/schema.prisma b/sites/backend/prisma/schema.prisma index 3159ae73183..5f2577c4fc7 100644 --- a/sites/backend/prisma/schema.prisma +++ b/sites/backend/prisma/schema.prisma @@ -54,7 +54,7 @@ model User { initial String imperial Boolean @default(false) language String @default("en") - lastLogin DateTime? + lastSignIn DateTime? lusername String @unique mfaSecret String @default("") mfaEnabled Boolean @default(false) diff --git a/sites/backend/src/controllers/users.mjs b/sites/backend/src/controllers/users.mjs index 8de48ca0a40..3df985845b3 100644 --- a/sites/backend/src/controllers/users.mjs +++ b/sites/backend/src/controllers/users.mjs @@ -29,14 +29,27 @@ UsersController.prototype.confirm = async (req, res, tools) => { } /* - * Login (with username and password) + * Sign in (with username and password) * - * This is the endpoint that provides traditional username/password login + * This is the endpoint that provides traditional username/password sign in * See: https://freesewing.dev/reference/backend/api */ -UsersController.prototype.login = async function (req, res, tools) { +UsersController.prototype.signin = async function (req, res, tools) { const User = new UserModel(tools) - await User.passwordLogin(req) + await User.passwordSignIn(req) + + return User.sendResponse(res) +} + +/* + * Send a magic link to sign in + * + * This is the endpoint that provides sign in via magic link + * See: https://freesewing.dev/reference/backend/api + */ +UsersController.prototype.signinlink = async function (req, res, tools) { + const User = new UserModel(tools) + await User.sendSigninlink(req) return User.sendResponse(res) } diff --git a/sites/backend/src/models/user.mjs b/sites/backend/src/models/user.mjs index 476975bed55..e14573f9bf9 100644 --- a/sites/backend/src/models/user.mjs +++ b/sites/backend/src/models/user.mjs @@ -279,42 +279,86 @@ UserModel.prototype.guardedCreate = async function ({ body }) { } /* - * Login based on username + password + * Sign in based on username + password */ -UserModel.prototype.passwordLogin = async function (req) { +UserModel.prototype.passwordSignIn = async function (req) { if (Object.keys(req.body).length < 1) return this.setResponse(400, 'postBodyMissing') if (!req.body.username) return this.setResponse(400, 'usernameMissing') if (!req.body.password) return this.setResponse(400, 'passwordMissing') await this.find(req.body) if (!this.exists) { - log.warn(`Login attempt for non-existing user: ${req.body.username} from ${req.ip}`) - return this.setResponse(401, 'loginFailed') + log.warn(`Sign-in attempt for non-existing user: ${req.body.username} from ${req.ip}`) + return this.setResponse(401, 'signInFailed') } // Account found, check password const [valid, updatedPasswordField] = verifyPassword(req.body.password, this.record.password) if (!valid) { log.warn(`Wrong password for existing user: ${req.body.username} from ${req.ip}`) - return this.setResponse(401, 'loginFailed') + return this.setResponse(401, 'signInFailed') } // Check for MFA if (this.record.mfaEnabled) { if (!req.body.token) return this.setResponse(403, 'mfaTokenRequired') else if (!this.mfa.verify(req.body.token, this.clear.mfaSecret)) { - return this.setResponse(401, 'loginFailed') + return this.setResponse(401, 'signInFailed') } } - // Login success - log.info(`Login by user ${this.record.id} (${this.record.username})`) + // Sign in success + log.info(`Sign-in by user ${this.record.id} (${this.record.username})`) if (updatedPasswordField) { // Update the password field with a v3 hash await this.unguardedUpdate({ password: updatedPasswordField }) } - return this.isOk() ? this.loginOk() : this.setResponse(401, 'loginFailed') + return this.isOk() ? this.signInOk() : this.setResponse(401, 'signInFailed') +} + +/* + * Send a magic link for user sign in + */ +UserModel.prototype.sendSigninlink = async function (req) { + if (Object.keys(req.body).length < 1) return this.setResponse(400, 'postBodyMissing') + if (!req.body.username) return this.setResponse(400, 'usernameMissing') + + await this.find(req.body) + if (!this.exists) { + log.warn(`Magic link attempt for non-existing user: ${req.body.username} from ${req.ip}`) + return this.setResponse(401, 'signinFailed') + } + + // Account found, create confirmation + const check = randomString() + this.confirmation = await this.Confirmation.create({ + type: 'signinlink', + data: { + language: this.record.language, + check, + }, + userId: this.record.id, + }) + const isUnitTest = this.isUnitTest(req.body) + if (!isUnitTest) { + // Send sign-in link email + await this.mailer.send({ + template: 'signinlink', + language: this.record.language, + to: this.clear.email, + replacements: { + actionUrl: i18nUrl( + this.record.language, + `/confirm/signin/${this.Confirmation.record.id}/${check}` + ), + whyUrl: i18nUrl(this.record.language, `/docs/faq/email/why-signin-link`), + supportUrl: i18nUrl(this.record.language, `/patrons/join`), + }, + }) + } + + return this.setResponse(200, 'emailSent') } /* @@ -348,19 +392,19 @@ UserModel.prototype.confirm = async function ({ body, params }) { await this.read({ id: this.Confirmation.record.user.id }) if (this.error) return this - // Update user status, consent, and last login + // Update user status, consent, and last sign in await this.unguardedUpdate({ status: 1, consent: body.consent, - lastLogin: new Date(), + lastSignIn: new Date(), }) if (this.error) return this // Before we return, remove the confirmation so it works only once await this.Confirmation.unguardedDelete() - // Account is now active, let's return a passwordless login - return this.loginOk() + // Account is now active, let's return a passwordless sign in + return this.signInOk() } /* @@ -599,7 +643,7 @@ UserModel.prototype.asAccount = function () { imperial: this.record.imperial, initial: this.clear.initial, language: this.record.language, - lastLogin: this.record.lastLogin, + lastSignIn: this.record.lastSignIn, mfaEnabled: this.record.mfaEnabled, newsletter: this.record.newsletter, patron: this.record.patron, @@ -689,9 +733,9 @@ UserModel.prototype.isOk = function () { } /* - * Helper method to return from successful login + * Helper method to return from successful sign in */ -UserModel.prototype.loginOk = function () { +UserModel.prototype.signInOk = function () { return this.setResponse(200, false, { result: 'success', token: this.getToken(), diff --git a/sites/backend/src/routes/users.mjs b/sites/backend/src/routes/users.mjs index f64b4aaaa6e..d115f68937e 100644 --- a/sites/backend/src/routes/users.mjs +++ b/sites/backend/src/routes/users.mjs @@ -7,14 +7,17 @@ const bsc = ['basic', { session: false }] export function usersRoutes(tools) { const { app, passport } = tools - // Sign up + // Sign Up app.post('/signup', (req, res) => Users.signup(req, res, tools)) // Confirm account app.post('/confirm/signup/:id', (req, res) => Users.confirm(req, res, tools)) - // Login - app.post('/login', (req, res) => Users.login(req, res, tools)) + // Sign In + app.post('/signin', (req, res) => Users.signin(req, res, tools)) + + // Send magic link to sign in + app.post('/signinlink', (req, res) => Users.signinlink(req, res, tools)) // Read current jwt app.get('/whoami/jwt', passport.authenticate(...jwt), (req, res) => Users.whoami(req, res, tools)) diff --git a/sites/backend/src/templates/email/index.mjs b/sites/backend/src/templates/email/index.mjs index 4bb3913f87c..346530eb5c5 100644 --- a/sites/backend/src/templates/email/index.mjs +++ b/sites/backend/src/templates/email/index.mjs @@ -1,6 +1,6 @@ import { emailchange, translations as emailchangeTranslations } from './emailchange/index.mjs' import { goodbye, translations as goodbyeTranslations } from './goodbye/index.mjs' -import { loginlink, translations as loginlinkTranslations } from './loginlink/index.mjs' +import { signinlink, translations as signinlinkTranslations } from './signinlink/index.mjs' import { newslettersub, translations as newslettersubTranslations } from './newslettersub/index.mjs' import { passwordreset, translations as passwordresetTranslations } from './passwordreset/index.mjs' import { signup, translations as signupTranslations } from './signup/index.mjs' @@ -19,7 +19,7 @@ import nl from '../../../public/locales/nl/shared.json' assert { type: 'json' } export const templates = { emailchange, goodbye, - loginlink, + signinlink, newslettersub, passwordreset, signup, @@ -30,7 +30,7 @@ export const templates = { export const translations = { emailchange: emailchangeTranslations, goodbye: goodbyeTranslations, - loginlink: loginlinkTranslations, + signinlink: signinlinkTranslations, newslettersub: newslettersubTranslations, passwordreset: passwordresetTranslations, signup: signupTranslations, diff --git a/sites/backend/src/templates/email/loginlink/loginlink.en.yaml b/sites/backend/src/templates/email/loginlink/loginlink.en.yaml deleted file mode 100644 index 1eaadb62dae..00000000000 --- a/sites/backend/src/templates/email/loginlink/loginlink.en.yaml +++ /dev/null @@ -1,5 +0,0 @@ -subject: '[FreeSewing] Loginlink fixme' -heading: FIXME -lead: fixme -text-lead: fixme -closing: fixme diff --git a/sites/backend/src/templates/email/loginlink/index.mjs b/sites/backend/src/templates/email/signinlink/index.mjs similarity index 60% rename from sites/backend/src/templates/email/loginlink/index.mjs rename to sites/backend/src/templates/email/signinlink/index.mjs index 897e33e9032..8cc212cd784 100644 --- a/sites/backend/src/templates/email/loginlink/index.mjs +++ b/sites/backend/src/templates/email/signinlink/index.mjs @@ -1,12 +1,12 @@ import { buttonRow, closingRow, headingRow, wrap } from '../shared/blocks.mjs' // Translations -import en from '../../../../public/locales/en/loginlink.json' assert { type: 'json' } -import de from '../../../../public/locales/de/loginlink.json' assert { type: 'json' } -import es from '../../../../public/locales/es/loginlink.json' assert { type: 'json' } -import fr from '../../../../public/locales/fr/loginlink.json' assert { type: 'json' } -import nl from '../../../../public/locales/nl/loginlink.json' assert { type: 'json' } +import en from '../../../../public/locales/en/signinlink.json' assert { type: 'json' } +import de from '../../../../public/locales/de/signinlink.json' assert { type: 'json' } +import es from '../../../../public/locales/es/signinlink.json' assert { type: 'json' } +import fr from '../../../../public/locales/fr/signinlink.json' assert { type: 'json' } +import nl from '../../../../public/locales/nl/signinlink.json' assert { type: 'json' } -export const loginlink = { +export const signinlink = { html: wrap.html(` ${headingRow} diff --git a/sites/backend/src/templates/email/signinlink/signinlink.en.yaml b/sites/backend/src/templates/email/signinlink/signinlink.en.yaml new file mode 100644 index 00000000000..e7b6a516d06 --- /dev/null +++ b/sites/backend/src/templates/email/signinlink/signinlink.en.yaml @@ -0,0 +1,6 @@ +subject: "[FreeSewing] Sign in to FreeSewing.org with this link" +heading: Password are so has-been +lead: 'To sign in to FreeSewing.org, click the big black rectangle below:' +text-lead: 'To sign in to FreeSewing.org, click the link below:' +button: Sign in +closing: That was easy, wasn't it? diff --git a/sites/backend/src/templates/email/signup/signup.en.yaml b/sites/backend/src/templates/email/signup/signup.en.yaml index f821ca52d91..da9da542e51 100644 --- a/sites/backend/src/templates/email/signup/signup.en.yaml +++ b/sites/backend/src/templates/email/signup/signup.en.yaml @@ -1,4 +1,4 @@ -subject: "[FreeSewing] Here's that signup link we promised you" +subject: "[FreeSewing] Here is your sign-up link for FreeSewing.org" heading: Join FreeSewing lead: 'To create a FreeSewing account linked to this email address, click the big black rectangle below:' text-lead: 'To create a FreeSewing account linked to this email address, click the link below:' diff --git a/sites/backend/tests/account.mjs b/sites/backend/tests/account.mjs index 145a581e02b..1455c004e2b 100644 --- a/sites/backend/tests/account.mjs +++ b/sites/backend/tests/account.mjs @@ -51,7 +51,7 @@ export const accountTests = async (chai, config, expect, store) => { }) } - // Update password - Check with login + // Update password - Check with sign in const password = store.randomString() it(`${store.icon('user', auth)} Should update the password (${auth})`, (done) => { const body = {} @@ -79,11 +79,11 @@ export const accountTests = async (chai, config, expect, store) => { it(`${store.icon( 'user', auth - )} Should be able to login with the updated password (${auth})`, (done) => { + )} Should be able to sign in with the updated password (${auth})`, (done) => { const body = {} chai .request(config.api) - .post(`/login`) + .post(`/signin`) .send({ username: store.account.username, password, @@ -122,11 +122,11 @@ export const accountTests = async (chai, config, expect, store) => { it(`${store.icon( 'user', auth - )} Should be able to login with the original password (${auth})`, (done) => { + )} Should be able to sign in with the original password (${auth})`, (done) => { const body = {} chai .request(config.api) - .post(`/login`) + .post(`/signin`) .send({ username: store.account.username, password: store.account.password, diff --git a/sites/backend/tests/mfa.mjs b/sites/backend/tests/mfa.mjs index 0ab84fd1cbd..52278093eda 100644 --- a/sites/backend/tests/mfa.mjs +++ b/sites/backend/tests/mfa.mjs @@ -133,10 +133,10 @@ export const mfaTests = async (chai, config, expect, store) => { }) }) - it(`${store.icon('mfa', auth)} Should not login with username/password only`, (done) => { + it(`${store.icon('mfa', auth)} Should not sign in with username/password only`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: secret[auth].username, password: secret[auth].password, @@ -150,10 +150,10 @@ export const mfaTests = async (chai, config, expect, store) => { }) }) - it(`${store.icon('mfa')} Should login with username/password/token`, (done) => { + it(`${store.icon('mfa')} Should sign in with username/password/token`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: secret[auth].username, password: secret[auth].password, @@ -168,10 +168,10 @@ export const mfaTests = async (chai, config, expect, store) => { }) }) - it(`${store.icon('mfa')} Should not login with a wrong token`, (done) => { + it(`${store.icon('mfa')} Should not sign in with a wrong token`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: secret[auth].username, password: secret[auth].password, @@ -181,7 +181,7 @@ export const mfaTests = async (chai, config, expect, store) => { expect(err === null).to.equal(true) expect(res.status).to.equal(401) expect(res.body.result).to.equal('error') - expect(res.body.error).to.equal('loginFailed') + expect(res.body.error).to.equal('signInFailed') done() }) }) diff --git a/sites/backend/tests/user.mjs b/sites/backend/tests/user.mjs index 246f32e67ca..2e9fac285a8 100644 --- a/sites/backend/tests/user.mjs +++ b/sites/backend/tests/user.mjs @@ -58,10 +58,10 @@ export const userTests = async (chai, config, expect, store) => { }) }) - step(`${store.icon('user')} Should not login with the wrong password`, (done) => { + step(`${store.icon('user')} Should not sign in with the wrong password`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: store.account.username, password: store.account.username, @@ -71,7 +71,7 @@ export const userTests = async (chai, config, expect, store) => { expect(res.type).to.equal('application/json') expect(res.charset).to.equal('utf-8') expect(res.body.result).to.equal(`error`) - expect(res.body.error).to.equal(`loginFailed`) + expect(res.body.error).to.equal(`signInFailed`) done() }) }) @@ -108,10 +108,10 @@ export const userTests = async (chai, config, expect, store) => { }) }) - step(`${store.icon('user')} Should login with username and password`, (done) => { + step(`${store.icon('user')} Should sign in with username and password`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: store.account.username, password: store.account.password, @@ -131,10 +131,10 @@ export const userTests = async (chai, config, expect, store) => { }) }) - step(`${store.icon('user')} Should login with USERNAME and password`, (done) => { + step(`${store.icon('user')} Should sign in with USERNAME and password`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: store.account.username.toUpperCase(), password: store.account.password, @@ -154,10 +154,10 @@ export const userTests = async (chai, config, expect, store) => { }) }) - step(`${store.icon('user')} Should login with email and password`, (done) => { + step(`${store.icon('user')} Should sign in with email and password`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: store.account.email, password: store.account.password, @@ -177,10 +177,10 @@ export const userTests = async (chai, config, expect, store) => { }) }) - step(`${store.icon('user')} Should login with EMAIL and password`, (done) => { + step(`${store.icon('user')} Should signin with EMAIL and password`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: store.account.email.toUpperCase(), password: store.account.password, @@ -200,10 +200,10 @@ export const userTests = async (chai, config, expect, store) => { }) }) - step(`${store.icon('user')} Should login with id and password`, (done) => { + step(`${store.icon('user')} Should signin with id and password`, (done) => { chai .request(config.api) - .post('/login') + .post('/signin') .send({ username: store.account.id, password: store.account.password,