fix(api): ApiKey auth guard performance #4972
Conversation
rifont
requested review from
djabarovgeorge and
LetItRock
and removed request for
djabarovgeorge
rifont
changed the title
|
|
||
| const AUTH_STRATEGIES: Provider[] = []; | ||
| const AUTH_STRATEGIES: Provider[] = [JwtStrategy, ApiKeyStrategy, JwtSubscriberStrategy]; |
There was a problem hiding this comment.
Moving the passport strategies out to where they belong.
| const user = jwt.decode(token) as IJwtPayload; | ||
| if (!user) return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
This is handled in the Authentication flow, and shouldn't be part of Authorization.
|
|
||
| const user = jwt.decode(token) as IJwtPayload; | ||
| if (!user) return false; | ||
| if (!user.environmentId) return false; |
There was a problem hiding this comment.
As above, this is handled in the Authentication flow, and shouldn't be part of Authorization.
| environmentId: payload.environmentId, | ||
| organizationId: payload.organizationId, | ||
| }); | ||
|
|
There was a problem hiding this comment.
This is now handled in a single place in the Auth Guard, for all Auth strategies.
|
|
||
| return jwt.decode(token); | ||
| return req.user; |
There was a problem hiding this comment.
req.user is always present now.
| const { environment, user, key, error } = await this.getUserData({ | ||
| apiKey, | ||
| }); | ||
| public async validateApiKey(apiKey: string): Promise<IJwtPayload> { |
There was a problem hiding this comment.
Making sure the ApiKey validation returns the same interface as Bearer authentication.
| } | ||
| ); | ||
| } | ||
|
|
| }) | ||
| private async getEnvironment({ apiKey }: { apiKey: string }) { | ||
| return await this.environmentRepository.findByApiKey(apiKey); | ||
| } |
|
|
||
| async verifyJwt(jwt: string) { | ||
| return this.jwtService.verify(jwt); | ||
| } |
|
|
||
| if (error) throw new UnauthorizedException(error); | ||
| if (!environment) throw new UnauthorizedException('API Key not found'); | ||
| if (!user) throw new UnauthorizedException('User not found'); |
There was a problem hiding this comment.
The if(error) check above already handles these exceptions.
packages/application-generic/src/usecases/trigger-event/trigger-event.usecase.ts
Outdated
Show resolved
Hide resolved
| }); | ||
| } | ||
| /** | ||
| * This helps with sentry and other tools that need to know who the user is based on `id` property. |
|
These changes broke a bunch of It may be a result of the Idempotency changes, but could be something else. Further investigation needed. |
I have ruled out Idempotency issues. It appears that the throttler guard is processing a different |
Turns out this was a simple fix. I'd changed the signature of the |
…-new-relic-alerts
…-new-relic-alerts
|
Added tests for both Bearer and ApiKey auth to make sure the auth logic and strategies are working as expected - 6ba5265 |
What change does this PR introduce?
RolesguardRootEnvironmentguardUserSessiondecoratorWhy was this change needed?
API requests using ApiKey authentication currently sign and decode a JWT token every request. The signing introduces considerable latency whilst the decoding adds unnecessary memory consumption.
We also needed an interface to retrieve User context from the request, regardless of auth scheme.
Other information (Screenshots)
Auth Guard Tests
ApiKey Auth Log
Bearer Auth Log
No Auth Log
