Browse Source

fix(core): Improve log level of ForbiddenError to reduce log noise

Fixes #1080. This change sets the default LogLevel of a ForbiddenError to `Error`, rather than
`Warn`. This means that the call stack will get logged too, which is useful for debugging. However,
in more routine cases of ForbiddenError, i.e. when a request is rejected by the AuthGuard, the log
level is set at `Verbose`, since AuthGuard rejections are very common and normal.
Michael Bromley 4 years ago
parent
commit
5be1dfec32

+ 2 - 1
packages/core/src/api/middleware/auth-guard.ts

@@ -7,6 +7,7 @@ import { GraphQLResolveInfo } from 'graphql';
 import { REQUEST_CONTEXT_KEY } from '../../common/constants';
 import { ForbiddenError } from '../../common/error/errors';
 import { ConfigService } from '../../config/config.service';
+import { LogLevel } from '../../config/logger/vendure-logger';
 import { CachedSession } from '../../config/session-cache/session-cache-strategy';
 import { Customer } from '../../entity/customer/customer.entity';
 import { ChannelService } from '../../service/services/channel.service';
@@ -76,7 +77,7 @@ export class AuthGuard implements CanActivate {
             const canActivate =
                 requestContext.userHasPermissions(permissions) || requestContext.authorizedAsOwnerOnly;
             if (!canActivate) {
-                throw new ForbiddenError();
+                throw new ForbiddenError(LogLevel.Verbose);
             } else {
                 return canActivate;
             }

+ 7 - 6
packages/core/src/api/resolvers/base/base-auth.resolver.ts

@@ -19,7 +19,7 @@ import {
 } from '../../../common/error/generated-graphql-shop-errors';
 import { NATIVE_AUTH_STRATEGY_NAME } from '../../../config/auth/native-authentication-strategy';
 import { ConfigService } from '../../../config/config.service';
-import { Logger } from '../../../config/logger/vendure-logger';
+import { Logger, LogLevel } from '../../../config/logger/vendure-logger';
 import { User } from '../../../entity/user/user.entity';
 import { getUserChannelsPermissions } from '../../../service/helpers/utils/get-user-channels-permissions';
 import { AdministratorService } from '../../../service/services/administrator.service';
@@ -39,9 +39,10 @@ export class BaseAuthResolver {
         protected administratorService: AdministratorService,
         protected configService: ConfigService,
     ) {
-        this.nativeAuthStrategyIsConfigured = !!this.configService.authOptions.shopAuthenticationStrategy.find(
-            strategy => strategy.name === NATIVE_AUTH_STRATEGY_NAME,
-        );
+        this.nativeAuthStrategyIsConfigured =
+            !!this.configService.authOptions.shopAuthenticationStrategy.find(
+                strategy => strategy.name === NATIVE_AUTH_STRATEGY_NAME,
+            );
     }
 
     /**
@@ -87,12 +88,12 @@ export class BaseAuthResolver {
     async me(ctx: RequestContext, apiType: ApiType) {
         const userId = ctx.activeUserId;
         if (!userId) {
-            throw new ForbiddenError();
+            throw new ForbiddenError(LogLevel.Verbose);
         }
         if (apiType === 'admin') {
             const administrator = await this.administratorService.findOneByUserId(ctx, userId);
             if (!administrator) {
-                throw new ForbiddenError();
+                throw new ForbiddenError(LogLevel.Verbose);
             }
         }
         const user = userId && (await this.userService.getUserById(ctx, userId));

+ 3 - 4
packages/core/src/api/resolvers/shop/shop-order.resolver.ts

@@ -26,17 +26,16 @@ import {
     UpdateOrderItemsResult,
 } from '@vendure/common/lib/generated-shop-types';
 import { QueryCountriesArgs } from '@vendure/common/lib/generated-types';
-import ms from 'ms';
 
 import { ErrorResultUnion, isGraphQlErrorResult } from '../../../common/error/error-result';
-import { ForbiddenError, InternalServerError } from '../../../common/error/errors';
+import { ForbiddenError } from '../../../common/error/errors';
 import {
     AlreadyLoggedInError,
     NoActiveOrderError,
 } from '../../../common/error/generated-graphql-shop-errors';
 import { Translated } from '../../../common/types/locale-types';
 import { idsAreEqual } from '../../../common/utils';
-import { ConfigService } from '../../../config';
+import { ConfigService, LogLevel } from '../../../config';
 import { Country } from '../../../entity';
 import { Order } from '../../../entity/order/order.entity';
 import { ActiveOrderService, CountryService } from '../../../service';
@@ -112,7 +111,7 @@ export class ShopOrderResolver {
             }
             // We throw even if the order does not exist, since giving a different response
             // opens the door to an enumeration attack to find valid order codes.
-            throw new ForbiddenError();
+            throw new ForbiddenError(LogLevel.Verbose);
         }
     }
 

+ 2 - 2
packages/core/src/common/error/errors.ts

@@ -65,8 +65,8 @@ export class UnauthorizedError extends I18nError {
  * @docsPage Error Types
  */
 export class ForbiddenError extends I18nError {
-    constructor() {
-        super('error.forbidden', {}, 'FORBIDDEN', LogLevel.Warn);
+    constructor(logLevel: LogLevel = LogLevel.Error) {
+        super('error.forbidden', {}, 'FORBIDDEN', logLevel);
     }
 }