aboutsummaryrefslogtreecommitdiff
path: root/eidas_modules/authmodule-eIDAS-v2/src/main
diff options
context:
space:
mode:
authorChristian Kollmann <christian.kollmann@a-sit.at>2020-12-18 10:22:59 +0100
committerAlexander Marsalek <amarsalek@iaik.tugraz.at>2021-01-04 14:17:18 +0100
commit06d395c2102cf6a8bb961ac6bb2113c12eda9651 (patch)
tree5a394fe5eb7823f448f2e4ae7822069ac88978de /eidas_modules/authmodule-eIDAS-v2/src/main
parent0e6e6b5da3382d8f733f8f0ed9caee725078badf (diff)
downloadNational_eIDAS_Gateway-06d395c2102cf6a8bb961ac6bb2113c12eda9651.tar.gz
National_eIDAS_Gateway-06d395c2102cf6a8bb961ac6bb2113c12eda9651.tar.bz2
National_eIDAS_Gateway-06d395c2102cf6a8bb961ac6bb2113c12eda9651.zip
Review code
Diffstat (limited to 'eidas_modules/authmodule-eIDAS-v2/src/main')
-rw-r--r--eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java1
-rw-r--r--eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java1
-rw-r--r--eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java1
-rw-r--r--eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java11
-rw-r--r--eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java1
5 files changed, 14 insertions, 1 deletions
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java
index 0c8a2f59..978be4d0 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/ernb/DummyErnbClient.java
@@ -11,6 +11,7 @@ public class DummyErnbClient implements IErnbClient {
@Override
public ArrayList<RegisterResult> searchWithPersonIdentifer(String personIdentifer) {
+ // NOTE: Are we using Mockito or these fixed strings for testing, why are those defined here?
switch (personIdentifer) {
case "a12345":
case "a12345-":
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java
index 51d6952f..b5e8551b 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/DeSpecificDetailSearchProcessor.java
@@ -10,6 +10,7 @@ public class DeSpecificDetailSearchProcessor extends ICountrySpecificDetailSearc
@Override
public boolean canHandle(String countryCode, SimpleEidasData eidData) {
+ // NOTE: Please extract constant for "de"
if (!countryCode.equalsIgnoreCase("de")) {
return false;
}
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java
index 3d6b35e9..b9ab2ceb 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/handler/ICountrySpecificDetailSearchProcessor.java
@@ -31,6 +31,7 @@ import org.springframework.beans.factory.annotation.Autowired;
public abstract class ICountrySpecificDetailSearchProcessor {
+ // NOTE: Please use constructor injection
protected IErnbClient ernbClient;
protected IZmrClient zmrClient;
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java
index 1580603c..20ee9325 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/tasks/InitialSearchTask.java
@@ -66,6 +66,8 @@ import java.util.Map;
*/
@Slf4j
@Component("InitialSearchTask")
+// NOTE: General: Please rebase git commit and squash them where useful, i.e. "remove unused import" should
+// not be a separate commit.
public class InitialSearchTask extends AbstractAuthServletTask {
private List<ICountrySpecificDetailSearchProcessor> handlers = new ArrayList<>();
@@ -92,6 +94,7 @@ public class InitialSearchTask extends AbstractAuthServletTask {
final ILightResponse eidasResponse = authProcessData
.getGenericDataFromSession(Constants.DATA_FULL_EIDAS_RESPONSE, ILightResponse.class);
+ // NOTE: Why is eidas first converted to a map, and then to a SimpleEidasData?
final Map<String, Object> simpleAttrMap = convertEidasAttrToSimpleMap(
eidasResponse.getAttributes().getAttributeMap());
@@ -106,14 +109,17 @@ public class InitialSearchTask extends AbstractAuthServletTask {
}
}
+ // NOTE: Please rename methods ... "step2" doesn't tell the reader anything
private String step2(SimpleEidasData eidData) throws TaskExecutionException {
String personIdentifier = eidData.getPseudonym();
+ // NOTE: Is that comment really necessary?
//search in register(step 2)
MergedRegisterSearchResult result = searchInZmrAndErnp(personIdentifier);
if (result.getResultCount() == 0) {
return step5(result, eidData);
} else if (result.getResultCount() == 1) {
return step3(result, eidData);
+ // NOTE: Why is that code commented-out?
} //else if (result.getResultCount() > 1) {
throw new TaskExecutionException(pendingReq, "Initial search - Kitt Process necessary.",
new ManualFixNecessaryException(personIdentifier));
@@ -179,7 +185,7 @@ public class InitialSearchTask extends AbstractAuthServletTask {
log.debug("Update " + result + " with " + eidData);
//TODO
-
+ // NOTE: Sometimes the bpk is returned, sometimes "105"?
return result.getBpk();
}
@@ -265,6 +271,7 @@ public class InitialSearchTask extends AbstractAuthServletTask {
log.debug("Automerge " + initialSearchResult + " with " + eidData + " " + mdsSearchResult);
//TODO
+ // NOTE: Sometimes the bpk is returned, sometimes "105"?
return "105";
}
@@ -329,6 +336,7 @@ public class InitialSearchTask extends AbstractAuthServletTask {
//just for testing
//TODO is there a nicer solution?
+ // There is: Constructor Injection see https://reflectoring.io/constructor-injection/ or https://www.baeldung.com/constructor-injection-in-spring
@Autowired
public void setErnbClient(IErnbClient ernbClient) {
this.ernbClient = ernbClient;
@@ -342,6 +350,7 @@ public class InitialSearchTask extends AbstractAuthServletTask {
@Autowired
public void setHandlers(List<ICountrySpecificDetailSearchProcessor> handlers) {
this.handlers = handlers;
+ // NOTE: There's a typo in "registrated"
log.info("# " + handlers.size() + " country specific detail search services are registrated");
}
diff --git a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java
index 0f3436d8..3af2e39e 100644
--- a/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java
+++ b/eidas_modules/authmodule-eIDAS-v2/src/main/java/at/asitplus/eidas/specific/modules/auth/eidas/v2/zmr/DummyZmrClient.java
@@ -11,6 +11,7 @@ public class DummyZmrClient implements IZmrClient {
@Override
public ArrayList<RegisterResult> searchWithPersonIdentifer(String personIdentifer) {
+ // NOTE: Are we using Mockito or these fixed strings for testing, why are those defined here?
switch (personIdentifer) {
case "a12345":
case "a12345-":