Add OCPP 1.6 security operations by juherr · Pull Request #97 · IZIVIA/ocpp-toolkit

juherr · 2026-07-07T12:55:02Z

Summary

Add optional ocpp-1-6-security module for OCPP 1.6-J Security Whitepaper operations.
Register transport-backed security send/receive operations without changing existing core operation interfaces.
Add official OCPP 1.6 security JSON schemas and adapter support for generic security calls.

Details

Align core16 security models with the official schemas where required.
Keep SOAP security support out of scope; the whitepaper support is JSON/WebSocket oriented.
Group ActionOcpp values by version and direction for readability.

Validation

git diff --check
./gradlew :ocpp-1-6-security:test :ocpp-1-6-json:test :ocpp-1-6-api-adapter:test
./gradlew build

Closes #90

pbourseau

Thanks for this — clean, well-structured addition, SOAP correctly left out of scope, and the security module comes with real unit tests.

Requesting changes mainly for two functional bugs (lazy init dropping inbound handlers and the serialNumber schema/model mismatch), plus a robustness concern around the Enum.valueOf(status.name) conversions. Details are in the inline comments; the rest are polish.

One coverage gap: SecurityMapper itself is untested, which is exactly where most of the risk lives (the enum conversions and the signingCertificate/signature null-guards). Round-trip tests per genToCore*/coreToGen* pair would be valuable.

pbourseau · 2026-07-20T14:43:19Z


    private val operations: ChargePointOperations = ChargePointOperations
        .newChargePointOperations(chargingStationId, transport, Ocpp16CSApiAdapter(csApi, transactionIds))
