Skip to content

1942 disable connections/communication from rejected stations#1943

Open
goekay wants to merge 3 commits into
masterfrom
1942-disable-connections-communication-from-rejected-stations
Open

1942 disable connections/communication from rejected stations#1943
goekay wants to merge 3 commits into
masterfrom
1942-disable-connections-communication-from-rejected-stations

Conversation

@goekay
Copy link
Copy Markdown
Member

@goekay goekay commented Jan 22, 2026

PR Type

Bug fix, Enhancement


Description

  • Block websocket connections from stations with "Rejected" registration status

  • Convert registration status from String to RegistrationStatus enum type

  • Update handshake validation logic to check registration status

  • Simplify registration status mapping in ChargePointService


Diagram Walkthrough

flowchart LR
  A["WebSocket Handshake Request"] --> B["Check ChargeBoxId Validity"]
  B --> C["Fetch Registration from DB"]
  C --> D["Validate Registration Status"]
  D --> E{"Status != REJECTED?"}
  E -->|Yes| F["Allow Connection"]
  E -->|No| G["Reject Connection"]
Loading

File Walkthrough

Relevant files
Bug fix
OcppWebSocketHandshakeHandler.java
Add registration status validation to websocket handshake

src/main/java/de/rwth/idsg/steve/ocpp/ws/OcppWebSocketHandshakeHandler.java

  • Import RegistrationStatus enum from OCPP schema
  • Add registration status check to connection validation logic
  • Reject connections if station status is "Rejected"
  • Update error message to clarify rejection reason
+4/-3     
Enhancement
ChargePointRegistration.java
Convert registration status to enum type                                 

src/main/java/de/rwth/idsg/steve/repository/dto/ChargePointRegistration.java

  • Change registrationStatus field type from String to RegistrationStatus
    enum
  • Import RegistrationStatus from OCPP schema
  • Enforce type safety for registration status values
+2/-1     
ChargePointRepositoryImpl.java
Map database status string to enum                                             

src/main/java/de/rwth/idsg/steve/repository/impl/ChargePointRepositoryImpl.java

  • Convert database string value to RegistrationStatus enum using
    fromValue()
  • Update record mapping in getRegistration() method
  • Ensure enum type consistency at data access layer
+1/-1     
ChargePointService.java
Simplify registration status retrieval                                     

src/main/java/de/rwth/idsg/steve/service/ChargePointService.java

  • Simplify getRegistrationStatus() method by removing redundant
    conversion
  • Use method reference to directly access enum value from registration
  • Leverage enum type already present in ChargePointRegistration
+1/-1     
Tests
SecurityProfileValidatorTest.java
Update test to use registration status enum                           

src/test/java/de/rwth/idsg/steve/web/validation/SecurityProfileValidatorTest.java

  • Import RegistrationStatus enum
  • Update test fixture to use RegistrationStatus.ACCEPTED enum value
  • Align test data with new enum-based registration status type
+2/-1     

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Jan 22, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫 #1942
🟢 WebSocket connection attempts from stations with registration_status 'Rejected' should be
rejected
SOAP messages from stations with registration_status 'Rejected' should not be
allowed/processed (verification needed)
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit trail: Connection rejection due to registration status lacks audit logging with user context and
outcome details for security analysis.

Referred Code
// Allow connections, if station is in DB and its registration_status is not Rejected
boolean allowConnection = registration.isPresent() && registration.get().registrationStatus() != RegistrationStatus.REJECTED;

// https://github.com/steve-community/steve/issues/1020
if (!allowConnection) {
    log.error("ChargeBoxId '{}' is not recognized or its registration status is 'Rejected'.", chargeBoxId);
    response.setStatusCode(HttpStatus.NOT_FOUND);
    return false;

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unhandled enum conversion: The RegistrationStatus.fromValue() call lacks error handling for invalid database values
that don't map to enum constants.

Referred Code
RegistrationStatus.fromValue(rec.value3()),
OcppSecurityProfile.fromValue(rec.value4()),

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Status disclosure risk: Error message reveals registration status information which could expose internal system
state to unauthorized parties attempting connections.

Referred Code
log.error("ChargeBoxId '{}' is not recognized or its registration status is 'Rejected'.", chargeBoxId);
response.setStatusCode(HttpStatus.NOT_FOUND);

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented Jan 22, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add null check for registration status

Add a null check before calling RegistrationStatus.fromValue() to prevent a
potential runtime exception if the registration status from the database is
null.

src/main/java/de/rwth/idsg/steve/repository/impl/ChargePointRepositoryImpl.java [93]

-rec.value3(),
+rec.value3() == null ? null : RegistrationStatus.fromValue(rec.value3()),

[Suggestion processed]

Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies that RegistrationStatus.fromValue(rec.value3()) will throw an exception if rec.value3() is null, which is a valid state for the database column. Applying this change prevents a runtime exception and makes the data mapping more robust.

Medium
Learned
best practice
Use safe null comparison order
Suggestion Impact:The commit directly implements the suggestion by changing the comparison order from `registration.get().registrationStatus() != RegistrationStatus.REJECTED` to `RegistrationStatus.REJECTED != registration.get().registrationStatus()` on line 6.

code diff:

-        boolean allowConnection = registration.isPresent() && registration.get().registrationStatus() != RegistrationStatus.REJECTED;
+        boolean allowConnection = registration.isPresent() && RegistrationStatus.REJECTED != registration.get().registrationStatus();

To prevent potential NullPointerException, swap the comparison order to place
the non-null enum constant on the left side of the inequality check.

src/main/java/de/rwth/idsg/steve/ocpp/ws/OcppWebSocketHandshakeHandler.java [93]

-boolean allowConnection = registration.isPresent() && registration.get().registrationStatus() != RegistrationStatus.REJECTED;
+boolean allowConnection = registration.isPresent() && RegistrationStatus.REJECTED != registration.get().registrationStatus();

[Suggestion processed]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - When performing operations that may throw NullPointerException due to null values, add explicit null checks before method calls or swap the order of comparison to use a guaranteed non-null object as the caller.

Low
  • Update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disable connections/communication from "Rejected" stations

1 participant