From 382bd67c4afeed8d49dcb18d70123ad2048e9996 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Sat, 21 Mar 2026 18:39:09 +0100 Subject: [PATCH] fix(auth): address audit findings in OCPP auth abstraction layer - Read AuthCtrlr.LocalAuthorizeOffline variable instead of hardcoding true - Delegate testConnectivity() to actual adapter availability checks - Remove duplicate mapOCPP20AuthStatus(), use canonical mapOCPP20AuthorizationStatus - Map strategies by name instead of fragile priority numbers - Make connectorId truly optional in OCPP20AuthAdapter.authorizeRemote() - Use ES2022 Error cause chain in AuthenticationError --- .../ocpp/auth/adapters/OCPP20AuthAdapter.ts | 58 ++----------------- .../ocpp/auth/services/OCPPAuthServiceImpl.ts | 24 ++++---- .../ocpp/auth/types/AuthTypes.ts | 4 +- 3 files changed, 19 insertions(+), 67 deletions(-) diff --git a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts index 27ce77d6..fa980921 100644 --- a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts +++ b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts @@ -16,7 +16,6 @@ import type { import { OCPP20VariableManager } from '../../2.0/OCPP20VariableManager.js' import { GetVariableStatusEnumType, - OCPP20AuthorizationStatusEnumType, OCPP20IdTokenEnumType, type OCPP20IdTokenType, OCPP20RequestCommand, @@ -28,6 +27,7 @@ import { AuthenticationMethod, AuthorizationStatus, IdentifierType, + mapOCPP20AuthorizationStatus, mapToOCPP20Status, } from '../types/AuthTypes.js' @@ -79,22 +79,6 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { } } - // Validate inputs - if (connectorId == null) { - logger.warn( - `${this.chargingStation.logPrefix()} ${moduleName}.${methodName}: No connector specified for authorization` - ) - return { - additionalInfo: { - error: 'Connector ID is required for OCPP 2.0 authorization', - }, - isOffline: false, - method: AuthenticationMethod.REMOTE_AUTHORIZATION, - status: AuthorizationStatus.INVALID, - timestamp: new Date(), - } - } - try { const idToken = this.convertFromUnifiedIdentifier(identifier) @@ -131,7 +115,7 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { const cacheExpiryDateTime = response.idTokenInfo.cacheExpiryDateTime // Map OCPP 2.0 authorization status to unified status - const unifiedStatus = this.mapOCPP20AuthStatus(authStatus) + const unifiedStatus = mapOCPP20AuthorizationStatus(authStatus) logger.debug( `${this.chargingStation.logPrefix()} ${moduleName}.${methodName}: Authorization result for ${idToken.idToken}: ${authStatus} (unified: ${unifiedStatus})` @@ -536,9 +520,8 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { */ private getOfflineAuthorizationConfig (): boolean { try { - // In OCPP 2.0, this would be controlled by LocalAuthorizeOffline variable - // For now, return a default value - return true + const value = this.getVariableValue('AuthCtrlr', 'LocalAuthorizeOffline') + return this.parseBooleanVariable(value, true) } catch (error) { logger.warn( `${this.chargingStation.logPrefix()} ${moduleName}.getOfflineAuthorizationConfig: Error getting offline authorization config`, @@ -631,39 +614,6 @@ export class OCPP20AuthAdapter implements OCPPAuthAdapter { } } - /** - * Maps OCPP 2.0 AuthorizationStatusEnumType to unified AuthorizationStatus - * @param ocpp20Status - OCPP 2.0 authorization status - * @returns Unified authorization status - */ - private mapOCPP20AuthStatus ( - ocpp20Status: OCPP20AuthorizationStatusEnumType - ): AuthorizationStatus { - switch (ocpp20Status) { - case OCPP20AuthorizationStatusEnumType.Accepted: - return AuthorizationStatus.ACCEPTED - case OCPP20AuthorizationStatusEnumType.Blocked: - return AuthorizationStatus.BLOCKED - case OCPP20AuthorizationStatusEnumType.ConcurrentTx: - return AuthorizationStatus.CONCURRENT_TX - case OCPP20AuthorizationStatusEnumType.Expired: - return AuthorizationStatus.EXPIRED - case OCPP20AuthorizationStatusEnumType.Invalid: - return AuthorizationStatus.INVALID - case OCPP20AuthorizationStatusEnumType.NoCredit: - return AuthorizationStatus.NO_CREDIT - case OCPP20AuthorizationStatusEnumType.NotAllowedTypeEVSE: - return AuthorizationStatus.NOT_ALLOWED_TYPE_EVSE - case OCPP20AuthorizationStatusEnumType.NotAtThisLocation: - return AuthorizationStatus.NOT_AT_THIS_LOCATION - case OCPP20AuthorizationStatusEnumType.NotAtThisTime: - return AuthorizationStatus.NOT_AT_THIS_TIME - case OCPP20AuthorizationStatusEnumType.Unknown: - default: - return AuthorizationStatus.UNKNOWN - } - } - /** * Map OCPP 2.0 IdToken type to unified identifier type * @param ocpp20Type - OCPP 2.0 IdTokenEnumType to convert diff --git a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts index 776050a3..7e717a7c 100644 --- a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts +++ b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts @@ -515,8 +515,18 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { return false } - // For now return true - real implementation would test remote connectivity - return true + // Check if any adapter reports remote availability + for (const adapter of this.adapters.values()) { + try { + if (adapter.isRemoteAvailable()) { + return true + } + } catch { + // Continue checking other adapters + } + } + + return false } public updateCacheEntry ( @@ -696,15 +706,9 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { this.config ) - // Map strategies by their priority to strategy names strategies.forEach(strategy => { - if (strategy.priority === 1) { - this.strategies.set('local', strategy) - } else if (strategy.priority === 2) { - this.strategies.set('remote', strategy) - } else if (strategy.priority === 3) { - this.strategies.set('certificate', strategy) - } + const key = strategy.name.replace('AuthStrategy', '').toLowerCase() + this.strategies.set(key, strategy) }) logger.info( diff --git a/src/charging-station/ocpp/auth/types/AuthTypes.ts b/src/charging-station/ocpp/auth/types/AuthTypes.ts index e71823e2..4d33bb40 100644 --- a/src/charging-station/ocpp/auth/types/AuthTypes.ts +++ b/src/charging-station/ocpp/auth/types/AuthTypes.ts @@ -274,7 +274,6 @@ export interface UnifiedIdentifier { * Authentication error with context */ export class AuthenticationError extends Error { - public override readonly cause?: Error public readonly code: AuthErrorCode public readonly context?: AuthContext public readonly identifier?: string @@ -292,12 +291,11 @@ export class AuthenticationError extends Error { ocppVersion?: OCPPVersion } ) { - super(message) + super(message, { cause: options?.cause }) this.code = code this.identifier = options?.identifier this.context = options?.context this.ocppVersion = options?.ocppVersion - this.cause = options?.cause } } -- 2.43.0