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.

Sign up for GitHub

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

Jump to bottom

validate CSMS-URL: opt. schema must fit with security-profile; chargepoint-ID deprecated in URL #201

Merged
merged 52 commits into from
Nov 13, 2023
Merged

validate CSMS-URL: opt. schema must fit with security-profile; chargepoint-ID deprecated in URL #201

merged 52 commits into from
Nov 13, 2023

Conversation

Dominik-K
Copy link
Contributor

No description provided.

@Pietfried
Copy link
Contributor

So far it was required to include the chargepoint_id in the configured CentralSystemURI (v16) or ocppCsmsUrl (v201, as part of the NetworkConnectionProfiles variable). This is indeed redundant so I like this initiative!

For v16 we can append the internal configuration key ChargePointId (like implemented in this PR) and for v201 we need to use the variable "Identity" from the "SecurityCtrlr", so this line is currently not correct:
https://github.com/EVerest/libocpp/blob/main/lib/ocpp/v201/charge_point.cpp#L585

Please add this to the PR.

Pietfried
Pietfried requested changes Sep 29, 2023
View reviewed changes
include/ocpp/helpers/uri.hpp Outdated Show resolved Hide resolved
include/ocpp/helpers/uri.hpp Outdated Show resolved Hide resolved
lib/ocpp/common/websocket/websocket_tls.cpp Outdated Show resolved Hide resolved
@Dominik-K
Copy link
Contributor Author

Next I'll use a Uri::set_path to allow backwards-compatibility with current URLs which contain the CP-ID as path.

@Dominik-K Dominik-K changed the title Fix/websockets/CP-ID deprecate chargepoint-ID in CSMS-URL config-field Oct 18, 2023
@Dominik-K Dominik-K self-assigned this Oct 18, 2023
@Dominik-K Dominik-K marked this pull request as ready for review October 18, 2023 13:49
@Dominik-K Dominik-K requested a review from Pietfried October 18, 2023 13:50
@Dominik-K
Copy link
Contributor Author

@Pietfried It's ready now. I'll do the returning an error/std::expected in a follow-up PR.

@Dominik-K
WIP: check CP-ID against path-base instead of full path
a919540 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
remove CP-ID from path if last segment
3bd6d62 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
WIP: check OCPP1.6 with security-profile=1
8b8e269 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
remove optional last slash
fbfcae7 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
Merge branch 'main' into fix/websockets/CP-ID
39ddcc4
@Dominik-K
fix correct path splitting
bf9bdc6 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
fix fallthrough in switch
f5f0091 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
1.6: connection to SteVe working
75a1705 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
path normalization
fe08ff2 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K
Merge branch 'main' into fix/websockets/CP-ID
30e81e2
@Dominik-K
(merge) fixes
3af51e3 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K Dominik-K force-pushed the fix/websockets/CP-ID branch from 6ed8c58 to 3af51e3 Compare November 7, 2023 14:43
@Dominik-K
fix doubled last_segment
2bfa19e 
Signed-off-by: Dominik K <[email protected]>
@Dominik-K Dominik-K changed the title deprecate chargepoint-ID in CSMS-URL config-field validate CSMS-URL: opt. schema must fit with security-profile; chargepoint-ID deprecated in URL Nov 8, 2023
@Dominik-K Dominik-K requested a review from Pietfried November 8, 2023 21:16
Pietfried
Pietfried requested changes Nov 9, 2023
View reviewed changes
include/ocpp/common/websocket/websocket_uri.hpp Outdated Show resolved Hide resolved
lib/ocpp/v16/charge_point_impl.cpp Outdated Show resolved Hide resolved
lib/ocpp/v201/charge_point.cpp
auto security_profile = network_connection_profile.securityProfile;
auto uri = Uri::parse_and_validate(
network_connection_profile.ocppCsmsUrl.get(),
this->device_model->get_value<std::string>(ControllerComponentVariables::SecurityCtrlrIdentity),
Copy link
Contributor

Choose a reason for hiding this comment

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

Identity is defined as not required in OCPP2.0.1 spec (so it should be requested from storage with get_optional_value). This seems to be a problem with the spec. Lets define Identity as requried in config/ and keep requesting storage with get_value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there sth. more needed except adding Identity to required: in SecurityCtrlr.json?

lib/ocpp/v201/charge_point.cpp Outdated Show resolved Hide resolved
include/ocpp/common/types.hpp Outdated Show resolved Hide resolved
lib/ocpp/common/websocket/websocket_plain.cpp Outdated Show resolved Hide resolved
@Dominik-K Dominik-K requested a review from Pietfried November 9, 2023 15:14
@Dominik-K Dominik-K merged commit 271e025 into main Nov 13, 2023
2 of 3 checks passed
@Dominik-K Dominik-K deleted the fix/websockets/CP-ID branch November 13, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maaikez maaikez maaikez left review comments

@Pietfried Pietfried Pietfried approved these changes

Assignees

@Dominik-K Dominik-K

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Dominik-K @Pietfried @maaikez