Skip to content

Commit 3e376c4

Browse files
authored
fix: retry attempts on session validation and more verbose logs (#898)
1 parent bfe2d4b commit 3e376c4

File tree

7 files changed

+50
-22
lines changed

7 files changed

+50
-22
lines changed

packages/api-gateway/src/Controller/AuthMiddleware.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,16 @@ export abstract class AuthMiddleware extends BaseMiddleware {
7474
response.locals.sharedVaultOwnerContext = decodedToken.shared_vault_owner_context
7575
response.locals.belongsToSharedVaults = decodedToken.belongs_to_shared_vaults ?? []
7676
} catch (error) {
77-
const errorMessage = (error as AxiosError).isAxiosError
78-
? JSON.stringify((error as AxiosError).response?.data)
79-
: (error as Error).message
77+
let detailedErrorMessage = (error as Error).message
78+
if (error instanceof AxiosError) {
79+
detailedErrorMessage = `Status: ${error.status}, code: ${error.code}, message: ${error.message}`
80+
}
8081

81-
this.logger.error(`Could not pass the request to sessions/validate on underlying service: ${errorMessage}`)
82+
this.logger.error(
83+
`Could not pass the request to sessions/validate on underlying service: ${detailedErrorMessage}`,
84+
)
8285

83-
this.logger.debug('Response error: %O', (error as AxiosError).response ?? error)
86+
this.logger.debug(`Response error: ${JSON.stringify(error)}`)
8487

8588
if ((error as AxiosError).response?.headers['content-type']) {
8689
response.setHeader('content-type', (error as AxiosError).response?.headers['content-type'] as string)
@@ -91,7 +94,14 @@ export abstract class AuthMiddleware extends BaseMiddleware {
9194
? +((error as AxiosError).code as string)
9295
: 500
9396

94-
response.status(errorCode).send(errorMessage)
97+
const responseErrorMessage = (error as AxiosError).response?.data
98+
99+
response
100+
.status(errorCode)
101+
.send(
102+
responseErrorMessage ??
103+
"Unfortunately, we couldn't handle your request. Please try again or contact our support if the error persists.",
104+
)
95105

96106
return
97107
}

packages/api-gateway/src/Service/Http/HttpServiceProxy.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,9 @@ export class HttpServiceProxy implements ServiceProxyInterface {
5555
},
5656
}
5757
} catch (error) {
58-
const requestTimedOut =
59-
'code' in (error as Record<string, unknown>) && (error as Record<string, unknown>).code === 'ETIMEDOUT'
58+
const requestDidNotMakeIt = this.requestTimedOutOrDidNotReachDestination(error as Record<string, unknown>)
6059
const tooManyRetryAttempts = retryAttempt && retryAttempt > 2
61-
if (!tooManyRetryAttempts && requestTimedOut) {
60+
if (!tooManyRetryAttempts && requestDidNotMakeIt) {
6261
await this.timer.sleep(50)
6362

6463
const nextRetryAttempt = retryAttempt ? retryAttempt + 1 : 1

packages/api-gateway/src/Service/Http/ServiceProxyInterface.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ export interface ServiceProxyInterface {
5050
endpointOrMethodIdentifier: string,
5151
payload?: Record<string, unknown> | string,
5252
): Promise<void>
53-
validateSession(headers: { authorization: string; sharedVaultOwnerContext?: string }): Promise<{
53+
validateSession(
54+
headers: {
55+
authorization: string
56+
sharedVaultOwnerContext?: string
57+
},
58+
retryAttempt?: number,
59+
): Promise<{
5460
status: number
5561
data: unknown
5662
headers: {

packages/api-gateway/src/Service/Proxy/DirectCallServiceProxy.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@ export class DirectCallServiceProxy implements ServiceProxyInterface {
99
private filesServerUrl: string,
1010
) {}
1111

12-
async validateSession(headers: {
13-
authorization: string
14-
sharedVaultOwnerContext?: string
15-
}): Promise<{ status: number; data: unknown; headers: { contentType: string } }> {
12+
async validateSession(
13+
headers: {
14+
authorization: string
15+
sharedVaultOwnerContext?: string
16+
},
17+
_retryAttempt?: number,
18+
): Promise<{ status: number; data: unknown; headers: { contentType: string } }> {
1619
const authService = this.serviceContainer.get(ServiceIdentifier.create(ServiceIdentifier.NAMES.Auth).getValue())
1720
if (!authService) {
1821
throw new Error('Auth service not found')

packages/auth/src/Domain/UseCase/AuthenticateRequest.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export class AuthenticateRequest implements UseCaseInterface {
1616

1717
async execute(dto: AuthenticateRequestDTO): Promise<AuthenticateRequestResponse> {
1818
if (!dto.authorizationHeader) {
19-
this.logger.debug('Authorization header not provided.')
19+
this.logger.debug('[authenticate-request] Authorization header not provided.')
2020

2121
return {
2222
success: false,
@@ -32,7 +32,9 @@ export class AuthenticateRequest implements UseCaseInterface {
3232
token: dto.authorizationHeader.replace('Bearer ', ''),
3333
})
3434
} catch (error) {
35-
this.logger.error('Error occurred during authentication of a user %o', error)
35+
this.logger.error(
36+
`[authenticate-request] Error occurred during authentication of a user ${(error as Error).message}`,
37+
)
3638

3739
return {
3840
success: false,

packages/auth/src/Domain/UseCase/AuthenticateUser.spec.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ describe('AuthenticateUser', () => {
2323
beforeEach(() => {
2424
logger = {} as jest.Mocked<Logger>
2525
logger.debug = jest.fn()
26+
logger.error = jest.fn()
27+
logger.warn = jest.fn()
2628

2729
user = {} as jest.Mocked<User>
2830
user.supportsSessions = jest.fn().mockReturnValue(false)

packages/auth/src/Domain/UseCase/AuthenticateUser.ts

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export class AuthenticateUser implements UseCaseInterface {
2424
async execute(dto: AuthenticateUserDTO): Promise<AuthenticateUserResponse> {
2525
const authenticationMethod = await this.authenticationMethodResolver.resolve(dto.token)
2626
if (!authenticationMethod) {
27-
this.logger.debug('No authentication method found for token.')
27+
this.logger.error(`[authenticate-user] No authentication method found for token: ${dto.token}`)
2828

2929
return {
3030
success: false,
@@ -33,6 +33,8 @@ export class AuthenticateUser implements UseCaseInterface {
3333
}
3434

3535
if (authenticationMethod.type === 'revoked') {
36+
this.logger.warn(`[authenticate-user] Session has been revoked: ${dto.token}`)
37+
3638
return {
3739
success: false,
3840
failureType: 'REVOKED_SESSION',
@@ -41,7 +43,7 @@ export class AuthenticateUser implements UseCaseInterface {
4143

4244
const user = authenticationMethod.user
4345
if (!user) {
44-
this.logger.debug('No user found for authentication method.')
46+
this.logger.error(`[authenticate-user] No user found for authentication method. Token: ${dto.token}`)
4547

4648
return {
4749
success: false,
@@ -50,7 +52,9 @@ export class AuthenticateUser implements UseCaseInterface {
5052
}
5153

5254
if (authenticationMethod.type == 'jwt' && user.supportsSessions()) {
53-
this.logger.debug('User supports sessions but is trying to authenticate with a JWT.')
55+
this.logger.error(
56+
`[authenticate-user][${user.uuid}] User supports sessions but is trying to authenticate with a JWT.`,
57+
)
5458

5559
return {
5660
success: false,
@@ -64,7 +68,7 @@ export class AuthenticateUser implements UseCaseInterface {
6468
const encryptedPasswordDigest = crypto.createHash('sha256').update(user.encryptedPassword).digest('hex')
6569

6670
if (!pwHash || !crypto.timingSafeEqual(Buffer.from(pwHash), Buffer.from(encryptedPasswordDigest))) {
67-
this.logger.debug('Password hash does not match.')
71+
this.logger.error(`[authenticate-user][${user.uuid}] Password hash does not match.`)
6872

6973
return {
7074
success: false,
@@ -76,7 +80,7 @@ export class AuthenticateUser implements UseCaseInterface {
7680
case 'session_token': {
7781
const session = authenticationMethod.session
7882
if (!session) {
79-
this.logger.debug('No session found for authentication method.')
83+
this.logger.error(`[authenticate-user][${user.uuid}] No session found for authentication method.`)
8084

8185
return {
8286
success: false,
@@ -85,7 +89,7 @@ export class AuthenticateUser implements UseCaseInterface {
8589
}
8690

8791
if (session.refreshExpiration < this.timer.getUTCDate()) {
88-
this.logger.debug('Session refresh token has expired.')
92+
this.logger.warn(`[authenticate-user][${user.uuid}] Session refresh token has expired.`)
8993

9094
return {
9195
success: false,
@@ -94,6 +98,8 @@ export class AuthenticateUser implements UseCaseInterface {
9498
}
9599

96100
if (this.sessionIsExpired(session)) {
101+
this.logger.warn(`[authenticate-user][${user.uuid}] Session access token has expired.`)
102+
97103
return {
98104
success: false,
99105
failureType: 'EXPIRED_TOKEN',

0 commit comments

Comments
 (0)