From 4b63c423b3eccef4d8ab1701e96581289c93fff2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 31 Mar 2026 22:06:21 +0200 Subject: [PATCH] refactor(auth): make updateCacheEntry API OCPP version-agnostic Decouple updateCacheEntry from OCPP 2.0-specific types by accepting AuthorizationStatus and optional expiryDate instead of pre-built AuthorizationResult and TTL. This centralizes result construction, date conversion, TTL computation, and expired-entry guards in the auth service, eliminating duplicated logic across OCPP 1.6 and 2.0 callers. Also change OCPPAuthAdapter generic default from OCPP20IdTokenType|string to unknown to remove the last OCPP version-specific type from the auth service interface. --- .../ocpp/1.6/OCPP16ServiceUtils.ts | 36 +---- .../ocpp/2.0/OCPP20ServiceUtils.ts | 13 +- .../ocpp/auth/adapters/OCPP20AuthAdapter.ts | 2 +- .../ocpp/auth/interfaces/OCPPAuthService.ts | 19 ++- .../ocpp/auth/services/OCPPAuthServiceImpl.ts | 40 +++--- .../OCPP20ResponseService-CacheUpdate.test.ts | 125 ++++++++++-------- 6 files changed, 115 insertions(+), 120 deletions(-) diff --git a/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts b/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts index d6faa6f8..718e2460 100644 --- a/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts +++ b/src/charging-station/ocpp/1.6/OCPP16ServiceUtils.ts @@ -58,12 +58,7 @@ import { roundTo, truncateId, } from '../../../utils/index.js' -import { - AuthenticationMethod, - type AuthorizationResult, - mapOCPP16Status, - OCPPAuthServiceFactory, -} from '../auth/index.js' +import { mapOCPP16Status, OCPPAuthServiceFactory } from '../auth/index.js' import { buildEmptyMeterValue, buildMeterValue, @@ -863,34 +858,7 @@ export class OCPP16ServiceUtils { ): void { try { const authService = OCPPAuthServiceFactory.getInstance(chargingStation) - const authCache = authService.getAuthCache() - if (authCache == null) { - return - } - const result: AuthorizationResult = { - isOffline: false, - method: AuthenticationMethod.REMOTE_AUTHORIZATION, - status: mapOCPP16Status(idTagInfo.status), - timestamp: new Date(), - } - let ttl: number | undefined - if (idTagInfo.expiryDate != null) { - const expiryDate = convertToDate(idTagInfo.expiryDate) - if (expiryDate != null) { - const ttlSeconds = Math.ceil((expiryDate.getTime() - Date.now()) / 1000) - if (ttlSeconds <= 0) { - logger.debug( - `${chargingStation.logPrefix()} ${moduleName}.updateAuthorizationCache: Skipping expired entry for '${truncateId(idTag)}'` - ) - return - } - ttl = ttlSeconds - } - } - authCache.set(idTag, result, ttl) - logger.debug( - `${chargingStation.logPrefix()} ${moduleName}.updateAuthorizationCache: Updated cache for '${truncateId(idTag)}' status=${result.status}${ttl != null ? `, ttl=${ttl.toString()}s` : ''}` - ) + authService.updateCacheEntry(idTag, mapOCPP16Status(idTagInfo.status), idTagInfo.expiryDate) } catch (error) { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.updateAuthorizationCache: Cache update failed for '${truncateId(idTag)}':`, diff --git a/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts b/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts index 19cdb4bd..fd1e0b2f 100644 --- a/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts +++ b/src/charging-station/ocpp/2.0/OCPP20ServiceUtils.ts @@ -47,7 +47,11 @@ import { validateIdentifierString, } from '../../../utils/index.js' import { buildConfigKey, getConfigurationKey } from '../../ConfigurationKeyUtils.js' -import { mapOCPP20TokenType, OCPPAuthServiceFactory } from '../auth/index.js' +import { + mapOCPP20AuthorizationStatus, + mapOCPP20TokenType, + OCPPAuthServiceFactory, +} from '../auth/index.js' import { buildMeterValue, createPayloadConfigs, @@ -1008,7 +1012,12 @@ export class OCPP20ServiceUtils { ): void { try { const authService = OCPPAuthServiceFactory.getInstance(chargingStation) - authService.updateCacheEntry(idToken.idToken, idTokenInfo, mapOCPP20TokenType(idToken.type)) + authService.updateCacheEntry( + idToken.idToken, + mapOCPP20AuthorizationStatus(idTokenInfo.status), + idTokenInfo.cacheExpiryDateTime, + mapOCPP20TokenType(idToken.type) + ) } catch (error: unknown) { logger.warn( `${chargingStation.logPrefix()} ${moduleName}.updateAuthorizationCache: Error updating auth cache:`, diff --git a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts index 514bc9b7..9be272bb 100644 --- a/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts +++ b/src/charging-station/ocpp/auth/adapters/OCPP20AuthAdapter.ts @@ -44,7 +44,7 @@ const moduleName = 'OCPP20AuthAdapter' * Handles authentication for OCPP 2.0/2.1 charging stations by translating * between auth types and OCPP 2.0 specific types and protocols. */ -export class OCPP20AuthAdapter implements OCPPAuthAdapter { +export class OCPP20AuthAdapter implements OCPPAuthAdapter { readonly ocppVersion = OCPPVersion.VERSION_20 constructor (private readonly chargingStation: ChargingStation) {} diff --git a/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts b/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts index e04e0c13..13edf9b1 100644 --- a/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts +++ b/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts @@ -1,16 +1,11 @@ -import type { - JsonObject, - OCPP20IdTokenInfoType, - OCPP20IdTokenType, - OCPPVersion, -} from '../../../../types/index.js' +import type { JsonObject, OCPPVersion } from '../../../../types/index.js' import type { AuthConfiguration, AuthorizationResult, AuthRequest, Identifier, } from '../types/AuthTypes.js' -import type { IdentifierType } from '../types/AuthTypes.js' +import type { AuthorizationStatus, IdentifierType } from '../types/AuthTypes.js' /** * Authorization cache interface @@ -333,7 +328,7 @@ export interface LocalAuthListManager { * Adapters handle the translation between auth types * and version-specific OCPP types and protocols. */ -export interface OCPPAuthAdapter { +export interface OCPPAuthAdapter { /** * Perform remote authorization using version-specific protocol * @param identifier - Identifier to authorize @@ -441,14 +436,16 @@ export interface OCPPAuthService { testConnectivity(): boolean /** - * Update a cache entry from TransactionEventResponse idTokenInfo (C10.FR.01/04/05) + * Update a cache entry from a CSMS authorization response (C10.FR.01/04/05) * @param identifier - The idToken string to cache - * @param idTokenInfo - The idTokenInfo from the CSMS response + * @param status - The authorization status (mapped from OCPP version-specific types) + * @param expiryDate - Optional expiry date from the CSMS response * @param identifierType - Optional identifier type for cache skip logic (C02.FR.03/C03.FR.02) */ updateCacheEntry( identifier: string, - idTokenInfo: OCPP20IdTokenInfoType, + status: AuthorizationStatus, + expiryDate?: Date | string, identifierType?: IdentifierType ): void diff --git a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts index d257dfe2..93f35497 100644 --- a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts +++ b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts @@ -1,4 +1,3 @@ -import type { OCPP20IdTokenInfoType } from '../../../../types/index.js' import type { OCPPAuthAdapter } from '../interfaces/OCPPAuthService.js' import { OCPPError } from '../../../../exception/index.js' @@ -27,7 +26,6 @@ import { type AuthRequest, type Identifier, IdentifierType, - mapOCPP20AuthorizationStatus, } from '../types/AuthTypes.js' import { AuthConfigValidator } from '../utils/ConfigValidator.js' @@ -534,7 +532,8 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { public updateCacheEntry ( identifier: string, - idTokenInfo: OCPP20IdTokenInfoType, + status: AuthorizationStatus, + expiryDate?: Date | string, identifierType?: IdentifierType ): void { if (!this.config.authorizationCacheEnabled) { @@ -560,32 +559,33 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { return } - const mappedStatus = mapOCPP20AuthorizationStatus(idTokenInfo.status) + let ttl: number | undefined + if (expiryDate != null) { + const parsed = convertToDate(expiryDate) + if (parsed != null) { + const ttlSeconds = Math.ceil((parsed.getTime() - Date.now()) / 1000) + if (ttlSeconds <= 0) { + logger.debug( + `${this.chargingStation.logPrefix()} ${moduleName}.updateCacheEntry: Skipping expired entry for ${truncateId(identifier)}` + ) + return + } + ttl = ttlSeconds + } + } + const effectiveTtl = ttl ?? this.config.authorizationCacheLifetime const result: AuthorizationResult = { isOffline: false, method: AuthenticationMethod.REMOTE_AUTHORIZATION, - status: mappedStatus, + status, timestamp: new Date(), } - let ttl: number | undefined - if (idTokenInfo.cacheExpiryDateTime != null) { - const expiryDate = convertToDate(idTokenInfo.cacheExpiryDateTime) - if (expiryDate != null) { - const expiryMs = expiryDate.getTime() - const ttlSeconds = Math.ceil((expiryMs - Date.now()) / 1000) - if (ttlSeconds > 0) { - ttl = ttlSeconds - } - } - } - ttl ??= this.config.authorizationCacheLifetime - - authCache.set(identifier, result, ttl) + authCache.set(identifier, result, effectiveTtl) logger.debug( - `${this.chargingStation.logPrefix()} ${moduleName}.updateCacheEntry: Updated cache for ${truncateId(identifier)} status=${mappedStatus}, ttl=${ttl != null ? ttl.toString() : 'default'}s` + `${this.chargingStation.logPrefix()} ${moduleName}.updateCacheEntry: Updated cache for ${truncateId(identifier)} status=${status}${effectiveTtl != null ? `, ttl=${effectiveTtl.toString()}s` : ''}` ) } diff --git a/tests/charging-station/ocpp/2.0/OCPP20ResponseService-CacheUpdate.test.ts b/tests/charging-station/ocpp/2.0/OCPP20ResponseService-CacheUpdate.test.ts index b4aea424..1cb6696b 100644 --- a/tests/charging-station/ocpp/2.0/OCPP20ResponseService-CacheUpdate.test.ts +++ b/tests/charging-station/ocpp/2.0/OCPP20ResponseService-CacheUpdate.test.ts @@ -1,7 +1,7 @@ /** - * @file Tests for OCPP20ResponseService cache update on TransactionEventResponse - * @description Unit tests for auth cache auto-update from TransactionEventResponse idTokenInfo - * per OCPP 2.0.1 C10.FR.01/05, C12.FR.06, C02.FR.03, C03.FR.02 + * @file Tests for OCPPAuthServiceImpl.updateCacheEntry + * @description Unit tests for auth cache updates per OCPP 2.0.1 + * C10.FR.01/05, C12.FR.06, C02.FR.03, C03.FR.02 */ import assert from 'node:assert/strict' @@ -17,7 +17,7 @@ import { OCPPAuthServiceFactory, OCPPAuthServiceImpl, } from '../../../../src/charging-station/ocpp/auth/index.js' -import { OCPP20AuthorizationStatusEnumType, OCPPVersion } from '../../../../src/types/index.js' +import { OCPPVersion } from '../../../../src/types/index.js' import { standardCleanup } from '../../../helpers/TestLifecycleHelpers.js' import { createMockChargingStation } from '../../ChargingStationTestUtils.js' @@ -55,13 +55,13 @@ await describe('C10 - TransactionEventResponse Cache Update', async () => { }) await it('C10.FR.05 - should update cache on TransactionEventResponse with Accepted idTokenInfo', () => { - // Arrange - const idTokenInfo = { - status: OCPP20AuthorizationStatusEnumType.Accepted, - } - // Act - authService.updateCacheEntry(TEST_IDENTIFIER, idTokenInfo, IdentifierType.ISO14443) + authService.updateCacheEntry( + TEST_IDENTIFIER, + AuthorizationStatus.ACCEPTED, + undefined, + IdentifierType.ISO14443 + ) // Assert const cached = authCache.get(TEST_IDENTIFIER) @@ -72,43 +72,44 @@ await describe('C10 - TransactionEventResponse Cache Update', async () => { await it('C10.FR.09 - should use cacheExpiryDateTime as TTL when present in idTokenInfo', () => { // Arrange — expiry 600 seconds from now const futureDate = new Date(Date.now() + 600_000) - const idTokenInfo = { - cacheExpiryDateTime: futureDate, - status: OCPP20AuthorizationStatusEnumType.Accepted, - } // Act - authService.updateCacheEntry(TEST_IDENTIFIER, idTokenInfo, IdentifierType.ISO14443) + authService.updateCacheEntry( + TEST_IDENTIFIER, + AuthorizationStatus.ACCEPTED, + futureDate, + IdentifierType.ISO14443 + ) - // Assert — entry is cached (TTL is explicit, checked via presence) + // Assert const cached = authCache.get(TEST_IDENTIFIER) assert.ok(cached != null, 'Cache entry should exist with explicit TTL') assert.strictEqual(cached.status, AuthorizationStatus.ACCEPTED) }) await it('C10.FR.08 - should use AuthCacheLifeTime as TTL when cacheExpiryDateTime absent', () => { - // Arrange — no cacheExpiryDateTime - const idTokenInfo = { - status: OCPP20AuthorizationStatusEnumType.Accepted, - } + // Act — no expiryDate, uses config.authorizationCacheLifetime + authService.updateCacheEntry( + TEST_IDENTIFIER, + AuthorizationStatus.ACCEPTED, + undefined, + IdentifierType.ISO14443 + ) - // Act - authService.updateCacheEntry(TEST_IDENTIFIER, idTokenInfo, IdentifierType.ISO14443) - - // Assert — entry is cached (uses config.authorizationCacheLifetime as default TTL) + // Assert const cached = authCache.get(TEST_IDENTIFIER) assert.ok(cached != null, 'Cache entry should exist with default TTL') assert.strictEqual(cached.status, AuthorizationStatus.ACCEPTED) }) await it('C02.FR.03 - should NOT cache NoAuthorization token type', () => { - // Arrange - const idTokenInfo = { - status: OCPP20AuthorizationStatusEnumType.Accepted, - } - // Act - authService.updateCacheEntry('', idTokenInfo, IdentifierType.NO_AUTHORIZATION) + authService.updateCacheEntry( + '', + AuthorizationStatus.ACCEPTED, + undefined, + IdentifierType.NO_AUTHORIZATION + ) // Assert const cached = authCache.get('') @@ -116,13 +117,13 @@ await describe('C10 - TransactionEventResponse Cache Update', async () => { }) await it('C03.FR.02 - should NOT cache Central token type', () => { - // Arrange - const idTokenInfo = { - status: OCPP20AuthorizationStatusEnumType.Accepted, - } - // Act - authService.updateCacheEntry('CENTRAL_TOKEN_001', idTokenInfo, IdentifierType.CENTRAL) + authService.updateCacheEntry( + 'CENTRAL_TOKEN_001', + AuthorizationStatus.ACCEPTED, + undefined, + IdentifierType.CENTRAL + ) // Assert const cached = authCache.get('CENTRAL_TOKEN_001') @@ -130,17 +131,19 @@ await describe('C10 - TransactionEventResponse Cache Update', async () => { }) await it('C10.FR.01 - should cache non-Accepted status (Blocked, Expired, etc.)', () => { - // Arrange — multiple non-Accepted statuses per C10.FR.01: cache ALL statuses - const blockedInfo = { - status: OCPP20AuthorizationStatusEnumType.Blocked, - } - const expiredInfo = { - status: OCPP20AuthorizationStatusEnumType.Expired, - } - // Act - authService.updateCacheEntry('BLOCKED_TOKEN', blockedInfo, IdentifierType.ISO14443) - authService.updateCacheEntry('EXPIRED_TOKEN', expiredInfo, IdentifierType.ISO14443) + authService.updateCacheEntry( + 'BLOCKED_TOKEN', + AuthorizationStatus.BLOCKED, + undefined, + IdentifierType.ISO14443 + ) + authService.updateCacheEntry( + 'EXPIRED_TOKEN', + AuthorizationStatus.EXPIRED, + undefined, + IdentifierType.ISO14443 + ) // Assert const cachedBlocked = authCache.get('BLOCKED_TOKEN') @@ -152,6 +155,23 @@ await describe('C10 - TransactionEventResponse Cache Update', async () => { assert.strictEqual(cachedExpired.status, AuthorizationStatus.EXPIRED) }) + await it('should skip caching when expiryDate is in the past', () => { + // Arrange + const pastDate = new Date(Date.now() - 60_000) + + // Act + authService.updateCacheEntry( + TEST_IDENTIFIER, + AuthorizationStatus.ACCEPTED, + pastDate, + IdentifierType.ISO14443 + ) + + // Assert + const cached = authCache.get(TEST_IDENTIFIER) + assert.strictEqual(cached, undefined, 'Expired entry must not be cached') + }) + await it('should not update cache when authorizationCacheEnabled is false', () => { // Arrange — create service with cache disabled const { station: disabledStation } = createMockChargingStation({ @@ -166,14 +186,15 @@ await describe('C10 - TransactionEventResponse Cache Update', async () => { disabledService.initialize() disabledService.updateConfiguration({ authorizationCacheEnabled: false }) - const idTokenInfo = { - status: OCPP20AuthorizationStatusEnumType.Accepted, - } - // Act - disabledService.updateCacheEntry(TEST_IDENTIFIER, idTokenInfo, IdentifierType.ISO14443) + disabledService.updateCacheEntry( + TEST_IDENTIFIER, + AuthorizationStatus.ACCEPTED, + undefined, + IdentifierType.ISO14443 + ) - // Assert — cache should not have been written to + // Assert const localStrategy = disabledService.getStrategy('local') as LocalAuthStrategy | undefined const cache = localStrategy?.getAuthCache() const cached = cache?.get(TEST_IDENTIFIER) -- 2.43.0