+    private val securityOperations: SecurityChargePointOperations by lazy {


Functional bug: by lazy can drop inbound CSMS→CP security requests.

RealSecurityChargePointOperations.init { … } is what registers the CertificateSigned / DeleteCertificate / GetLog / InstallCertificate / SignedUpdateFirmware receive handlers. Because this field is by lazy, that init block doesn't run until the CP first sends a security message (logStatusNotification / securityEventNotification / signCertificate). If the CSMS sends any of those CSMS→CP requests beforehand, no handler is registered and the message is dropped.

Note operations just above is deliberately eager for exactly this reason — securityOperations should be eager too, or be initialized in connect().

Fixed. securityOperations is now initialized eagerly, like the core operations, so the CSMS->CP security receive handlers are registered when the adapter is constructed. I added an adapter test that verifies the security receive handlers are registered on construction.

pbourseau · 2026-07-20T14:43:20Z

+    fun genToCoreReq(req: GenSignCertificateReq) = CoreSignCertificateReq(req.csr)
+
+    fun coreToGenResp(resp: CoreSignCertificateResp) =
+        GenSignCertificateResp(GenGenericStatusEnumType.valueOf(resp.status.name))


Enum.valueOf(status.name) is fragile: it throws IllegalArgumentException at runtime the moment a core and generic enum constant name diverges. This file already uses exhaustive when elsewhere (DeleteCertificate, GetInstalledCertificateIds), which gives a compile-time guarantee instead of a production crash. Recommend converting all the valueOf(...name) conversions (here, CertificateSigned, GetLog, InstallCertificate, and the UpdateFirmware status at line 157) to exhaustive when.

Fixed. The security mapper now uses exhaustive when mappings instead of Enum.valueOf(status.name) for the OCPP 1.6 security conversions called out here, including CertificateSigned, GetLog, InstallCertificate, and signed firmware update statuses. I also added targeted mapper tests for the corrected mappings.

pbourseau · 2026-07-20T14:43:20Z

+    fun genToCoreReq(req: GenLogStatusNotificationReq) =
+        CoreLogStatusNotificationReq(
+            status = when (req.status) {
+                GenUploadLogStatusEnumType.AcceptedCanceled -> CoreUploadLogStatusEnumType.UploadFailure


Lossy mapping: AcceptedCanceled → UploadFailure. Semantically a cancel is being reported as a failure. If intentional, please add a comment explaining why; otherwise this will surface as a misleading upload status.

Fixed. This mapping is intentional because the OCPP 1.6 security whitepaper LogStatusNotification enum has no canceled status. I added an explicit comment documenting that AcceptedCanceled falls back to the closest non-success terminal status, UploadFailure, and replaced the remaining status mapping with an exhaustive when.

pbourseau · 2026-07-20T14:43:20Z

+        CoreGetInstalledCertificateIdsResp(
+            status = when (resp.status) {
+                GenGetInstalledCertificateStatusEnumType.Accepted -> CoreGetInstalledCertificateStatusEnumType.Accepted
+                GenGetInstalledCertificateStatusEnumType.NotFound -> CoreGetInstalledCertificateStatusEnumType.Rejected


NotFound → Rejected looks wrong. The 1.6 whitepaper GetInstalledCertificateIds status set is Accepted/NotFound; if core16 has a NotFound value it should map straight through rather than becoming Rejected.

Fixed. Core 1.6 GetInstalledCertificateStatusEnumType now has NotFound, and the mapper now maps generic NotFound directly to core NotFound. Added a targeted mapper assertion for this case.

pbourseau · 2026-07-20T14:43:20Z

+            hashAlgorithm = GenHashAlgorithmEnumType.valueOf(hash.hashAlgorithm.name),
+            issuerNameHash = hash.issuerNameHash,
+            issuerKeyHash = hash.issuerKeyHash,
+            serialNumber = hash.serialNumber ?: ""


serialNumber ?: "" sends an empty string, which is not a valid certificate serial. This is a symptom of the model/schema mismatch: CertificateHashDataType.serialNumber is now nullable, but DeleteCertificateRequest.json (and the GetInstalledCertificateIds response schema) mark serialNumber as required. Either keep it required in the model or make the schema optional so the two agree — a null serialNumber currently serializes to JSON that fails its own schema.

Fixed. CertificateHashDataType.serialNumber is required again in the core model to match the official security schemas and the generic model. The mapper no longer sends serialNumber ?: ""; it passes the required serial number through directly.

pbourseau · 2026-07-20T14:43:20Z

    val issuerNameHash: String,
-    val issuerKeyHash: String
+    val issuerKeyHash: String,
+    val serialNumber: String? = null


Making serialNumber optional here conflicts with DeleteCertificateRequest.json, which lists serialNumber in required for CertificateHashDataType. A request built with serialNumber = null will serialize to JSON that fails schema validation. Please align the model and the schema (see related comment in SecurityMapper).

Fixed. The model and schema are aligned now: serialNumber is required in CertificateHashDataType, matching DeleteCertificateRequest.json and GetInstalledCertificateIdsResponse.json.

pbourseau · 2026-07-20T14:43:20Z

+        meta: RequestMetadata,
+        req: ExtendedTriggerMessageReq
+    ): OperationExecution<ExtendedTriggerMessageReq, ExtendedTriggerMessageResp> {
+        throw IllegalStateException("ExtendedTriggerMessage is not available through the generic API")


A receive handler for ExtendedTriggerMessage is registered in RealSecurityChargePointOperations, but this adapter unconditionally throws for it. So a compliant CSMS sending ExtendedTriggerMessage makes the CP error rather than reject gracefully. Fine as a known gap, but please reference a TODO/issue rather than a bare throw.

Fixed. I kept the current behavior as a known generic API gap and added a TODO referencing #90 before the throw: ExtendedTriggerMessage is supported by the security transport module, but the generic API does not expose a callback for it yet.

Implemented. ExtendedTriggerMessage now delegates to the existing generic triggerMessage API with explicit OCPP 1.6 security mappings, including SignChargePointCertificate -> SignChargingStationCertificate, and maps the generic trigger response back to the OCPP 1.6 extended response. Added mapper and adapter coverage, and the targeted checks plus full build pass.

- extract a shared `ok(...)` helper in Ocpp16SecurityCSApiAdapter to drop the repeated OperationExecution/ExecutionMetadata boilerplate across the 7 overrides - hoist the acceptConnection(ocppId) call out of the server scan in RealSecurityCSMSOperations.getTransport so the ChargingStationConfig is built once per send instead of once per server Behavior-preserving; existing tests unchanged.

The server-side RealSecurityCSMSOperations was implemented and unit-tested but never reachable through the public factory: CSMS only built RealCSMSOperations16 for core operations, so a CSMS could not send CertificateSigned/DeleteCertificate/ GetLog/... nor receive the CP-initiated security notifications. - add ocpp-1-6-security as an api dependency of the toolkit module (it was only an implementation dep of the adapter, hence not visible here) - register a SecurityChargePointOperations callback into a new OcppSecurityCsApiType(OCPP_1_6) entry, mirroring the core ChargePointOperations16 -> RealCSMSOperations16 flow - expose CSMS.getSecurityCSApi16(): SecurityCSMSOperations csmsOcppServer needs no signature change: SecurityChargePointOperations already extends CSMSCallbacks, so callers add it to the existing csmsApiCallbacks list.

Code-review follow-up. A single callback object implementing BOTH ChargePointOperations16 and SecurityChargePointOperations was silently reduced to core-only: the sequential `when` + `associate` matched the first branch and dropped the security facet, later surfacing as a misleading "No 1.6 security api is available". Switch to `flatMap` so the security facet is registered as an independent entry regardless of whether the same object also serves as a core callback. Tests: - CSMSSecurityWiringTest: fix the mislabeled negative test and its dead `as CSMSOperations16` cast; add the genuinely-missing case (getSecurityCSApi16 throws when no security callback is registered) and a combined core+security callback test that pins the fix above. - MapperTest: add red-path coverage for the signed-firmware null guard (missing signature -> IllegalArgumentException) and a multi-element installed certificate chain mapping.

sonarqubecloud · 2026-07-20T16:55:57Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

pbourseau requested changes Jul 20, 2026

View reviewed changes

juherr force-pushed the ocpp-16-security-messages branch from d28bfe8 to a58d828 Compare July 20, 2026 15:27

feat: add OCPP 1.6 security operations

b8d8b08

juherr force-pushed the ocpp-16-security-messages branch from a58d828 to b8d8b08 Compare July 20, 2026 15:48

juherr added 3 commits July 20, 2026 18:36

juherr force-pushed the ocpp-16-security-messages branch from 669d3d2 to 3eb93f3 Compare July 20, 2026 16:55

juherr requested a review from pbourseau July 20, 2026 18:45

Uh oh!

Conversation

juherr commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Details

Validation

Uh oh!

pbourseau left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jul 20, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

juherr commented Jul 7, 2026 •

edited

Loading