From 48826f019cc7d73ce65eb4a0c77bac54cbd79631 Mon Sep 17 00:00:00 2001 From: Matheus-Aguilar <43484640+Matheus-Aguilar@users.noreply.github.com> Date: Mon, 1 Jul 2024 09:15:53 -0300 Subject: [PATCH] fix: add token validation directive (#143) * fix: add token validation directive * fix: add additional check to admin token * chore: fix cognitive load from the code * fix: rename access directive * fix: reduce cognitive load of the functions * fix: log errors that happened on the validation * fix: catch and log exceptions on token validations --- CHANGELOG.md | 3 + graphql/directives.graphql | 1 + graphql/schema.graphql | 8 +- node/directives/helper.ts | 22 +++- node/directives/index.ts | 2 + node/directives/validateStoreUserAccess.ts | 132 +++++++++++++++++++++ node/metrics/auth.ts | 6 +- node/resolvers/Queries/Users.ts | 37 +++--- node/utils/LicenseManager.ts | 10 ++ 9 files changed, 189 insertions(+), 32 deletions(-) create mode 100644 node/directives/validateStoreUserAccess.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ab1c9da..fca3279 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Added +- Add token validation directive + ## [1.40.7] - 2024-06-11 ### Fixed diff --git a/graphql/directives.graphql b/graphql/directives.graphql index 7a3de32..20e35b9 100644 --- a/graphql/directives.graphql +++ b/graphql/directives.graphql @@ -1,4 +1,5 @@ directive @checkUserAccess on FIELD | FIELD_DEFINITION +directive @validateStoreUserAccess on FIELD | FIELD_DEFINITION directive @checkAdminAccess on FIELD | FIELD_DEFINITION directive @withSession on FIELD_DEFINITION directive @withSender on FIELD_DEFINITION diff --git a/graphql/schema.graphql b/graphql/schema.graphql index cfd2a7e..778a93f 100644 --- a/graphql/schema.graphql +++ b/graphql/schema.graphql @@ -36,7 +36,7 @@ type Query { listAllUsers: [User] @cacheControl(scope: PRIVATE, maxAge: SHORT) @withSender - @checkUserAccess + @validateStoreUserAccess listUsers(organizationId: ID, costCenterId: ID, roleId: ID): [User] @cacheControl(scope: PRIVATE, maxAge: SHORT) @@ -71,11 +71,7 @@ type Query { getSessionWatcher: Boolean @cacheControl(scope: PRIVATE) - getUsersByEmail( - email: String! - orgId: ID - costId: ID - ): [User] + getUsersByEmail(email: String!, orgId: ID, costId: ID): [User] @cacheControl(scope: PRIVATE) @checkUserAccess diff --git a/node/directives/helper.ts b/node/directives/helper.ts index 76d850a..524eeeb 100644 --- a/node/directives/helper.ts +++ b/node/directives/helper.ts @@ -9,7 +9,8 @@ export const validateAdminToken = async ( hasCurrentValidAdminToken: boolean }> => { const { - clients: { identity }, + clients: { identity, lm }, + vtex: { logger }, } = context // check if has admin token and if it is valid @@ -29,10 +30,17 @@ export const validateAdminToken = async ( hasCurrentValidAdminToken = true if (authUser?.audience === 'admin') { - hasValidAdminToken = true + hasValidAdminToken = await lm.getUserAdminPermissions( + authUser.account, + authUser.id + ) } } catch (err) { // noop so we leave hasValidAdminToken as false + logger.warn({ + message: 'Error validating admin token', + err, + }) } } @@ -47,6 +55,7 @@ export const validateApiToken = async ( }> => { const { clients: { identity }, + vtex: { logger }, } = context // check if has api token and if it is valid @@ -71,6 +80,10 @@ export const validateApiToken = async ( } } catch (err) { // noop so we leave hasValidApiToken as false + logger.warn({ + message: 'Error validating API token', + err, + }) } } @@ -87,6 +100,7 @@ export const validateStoreToken = async ( }> => { const { clients: { vtexId }, + vtex: { logger }, } = context // check if has store token and if it is valid @@ -115,6 +129,10 @@ export const validateStoreToken = async ( } } catch (err) { // noop so we leave hasValidStoreToken as false + logger.warn({ + message: 'Error validating store token:', + err, + }) } } diff --git a/node/directives/index.ts b/node/directives/index.ts index 35b6665..28a0979 100644 --- a/node/directives/index.ts +++ b/node/directives/index.ts @@ -3,11 +3,13 @@ import { WithSender } from './withSender' import { WithUserPermissions } from './withUserPermissions' import { CheckAdminAccess } from './checkAdminAccess' import { CheckUserAccess } from './checkUserAccess' +import { ValidateStoreUserAccess } from './validateStoreUserAccess' import { AuditAccess } from './auditAccess' export const schemaDirectives = { checkAdminAccess: CheckAdminAccess as any, checkUserAccess: CheckUserAccess as any, + validateStoreUserAccess: ValidateStoreUserAccess as any, withSession: WithSession, withSender: WithSender, withUserPermissions: WithUserPermissions, diff --git a/node/directives/validateStoreUserAccess.ts b/node/directives/validateStoreUserAccess.ts new file mode 100644 index 0000000..24ef80e --- /dev/null +++ b/node/directives/validateStoreUserAccess.ts @@ -0,0 +1,132 @@ +import { AuthenticationError, ForbiddenError } from '@vtex/api' +import type { GraphQLField } from 'graphql' +import { defaultFieldResolver } from 'graphql' +import { SchemaDirectiveVisitor } from 'graphql-tools' + +import type { AuthAuditMetric } from '../metrics/auth' +import sendAuthMetric, { AuthMetric } from '../metrics/auth' +import { + validateAdminToken, + validateApiToken, + validateStoreToken, +} from './helper' + +export class ValidateStoreUserAccess extends SchemaDirectiveVisitor { + public visitFieldDefinition(field: GraphQLField) { + const { resolve = defaultFieldResolver } = field + + field.resolve = async ( + root: any, + args: any, + context: Context, + info: any + ) => { + const { + vtex: { adminUserAuthToken, storeUserAuthToken, logger }, + } = context + + // get metrics data + const operation = field?.astNode?.name?.value ?? context?.request?.url + const userAgent = context?.request?.headers['user-agent'] as string + const caller = context?.request?.headers['x-vtex-caller'] as string + const forwardedHost = context?.request?.headers[ + 'x-forwarded-host' + ] as string + + // set metric fields with initial data + let metricFields: AuthAuditMetric = { + operation, + forwardedHost, + caller, + userAgent, + } + + const { hasAdminToken, hasValidAdminToken } = await validateAdminToken( + context, + adminUserAuthToken as string + ) + + // add admin token metrics + metricFields = { ...metricFields, hasAdminToken, hasValidAdminToken } + + // allow access if has valid admin token + if (hasValidAdminToken) { + sendAuthMetric( + logger, + new AuthMetric( + context?.vtex?.account, + metricFields, + 'ValidateStoreUserAccessAudit' + ) + ) + + return resolve(root, args, context, info) + } + + const { hasApiToken, hasValidApiToken } = await validateApiToken(context) + + // add API token metrics + metricFields = { + ...metricFields, + hasApiToken, + hasValidApiToken, + } + + // allow access if has valid API token + if (hasValidApiToken) { + sendAuthMetric( + logger, + new AuthMetric( + context?.vtex?.account, + metricFields, + 'ValidateStoreUserAccessAudit' + ) + ) + + return resolve(root, args, context, info) + } + + const { hasStoreToken, hasValidStoreToken } = await validateStoreToken( + context, + storeUserAuthToken as string + ) + + // add store token metrics + metricFields = { + ...metricFields, + hasStoreToken, + hasValidStoreToken, + } + + // allow access if has valid store token + if (hasValidStoreToken) { + sendAuthMetric( + logger, + new AuthMetric( + context?.vtex?.account, + metricFields, + 'ValidateStoreUserAccessAudit' + ) + ) + + return resolve(root, args, context, info) + } + + // deny access if no tokens were provided + if (!hasAdminToken && !hasApiToken && !hasStoreToken) { + logger.warn({ + message: 'ValidateStoreUserAccess: No token provided', + ...metricFields, + }) + throw new AuthenticationError('No token was provided') + } + + // deny access if no valid tokens were provided + logger.warn({ + message: `ValidateStoreUserAccess: Invalid token`, + ...metricFields, + }) + throw new ForbiddenError('Unauthorized Access') + } + } +} diff --git a/node/metrics/auth.ts b/node/metrics/auth.ts index c2c675c..f58f0a6 100644 --- a/node/metrics/auth.ts +++ b/node/metrics/auth.ts @@ -10,11 +10,11 @@ export interface AuthAuditMetric { userAgent: string role?: string permissions?: string[] - hasAdminToken: boolean + hasAdminToken?: boolean hasValidAdminToken?: boolean - hasStoreToken: boolean + hasStoreToken?: boolean hasValidStoreToken?: boolean - hasApiToken: boolean + hasApiToken?: boolean hasValidApiToken?: boolean } diff --git a/node/resolvers/Queries/Users.ts b/node/resolvers/Queries/Users.ts index 21198d6..b0385a9 100644 --- a/node/resolvers/Queries/Users.ts +++ b/node/resolvers/Queries/Users.ts @@ -21,29 +21,24 @@ export const isUserPartOfBuyerOrg = async (email: string, ctx: Context) => { clients: { masterdata }, } = ctx - try { - const where = `email=${email}` - const resp = await masterdata.searchDocumentsWithPaginationInfo({ - dataEntity: config.name, - fields: ['id'], // we don't need to fetch all fields, only if there is an entry or not - pagination: { - page: 1, - pageSize: 1, // we only need to know if there is at least one user entry - }, - schema: config.version, - ...(where ? { where } : {}), - }) + const where = `email=${email}` + const resp = await masterdata.searchDocumentsWithPaginationInfo({ + dataEntity: config.name, + fields: ['id'], // we don't need to fetch all fields, only if there is an entry or not + pagination: { + page: 1, + pageSize: 1, // we only need to know if there is at least one user entry + }, + schema: config.version, + ...(where ? { where } : {}), + }) - const { data } = resp as unknown as { - data: any - } + const { data } = resp as unknown as { + data: any + } - if (data.length > 0) { - return true - } - } catch (error) { - // if it fails at somepoint, we treat it like no user was found - // on any buyer org, so we just let the function return false + if (data.length > 0) { + return true } return false diff --git a/node/utils/LicenseManager.ts b/node/utils/LicenseManager.ts index e37dc79..b91bdf4 100644 --- a/node/utils/LicenseManager.ts +++ b/node/utils/LicenseManager.ts @@ -73,6 +73,14 @@ export class LMClient extends ExternalClient { return user ? this.delete(this.routes.deleteUser(userId, '957'), {}) : {} } + public getUserAdminPermissions = async (account: string, userId: string) => { + return this.get(this.routes.getUserAdminPermissions(account, userId)).then( + (res: any) => { + return res + } + ) + } + protected get = (url: string) => { return this.http.get(url).catch(statusToError) } @@ -103,6 +111,8 @@ export class LMClient extends ExternalClient { userByEmail: (email: string) => `api/license-manager/pvt/users/${encodeURIComponent(email)}`, userById: (id: string) => `api/license-manager/pvt/users/${id}`, + getUserAdminPermissions: (account: string, userId: string) => + `/api/license-manager/pvt/accounts/${account}/logins/${userId}/granted`, } } }