]> Piment Noir Git Repositories - e-mobility-charging-stations-simulator.git/commitdiff
fix(auth): address audit findings in OCPP auth abstraction layer
authorJérôme Benoit <jerome.benoit@sap.com>
Sat, 21 Mar 2026 17:39:09 +0000 (18:39 +0100)
committerJérôme Benoit <jerome.benoit@sap.com>
Sat, 21 Mar 2026 17:39:09 +0000 (18:39 +0100)
- 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

src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts
src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts
src/charging-station/ocpp/auth/types/AuthTypes.ts

index 27ce77d6e6d34e0d8213fe501f98ab10eb22f890..fa98092194286dd2841e0ed6b8e118f53e5bd909 100644 (file)
@@ -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
index 776050a3b18ced5d9dc6198a54308a0b6195c109..7e717a7cf95a50a1d4bbc08bce4ea062c9a1e945 100644 (file)
@@ -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(
index e71823e27cca542e357753982c3e80a074310f63..4d33bb401ce49d3303f705ebbeeb342b0c843f4a 100644 (file)
@@ -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
   }
 }