Skip to content

Commit 036e217

Browse files
goekaygoekay
committed
refactor: move password validation to ChargePointService
* and also make getRegistrationStatus return the password additionally, in order prevent another DB lookup later
1 parent eb13078 commit 036e217

File tree

6 files changed

+53
-62
lines changed

6 files changed

+53
-62
lines changed

‎src/main/java/de/rwth/idsg/steve/repository/ChargePointRepository.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@
2020

2121
import de.rwth.idsg.steve.ocpp.OcppProtocol;
2222
import de.rwth.idsg.steve.repository.dto.ChargePoint;
23+
import de.rwth.idsg.steve.repository.dto.ChargePointRegistration;
2324
import de.rwth.idsg.steve.repository.dto.ChargePointSelect;
2425
import de.rwth.idsg.steve.repository.dto.ConnectorStatus;
2526
import de.rwth.idsg.steve.web.dto.ChargePointForm;
2627
import de.rwth.idsg.steve.web.dto.ChargePointQueryForm;
2728
import de.rwth.idsg.steve.web.dto.ConnectorStatusForm;
2829
import org.jetbrains.annotations.Nullable;
2930

30-
import java.util.Collections;
3131
import java.util.List;
3232
import java.util.Map;
3333
import java.util.Optional;
@@ -37,7 +37,7 @@
3737
* @since 19.08.2014
3838
*/
3939
public interface ChargePointRepository {
40-
Optional<String> getRegistrationStatus(String chargeBoxId);
40+
Optional<ChargePointRegistration> getRegistration(String chargeBoxId);
4141

4242
List<ChargePointSelect> getChargePointSelect(OcppProtocol protocol, List<String> inStatusFilter, List<String> chargeBoxIdFilter);
4343

@@ -55,7 +55,4 @@ public interface ChargePointRepository {
5555
int addChargePoint(ChargePointForm form);
5656
void updateChargePoint(ChargePointForm form);
5757
void deleteChargePoint(int chargeBoxPk);
58-
59-
boolean isRegistered(String chargeBoxId);
60-
boolean validatePassword(String chargeBoxId, String password);
6158
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package de.rwth.idsg.steve.repository.dto;
2+
3+
public record ChargePointRegistration(
4+
String registrationStatus,
5+
String hashedAuthPassword
6+
) {
7+
}

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

Lines changed: 11 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import de.rwth.idsg.steve.repository.AddressRepository;
2424
import de.rwth.idsg.steve.repository.ChargePointRepository;
2525
import de.rwth.idsg.steve.repository.dto.ChargePoint;
26+
import de.rwth.idsg.steve.repository.dto.ChargePointRegistration;
2627
import de.rwth.idsg.steve.repository.dto.ChargePointSelect;
2728
import de.rwth.idsg.steve.repository.dto.ConnectorStatus;
2829
import de.rwth.idsg.steve.utils.DateTimeUtils;
@@ -46,7 +47,6 @@
4647
import org.jooq.Table;
4748
import org.jooq.exception.DataAccessException;
4849
import org.jooq.impl.DSL;
49-
import org.springframework.security.crypto.password.PasswordEncoder;
5050
import org.springframework.stereotype.Repository;
5151
import org.springframework.util.CollectionUtils;
5252

@@ -72,16 +72,18 @@ public class ChargePointRepositoryImpl implements ChargePointRepository {
7272

7373
private final DSLContext ctx;
7474
private final AddressRepository addressRepository;
75-
private final PasswordEncoder passwordEncoder;
7675

7776
@Override
78-
public Optional<String> getRegistrationStatus(String chargeBoxId) {
79-
String status = ctx.select(CHARGE_BOX.REGISTRATION_STATUS)
80-
.from(CHARGE_BOX)
81-
.where(CHARGE_BOX.CHARGE_BOX_ID.eq(chargeBoxId))
82-
.fetchOne(CHARGE_BOX.REGISTRATION_STATUS);
83-
84-
return Optional.ofNullable(status);
77+
public Optional<ChargePointRegistration> getRegistration(String chargeBoxId) {
78+
var status = ctx.select(CHARGE_BOX.REGISTRATION_STATUS, CHARGE_BOX.AUTH_PASSWORD)
79+
.from(CHARGE_BOX)
80+
.where(CHARGE_BOX.CHARGE_BOX_ID.eq(chargeBoxId))
81+
.fetch()
82+
.map(rec -> new ChargePointRegistration(rec.value1(), rec.value2()));
83+
84+
return status.isEmpty()
85+
? Optional.empty()
86+
: Optional.ofNullable(status.getFirst());
8587
}
8688

8789
@Override
@@ -376,41 +378,4 @@ private void deleteChargePointInternal(DSLContext ctx, int chargeBoxPk) {
376378
.where(CHARGE_BOX.CHARGE_BOX_PK.equal(chargeBoxPk))
377379
.execute();
378380
}
379-
380-
@Override
381-
public boolean isRegistered(String chargeBoxId) {
382-
return ctx.fetchExists(
383-
ctx.selectFrom(CHARGE_BOX)
384-
.where(CHARGE_BOX.CHARGE_BOX_ID.eq(chargeBoxId))
385-
);
386-
}
387-
388-
@Override
389-
public boolean validatePassword(String chargeBoxId, String password) {
390-
if (password == null || password.isEmpty()) {
391-
log.warn("Empty password provided for charge point '{}'", chargeBoxId);
392-
return false;
393-
}
394-
395-
var storedHashedPassword = ctx.select(CHARGE_BOX.AUTH_PASSWORD)
396-
.from(CHARGE_BOX)
397-
.where(CHARGE_BOX.CHARGE_BOX_ID.eq(chargeBoxId))
398-
.fetchOne(CHARGE_BOX.AUTH_PASSWORD);
399-
400-
if (storedHashedPassword == null || storedHashedPassword.isEmpty()) {
401-
log.warn("No password configured for charge point '{}' - authentication disabled", chargeBoxId);
402-
return true;
403-
}
404-
405-
try {
406-
var matches = passwordEncoder.matches(password, storedHashedPassword);
407-
if (!matches) {
408-
log.warn("Invalid password attempt for charge point '{}'", chargeBoxId);
409-
}
410-
return matches;
411-
} catch (IllegalArgumentException e) {
412-
log.error("Invalid BCrypt hash stored for charge point '{}'. Hash might be corrupted.", chargeBoxId, e);
413-
return false;
414-
}
415-
}
416381
}

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@
4343
import lombok.extern.slf4j.Slf4j;
4444
import ocpp.cs._2015._10.RegistrationStatus;
4545
import org.joda.time.DateTime;
46+
import org.springframework.security.crypto.password.PasswordEncoder;
4647
import org.springframework.stereotype.Service;
4748
import org.springframework.util.CollectionUtils;
49+
import org.springframework.util.StringUtils;
4850

4951
import java.util.ArrayList;
5052
import java.util.Arrays;
@@ -74,6 +76,7 @@ public class ChargePointService {
7476
private final ChargePointRepository chargePointRepository;
7577
private final GenericRepository genericRepository;
7678
private final SteveProperties steveProperties;
79+
private final PasswordEncoder passwordEncoder;
7780

7881
// SOAP-based charge points are stored in DB with an endpoint address.
7982
// But, for WebSocket-based charge points, the active sessions are stored in memory.
@@ -127,6 +130,31 @@ public void removeUnknown(List<String> chargeBoxIdList) {
127130
// Registration status
128131
// -------------------------------------------------------------------------
129132

133+
public boolean validatePassword(String chargeBoxId, String passwordFromHeaders, String storedHashedPassword) {
134+
// if no password in DB, this station needs no validation
135+
if (StringUtils.isEmpty(storedHashedPassword)) {
136+
log.info("No password configured for charge point '{}' - authentication disabled", chargeBoxId);
137+
return true;
138+
}
139+
140+
// there is a password in DB, but the station provided no password
141+
if (StringUtils.isEmpty(passwordFromHeaders)) {
142+
log.warn("Empty password provided for charge point '{}'", chargeBoxId);
143+
return false;
144+
}
145+
146+
try {
147+
var matches = passwordEncoder.matches(passwordFromHeaders, storedHashedPassword);
148+
if (!matches) {
149+
log.warn("Invalid password attempt for charge point '{}'", chargeBoxId);
150+
}
151+
return matches;
152+
} catch (Exception e) {
153+
log.error("Exception while checking password for charge point '{}'", chargeBoxId, e);
154+
return false;
155+
}
156+
}
157+
130158
public Optional<RegistrationStatus> getRegistrationStatus(String chargeBoxId) {
131159
Lock l = isRegisteredLocks.get(chargeBoxId);
132160
l.lock();
@@ -143,10 +171,10 @@ public Optional<RegistrationStatus> getRegistrationStatus(String chargeBoxId) {
143171

144172
private Optional<RegistrationStatus> getRegistrationStatusInternal(String chargeBoxId) {
145173
// 1. exit if already registered
146-
Optional<String> status = chargePointRepository.getRegistrationStatus(chargeBoxId);
174+
var status = chargePointRepository.getRegistration(chargeBoxId);
147175
if (status.isPresent()) {
148176
try {
149-
return Optional.ofNullable(RegistrationStatus.fromValue(status.get()));
177+
return Optional.ofNullable(RegistrationStatus.fromValue(status.get().registrationStatus()));
150178
} catch (Exception e) {
151179
// in cases where the database entry (string) is altered, and therefore cannot be converted to enum
152180
log.error("Exception happened", e);

‎src/test/java/de/rwth/idsg/steve/issues/Issue1219.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@
4242
import org.jooq.impl.DataSourceConnectionProvider;
4343
import org.jooq.impl.DefaultConfiguration;
4444
import org.jooq.tools.jdbc.SingleConnectionDataSource;
45-
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
46-
import org.springframework.security.crypto.password.PasswordEncoder;
4745

4846
import java.sql.Connection;
4947
import java.sql.DriverManager;
@@ -61,7 +59,6 @@ public class Issue1219 {
6159
private static final String url = "jdbc:mysql://localhost:3306/stevedb";
6260
private static final String userName = "steve";
6361
private static final String password = "changeme";
64-
private static final PasswordEncoder PASSWORD_ENCODER = new BCryptPasswordEncoder();
6562

6663
private final DSLContext ctx;
6764

@@ -155,7 +152,7 @@ private List<Integer> insertStartTransactions(int count, List<String> ocppTags,
155152
}
156153

157154
private List<String> insertChargeBoxes(int count) {
158-
var repository = new ChargePointRepositoryImpl(ctx, new AddressRepositoryImpl(), PASSWORD_ENCODER);
155+
var repository = new ChargePointRepositoryImpl(ctx, new AddressRepositoryImpl());
159156

160157
List<String> ids = IntStream.range(0, count).mapToObj(val -> UUID.randomUUID().toString()).collect(Collectors.toList());
161158
repository.addChargePointList(ids);

‎src/test/java/de/rwth/idsg/steve/utils/__DatabasePreparer__.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,6 @@
4545
import org.jooq.Schema;
4646
import org.jooq.Table;
4747
import org.jooq.impl.DSL;
48-
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
49-
import org.springframework.security.crypto.password.PasswordEncoder;
5048

5149
import java.util.Arrays;
5250
import java.util.List;
@@ -74,7 +72,6 @@ public class __DatabasePreparer__ {
7472
private static final String REGISTERED_CHARGE_BOX_ID = "charge_box_2aa6a783d47d";
7573
private static final String REGISTERED_CHARGE_BOX_ID_2 = "charge_box_2aa6a783d47d_2";
7674
private static final String REGISTERED_OCPP_TAG = "id_tag_2aa6a783d47d";
77-
private static final PasswordEncoder PASSWORD_ENCODER = new BCryptPasswordEncoder();
7875

7976
private final DSLContext dslContext;
8077

@@ -129,7 +126,7 @@ public List<Reservation> getReservations() {
129126
}
130127

131128
public List<ConnectorStatus> getChargePointConnectorStatus() {
132-
ChargePointRepositoryImpl impl = new ChargePointRepositoryImpl(dslContext, new AddressRepositoryImpl(), PASSWORD_ENCODER);
129+
ChargePointRepositoryImpl impl = new ChargePointRepositoryImpl(dslContext, new AddressRepositoryImpl());
133130
return impl.getChargePointConnectorStatus(null);
134131
}
135132

@@ -144,7 +141,7 @@ public OcppTagActivityRecord getOcppTagRecord(String idTag) {
144141
}
145142

146143
public ChargePoint.Details getCBDetails(String chargeboxID) {
147-
ChargePointRepositoryImpl impl = new ChargePointRepositoryImpl(dslContext, new AddressRepositoryImpl(), PASSWORD_ENCODER);
144+
ChargePointRepositoryImpl impl = new ChargePointRepositoryImpl(dslContext, new AddressRepositoryImpl());
148145
Map<String, Integer> pkMap = impl.getChargeBoxIdPkPair(Arrays.asList(chargeboxID));
149146
int pk = pkMap.get(chargeboxID);
150147
return impl.getDetails(pk);

0 commit comments

Comments
 (0)