refactor(auth.client): (1) extract email validation and send login ot…

…p flow to Typescript (#2084)

* feat(AdminAuthService):  add checkIsEmailAllowed service function

* ref: condense vm.checkEmail and vm.checkUser into single vm.login fn

* feat(AdminAuthService): add sendLoginOtp function

* ref: use AdminAuthService.sendLoginOtp

* ref: rename AdminAuthService to AuthService

better fit API prefix

* feat: remove all catch transformations from AuthService

not needed, may interfere with React migration. Should just handle it in the caller

# Conflicts:
#	src/public/modules/users/controllers/authentication.client.controller.js
#	src/public/services/AuthService.ts

* test(AuthService): unit tests for checkIsEmailAllowed + sendLoginOtp

* feat: remove unused functions from old auth.client

* fix(auth.client.ctl): correctly retrieve error msg when sendOtp fails

* test(AuthService): add clarity to test block

Co-authored-by: tshuli <[email protected]>

* feat(AuthService): remove throw documentation on sendLoginOtp

Co-authored-by: tshuli <[email protected]>

src/public/modules/users/controllers/authentication.client.controller.js

@@ -1,10 +1,14 @@
 'use strict'
 
 const HttpStatus = require('http-status-codes')
+const get = require('lodash/get')
+const validator = require('validator').default
+const AuthService = require('../../../services/AuthService')
 
 angular
   .module('users')
   .controller('AuthenticationController', [
+    '$q',
     '$scope',
     '$state',
     '$timeout',
@@ -15,6 +19,7 @@ angular
   ])
 
 function AuthenticationController(
+  $q,
   $scope,
   $state,
   $timeout,
@@ -54,7 +59,7 @@ function AuthenticationController(
 
   vm.handleEmailKeyUp = function (e) {
     if (e.keyCode == 13) {
-      vm.isSubmitEmailDisabled === false && vm.checkEmail()
+      vm.isSubmitEmailDisabled === false && vm.login()
       // condition vm.isSubmitEmailDisabled == false is necessary to prevent retries using enter key
       // when submit button is disabled
     } else {
@@ -115,52 +120,45 @@ function AuthenticationController(
     }
   }
 
+  const setEmailSignInError = (errorMsg) => {
+    vm.buttonClicked = false
+    vm.signInMsg = {
+      isMsg: true,
+      isError: true,
+      msg: errorMsg,
+    }
+    vm.isSubmitEmailDisabled = true
+  }
+
   // Steps of sign-in process
   // 1 - Check if user email is in valid format
   // 2 - Check if user's email domain (i.e. agency) is valid
   // 3 - Send OTP to user email
   // 4 - Verify OTP
 
   /**
-   * Conducts front-end check of user email format
+   * Checks validity of email domain (i.e. agency) and sends login OTP if email
+   * is valid.
    */
-  vm.checkEmail = function () {
+  vm.login = function () {
     vm.buttonClicked = true
-    if (/\S+@\S+\.\S+/.test(vm.credentials.email)) {
-      vm.credentials.email = vm.credentials.email.toLowerCase()
-      vm.checkUser()
-    } else {
-      // Invalid email
-      vm.buttonClicked = false
-      vm.signInMsg = {
-        isMsg: true,
-        isError: true,
-        msg: 'Please enter a valid email',
-      }
-      vm.isSubmitEmailDisabled = true
+
+    const email = vm.credentials.email
+    if (!validator.isEmail(email)) {
+      setEmailSignInError('Please enter a valid email address')
+      return
     }
-  }
 
-  /**
-   * Checks validity of email domain (i.e. agency)
-   * Directs user to otp input page
-   */
-  vm.checkUser = function () {
-    Auth.checkUser(vm.credentials).then(
-      function () {
-        vm.sendOtp()
-      },
-      function (error) {
-        // Invalid agency
-        vm.buttonClicked = false
-        vm.signInMsg = {
-          isMsg: true,
-          isError: true,
-          msg: error,
-        }
-        vm.isSubmitEmailDisabled = true
-      },
-    )
+    $q.when(AuthService.checkIsEmailAllowed(vm.credentials.email))
+      .then(() => vm.sendOtp())
+      .catch((error) => {
+        const errorMsg = get(
+          error,
+          'response.data',
+          'Something went wrong while validating your email. Please refresh and try again',
+        )
+        setEmailSignInError(errorMsg)
+      })
   }
 
   /**
@@ -173,9 +171,9 @@ function AuthenticationController(
     vm.isOtpSending = true
     vm.buttonClicked = true
     vm.showOtpDelayNotification = false
-    const { email } = vm.credentials
-    Auth.sendOtp({ email }).then(
-      function (success) {
+
+    $q.when(AuthService.sendLoginOtp(vm.credentials.email))
+      .then((success) => {
         vm.isOtpSending = false
         vm.buttonClicked = false
         // Configure message to be show
@@ -195,21 +193,22 @@ function AuthenticationController(
           vm.signInMsg.isMsg = false
           vm.showOtpDelayNotification = true
         }, 20000)
-      },
-      function (error) {
+      })
+      .catch((error) => {
+        const errorMsg = get(
+          error,
+          'response.data.message',
+          'Failed to send login OTP. Please try again later and if the problem persists, contact us.',
+        )
         vm.isOtpSending = false
         vm.buttonClicked = false
         // Configure message to be shown
-        const msg =
-          (error && error.data && error.data.message) ||
-          'Failed to send login OTP. Please try again later and if the problem persists, contact us.'
         vm.signInMsg = {
           isMsg: true,
           isError: true,
-          msg,
+          msg: errorMsg,
         }
-      },
-    )
+      })
   }
 
   /**

src/public/modules/users/services/auth.client.service.js

@@ -21,8 +21,6 @@ function Auth($q, $http, $state, $window) {
    */
 
   let authService = {
-    checkUser,
-    sendOtp,
     verifyOtp,
     getUser,
     setUser,
@@ -66,33 +64,6 @@ function Auth($q, $http, $state, $window) {
         return null
       })
   }
-
-  function checkUser(credentials) {
-    let deferred = $q.defer()
-    $http.post('/api/v3/auth/email/validate', credentials).then(
-      function (response) {
-        deferred.resolve(response.data)
-      },
-      function (error) {
-        deferred.reject(error.data)
-      },
-    )
-    return deferred.promise
-  }
-
-  function sendOtp(credentials) {
-    let deferred = $q.defer()
-    $http.post('/api/v3/auth/otp/generate', credentials).then(
-      function (response) {
-        deferred.resolve(response.data)
-      },
-      function (error) {
-        deferred.reject(error)
-      },
-    )
-    return deferred.promise
-  }
-
   function verifyOtp(credentials) {
     let deferred = $q.defer()
     $http.post('/api/v3/auth/otp/verify', credentials).then(

src/public/modules/users/views/authentication/signin.client.view.html

@@ -40,7 +40,7 @@
         <div class="btn-grp">
           <button
             class="btn-custom btn-medium"
-            ng-click="vm.checkEmail()"
+            ng-click="vm.login()"
             ng-disabled="vm.buttonClicked || vm.isSubmitEmailDisabled"
           >
             Get Started

src/public/services/AuthService.ts

@@ -0,0 +1,33 @@
+import axios from 'axios'
+import { Opaque } from 'type-fest'
+
+// Exported for testing.
+export const AUTH_ENDPOINT = '/api/v3/auth'
+
+type Email = Opaque<string, 'Email'>
+
+/**
+ * Check whether the given email string is from a whitelisted email domain.
+ * @param email the email to check
+ * @returns original email if email is valid
+ */
+export const checkIsEmailAllowed = async (email: string): Promise<Email> => {
+  return axios
+    .post(`${AUTH_ENDPOINT}/email/validate`, {
+      email: email.toLowerCase(),
+    })
+    .then(() => email as Email)
+}
+
+/**
+ * Sends login OTP to given email
+ * @param email email to send login OTP to
+ * @returns success string if login OTP is sent successfully
+ */
+export const sendLoginOtp = async (email: Email): Promise<string> => {
+  return axios
+    .post<string>(`${AUTH_ENDPOINT}/otp/generate`, {
+      email: email.toLowerCase(),
+    })
+    .then(({ data }) => data)
+}

src/public/services/__tests__/AuthService.test.ts

@@ -0,0 +1,60 @@
+import axios from 'axios'
+import { mocked } from 'ts-jest/utils'
+import { Opaque } from 'type-fest'
+
+import {
+  AUTH_ENDPOINT,
+  checkIsEmailAllowed,
+  sendLoginOtp,
+} from '../AuthService'
+
+jest.mock('axios')
+
+const MockAxios = mocked(axios, true)
+
+// Duplicated here instead of exporting from AuthService to prevent production
+// code from casting to Email type without going through a type guard/validator.
+type TestEmail = Opaque<string, 'Email'>
+
+describe('AuthService', () => {
+  describe('checkIsEmailAllowed', () => {
+    const EXPECTED_POST_ENDPOINT = `${AUTH_ENDPOINT}/email/validate`
+
+    it('should return given email argument when email is allowed', async () => {
+      // Arrange
+      const mockEmail = '[email protected]'
+      MockAxios.post.mockResolvedValueOnce({ status: 200 })
+
+      // Act
+      const actual = await checkIsEmailAllowed(mockEmail)
+
+      // Assert
+      expect(actual).toEqual(mockEmail)
+      expect(MockAxios.post).toHaveBeenCalledWith(EXPECTED_POST_ENDPOINT, {
+        email: mockEmail.toLowerCase(),
+      })
+    })
+  })
+
+  describe('sendLoginOtp', () => {
+    const EXPECTED_POST_ENDPOINT = `${AUTH_ENDPOINT}/otp/generate`
+
+    it('should return success string when OTP is successfully generated', async () => {
+      // Arrange
+      const mockEmail = '[email protected]'
+      const mockSuccessStr = 'yippee ki yay'
+      MockAxios.post.mockResolvedValueOnce({
+        data: mockSuccessStr,
+      })
+
+      // Act
+      const actual = await sendLoginOtp(mockEmail as TestEmail)
+
+      // Assert
+      expect(actual).toEqual(mockSuccessStr)
+      expect(MockAxios.post).toHaveBeenCalledWith(EXPECTED_POST_ENDPOINT, {
+        email: mockEmail.toLowerCase(),
+      })
+    })
+  })
+})