Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/persist installed certificate hashes #284

Merged
merged 4 commits into from
Nov 5, 2024

Conversation

elliot-sabitov
Copy link
Contributor

@elliot-sabitov elliot-sabitov commented Oct 19, 2024

  • feat: adjusting implementation such that a GetInstalledCertificateIds message is triggered after a successful InstallCertificate response is received from the charger. The GetInstalledCertificateIds body that is received from the charger will now also be persisted in the new InstalledCertificate table to keep track of the certificates that are installed on the charger allowing easy access to issuerName and issuerKey hash values. Ensuring that a successful DeleteCertificates response from the charger also triggers the GetInstalledCertificateIds to ensure that our Installed Certificates table data is refreshed.
  • feat: ensuring that we persist issuerKey of the certs that are generated in certificateChain request.

Elliot Sabitov added 2 commits October 18, 2024 15:28
…InstalledCertificateIds message is triggered after a successful InstallCertificate response is received from the charger
…ificateIds handler and adjusting delete handler to re-trigger getInstalledCertificateIds so that we can keep latest state in sync with DB
03_Modules/Certificates/src/module/api.ts Outdated Show resolved Hide resolved
if (certificateHashDataList && certificateHashDataList.length > 0) {
// delete previous hashes for station
try {
await this._installedCertificateRepository.deleteAllByQuery({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot always delete all certificates in a station because user can send the GetInstalledCertificateIdsRequest to get only one or few GetCertificateIdUseEnumTypes, e.g., V2GRootCertificate. How about finding all certificate types inside the certificateHashDataList at first and only delete these types of certificates?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now adjusting module to not automatically trigger GetInstalledCertificateIds, and when GetInstalledCertificateIds payload comes in from the charger, deleting only installed certificate records with matching certificate types.

message.context.stationId,
);
try {
await this._installedCertificateRepository.deleteAllByQuery(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the same reason as in the previous comment, so we cannot simply delete all installed certificates in db.

Another idea I have is that we can add a new optional field certificateType in this CallMessage table. Whenever GetInstalledCertificateIdsRequest is called, we stores the correlation id and certificateTypes in this CallMessage table. Then when getting response, we can retrieve the CallMessage entity to know what types of certificate are requested. An example is here:

await this._module.callMessageRepository.create(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now adjusting module to not automatically trigger GetInstalledCertificateIds, and when GetInstalledCertificateIds payload comes in from the charger, deleting only installed certificate records with matching certificate types.

Elliot Sabitov added 2 commits November 1, 2024 13:12
…ashDataType and HashAlgorithmEnumType to better match spec, and adjusting module to not automatically trigger GetInstalledCertificateIds, and when GetInstalledCertificateIds payload comes in from the charger, deleting only installed certificate records with matching certificate types.
…shes

* rc-1.5.0:
  fix: adjusting concurrent check to only happen after authorizations have been applied to ensure that eventType = Updated does not result in unknown status
  feat: created LatestStatusNotification model and adjusting logic such that when a new status notification is added for a station we also update the LatestStatusNotifications table ensuring that the latest record for the evse/connector is saved in the DB
  updating in order to work with operator ui

# Conflicts:
#	01_Data/src/interfaces/repositories.ts
Copy link
Contributor

@lydiazcheng lydiazcheng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Elliot, why we decide not to send get certificate ids request in the delete cert response? I think that one is still helpful.

@elliot-sabitov
Copy link
Contributor Author

Hi Elliot, why we decide not to send get certificate ids request in the delete cert response? I think that one is still helpful.

For the OCA Certification, it looks like the get cert ids message will be triggered manually. In order to simplify the setup for now, we decided to take this out

@thanaParis thanaParis merged commit 2bc0f49 into rc-1.5.0 Nov 5, 2024
3 checks passed
@thanaParis thanaParis deleted the feature/persist-installed-certificate-hashes branch December 2, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants