[ES-1640] added schema validation for claims query parameter #887
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 8 commits
September 16, 2024 17:18
Signed-off-by: Mohd Kaif Siddique <[email protected]>
Signed-off-by: Mohd Kaif Siddique <[email protected]>
Signed-off-by: Mohd Kaif Siddique <[email protected]>
Signed-off-by: Mohd Kaif Siddique <[email protected]>
Signed-off-by: Mohd Kaif Siddique <[email protected]>
Signed-off-by: Mohd Kaif Siddique <[email protected]>
ase-101
changed the title
ase-101
reviewed
Oct 18, 2024
| @@ -76,6 +72,7 @@ public class OAuthDetailRequest { | |||
| * names of the individual Claims being requested as the member names. | |||
| */ | |||
| @Valid | |||
| @ClaimSchema(message = ErrorConstants.INVALID_CLAIMS_SCHEMA) | |||
There was a problem hiding this comment.
As the query parameter is named as "claims" it is better to name as "ClaimsSchema". And also the error code can state as "INVALID_CLAIMS"
ase-101
reviewed
Oct 18, 2024
| public class ClaimSchemaValidator implements ConstraintValidator<ClaimSchema, ClaimsV2> { | ||
|
|
||
|
|
||
| @Value("${mosip.esignet.json.validation.schema.url}") |
There was a problem hiding this comment.
Rename the property as "mosip.esignet.claims.schema.url"
ase-101
reviewed
Oct 18, 2024
| private volatile JsonSchema cachedSchema; | ||
|
|
||
| @Autowired | ||
| private RestTemplate restTemplate; |
ase-101
reviewed
Oct 18, 2024
| @Value("${mosip.esignet.json.validation.schema.url}") | ||
| private String schemaUrl; | ||
|
|
||
| private volatile JsonSchema cachedSchema; |
There was a problem hiding this comment.
can we store this in Redis instead?
ase-101
reviewed
Oct 18, 2024
Comment on lines
+63
to
+65
| if (cachedSchema == null) { | ||
| synchronized (this) { | ||
| if (cachedSchema == null) { |
There was a problem hiding this comment.
is it required to check for null twice - line 63 & 65?
ase-101
reviewed
Oct 18, 2024
| return cachedSchema; | ||
| } | ||
|
|
||
| private String getResource(String url) { |
There was a problem hiding this comment.
I think we have written similar method to download the amr-acr mapping file. If its same common method we can move this method to common util class.
ase-101
reviewed
Oct 18, 2024
| @Mock | ||
| private JsonSchemaFactory jsonSchemaFactory; | ||
|
|
||
| private final String claimSchema="{\"$id\":\"https://bitbucket.org/openid/ekyc-ida/raw/master/schema/verified_claims_request.json\",\"$schema\":\"https://json-schema.org/draft/2020-12/schema\",\"$defs\":{\"check_details\":{\"type\":\"array\",\"prefixItems\":[{\"check_id\":{\"type\":\"string\"},\"check_method\":{\"type\":\"string\"},\"organization\":{\"type\":\"string\"},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}]},\"claims_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"additionalProperties\":{\"anyOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3},\"value\":{\"type\":\"string\",\"minLength\":3},\"values\":{\"type\":\"array\",\"items\":{\"type\":\"string\"},\"minItems\":1}}}]},\"minProperties\":1}]},\"constrainable_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3},\"value\":{\"type\":\"string\"},\"values\":{\"type\":\"array\",\"items\":{\"type\":\"string\"},\"minItems\":1}}}]},\"datetime_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"max_age\":{\"type\":\"integer\",\"minimum\":0},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3}}}]},\"document_details\":{\"type\":\"object\",\"properties\":{\"type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"date_of_expiry\":{\"$ref\":\"#/$defs/datetime_element\"},\"date_of_issuance\":{\"$ref\":\"#/$defs/datetime_element\"},\"document_number\":{\"$ref\":\"#/$defs/simple_element\"},\"issuer\":{\"type\":\"object\",\"properties\":{\"country\":{\"$ref\":\"#/$defs/simple_element\"},\"country_code\":{\"$ref\":\"#/$defs/simple_element\"},\"formatted\":{\"$ref\":\"#/$defs/simple_element\"},\"jurisdiction\":{\"$ref\":\"#/$defs/simple_element\"},\"locality\":{\"$ref\":\"#/$defs/simple_element\"},\"name\":{\"$ref\":\"#/$defs/simple_element\"},\"postal_code\":{\"$ref\":\"#/$defs/simple_element\"},\"region\":{\"$ref\":\"#/$defs/simple_element\"},\"street_address\":{\"$ref\":\"#/$defs/simple_element\"}}},\"personal_number\":{\"$ref\":\"#/$defs/simple_element\"},\"serial_number\":{\"$ref\":\"#/$defs/simple_element\"}}},\"evidence\":{\"type\":\"object\",\"required\":[\"type\"],\"properties\":{\"type\":{\"type\":\"object\",\"properties\":{\"value\":{\"enum\":[\"document\",\"electronic_record\",\"vouch\",\"electronic_signature\"]}}},\"attachments\":{\"$ref\":\"#/$defs/simple_element\"}},\"allOf\":[{\"if\":{\"properties\":{\"type\":{\"value\":\"electronic_signature\"}}},\"then\":{\"properties\":{\"created_at\":{\"$ref\":\"#/$defs/datetime_element\"},\"issuer\":{\"$ref\":\"#/$defs/simple_element\"},\"serial_number\":{\"$ref\":\"#/$defs/simple_element\"},\"signature_type\":{\"$ref\":\"#/$defs/simple_element\"}}},\"else\":true},{\"if\":{\"properties\":{\"type\":{\"value\":\"document\"}}},\"then\":{\"properties\":{\"check_details\":{\"$ref\":\"#/$defs/check_details\"},\"document_details\":{\"$ref\":\"#/$defs/document_details\"},\"method\":{\"$ref\":\"#/$defs/constrainable_element\"},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}},\"else\":true},{\"if\":{\"properties\":{\"type\":{\"value\":\"electronic_record\"}}},\"then\":{\"properties\":{\"check_details\":{\"$ref\":\"#/$defs/check_details\"},\"record\":{\"type\":\"object\",\"properties\":{\"type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"created_at\":{\"$ref\":\"#/$defs/datetime_element\"},\"date_of_expiry\":{\"$ref\":\"#/$defs/datetime_element\"},\"derived_claims\":{\"$ref\":\"#/$defs/claims_element\"},\"source\":{\"type\":\"object\",\"properties\":{\"country\":{\"$ref\":\"#/$defs/simple_element\"},\"country_code\":{\"$ref\":\"#/$defs/simple_element\"},\"formatted\":{\"$ref\":\"#/$defs/simple_element\"},\"locality\":{\"$ref\":\"#/$defs/simple_element\"},\"name\":{\"$ref\":\"#/$defs/simple_element\"},\"postal_code\":{\"$ref\":\"#/$defs/simple_element\"},\"region\":{\"$ref\":\"#/$defs/simple_element\"},\"street_address\":{\"$ref\":\"#/$defs/simple_element\"}}}}},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}},\"else\":true},{\"if\":{\"properties\":{\"type\":{\"value\":\"vouch\"}}},\"then\":{\"properties\":{\"attestation\":{\"type\":\"object\",\"properties\":{\"type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"date_of_expiry\":{\"$ref\":\"#/$defs/datetime_element\"},\"date_of_issuance\":{\"$ref\":\"#/$defs/datetime_element\"},\"derived_claims\":{\"$ref\":\"#/$defs/claims_element\"},\"reference_number\":{\"$ref\":\"#/$defs/simple_element\"},\"voucher\":{\"type\":\"object\",\"properties\":{\"birthdate\":{\"$ref\":\"#/$defs/datetime_element\"},\"country\":{\"$ref\":\"#/$defs/simple_element\"},\"formatted\":{\"$ref\":\"#/$defs/simple_element\"},\"locality\":{\"$ref\":\"#/$defs/simple_element\"},\"name\":{\"$ref\":\"#/$defs/simple_element\"},\"occupation\":{\"$ref\":\"#/$defs/simple_element\"},\"organization\":{\"$ref\":\"#/$defs/simple_element\"},\"postal_code\":{\"$ref\":\"#/$defs/simple_element\"},\"region\":{\"$ref\":\"#/$defs/simple_element\"},\"street_address\":{\"$ref\":\"#/$defs/simple_element\"}}}}},\"check_details\":{\"$ref\":\"#/$defs/check_details\"},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}},\"else\":true}]},\"simple_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3}}}]},\"verified_claims\":{\"oneOf\":[{\"type\":\"array\",\"items\":{\"anyOf\":[{\"$ref\":\"#/$defs/verified_claims_def\"}]}},{\"$ref\":\"#/$defs/verified_claims_def\"}]},\"verified_claims_def\":{\"type\":\"object\",\"required\":[\"verification\",\"claims\"],\"additionalProperties\":false,\"properties\":{\"claims\":{\"$ref\":\"#/$defs/claims_element\"},\"verification\":{\"type\":\"object\",\"required\":[\"trust_framework\"],\"additionalProperties\":true,\"properties\":{\"assurance_level\":{\"$ref\":\"#/$defs/constrainable_element\"},\"assurance_process\":{\"type\":\"object\",\"properties\":{\"assurance_details\":{\"type\":\"array\",\"items\":{\"oneOf\":[{\"assurance_classification\":{\"$ref\":\"#/$defs/constrainable_element\"},\"assurance_type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"evidence_ref\":{\"type\":\"object\",\"required\":[\"txn\"],\"additionalProperties\":true,\"properties\":{\"evidence_classification\":{\"$ref\":\"#/$defs/constrainable_element\"},\"evidence_metadata\":{\"$ref\":\"#/$defs/constrainable_element\"},\"txn\":{\"$ref\":\"#/$defs/constrainable_element\"}}}}]},\"minItems\":1},\"policy\":{\"$ref\":\"#/$defs/constrainable_element\"},\"procedure\":{\"$ref\":\"#/$defs/constrainable_element\"}}},\"evidence\":{\"type\":\"array\",\"items\":{\"oneOf\":[{\"$ref\":\"#/$defs/evidence\"}]},\"minItems\":1},\"time\":{\"$ref\":\"#/$defs/datetime_element\"},\"trust_framework\":{\"$ref\":\"#/$defs/constrainable_element\"},\"verification_process\":{\"$ref\":\"#/$defs/simple_element\"}}}}}},\"properties\":{\"id_token\":{\"type\":\"object\",\"additionalProperties\":true,\"properties\":{\"verified_claims\":{\"$ref\":\"#/$defs/verified_claims\"}}},\"userinfo\":{\"type\":\"object\",\"additionalProperties\":true,\"properties\":{\"verified_claims\":{\"$ref\":\"#/$defs/verified_claims\"}}}}}"; |
There was a problem hiding this comment.
Can we keep a copy of schema file under resources folder and load it during the test?
ase-101
reviewed
Oct 18, 2024
| @Test | ||
| public void test_ClaimSchemaValidator_withEssentialAsInteger_thenFail() throws IOException { | ||
|
|
||
| ObjectMapper mapper = new ObjectMapper(); |
There was a problem hiding this comment.
as this is used in more than one method better to declare objectmapper as class variable.
ase-101
reviewed
Oct 18, 2024
| // =============================ClaimSchemaValidator=============================// | ||
|
|
||
| @Test | ||
| public void test_ClaimSchemaValidator_withValidDetails_thenPass() throws IOException { |
There was a problem hiding this comment.
Why are we deviating from the test method naming convention?
ase-101
reviewed
Oct 18, 2024
| } | ||
|
|
||
| @Test | ||
| public void test_ClaimSchemaValidator_withTrustFrameWorkAsNull_thenFail() throws IOException { |
There was a problem hiding this comment.
Why are we deviating from the test method naming convention?
ase-101
reviewed
Oct 18, 2024
|
|
||
| ClaimsV2 claimsV2; | ||
|
|
||
| private final String claimSchema="{\"$id\":\"https://bitbucket.org/openid/ekyc-ida/raw/master/schema/verified_claims_request.json\",\"$schema\":\"https://json-schema.org/draft/2020-12/schema\",\"$defs\":{\"check_details\":{\"type\":\"array\",\"prefixItems\":[{\"check_id\":{\"type\":\"string\"},\"check_method\":{\"type\":\"string\"},\"organization\":{\"type\":\"string\"},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}]},\"claims_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"additionalProperties\":{\"anyOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3}}}]},\"minProperties\":1}]},\"constrainable_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3},\"value\":{\"type\":\"string\"},\"values\":{\"type\":\"array\",\"items\":{\"type\":\"string\"},\"minItems\":1}}}]},\"datetime_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"max_age\":{\"type\":\"integer\",\"minimum\":0},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3}}}]},\"document_details\":{\"type\":\"object\",\"properties\":{\"type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"date_of_expiry\":{\"$ref\":\"#/$defs/datetime_element\"},\"date_of_issuance\":{\"$ref\":\"#/$defs/datetime_element\"},\"document_number\":{\"$ref\":\"#/$defs/simple_element\"},\"issuer\":{\"type\":\"object\",\"properties\":{\"country\":{\"$ref\":\"#/$defs/simple_element\"},\"country_code\":{\"$ref\":\"#/$defs/simple_element\"},\"formatted\":{\"$ref\":\"#/$defs/simple_element\"},\"jurisdiction\":{\"$ref\":\"#/$defs/simple_element\"},\"locality\":{\"$ref\":\"#/$defs/simple_element\"},\"name\":{\"$ref\":\"#/$defs/simple_element\"},\"postal_code\":{\"$ref\":\"#/$defs/simple_element\"},\"region\":{\"$ref\":\"#/$defs/simple_element\"},\"street_address\":{\"$ref\":\"#/$defs/simple_element\"}}},\"personal_number\":{\"$ref\":\"#/$defs/simple_element\"},\"serial_number\":{\"$ref\":\"#/$defs/simple_element\"}}},\"evidence\":{\"type\":\"object\",\"required\":[\"type\"],\"properties\":{\"type\":{\"type\":\"object\",\"properties\":{\"value\":{\"enum\":[\"document\",\"electronic_record\",\"vouch\",\"electronic_signature\"]}}},\"attachments\":{\"$ref\":\"#/$defs/simple_element\"}},\"allOf\":[{\"if\":{\"properties\":{\"type\":{\"value\":\"electronic_signature\"}}},\"then\":{\"properties\":{\"created_at\":{\"$ref\":\"#/$defs/datetime_element\"},\"issuer\":{\"$ref\":\"#/$defs/simple_element\"},\"serial_number\":{\"$ref\":\"#/$defs/simple_element\"},\"signature_type\":{\"$ref\":\"#/$defs/simple_element\"}}},\"else\":true},{\"if\":{\"properties\":{\"type\":{\"value\":\"document\"}}},\"then\":{\"properties\":{\"check_details\":{\"$ref\":\"#/$defs/check_details\"},\"document_details\":{\"$ref\":\"#/$defs/document_details\"},\"method\":{\"$ref\":\"#/$defs/constrainable_element\"},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}},\"else\":true},{\"if\":{\"properties\":{\"type\":{\"value\":\"electronic_record\"}}},\"then\":{\"properties\":{\"check_details\":{\"$ref\":\"#/$defs/check_details\"},\"record\":{\"type\":\"object\",\"properties\":{\"type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"created_at\":{\"$ref\":\"#/$defs/datetime_element\"},\"date_of_expiry\":{\"$ref\":\"#/$defs/datetime_element\"},\"derived_claims\":{\"$ref\":\"#/$defs/claims_element\"},\"source\":{\"type\":\"object\",\"properties\":{\"country\":{\"$ref\":\"#/$defs/simple_element\"},\"country_code\":{\"$ref\":\"#/$defs/simple_element\"},\"formatted\":{\"$ref\":\"#/$defs/simple_element\"},\"locality\":{\"$ref\":\"#/$defs/simple_element\"},\"name\":{\"$ref\":\"#/$defs/simple_element\"},\"postal_code\":{\"$ref\":\"#/$defs/simple_element\"},\"region\":{\"$ref\":\"#/$defs/simple_element\"},\"street_address\":{\"$ref\":\"#/$defs/simple_element\"}}}}},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}},\"else\":true},{\"if\":{\"properties\":{\"type\":{\"value\":\"vouch\"}}},\"then\":{\"properties\":{\"attestation\":{\"type\":\"object\",\"properties\":{\"type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"date_of_expiry\":{\"$ref\":\"#/$defs/datetime_element\"},\"date_of_issuance\":{\"$ref\":\"#/$defs/datetime_element\"},\"derived_claims\":{\"$ref\":\"#/$defs/claims_element\"},\"reference_number\":{\"$ref\":\"#/$defs/simple_element\"},\"voucher\":{\"type\":\"object\",\"properties\":{\"birthdate\":{\"$ref\":\"#/$defs/datetime_element\"},\"country\":{\"$ref\":\"#/$defs/simple_element\"},\"formatted\":{\"$ref\":\"#/$defs/simple_element\"},\"locality\":{\"$ref\":\"#/$defs/simple_element\"},\"name\":{\"$ref\":\"#/$defs/simple_element\"},\"occupation\":{\"$ref\":\"#/$defs/simple_element\"},\"organization\":{\"$ref\":\"#/$defs/simple_element\"},\"postal_code\":{\"$ref\":\"#/$defs/simple_element\"},\"region\":{\"$ref\":\"#/$defs/simple_element\"},\"street_address\":{\"$ref\":\"#/$defs/simple_element\"}}}}},\"check_details\":{\"$ref\":\"#/$defs/check_details\"},\"time\":{\"$ref\":\"#/$defs/datetime_element\"}}},\"else\":true}]},\"simple_element\":{\"oneOf\":[{\"type\":\"null\"},{\"type\":\"object\",\"properties\":{\"essential\":{\"type\":\"boolean\"},\"purpose\":{\"type\":\"string\",\"maxLength\":300,\"minLength\":3}}}]},\"verified_claims\":{\"oneOf\":[{\"type\":\"array\",\"items\":{\"anyOf\":[{\"$ref\":\"#/$defs/verified_claims_def\"}]}},{\"$ref\":\"#/$defs/verified_claims_def\"}]},\"verified_claims_def\":{\"type\":\"object\",\"required\":[\"verification\",\"claims\"],\"additionalProperties\":false,\"properties\":{\"claims\":{\"$ref\":\"#/$defs/claims_element\"},\"verification\":{\"type\":\"object\",\"required\":[\"trust_framework\"],\"additionalProperties\":true,\"properties\":{\"assurance_level\":{\"$ref\":\"#/$defs/constrainable_element\"},\"assurance_process\":{\"type\":\"object\",\"properties\":{\"assurance_details\":{\"type\":\"array\",\"items\":{\"oneOf\":[{\"assurance_classification\":{\"$ref\":\"#/$defs/constrainable_element\"},\"assurance_type\":{\"$ref\":\"#/$defs/constrainable_element\"},\"evidence_ref\":{\"type\":\"object\",\"required\":[\"txn\"],\"additionalProperties\":true,\"properties\":{\"evidence_classification\":{\"$ref\":\"#/$defs/constrainable_element\"},\"evidence_metadata\":{\"$ref\":\"#/$defs/constrainable_element\"},\"txn\":{\"$ref\":\"#/$defs/constrainable_element\"}}}}]},\"minItems\":1},\"policy\":{\"$ref\":\"#/$defs/constrainable_element\"},\"procedure\":{\"$ref\":\"#/$defs/constrainable_element\"}}},\"evidence\":{\"type\":\"array\",\"items\":{\"oneOf\":[{\"$ref\":\"#/$defs/evidence\"}]},\"minItems\":1},\"time\":{\"$ref\":\"#/$defs/datetime_element\"},\"trust_framework\":{\"$ref\":\"#/$defs/constrainable_element\"},\"verification_process\":{\"$ref\":\"#/$defs/simple_element\"}}}}}},\"properties\":{\"id_token\":{\"type\":\"object\",\"additionalProperties\":true,\"properties\":{\"verified_claims\":{\"$ref\":\"#/$defs/verified_claims\"}}},\"userinfo\":{\"type\":\"object\",\"additionalProperties\":true,\"properties\":{\"verified_claims\":{\"$ref\":\"#/$defs/verified_claims\"}}}}}"; |
There was a problem hiding this comment.
can we create this file under test/resources instead of defining this in all the test classes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.