Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

Submission Service - Enhance TEK's #712

Merged
merged 30 commits into from
Aug 25, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c8c81d8
-initial draft commit
ioangut Aug 13, 2020
6b20839
updated checkDuplicateStartIntervalNumberLimit wip
emmetsap Aug 13, 2020
7487eeb
updated checkDuplicateStartIntervalNumberLimit
emmetsap Aug 13, 2020
b9a94db
Merge remote-tracking branch 'origin/master' into enhancement/change-…
ioangut Aug 13, 2020
ec4fd7a
-fix test broke after solved conflict
ioangut Aug 13, 2020
5bbb817
changed checkDuplicateStartIntervalNumberLimit to use HashMap
emmetsap Aug 14, 2020
604ad46
-fix checklist reports
ioangut Aug 14, 2020
d39f93a
add env variables for submission service
ioangut Aug 14, 2020
8fe54b9
Merge remote-tracking branch 'origin/master' into enhancement/change-…
ioangut Aug 14, 2020
bdd4cb4
review update add more tests
ioangut Aug 17, 2020
7468aaa
solved code smells
ioangut Aug 17, 2020
b06d6d0
rename attribute maxRollingPeriod
ioangut Aug 18, 2020
845002a
renamed getter getMaxRollingPeriod
ioangut Aug 18, 2020
adff6f1
Update ValidSubmissionPayload.java
ioangut Aug 18, 2020
268a3a6
refactor implementation for consistency
ioangut Aug 18, 2020
44640da
fix checkstyle errors
ioangut Aug 18, 2020
41b0902
fix code smell
ioangut Aug 18, 2020
36b702a
Update ValidSubmissionPayload.java
ioangut Aug 18, 2020
2ada526
Change validation for payloads based on rolling period
EugenM-SAP Aug 20, 2020
bae6653
Capture more test scenarios
EugenM-SAP Aug 20, 2020
fe701a4
add checkstyle indentation
ioangut Aug 20, 2020
ae59664
create test data generation class
ioangut Aug 20, 2020
c7e8742
add header to the new class
ioangut Aug 20, 2020
13854ca
resolved review conversation
ioangut Aug 21, 2020
29e25f6
fixed checkstyle errors
ioangut Aug 21, 2020
29a52cd
Merge remote-tracking branch 'origin/master' into enhancement/change-…
ioangut Aug 21, 2020
8bb7e07
resolved code smell
ioangut Aug 21, 2020
b57ca0a
fix compile error
ioangut Aug 21, 2020
b4abfca
create valid TEK, fix test failing
ioangut Aug 24, 2020
22bb542
Merge branch 'master' into enhancement/change-tek-approach
pithumke Aug 24, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ public class DiagnosisKey {
* reject any diagnosis keys that do not have a rolling period of a certain fixed value. See
* https://developer.apple.com/documentation/exposurenotification/setting_up_an_exposure_notification_server
*/
public static final int EXPECTED_ROLLING_PERIOD = 144;
public static final int MIN_ROLLING_PERIOD = 0;
public static final int MAX_ROLLING_PERIOD = 144;

private static final Validator VALIDATOR = Validation.buildDefaultValidatorFactory().getValidator();

Expand All @@ -58,8 +59,8 @@ public class DiagnosisKey {
@ValidRollingStartIntervalNumber
private final int rollingStartIntervalNumber;

@Range(min = EXPECTED_ROLLING_PERIOD, max = EXPECTED_ROLLING_PERIOD,
message = "Rolling period must be " + EXPECTED_ROLLING_PERIOD + ".")
@Range(min = MIN_ROLLING_PERIOD, max = MAX_ROLLING_PERIOD,
message = "Rolling period must be between " + MIN_ROLLING_PERIOD + " and " + MAX_ROLLING_PERIOD + ".")
private final int rollingPeriod;

@Range(min = 0, max = 8, message = "Risk level must be between 0 and 8.")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class DiagnosisKeyBuilder implements

private byte[] keyData;
private int rollingStartIntervalNumber;
private int rollingPeriod = DiagnosisKey.EXPECTED_ROLLING_PERIOD;
private int rollingPeriod = DiagnosisKey.MAX_ROLLING_PERIOD;
private int transmissionRiskLevel;
private Long submissionTimestamp = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ interface FinalBuilder {

/**
* Adds the specified rolling period to this builder. If not specified, the rolling period defaults to {@link
* DiagnosisKey#EXPECTED_ROLLING_PERIOD}
* DiagnosisKey#MAX_ROLLING_PERIOD}
*
* @param rollingPeriod Number describing how long a key is valid. It is expressed in increments of 10 minutes (e.g.
* 144 for 24 hours).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void buildFromProtoBufObjWithSubmissionTimestamp() {
.newBuilder()
.setKeyData(ByteString.copyFrom(this.expKeyData))
.setRollingStartIntervalNumber(this.expRollingStartIntervalNumber)
.setRollingPeriod(DiagnosisKey.EXPECTED_ROLLING_PERIOD)
.setRollingPeriod(DiagnosisKey.MAX_ROLLING_PERIOD)
.setTransmissionRiskLevel(this.expTransmissionRiskLevel)
.build();

Expand All @@ -69,7 +69,7 @@ void buildFromProtoBufObjWithoutSubmissionTimestamp() {
.newBuilder()
.setKeyData(ByteString.copyFrom(this.expKeyData))
.setRollingStartIntervalNumber(this.expRollingStartIntervalNumber)
.setRollingPeriod(DiagnosisKey.EXPECTED_ROLLING_PERIOD)
.setRollingPeriod(DiagnosisKey.MAX_ROLLING_PERIOD)
.setTransmissionRiskLevel(this.expTransmissionRiskLevel)
.build();

Expand Down Expand Up @@ -106,7 +106,7 @@ void buildSuccessivelyWithRollingPeriod() {
.withRollingStartIntervalNumber(this.expRollingStartIntervalNumber)
.withTransmissionRiskLevel(this.expTransmissionRiskLevel)
.withSubmissionTimestamp(this.expSubmissionTimestamp)
.withRollingPeriod(DiagnosisKey.EXPECTED_ROLLING_PERIOD).build();
.withRollingPeriod(DiagnosisKey.MAX_ROLLING_PERIOD).build();

assertDiagnosisKeyEquals(actDiagnosisKey, this.expSubmissionTimestamp);
}
Expand Down Expand Up @@ -165,17 +165,18 @@ void transmissionRiskLevelDoesNotThrowForValid(int validRiskLevel) {
}

@ParameterizedTest
@ValueSource(ints = {-3, 143, 145})
@ValueSource(ints = {-3, 145})
void rollingPeriodMustBeEpectedValue(int invalidRollingPeriod) {
assertThat(catchThrowable(() -> keyWithRollingPeriod(invalidRollingPeriod)))
.isInstanceOf(InvalidDiagnosisKeyException.class)
.hasMessage("[Rolling period must be " + DiagnosisKey.EXPECTED_ROLLING_PERIOD
.hasMessage("[Rolling period must be between "+ DiagnosisKey.MIN_ROLLING_PERIOD + " and " + DiagnosisKey.MAX_ROLLING_PERIOD
+ ". Invalid Value: " + invalidRollingPeriod + "]");
}

@Test
void rollingPeriodDoesNotThrowForValid() {
assertThatCode(() -> keyWithRollingPeriod(DiagnosisKey.EXPECTED_ROLLING_PERIOD)).doesNotThrowAnyException();
@ParameterizedTest
@ValueSource(ints = {DiagnosisKey.MIN_ROLLING_PERIOD, 100, DiagnosisKey.MAX_ROLLING_PERIOD})
void rollingPeriodDoesNotThrowForValid(int validRollingPeriod) {
assertThatCode(() -> keyWithRollingPeriod(validRollingPeriod)).doesNotThrowAnyException();
}

@ParameterizedTest
Expand Down Expand Up @@ -218,7 +219,7 @@ void submissionTimestampDoesNotThrowOnValid() {
() -> buildDiagnosisKeyForSubmissionTimestamp(Instant.now().minus(Duration.ofHours(2)).getEpochSecond() / SECONDS_PER_HOUR))
.doesNotThrowAnyException();
}

private DiagnosisKey keyWithKeyData(byte[] expKeyData) {
return DiagnosisKey.builder()
.withKeyData(expKeyData)
Expand Down Expand Up @@ -260,7 +261,7 @@ private void assertDiagnosisKeyEquals(DiagnosisKey actDiagnosisKey, long expSubm
assertThat(actDiagnosisKey.getSubmissionTimestamp()).isEqualTo(expSubmissionTimestamp);
assertThat(actDiagnosisKey.getKeyData()).isEqualTo(this.expKeyData);
assertThat(actDiagnosisKey.getRollingStartIntervalNumber()).isEqualTo(this.expRollingStartIntervalNumber);
assertThat(actDiagnosisKey.getRollingPeriod()).isEqualTo(DiagnosisKey.EXPECTED_ROLLING_PERIOD);
assertThat(actDiagnosisKey.getRollingPeriod()).isEqualTo(DiagnosisKey.MAX_ROLLING_PERIOD);
assertThat(actDiagnosisKey.getTransmissionRiskLevel()).isEqualTo(this.expTransmissionRiskLevel);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ private void mockInvalidKeyInDb(List<DiagnosisKey> keys) {

private DiagnosisKey validKey(long expSubmissionTimestamp) {
return new DiagnosisKey(expKeyData, expRollingStartIntervalNumber,
DiagnosisKey.EXPECTED_ROLLING_PERIOD, expTransmissionRiskLevel, expSubmissionTimestamp);
DiagnosisKey.MAX_ROLLING_PERIOD, expTransmissionRiskLevel, expSubmissionTimestamp);
}

private DiagnosisKey invalidKey(long expSubmissionTimestamp) {
byte[] expKeyData = "17--bytelongarray".getBytes(StandardCharsets.US_ASCII);
return new DiagnosisKey(expKeyData, expRollingStartIntervalNumber,
DiagnosisKey.EXPECTED_ROLLING_PERIOD, expTransmissionRiskLevel, expSubmissionTimestamp);
DiagnosisKey.MAX_ROLLING_PERIOD, expTransmissionRiskLevel, expSubmissionTimestamp);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public class SubmissionServiceConfig {
private Verification verification;
private Monitoring monitoring;
private Client client;
private DiagnosisKey diagnosisKey;

public Long getInitialFakeDelayMilliseconds() {
return initialFakeDelayMilliseconds;
Expand Down Expand Up @@ -119,7 +120,7 @@ public void setPayload(Payload payload) {
private static class Payload {

@Min(7)
@Max(28)
@Max(100)
private Integer maxNumberOfKeys;

public Integer getMaxNumberOfKeys() {
Expand Down Expand Up @@ -268,4 +269,27 @@ public void setTrustStorePassword(String trustStorePassword) {
}
}
}

public void setDiagnosisKey(DiagnosisKey diagnosisKey) {
this.diagnosisKey = diagnosisKey;
}

private static class DiagnosisKey {
ioangut marked this conversation as resolved.
Show resolved Hide resolved

@Min(0)
@Max(144)
private Integer rollingPeriod;

public Integer getRollingPeriod() {
return rollingPeriod;
}

public void setRollingPeriod(Integer rollingPeriod) {
this.rollingPeriod = rollingPeriod;
}
}

public Integer getRollingPeriod() {
return diagnosisKey.getRollingPeriod();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

package app.coronawarn.server.services.submission.validation;

import static app.coronawarn.server.common.persistence.domain.DiagnosisKey.EXPECTED_ROLLING_PERIOD;

import app.coronawarn.server.common.protocols.external.exposurenotification.TemporaryExposureKey;
import app.coronawarn.server.common.protocols.internal.SubmissionPayload;
import app.coronawarn.server.services.submission.config.SubmissionServiceConfig;
Expand All @@ -31,6 +29,7 @@
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import javax.validation.Constraint;
import javax.validation.ConstraintValidator;
Expand Down Expand Up @@ -68,9 +67,11 @@ class SubmissionPayloadValidator implements
ConstraintValidator<ValidSubmissionPayload, SubmissionPayload> {

private final int maxNumberOfKeys;
private final int rollingPeriod;

public SubmissionPayloadValidator(SubmissionServiceConfig submissionServiceConfig) {
maxNumberOfKeys = submissionServiceConfig.getMaxNumberOfKeys();
rollingPeriod = submissionServiceConfig.getRollingPeriod();
}

/**
Expand All @@ -86,10 +87,17 @@ public SubmissionPayloadValidator(SubmissionServiceConfig submissionServiceConfi
public boolean isValid(SubmissionPayload submissionPayload, ConstraintValidatorContext validatorContext) {
List<TemporaryExposureKey> exposureKeys = submissionPayload.getKeysList();
validatorContext.disableDefaultConstraintViolation();
ioangut marked this conversation as resolved.
Show resolved Hide resolved

boolean isValid = checkKeyCollectionSize(exposureKeys, validatorContext);
isValid &= checkUniqueStartIntervalNumbers(exposureKeys, validatorContext);
isValid &= checkNoOverlapsInTimeWindow(exposureKeys, validatorContext);
boolean isValid = false;

for (TemporaryExposureKey exposureKey: exposureKeys) {
ioangut marked this conversation as resolved.
Show resolved Hide resolved
if (exposureKey.getRollingPeriod() < rollingPeriod) {
isValid = checkDuplicateStartIntervalNumberLimit(exposureKeys, validatorContext);
} else {
isValid = checkKeyCollectionSize(exposureKeys, validatorContext);
isValid &= checkUniqueStartIntervalNumbers(exposureKeys, validatorContext);
isValid &= checkNoOverlapsInTimeWindow(exposureKeys, validatorContext);
}
}

return isValid;
}
Expand Down Expand Up @@ -135,13 +143,37 @@ private boolean checkNoOverlapsInTimeWindow(List<TemporaryExposureKey> exposureK
.sorted().boxed().toArray(Integer[]::new);

for (int i = 1; i < sortedStartIntervalNumbers.length; i++) {
if ((sortedStartIntervalNumbers[i - 1] + EXPECTED_ROLLING_PERIOD) > sortedStartIntervalNumbers[i]) {
if ((sortedStartIntervalNumbers[i - 1] + rollingPeriod) > sortedStartIntervalNumbers[i]) {
addViolation(validatorContext, String.format(
"Subsequent intervals overlap. StartIntervalNumbers: %s", sortedStartIntervalNumbers));
return false;
}
}
return true;
}

private boolean checkDuplicateStartIntervalNumberLimit(List<TemporaryExposureKey> exposureKeys,
ConstraintValidatorContext validatorContext) {


HashMap<Integer, Integer> totalKeysPerDay = new HashMap<>();

for (TemporaryExposureKey exposureKey: exposureKeys) {
int numberOfKeys = exposureKey.getRollingPeriod();
if (totalKeysPerDay.containsKey(exposureKey.getRollingStartIntervalNumber())) {
numberOfKeys += totalKeysPerDay.get(exposureKey.getRollingStartIntervalNumber());
ioangut marked this conversation as resolved.
Show resolved Hide resolved

if (numberOfKeys > rollingPeriod) {
addViolation(validatorContext, String.format(
"Keys in excess of %s per day.", exposureKeys.size()));
return false;
}
}

totalKeysPerDay.put(exposureKey.getRollingStartIntervalNumber(), numberOfKeys);
}

return true;
}
}
}
6 changes: 4 additions & 2 deletions services/submission/src/main/resources/application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ services:
maximum-request-size: ${MAXIMUM_REQUEST_SIZE:100KB}
payload:
# The maximum number of keys accepted for any submission.
max-number-of-keys: 14
max-number-of-keys: ${MAX_NUMBER_OF_KEYS:10}
verification:
base-url: ${VERIFICATION_BASE_URL:http://localhost:8004}
path: /version/v1/tan/verify
monitoring:
# The batch size (number of requests) to use for monitoring request count.
batch-size: 5
diagnosis-key:
rolling-period: ${MAX_ROLLING_PERIOD:144}
client:
ssl:
key-password: ${SSL_SUBMISSION_KEYSTORE_PASSWORD}
Expand Down Expand Up @@ -116,4 +118,4 @@ feign:
config:
default:
connect-timeout: 5000
read-timeout: 5000
read-timeout: 5000