Pārlūkot izejas kodu

feat(server): Refine customer signup flow

Relates to #33
Michael Bromley 7 gadi atpakaļ
vecāks
revīzija
91de6beb2b

+ 2 - 2
docs/diagrams/customer-account-order-activity-diagram.puml

@@ -12,7 +12,7 @@ if (Has existing account?) then (yes)
 else (no)
     if (Create account?) then (no)
     else (yes)
-        #66aa66:Create authenticated Customer;
+        GREEN:Create authenticated Customer;
         stop
     endif
 endif
@@ -31,7 +31,7 @@ else (no)
 endif
 :Complete Order;
 if (Create account?) then (yes)
-    #66aa66:Create authenticated Customer;
+    GREEN:Create authenticated Customer;
 endif
 stop
 @enduml

+ 39 - 0
docs/diagrams/customer-registration-activity-diagram.puml

@@ -0,0 +1,39 @@
+' This diagram illustrates the Customer signup flow
+@startuml
+
+!include theme.puml
+title Customer Account Registration
+start
+GREEN:registerCustomerAccount;
+note left
+    Args:
+    * emailAddress
+    * title?
+    * firstName?
+    * lastName?
+end note
+:create & store new token;
+repeat
+    :Token sent to email address;
+    :Follow link;
+    :Customer creates password;
+    GREEN:verifyCustomerEmailAddress;
+    note left
+        Args:
+        * token
+        * password
+    end note
+    if (token has expired?) then (yes)
+        GREEN:resendVerification;
+        note left
+            Args:
+            * emailAddress
+        end note
+        :create & store new token;
+    else (no)
+    endif
+repeat while (had to re-verify due to expired token?)
+
+:Verified and logged in;
+stop
+@enduml

+ 1 - 0
docs/diagrams/theme.puml

@@ -3,6 +3,7 @@
 !define LINE    #1a3164
 !define BACKGROUND #b9cefc
 !define BORDER  #e58e26
+!define GREEN   #66aa66
 !define BLUE  #4067af
 !define BLUE_LIGHT  #c6dbff
 

Failā izmaiņas netiks attēlotas, jo tās ir par lielu
+ 0 - 0
schema.json


+ 227 - 63
server/e2e/auth.e2e-spec.ts

@@ -6,6 +6,7 @@ import {
     CreateRole,
     LoginMutationArgs,
     Permission,
+    RegisterCustomerInput,
     UpdateProductMutationArgs,
 } from 'shared/generated-types';
 import { SUPER_ADMIN_USER_IDENTIFIER, SUPER_ADMIN_USER_PASSWORD } from 'shared/shared-constants';
@@ -20,6 +21,8 @@ import {
     GET_PRODUCT_LIST,
     UPDATE_PRODUCT,
 } from '../../admin-ui/src/app/data/definitions/product-definitions';
+import { NoopEmailGenerator } from '../src/config/email/noop-email-generator';
+import { defaultEmailTypes } from '../src/email/default-email-types';
 
 import { TEST_SETUP_TIMEOUT_MS } from './config/test-config';
 import { TestClient } from './test-client';
