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

Fix for the default password max length (as per the 1.6 spec) and other minor changes #345

Merged
merged 5 commits into from
Apr 28, 2024

Conversation

rpseng
Copy link

@rpseng rpseng commented Apr 24, 2024

  • As per the 1.6 spec the authorization key should be 20 bytes long, in hex format that is 40 chars
  • gson library updated due to a reported vulnerability
  • Keep a single instance of JSONConfiguration.java so users can actually configure it

@rpseng
Copy link
Author

rpseng commented Apr 24, 2024

I also found that the authentication key (aka password) should be handled as a hex string and not as a byte array.

These changes I'm sending were tested with the following simulator: https://github.com/matth-x/MicroOcppSimulator

@TVolden
Copy link
Member

TVolden commented Apr 28, 2024

This should fix #304

@TVolden TVolden merged commit 088b78c into ChargeTimeEU:master Apr 28, 2024
3 checks passed
@TVolden
Copy link
Member

TVolden commented Apr 28, 2024

Hi @rpseng,

Thanks for your contribution. Everything look good, so I merged the PR.

@EltonSaraci99
Copy link

@TVolden that's a very useful contribution, we were also facing issues because of the password field. When do you plan to publish the newest version on maven repository?

@rpseng
Copy link
Author

rpseng commented Apr 30, 2024

Hello @EltonSaraci99 you can use jitpack to get any commit integrated with a gradle or maven build. For this case, please check:
https://jitpack.io/#ChargeTimeEU/Java-OCA-OCPP/088b78cd3a

@EltonSaraci99
Copy link

EltonSaraci99 commented Apr 30, 2024

hello @rpseng thanks a lot, appreciate it! Just added:

<dependency>
			<groupId>com.github.ChargeTimeEU.Java-OCA-OCPP</groupId>
			<artifactId>OCPP-J</artifactId>
			<version>088b78cd3a</version>
		</dependency>
    </dependencies>
	<repositories>
		<repository>
			<id>jitpack.io</id>
			<url>https://jitpack.io</url>
		</repository>
	</repositories> 

and now I have a password string. The problem is that if I remove the dependency below that I got from maven I get a lot of import errors.


<dependency>
			<groupId>eu.chargetime.ocpp</groupId>
			<artifactId>v1_6</artifactId>
			<version>1.1.0</version>
		</dependency>

any idea what I can possibly be doing wrong?
Screenshot 2024-04-30 at 20 08 57

and if I add both the dependencies it requires me to put again a byte array password in the Override method.

@rpseng
Copy link
Author

rpseng commented Apr 30, 2024

I'm not a maven expert, but I believe the repositories group should come before the dependencies one.

@robert-s-ubi
Copy link
Contributor

I also found that the authentication key (aka password) should be handled as a hex string and not as a byte array.

Unfortunately, that is WRONG. The authorization key is only CONFIGURED as a hex string in an OCPP 1.6 ChangeConfiguration key, but the charge point MUST treat it as a binary byte array. Please re-read the OCPP-J 1.6 specification page 16:

Example
If we have a charge point with:
• charge point identity "AL1000"
• authorization key 0001020304050607FFFFFFFFFFFFFFFFFFFFFFFF the HTTP authorization header should be:
Authorization: Basic QUwxMDAwOgABAgMEBQYH////////////////

If you base64-decode the end of the last line, you see that the BYTES are in there, not the hex string.

This entire change is wrong and only breaks things. Please revert ASAP.

public static JSONConfiguration get() {
return new JSONConfiguration();
return instance;
Copy link
Contributor

Choose a reason for hiding this comment

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

API breaking change. Instead of getting a default configuration, you now get whatever configuration the last library user left behind.

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.

4 participants