diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index 0ba6ac410..6150cd145 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -73,7 +73,6 @@ import { setShareHighlightResolver, setUserPersonalizationResolver, setWebhookResolver, - signupResolver, subscribeResolver, subscriptionsResolver, typeaheadSearchResolver, @@ -155,7 +154,6 @@ export const functionResolvers = { updateLabel: updateLabelResolver, deleteLabel: deleteLabelResolver, login: loginResolver, - signup: signupResolver, setLabels: setLabelsResolver, generateApiKey: generateApiKeyResolver, unsubscribe: unsubscribeResolver, @@ -573,7 +571,6 @@ export const functionResolvers = { ...resultResolveTypeResolver('CreateLabel'), ...resultResolveTypeResolver('DeleteLabel'), ...resultResolveTypeResolver('Login'), - ...resultResolveTypeResolver('Signup'), ...resultResolveTypeResolver('SetLabels'), ...resultResolveTypeResolver('GenerateApiKey'), ...resultResolveTypeResolver('Search'), diff --git a/packages/api/src/resolvers/user/index.ts b/packages/api/src/resolvers/user/index.ts index fbd747124..df35f5e2a 100644 --- a/packages/api/src/resolvers/user/index.ts +++ b/packages/api/src/resolvers/user/index.ts @@ -11,14 +11,12 @@ import { MutationGoogleLoginArgs, MutationGoogleSignupArgs, MutationLoginArgs, - MutationSignupArgs, MutationUpdateUserArgs, MutationUpdateUserProfileArgs, QueryUserArgs, QueryValidateUsernameArgs, ResolverFn, SignupErrorCode, - SignupResult, UpdateUserError, UpdateUserErrorCode, UpdateUserProfileError, @@ -37,7 +35,7 @@ import { env } from '../../env' import { validateUsername } from '../../utils/usernamePolicy' import * as jwt from 'jsonwebtoken' import { createUser } from '../../services/create_user' -import { comparePassword, hashPassword } from '../../utils/auth' +import { comparePassword } from '../../utils/auth' import { deletePagesByParam } from '../../elastic/pages' import { setClaims } from '../../entity/utils' import { User as UserEntity } from '../../entity/user' @@ -328,43 +326,6 @@ export const loginResolver: ResolverFn< return { me: userDataToUser(user) } } -export const signupResolver: ResolverFn< - SignupResult, - Record, - WithDataSourcesContext, - MutationSignupArgs -> = async (_obj, { input }) => { - const { email, username, name, bio, password, pictureUrl } = input - const lowerCasedUsername = username.toLowerCase() - - try { - // hash password - const hashedPassword = await hashPassword(password) - - const [user, profile] = await createUser({ - email, - provider: 'EMAIL', - sourceUserId: email, - name, - username: lowerCasedUsername, - pictureUrl: pictureUrl || undefined, - bio: bio || undefined, - password: hashedPassword, - pendingConfirmation: true, - }) - - return { - me: userDataToUser({ ...user, profile: { ...profile, private: false } }), - } - } catch (err) { - console.log('error', err) - if (isErrorWithCode(err)) { - return { errorCodes: [err.errorCode as SignupErrorCode] } - } - return { errorCodes: [SignupErrorCode.Unknown] } - } -} - export const deleteAccountResolver = authorized< DeleteAccountSuccess, DeleteAccountError, diff --git a/packages/api/src/routers/auth/auth_router.ts b/packages/api/src/routers/auth/auth_router.ts index d9289111e..661bf701f 100644 --- a/packages/api/src/routers/auth/auth_router.ts +++ b/packages/api/src/routers/auth/auth_router.ts @@ -37,6 +37,9 @@ import { RegistrationType, UserData, } from '../../datalayer/user/model' +import { hashPassword } from '../../utils/auth' +import { createUser } from '../../services/create_user' +import { isErrorWithCode } from '../../resolvers' const logger = buildLogger('app.dispatch') const signToken = promisify(jwt.sign) @@ -422,53 +425,33 @@ export function authRouter() { '/email-signup', cors(corsConfig), async (req: express.Request, res: express.Response) => { - const { email, password, name, username, bio } = req.body - if (!email || !password || !name || !username) { - res.redirect(`${env.client.url}/email-signup?errorCodes=BAD_DATA`) - return - } - - const query = ` - mutation signup { - signup(input: { - email: "${email}", - password: "${password}", - name: "${name}", - username: "${username}", - bio: "${bio ?? ''}" - }) { - __typename - ... on SignupSuccess { - me { - id - name - profile { - username - } - } - } - ... on SignupError { - errorCodes - } - } - }` + const { email, password, name, username, bio, pictureUrl } = req.body + const lowerCasedUsername = username.toLowerCase() try { - const result = await axios.post(env.server.gateway_url + '/graphql', { - query, - }) - const { data } = result.data + // hash password + const hashedPassword = await hashPassword(password) - if (data.signup.__typename === 'SignupError') { - const errorCodes = data.signup.errorCodes.join(',') - return res.redirect( - `${env.client.url}/email-signup?errorCodes=${errorCodes}` - ) - } + await createUser({ + email, + provider: 'EMAIL', + sourceUserId: email, + name, + username: lowerCasedUsername, + pictureUrl, + bio, + password: hashedPassword, + pendingConfirmation: true, + }) res.redirect(`${env.client.url}/email-login?message=SIGNUP_SUCCESS`) } catch (e) { - logger.info('email-signup exception:', e) + logger.error('email-signup exception:', e) + if (isErrorWithCode(e)) { + return res.redirect( + `${env.client.url}/email-signup?errorCodes=${e.errorCode}` + ) + } res.redirect(`${env.client.url}/email-signup?errorCodes=UNKNOWN`) } } diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 259e66c9d..3ab828e1a 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -1415,25 +1415,6 @@ const schema = gql` email: String! } - input SignupInput { - email: String! - password: String! @sanitize(maxLength: 40) - username: String! - name: String! - pictureUrl: String - bio: String - } - - type SignupSuccess { - me: User! - } - - type SignupError { - errorCodes: [SignupErrorCode]! - } - - union SignupResult = SignupSuccess | SignupError - input SetLabelsInput { pageId: ID! labelIds: [ID!]! @@ -1845,7 +1826,6 @@ const schema = gql` updateLabel(input: UpdateLabelInput!): UpdateLabelResult! deleteLabel(id: ID!): DeleteLabelResult! login(input: LoginInput!): LoginResult! - signup(input: SignupInput!): SignupResult! setLabels(input: SetLabelsInput!): SetLabelsResult! generateApiKey(input: GenerateApiKeyInput!): GenerateApiKeyResult! unsubscribe(name: String!): UnsubscribeResult! diff --git a/packages/api/test/resolvers/user.test.ts b/packages/api/test/resolvers/user.test.ts index 78abb33df..afe6e7f53 100644 --- a/packages/api/test/resolvers/user.test.ts +++ b/packages/api/test/resolvers/user.test.ts @@ -3,16 +3,12 @@ import { graphqlRequest, request } from '../util' import { expect } from 'chai' import { LoginErrorCode, - SignupErrorCode, UpdateUserErrorCode, UpdateUserProfileErrorCode, } from '../../src/generated/graphql' import { User } from '../../src/entity/user' import { hashPassword } from '../../src/utils/auth' import 'mocha' -import { MailDataRequired } from '@sendgrid/helpers/classes/mail' -import sinon from 'sinon' -import * as util from '../../src/utils/sendEmail' describe('User API', () => { const username = 'fake_user' @@ -327,132 +323,4 @@ describe('User API', () => { }) }) }) - - describe('signup', () => { - let query: string - let email: string - let password: string - let username: string - let fake: (msg: MailDataRequired) => Promise - - beforeEach(() => { - query = ` - mutation { - signup( - input: { - email: "${email}" - password: "${password}" - name: "Some name" - username: "${username}" - } - ) { - ... on SignupSuccess { - me { - id - name - profile { - username - } - } - } - ... on SignupError { - errorCodes - } - } - } - ` - }) - - context('when inputs are valid and user not exists', () => { - beforeEach(() => { - password = correctPassword - username = 'Some_username' - email = `${username}@fake.com` - }) - - afterEach(async () => { - await deleteTestUser(username) - }) - - context('when confirmation email sent', () => { - beforeEach(() => { - fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) - }) - - afterEach(() => { - sinon.restore() - }) - - it('responds with 200', async () => { - return graphqlRequest(query).expect(200) - }) - - it('returns the user with the lowercase username', async () => { - const res = await graphqlRequest(query).expect(200) - expect(res.body.data.signup.me.profile.username).to.eql( - username.toLowerCase() - ) - }) - }) - - context('when confirmation email not sent', () => { - before(() => { - fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(false)) - }) - - after(() => { - sinon.restore() - }) - - it('responds with error code INVALID_EMAIL', async () => { - const res = await graphqlRequest(query).expect(200) - expect(res.body.data.signup.errorCodes).to.eql([ - SignupErrorCode.InvalidEmail, - ]) - }) - }) - }) - - context('when password is too long', () => { - before(() => { - email = 'Some_email' - password = 'Some_password_that_is_too_long_for_database' - username = 'Some_username' - }) - - it('responds with status code 400', async () => { - return graphqlRequest(query).expect(400) - }) - }) - - context('when user exists', () => { - before(() => { - email = user.email - password = 'Some password' - username = 'Some username' - }) - - it('responds with error code UserExists', async () => { - const response = await graphqlRequest(query).expect(200) - expect(response.body.data.signup.errorCodes).to.eql([ - SignupErrorCode.UserExists, - ]) - }) - }) - - context('when username is invalid', () => { - before(() => { - email = 'Some_email' - password = correctPassword - username = 'omnivore_admin' - }) - - it('responds with error code InvalidUsername', async () => { - const response = await graphqlRequest(query).expect(200) - expect(response.body.data.signup.errorCodes).to.eql([ - SignupErrorCode.InvalidUsername, - ]) - }) - }) - }) }) diff --git a/packages/api/test/routers/article_router.test.ts b/packages/api/test/routers/article.test.ts similarity index 100% rename from packages/api/test/routers/article_router.test.ts rename to packages/api/test/routers/article.test.ts diff --git a/packages/api/test/routers/auth.test.ts b/packages/api/test/routers/auth.test.ts new file mode 100644 index 000000000..1086acb56 --- /dev/null +++ b/packages/api/test/routers/auth.test.ts @@ -0,0 +1,142 @@ +import { createTestUser, deleteTestUser } from '../db' +import { request } from '../util' +import { expect } from 'chai' +import { StatusType } from '../../src/datalayer/user/model' +import { getRepository } from '../../src/entity/utils' +import { User } from '../../src/entity/user' +import { MailDataRequired } from '@sendgrid/helpers/classes/mail' +import sinon from 'sinon' +import * as util from '../../src/utils/sendEmail' +import supertest from 'supertest' + +describe('auth router', () => { + const route = '/api/auth' + const signupRequest = ( + email: string, + password: string, + name: string, + username: string + ): supertest.Test => { + return request.post(`${route}/email-signup`).send({ + email, + password, + name, + username, + }) + } + + describe('email signup', () => { + const validPassword = 'validPassword' + + let email: string + let password: string + let username: string + let name: string + + context('when inputs are valid and user not exists', () => { + let fake: (msg: MailDataRequired) => Promise + + before(() => { + password = validPassword + username = 'Some_username' + email = `${username}@fake.com` + name = 'Some name' + }) + + afterEach(async () => { + await deleteTestUser(username) + }) + + context('when confirmation email sent', () => { + beforeEach(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(true)) + }) + + afterEach(() => { + sinon.restore() + }) + + it('redirects to login page', async () => { + const res = await signupRequest( + email, + password, + name, + username + ).expect(302) + expect(res.header.location).to.endWith( + '/email-login?message=SIGNUP_SUCCESS' + ) + }) + + it('creates the user with pending status and correct name', async () => { + await signupRequest(email, password, name, username).expect(302) + const user = await getRepository(User).findOneBy({ name }) + + expect(user?.status).to.eql(StatusType.Pending) + expect(user?.name).to.eql(name) + }) + }) + + context('when confirmation email not sent', () => { + before(() => { + fake = sinon.replace(util, 'sendEmail', sinon.fake.resolves(false)) + }) + + after(() => { + sinon.restore() + }) + + it('redirects to sign up page with error code INVALID_EMAIL', async () => { + const res = await signupRequest( + email, + password, + name, + username + ).expect(302) + expect(res.header.location).to.endWith( + '/email-registration?errorCodes=INVALID_EMAIL' + ) + }) + }) + }) + + context('when user exists', () => { + before(async () => { + username = 'Some_username' + const user = await createTestUser(username) + email = user.email + password = 'Some password' + }) + + after(async () => { + await deleteTestUser(username) + }) + + it('redirects to sign up page with error code USER_EXISTS', async () => { + const res = await signupRequest(email, password, name, username).expect( + 302 + ) + expect(res.header.location).to.endWith( + '/email-registration?errorCodes=USER_EXISTS' + ) + }) + }) + + context('when username is invalid', () => { + before(() => { + email = 'Some_email' + password = validPassword + username = 'omnivore_admin' + }) + + it('redirects to sign up page with error code INVALID_USERNAME', async () => { + const res = await signupRequest(email, password, name, username).expect( + 302 + ) + expect(res.header.location).to.endWith( + '/email-registration?errorCodes=INVALID_USERNAME' + ) + }) + }) + }) +}) diff --git a/packages/api/test/routers/reminders_router.test.ts b/packages/api/test/routers/reminders.test.ts similarity index 100% rename from packages/api/test/routers/reminders_router.test.ts rename to packages/api/test/routers/reminders.test.ts