@@ -28,12 +31,25 @@ import { TestServer } from './test-server';
 describe('Authorization & permissions', () => {
     const client = new TestClient();
     const server = new TestServer();
+    let sendEmailFn: jest.Mock;
 
     beforeAll(async () => {
-        const token = await server.init({
-            productCount: 1,
-            customerCount: 1,
-        });
+        const token = await server.init(
+            {
+                productCount: 1,
+                customerCount: 1,
+            },
+            {
+                emailOptions: {
+                    emailTypes: defaultEmailTypes,
+                    generator: new NoopEmailGenerator(),
+                    transport: {
+                        type: 'testing',
+                        onSend: ctx => sendEmailFn(ctx),
+                    },
+                },
+            },
+        );
         await client.init();
     }, TEST_SETUP_TIMEOUT_MS);
 
@@ -41,87 +57,212 @@ describe('Authorization & permissions', () => {
         await server.destroy();
     });
 
-    describe('Anonymous user', () => {
-        beforeAll(async () => {
-            await client.asAnonymousUser();
+    beforeEach(() => {
+        sendEmailFn = jest.fn();
+    });
+
+    describe('admin permissions', () => {
+        describe('Anonymous user', () => {
+            beforeAll(async () => {
+                await client.asAnonymousUser();
+            });
+
+            it('can attempt login', async () => {
+                await assertRequestAllowed<LoginMutationArgs>(ATTEMPT_LOGIN, {
+                    username: SUPER_ADMIN_USER_IDENTIFIER,
+                    password: SUPER_ADMIN_USER_PASSWORD,
+                    rememberMe: false,
+                });
+            });
         });
 
-        it('can attempt login', async () => {
-            await assertRequestAllowed<LoginMutationArgs>(ATTEMPT_LOGIN, {
-                username: SUPER_ADMIN_USER_IDENTIFIER,
-                password: SUPER_ADMIN_USER_PASSWORD,
-                rememberMe: false,
+        describe('ReadCatalog', () => {
+            beforeAll(async () => {
+                await client.asSuperAdmin();
+                const { identifier, password } = await createAdministratorWithPermissions('ReadCatalog', [
+                    Permission.ReadCatalog,
+                ]);
+                await client.asUserWithCredentials(identifier, password);
+            });
+
+            it('can read', async () => {
+                await assertRequestAllowed(GET_PRODUCT_LIST);
+            });
+
+            it('cannot uppdate', async () => {
+                await assertRequestForbidden<UpdateProductMutationArgs>(UPDATE_PRODUCT, {
+                    input: {
+                        id: '1',
+                        translations: [],
+                    },
+                });
+            });
+
+            it('cannot create', async () => {
+                await assertRequestForbidden<CreateProductMutationArgs>(CREATE_PRODUCT, {
+                    input: {
+                        translations: [],
+                    },
+                });
             });
         });
-    });
 
-    describe('ReadCatalog', () => {
-        beforeAll(async () => {
-            await client.asSuperAdmin();
-            const { identifier, password } = await createAdministratorWithPermissions('ReadCatalog', [
-                Permission.ReadCatalog,
-            ]);
-            await client.asUserWithCredentials(identifier, password);
+        describe('CRUD on Customers', () => {
+            beforeAll(async () => {
+                await client.asSuperAdmin();
+                const { identifier, password } = await createAdministratorWithPermissions('CRUDCustomer', [
+                    Permission.CreateCustomer,
+                    Permission.ReadCustomer,
+                    Permission.UpdateCustomer,
+                    Permission.DeleteCustomer,
+                ]);
+                await client.asUserWithCredentials(identifier, password);
+            });
+
+            it('can create', async () => {
+                await assertRequestAllowed(
+                    gql`
+                        mutation CreateCustomer($input: CreateCustomerInput!) {
+                            createCustomer(input: $input) {
+                                id
+                            }
+                        }
+                    `,
+                    { input: { emailAddress: '', firstName: '', lastName: '' } },
+                );
+            });
+
+            it('can read', async () => {
+                await assertRequestAllowed(gql`
+                    query {
+                        customers {
+                            totalItems
+                        }
+                    }
+                `);
+            });
         });
+    });
+
+    describe('customer account creation', () => {
+        const password = 'password';
+        const emailAddress = 'test1@test.com';
+        let verificationToken: string;
+
+        it('register a new account', async () => {
+            const verificationTokenPromise = getVerificationTokenPromise();
+            const input: RegisterCustomerInput = {
+                firstName: 'Sean',
+                lastName: 'Tester',
+                emailAddress,
+            };
+            const result = await client.query(REGISTER_ACCOUNT, { input });
+
+            verificationToken = await verificationTokenPromise;
 
-        it('can read', async () => {
-            await assertRequestAllowed(GET_PRODUCT_LIST);
+            expect(result.registerCustomerAccount).toBe(true);
+            expect(sendEmailFn).toHaveBeenCalled();
+            expect(verificationToken).toBeDefined();
         });
 
-        it('cannot uppdate', async () => {
-            await assertRequestForbidden<UpdateProductMutationArgs>(UPDATE_PRODUCT, {
-                input: {
-                    id: '1',
-                    translations: [],
-                },
+        it('issues a new token if attempting to register a second time', async () => {
+            const sendEmail = new Promise<string>(resolve => {
+                sendEmailFn.mockImplementation(ctx => {
+                    resolve(ctx.event.user.verificationToken);
+                });
             });
+            const input: RegisterCustomerInput = {
+                firstName: 'Sean',
+                lastName: 'Tester',
+                emailAddress,
+            };
+            const result = await client.query(REGISTER_ACCOUNT, { input });
+
+            const newVerificationToken = await sendEmail;
+
+            expect(result.registerCustomerAccount).toBe(true);
+            expect(sendEmailFn).toHaveBeenCalled();
+            expect(newVerificationToken).not.toBe(verificationToken);
+
+            verificationToken = newVerificationToken;
         });
 
-        it('cannot create', async () => {
-            await assertRequestForbidden<CreateProductMutationArgs>(CREATE_PRODUCT, {
-                input: {
-                    translations: [],
-                },
+        it('refreshCustomerVerification issues a new token', async () => {
+            const sendEmail = new Promise<string>(resolve => {
+                sendEmailFn.mockImplementation(ctx => {
+                    resolve(ctx.event.user.verificationToken);
+                });
             });
+            const result = await client.query(REFRESH_TOKEN, { emailAddress });
+
+            const newVerificationToken = await sendEmail;
+
+            expect(result.refreshCustomerVerification).toBe(true);
+            expect(sendEmailFn).toHaveBeenCalled();
+            expect(newVerificationToken).not.toBe(verificationToken);
+
+            verificationToken = newVerificationToken;
         });
-    });
+        it('refreshCustomerVerification does nothing with an unrecognized emailAddress', async () => {
+            const result = await client.query(REFRESH_TOKEN, {
+                emailAddress: 'never-been-registered@test.com',
+            });
 
-    describe('CRUD on Customers', () => {
-        beforeAll(async () => {
-            await client.asSuperAdmin();
-            const { identifier, password } = await createAdministratorWithPermissions('CRUDCustomer', [
-                Permission.CreateCustomer,
-                Permission.ReadCustomer,
-                Permission.UpdateCustomer,
-                Permission.DeleteCustomer,
-            ]);
-            await client.asUserWithCredentials(identifier, password);
+            expect(result.refreshCustomerVerification).toBe(true);
+            expect(sendEmailFn).not.toHaveBeenCalled();
         });
 
-        it('can create', async () => {
-            await assertRequestAllowed(
-                gql`
-                    mutation CreateCustomer($input: CreateCustomerInput!) {
-                        createCustomer(input: $input) {
-                            id
-                        }
-                    }
-                `,
-                { input: { emailAddress: '', firstName: '', lastName: '' } },
-            );
+        it('login fails before verification', async () => {
+            try {
+                await client.asUserWithCredentials(emailAddress, '');
+                fail('should have thrown');
+            } catch (err) {
+                expect(getErrorStatusCode(err)).toBe(401);
+            }
         });
 
-        it('can read', async () => {
-            await assertRequestAllowed(gql`
-                query {
-                    customers {
-                        totalItems
-                    }
-                }
-            `);
+        it('verification fails with wrong token', async () => {
+            try {
+                await client.query(VERIFY_EMAIL, {
+                    password,
+                    token: 'bad-token',
+                });
+                fail('should have thrown');
+            } catch (err) {
+                expect(err.message).toEqual(expect.stringContaining(`Verification token not recognized`));
+            }
+        });
+
+        it('verification succeeds with correct token', async () => {
+            const result = await client.query(VERIFY_EMAIL, {
+                password,
+                token: verificationToken,
+            });
+
+            expect(result.verifyCustomerAccount.user.identifier).toBe('test1@test.com');
+        });
+
+        it('verification fails if attempted a second time', async () => {
+            try {
+                await client.query(VERIFY_EMAIL, {
+                    password,
+                    token: verificationToken,
+                });
+                fail('should have thrown');
+            } catch (err) {
+                expect(err.message).toEqual(expect.stringContaining(`Verification token not recognized`));
+            }
         });
     });
 
+    function getVerificationTokenPromise(): Promise<string> {
+        return new Promise<string>(resolve => {
+            sendEmailFn.mockImplementation(ctx => {
+                resolve(ctx.event.user.verificationToken);
+            });
+        });
+    }
+
     async function assertRequestAllowed<V>(operation: DocumentNode, variables?: V) {
         try {
             const status = await client.queryStatus(operation, variables);
@@ -187,4 +328,27 @@ describe('Authorization & permissions', () => {
             password,
         };
     }
+
+    const REGISTER_ACCOUNT = gql`
+        mutation Register($input: RegisterCustomerInput!) {
+            registerCustomerAccount(input: $input)
+        }
+    `;
+
+    const VERIFY_EMAIL = gql`
+        mutation Verify($password: String!, $token: String!) {
+            verifyCustomerAccount(password: $password, token: $token) {
+                user {
+                    id
+                    identifier
+                }
+            }
+        }
+    `;
+
+    const REFRESH_TOKEN = gql`
+        mutation RefreshToken($emailAddress: String!) {
+            refreshCustomerVerification(emailAddress: $emailAddress)
+        }
+    `;
 });

+ 1 - 0
server/e2e/config/test-config.ts

@@ -25,6 +25,7 @@ export const testConfig: VendureConfig = {
     authOptions: {
         sessionSecret: 'some-secret',
         tokenMethod: 'bearer',
+        requireVerification: true,
     },
     dbConnectionOptions: {
         type: 'sqljs',

+ 7 - 0
server/e2e/test-server.ts

@@ -59,13 +59,20 @@ export class TestServer {
      * Populates an .sqlite database file based on the PopulateOptions.
      */
     private async populateInitialData(testingConfig: VendureConfig, options: PopulateOptions): Promise<void> {
+        // Make some specific modifications to the config for the population to work correctly
         (testingConfig.dbConnectionOptions as Mutable<SqljsConnectionOptions>).autoSave = true;
+        const originalRequireVerification = testingConfig.authOptions.requireVerification;
+        testingConfig.authOptions.requireVerification = false;
+
         const app = await populate(testingConfig, this.bootstrapForTesting, {
             logging: false,
             ...options,
         });
         await app.close();
+
+        // Revert the config back to its initial state
         (testingConfig.dbConnectionOptions as Mutable<SqljsConnectionOptions>).autoSave = false;
+        testingConfig.authOptions.requireVerification = originalRequireVerification;
     }
 
     /**

+ 13 - 3
server/src/api/resolvers/auth.resolver.ts

@@ -4,8 +4,9 @@ import {
     LoginMutationArgs,
     LoginResult,
     Permission,
+    RefreshCustomerVerificationMutationArgs,
     RegisterCustomerAccountMutationArgs,
-    VerifyCustomerEmailAddressMutationArgs,
+    VerifyCustomerAccountMutationArgs,
 } from 'shared/generated-types';
 
 import { ConfigService } from '../../config/config.service';
@@ -75,9 +76,9 @@ export class AuthResolver {
 
     @Mutation()
     @Allow(Permission.Public)
-    async verifyCustomerEmailAddress(
+    async verifyCustomerAccount(
         @Ctx() ctx: RequestContext,
-        @Args() args: VerifyCustomerEmailAddressMutationArgs,
+        @Args() args: VerifyCustomerAccountMutationArgs,
         @Context('req') req: Request,
         @Context('res') res: Response,
     ) {
@@ -102,6 +103,15 @@ export class AuthResolver {
         }
     }
 
+    @Mutation()
+    @Allow(Permission.Public)
+    async refreshCustomerVerification(
+        @Ctx() ctx: RequestContext,
+        @Args() args: RefreshCustomerVerificationMutationArgs,
+    ) {
+        return this.customerService.refreshVerificationToken(ctx, args.emailAddress).then(() => true);
+    }
+
     /**
      * Returns information about the current authenticated user.
      */

+ 3 - 2
server/src/api/types/auth.api.graphql

@@ -8,7 +8,9 @@ type Mutation {
     "Register a Customer account with the given credentials"
     registerCustomerAccount(input: RegisterCustomerInput!): Boolean!
     "Verify a Customer email address with the token sent to that address"
-    verifyCustomerEmailAddress(token: String!, password: String!): LoginResult!
+    verifyCustomerAccount(token: String!, password: String!): LoginResult!
+    "Regenerate and send a verification token for a new Customer registration"
+    refreshCustomerVerification(emailAddress: String!): Boolean!
 }
 
 type LoginResult {
@@ -26,5 +28,4 @@ input RegisterCustomerInput {
     title: String
     firstName: String
     lastName: String
-    password: String!
 }

+ 16 - 1
server/src/service/services/customer.service.ts

@@ -87,7 +87,12 @@ export class CustomerService {
             firstName: input.firstName || '',
             lastName: input.lastName || '',
         });
-        const user = await this.userService.createCustomerUser(input.emailAddress, input.password);
+        let user = await this.userService.getUserByEmailAddress(input.emailAddress);
+        if (!user) {
+            user = await this.userService.createCustomerUser(input.emailAddress);
+        } else {
+            user = await this.userService.setVerificationToken(user);
+        }
         customer.user = user;
         await this.connection.getRepository(Customer).save(customer);
         if (!user.verified) {
@@ -96,6 +101,16 @@ export class CustomerService {
         return customer;
     }
 
+    async refreshVerificationToken(ctx: RequestContext, emailAddress: string) {
+        const user = await this.userService.getUserByEmailAddress(emailAddress);
+        if (user) {
+            await this.userService.setVerificationToken(user);
+            if (!user.verified) {
+                this.eventBus.publish(new AccountRegistrationEvent(ctx, user));
+            }
+        }
+    }
+
     async verifyCustomerEmailAddress(
         ctx: RequestContext,
         verificationToken: string,

+ 23 - 4
server/src/service/services/user.service.ts

@@ -26,7 +26,16 @@ export class UserService {
         });
     }
 
-    async createCustomerUser(identifier: string, password: string): Promise<User> {
+    async getUserByEmailAddress(emailAddress: string): Promise<User | undefined> {
+        return this.connection.getRepository(User).findOne({
+            where: {
+                identifier: emailAddress,
+            },
+            relations: ['roles', 'roles.channels'],
+        });
+    }
+
+    async createCustomerUser(identifier: string, password?: string): Promise<User> {
         const user = new User();
         if (this.configService.authOptions.requireVerification) {
             user.verificationToken = this.generateVerificationToken();
@@ -34,7 +43,11 @@ export class UserService {
         } else {
             user.verified = true;
         }
-        user.passwordHash = await this.passwordCipher.hash(password);
+        if (password) {
+            user.passwordHash = await this.passwordCipher.hash(password);
+        } else {
+            user.passwordHash = '';
+        }
         user.identifier = identifier;
         const customerRole = await this.roleService.getCustomerRole();
         user.roles = [customerRole];
@@ -50,13 +63,19 @@ export class UserService {
         return this.connection.manager.save(user);
     }
 
+    async setVerificationToken(user: User): Promise<User> {
+        user.verificationToken = this.generateVerificationToken();
+        user.verified = false;
+        return this.connection.manager.save(user);
+    }
+
     async verifyUserByToken(verificationToken: string, password: string): Promise<User | undefined> {
         const user = await this.connection.getRepository(User).findOne({
             where: { verificationToken },
         });
         if (user) {
-            const passwordMatches = await this.passwordCipher.check(password, user.passwordHash);
-            if (passwordMatches && this.verifyVerificationToken(verificationToken)) {
+            if (this.verifyVerificationToken(verificationToken)) {
+                user.passwordHash = await this.passwordCipher.hash(password);
                 user.verificationToken = null;
                 user.verified = true;
                 await this.connection.getRepository(User).save(user);

+ 21 - 7
shared/generated-types.ts

@@ -576,7 +576,8 @@ export interface Mutation {
     login: LoginResult;
     logout: boolean;
     registerCustomerAccount: boolean;
-    verifyCustomerEmailAddress: LoginResult;
+    verifyCustomerAccount: LoginResult;
+    refreshCustomerVerification: boolean;
     createChannel: Channel;
     updateChannel: Channel;
     createCountry: Country;
@@ -955,7 +956,6 @@ export interface RegisterCustomerInput {
     title?: string | null;
     firstName?: string | null;
     lastName?: string | null;
-    password: string;
 }
 
 export interface CreateChannelInput {
@@ -1422,10 +1422,13 @@ export interface LoginMutationArgs {
 export interface RegisterCustomerAccountMutationArgs {
     input: RegisterCustomerInput;
 }
-export interface VerifyCustomerEmailAddressMutationArgs {
+export interface VerifyCustomerAccountMutationArgs {
     token: string;
     password: string;
 }
+export interface RefreshCustomerVerificationMutationArgs {
+    emailAddress: string;
+}
 export interface CreateChannelMutationArgs {
     input: CreateChannelInput;
 }
@@ -3624,7 +3627,8 @@ export namespace MutationResolvers {
         login?: LoginResolver<LoginResult, any, Context>;
         logout?: LogoutResolver<boolean, any, Context>;
         registerCustomerAccount?: RegisterCustomerAccountResolver<boolean, any, Context>;
-        verifyCustomerEmailAddress?: VerifyCustomerEmailAddressResolver<LoginResult, any, Context>;
+        verifyCustomerAccount?: VerifyCustomerAccountResolver<LoginResult, any, Context>;
+        refreshCustomerVerification?: RefreshCustomerVerificationResolver<boolean, any, Context>;
         createChannel?: CreateChannelResolver<Channel, any, Context>;
         updateChannel?: UpdateChannelResolver<Channel, any, Context>;
         createCountry?: CreateCountryResolver<Country, any, Context>;
@@ -3748,17 +3752,27 @@ export namespace MutationResolvers {
         input: RegisterCustomerInput;
     }
 
-    export type VerifyCustomerEmailAddressResolver<R = LoginResult, Parent = any, Context = any> = Resolver<
+    export type VerifyCustomerAccountResolver<R = LoginResult, Parent = any, Context = any> = Resolver<
         R,
         Parent,
         Context,
-        VerifyCustomerEmailAddressArgs
+        VerifyCustomerAccountArgs
     >;
-    export interface VerifyCustomerEmailAddressArgs {
+    export interface VerifyCustomerAccountArgs {
         token: string;
         password: string;
     }
 
+    export type RefreshCustomerVerificationResolver<R = boolean, Parent = any, Context = any> = Resolver<
+        R,
+        Parent,
+        Context,
+        RefreshCustomerVerificationArgs
+    >;
+    export interface RefreshCustomerVerificationArgs {
+        emailAddress: string;
+    }
+
     export type CreateChannelResolver<R = Channel, Parent = any, Context = any> = Resolver<
         R,
         Parent,

Daži faili netika attēloti, jo izmaiņu fails ir pārāk liels