Przeglądaj źródła

fix: retry attempts on session validation and more verbose logs (#898)

Karol Sójko 1 rok temu
rodzic
commit
3e376c44e3

+ 16 - 6
packages/api-gateway/src/Controller/AuthMiddleware.ts

@@ -74,13 +74,16 @@ export abstract class AuthMiddleware extends BaseMiddleware {
       response.locals.sharedVaultOwnerContext = decodedToken.shared_vault_owner_context
       response.locals.belongsToSharedVaults = decodedToken.belongs_to_shared_vaults ?? []
     } catch (error) {
-      const errorMessage = (error as AxiosError).isAxiosError
-        ? JSON.stringify((error as AxiosError).response?.data)
-        : (error as Error).message
+      let detailedErrorMessage = (error as Error).message
+      if (error instanceof AxiosError) {
+        detailedErrorMessage = `Status: ${error.status}, code: ${error.code}, message: ${error.message}`
+      }
 
-      this.logger.error(`Could not pass the request to sessions/validate on underlying service: ${errorMessage}`)
+      this.logger.error(
+        `Could not pass the request to sessions/validate on underlying service: ${detailedErrorMessage}`,
+      )
 
-      this.logger.debug('Response error: %O', (error as AxiosError).response ?? error)
+      this.logger.debug(`Response error: ${JSON.stringify(error)}`)
 
       if ((error as AxiosError).response?.headers['content-type']) {
         response.setHeader('content-type', (error as AxiosError).response?.headers['content-type'] as string)
@@ -91,7 +94,14 @@ export abstract class AuthMiddleware extends BaseMiddleware {
           ? +((error as AxiosError).code as string)
           : 500
 
-      response.status(errorCode).send(errorMessage)
+      const responseErrorMessage = (error as AxiosError).response?.data
+
+      response
+        .status(errorCode)
+        .send(
+          responseErrorMessage ??
+            "Unfortunately, we couldn't handle your request. Please try again or contact our support if the error persists.",
+        )
 
       return
     }

+ 2 - 3
packages/api-gateway/src/Service/Http/HttpServiceProxy.ts

@@ -55,10 +55,9 @@ export class HttpServiceProxy implements ServiceProxyInterface {
         },
       }
     } catch (error) {
-      const requestTimedOut =
-        'code' in (error as Record<string, unknown>) && (error as Record<string, unknown>).code === 'ETIMEDOUT'
+      const requestDidNotMakeIt = this.requestTimedOutOrDidNotReachDestination(error as Record<string, unknown>)
       const tooManyRetryAttempts = retryAttempt && retryAttempt > 2
-      if (!tooManyRetryAttempts && requestTimedOut) {
+      if (!tooManyRetryAttempts && requestDidNotMakeIt) {
         await this.timer.sleep(50)
 
         const nextRetryAttempt = retryAttempt ? retryAttempt + 1 : 1

+ 7 - 1
packages/api-gateway/src/Service/Http/ServiceProxyInterface.ts

@@ -50,7 +50,13 @@ export interface ServiceProxyInterface {
     endpointOrMethodIdentifier: string,
     payload?: Record<string, unknown> | string,
   ): Promise<void>
-  validateSession(headers: { authorization: string; sharedVaultOwnerContext?: string }): Promise<{
+  validateSession(
+    headers: {
+      authorization: string
+      sharedVaultOwnerContext?: string
+    },
+    retryAttempt?: number,
+  ): Promise<{
     status: number
     data: unknown
     headers: {

+ 7 - 4
packages/api-gateway/src/Service/Proxy/DirectCallServiceProxy.ts

@@ -9,10 +9,13 @@ export class DirectCallServiceProxy implements ServiceProxyInterface {
     private filesServerUrl: string,
   ) {}
 
-  async validateSession(headers: {
-    authorization: string
-    sharedVaultOwnerContext?: string
-  }): Promise<{ status: number; data: unknown; headers: { contentType: string } }> {
+  async validateSession(
+    headers: {
+      authorization: string
+      sharedVaultOwnerContext?: string
+    },
+    _retryAttempt?: number,
+  ): Promise<{ status: number; data: unknown; headers: { contentType: string } }> {
     const authService = this.serviceContainer.get(ServiceIdentifier.create(ServiceIdentifier.NAMES.Auth).getValue())
     if (!authService) {
       throw new Error('Auth service not found')

+ 4 - 2
packages/auth/src/Domain/UseCase/AuthenticateRequest.ts

@@ -16,7 +16,7 @@ export class AuthenticateRequest implements UseCaseInterface {
 
   async execute(dto: AuthenticateRequestDTO): Promise<AuthenticateRequestResponse> {
     if (!dto.authorizationHeader) {
-      this.logger.debug('Authorization header not provided.')
+      this.logger.debug('[authenticate-request] Authorization header not provided.')
 
       return {
         success: false,
@@ -32,7 +32,9 @@ export class AuthenticateRequest implements UseCaseInterface {
         token: dto.authorizationHeader.replace('Bearer ', ''),
       })
     } catch (error) {
-      this.logger.error('Error occurred during authentication of a user %o', error)
+      this.logger.error(
+        `[authenticate-request] Error occurred during authentication of a user ${(error as Error).message}`,
+      )
 
       return {
         success: false,

+ 2 - 0
packages/auth/src/Domain/UseCase/AuthenticateUser.spec.ts

@@ -23,6 +23,8 @@ describe('AuthenticateUser', () => {
   beforeEach(() => {
     logger = {} as jest.Mocked<Logger>
     logger.debug = jest.fn()
+    logger.error = jest.fn()
+    logger.warn = jest.fn()
 
     user = {} as jest.Mocked<User>
     user.supportsSessions = jest.fn().mockReturnValue(false)

+ 12 - 6
packages/auth/src/Domain/UseCase/AuthenticateUser.ts

@@ -24,7 +24,7 @@ export class AuthenticateUser implements UseCaseInterface {
   async execute(dto: AuthenticateUserDTO): Promise<AuthenticateUserResponse> {
     const authenticationMethod = await this.authenticationMethodResolver.resolve(dto.token)
     if (!authenticationMethod) {
-      this.logger.debug('No authentication method found for token.')
+      this.logger.error(`[authenticate-user] No authentication method found for token: ${dto.token}`)
 
       return {
         success: false,
@@ -33,6 +33,8 @@ export class AuthenticateUser implements UseCaseInterface {
     }
 
     if (authenticationMethod.type === 'revoked') {
+      this.logger.warn(`[authenticate-user] Session has been revoked: ${dto.token}`)
+
       return {
         success: false,
         failureType: 'REVOKED_SESSION',
@@ -41,7 +43,7 @@ export class AuthenticateUser implements UseCaseInterface {
 
     const user = authenticationMethod.user
     if (!user) {
-      this.logger.debug('No user found for authentication method.')
+      this.logger.error(`[authenticate-user] No user found for authentication method. Token: ${dto.token}`)
 
       return {
         success: false,
@@ -50,7 +52,9 @@ export class AuthenticateUser implements UseCaseInterface {
     }
 
     if (authenticationMethod.type == 'jwt' && user.supportsSessions()) {
-      this.logger.debug('User supports sessions but is trying to authenticate with a JWT.')
+      this.logger.error(
+        `[authenticate-user][${user.uuid}] User supports sessions but is trying to authenticate with a JWT.`,
+      )
 
       return {
         success: false,
@@ -64,7 +68,7 @@ export class AuthenticateUser implements UseCaseInterface {
         const encryptedPasswordDigest = crypto.createHash('sha256').update(user.encryptedPassword).digest('hex')
 
         if (!pwHash || !crypto.timingSafeEqual(Buffer.from(pwHash), Buffer.from(encryptedPasswordDigest))) {
-          this.logger.debug('Password hash does not match.')
+          this.logger.error(`[authenticate-user][${user.uuid}] Password hash does not match.`)
 
           return {
             success: false,
@@ -76,7 +80,7 @@ export class AuthenticateUser implements UseCaseInterface {
       case 'session_token': {
         const session = authenticationMethod.session
         if (!session) {
-          this.logger.debug('No session found for authentication method.')
+          this.logger.error(`[authenticate-user][${user.uuid}] No session found for authentication method.`)
 
           return {
             success: false,
@@ -85,7 +89,7 @@ export class AuthenticateUser implements UseCaseInterface {
         }
 
         if (session.refreshExpiration < this.timer.getUTCDate()) {
-          this.logger.debug('Session refresh token has expired.')
+          this.logger.warn(`[authenticate-user][${user.uuid}] Session refresh token has expired.`)
 
           return {
             success: false,
@@ -94,6 +98,8 @@ export class AuthenticateUser implements UseCaseInterface {
         }
 
         if (this.sessionIsExpired(session)) {
+          this.logger.warn(`[authenticate-user][${user.uuid}] Session access token has expired.`)
+
           return {
             success: false,
             failureType: 'EXPIRED_TOKEN',