From 9152c8197f3cdd5807174e0cce61ac599a5df535 Mon Sep 17 00:00:00 2001 From: kaifk468 <74772315+kaifk468@users.noreply.github.com> Date: Fri, 8 Sep 2023 13:46:00 +0530 Subject: [PATCH] review changes (#346) * review changes * review changes --- .../services/KeyBindingHelperService.java | 2 +- .../core/util/IdentityProviderUtil.java | 3 +- .../core/IdentityProviderUtilTest.java | 4 +- .../services/AuthorizationServiceImpl.java | 2 +- .../services/ConsentHelperService.java | 19 ++++---- .../LinkedAuthorizationServiceImpl.java | 6 +-- .../services/ConsentHelperServiceTest.java | 45 ++++--------------- 7 files changed, 25 insertions(+), 56 deletions(-) diff --git a/binding-service-impl/src/main/java/io/mosip/esignet/services/KeyBindingHelperService.java b/binding-service-impl/src/main/java/io/mosip/esignet/services/KeyBindingHelperService.java index 3460eafb0..c88c9340f 100644 --- a/binding-service-impl/src/main/java/io/mosip/esignet/services/KeyBindingHelperService.java +++ b/binding-service-impl/src/main/java/io/mosip/esignet/services/KeyBindingHelperService.java @@ -72,7 +72,7 @@ public PublicKeyRegistry storeKeyBindingDetailsInRegistry(String individualId, S publicKeyRegistry.setExpiredtimes(expireDTimes); publicKeyRegistry.setWalletBindingId(walletBindingId == null ? generateWalletBindingId(partnerSpecificUserToken) : walletBindingId); publicKeyRegistry.setCertificate(certificateData); - publicKeyRegistry.setThumbprint(IdentityProviderUtil.generateThumbprintByCertificate(certificateData)); + publicKeyRegistry.setThumbprint(IdentityProviderUtil.generateCertificateThumbprint(certificateData)); publicKeyRegistry.setCreatedtimes(LocalDateTime.now(ZoneId.of("UTC"))); publicKeyRegistry = publicKeyRegistryRepository.save(publicKeyRegistry); log.info("Saved PublicKeyRegistry details successfully"); diff --git a/esignet-core/src/main/java/io/mosip/esignet/core/util/IdentityProviderUtil.java b/esignet-core/src/main/java/io/mosip/esignet/core/util/IdentityProviderUtil.java index d21e3cb07..fd3c60dd1 100644 --- a/esignet-core/src/main/java/io/mosip/esignet/core/util/IdentityProviderUtil.java +++ b/esignet-core/src/main/java/io/mosip/esignet/core/util/IdentityProviderUtil.java @@ -240,7 +240,8 @@ public static Certificate convertToCertificate(String certData) { throw new EsignetException(ErrorConstants.INVALID_CERTIFICATE); } } - public static String generateThumbprintByCertificate(String cerifacate) + + public static String generateCertificateThumbprint(String cerifacate) { Object certObj = convertToCertificate(cerifacate); if (certObj instanceof X509Certificate) { diff --git a/esignet-core/src/test/java/io/mosip/esignet/core/IdentityProviderUtilTest.java b/esignet-core/src/test/java/io/mosip/esignet/core/IdentityProviderUtilTest.java index d4290f211..88e79b97e 100644 --- a/esignet-core/src/test/java/io/mosip/esignet/core/IdentityProviderUtilTest.java +++ b/esignet-core/src/test/java/io/mosip/esignet/core/IdentityProviderUtilTest.java @@ -167,9 +167,9 @@ public void test_getCertificateThumbprint() throws Exception { @Test public void test_generateThumbprintByCertificate()throws Exception{ String certificateString="-----BEGIN CERTIFICATE-----\nMIIDYjCCAkqgAwIBAgIIyF+UqkzoF9kwDQYJKoZIhvcNAQELBQAwgYIxCzAJBgNV\nBAYTAklOMQswCQYDVQQIDAJLQTESMBAGA1UEBwwJQkFOR0FMT1JFMQ0wCwYDVQQK\nDARJSVRCMSwwKgYDVQQLDCNNT1NJUC1URUNILUNFTlRFUiAoSURBX0tFWV9CSU5E\nSU5HKTEVMBMGA1UEAwwMd3d3Lm1vc2lwLmlvMB4XDTIzMDgyMjEyMTU1OVoXDTIz\nMTIyMDEyMTU1OVowGzEZMBcGA1UEAwwQVEVTVF9GVUxMTkFNRWVuZzCCASIwDQYJ\nKoZIhvcNAQEBBQADggEPADCCAQoCggEBANR6slnf+yDgQ8Z2oU5wcV7c7YEGx4J2\nk6RL5KxvigISwrxy3d7ZwdJvASuolzOSdspvAf32EoLjlMgCPCAUJ5VacbPa0YgX\n8srYwBdEzgecTorU9XGDQoAnAaI28JEDtQbuZJ8dMiwYk//g8QMj2xs1sw1t1A3q\n4lAfH4UxnmBNipq2p7wtn7PdOjq2TfKUkNDYx7hNbNQe75Z1KaXfN1mr02k7V0F+\nFgKScaBXtxWFZt6OoUdJQixTF9vz13Skl+J+/sCP3lyC4OptAbD0chYrvXlsAMDj\n9r0OXFPzT9pchi4ZojgGhu9HXV1REadfvVSyeysv2VSHGvuooKiyik0CAwEAAaNC\nMEAwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQUV7X8HklamaYsSF8SKH3YlCQ7\nop4wDgYDVR0PAQH/BAQDAgKEMA0GCSqGSIb3DQEBCwUAA4IBAQC+eRZJoeBFyK5P\niOSnaSeXk6hFyF9tgh7QAXho/5nJ4QgYwU+16GjatMZNuYh5VFjmrmuf7fVnHI5R\nzGj+D4T3F78CB2B5l/Fh3v73C7fU8G6th5Le4aLhpEtOOqAYpx99uiDNs07+Up59\nHdYDQrX6k9IThAa+JrYjllShGJoPJYrRFOo/amBTFjQnKxFQ1IvUfBsuqalIGulh\n4K/5H4lC8aC9U2LaP+Ncu6six/MF/OiNurF86F9uAQucuxsay7hZSZk5I+iGGkcl\nGAJCVFB37kXviV4n4m6o/YPw/xeZ4tuRlFZ7+KkWekcXt7QwdNiCpmPrPYIshTYh\n5DMsAUrK\n-----END CERTIFICATE-----\n"; - Assert.assertNotNull(IdentityProviderUtil.generateThumbprintByCertificate(certificateString)); + Assert.assertNotNull(IdentityProviderUtil.generateCertificateThumbprint(certificateString)); try { - IdentityProviderUtil.generateThumbprintByCertificate("test"); + IdentityProviderUtil.generateCertificateThumbprint("test"); Assert.fail(); } catch (EsignetException e) { Assert.assertEquals(e.getMessage(),ErrorConstants.INVALID_CERTIFICATE); diff --git a/oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java b/oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java index 4b9670b32..9c8ed5ec3 100644 --- a/oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java +++ b/oidc-service-impl/src/main/java/io/mosip/esignet/services/AuthorizationServiceImpl.java @@ -249,7 +249,7 @@ public AuthCodeResponse getAuthCode(AuthCodeRequest authCodeRequest) throws Esig transaction.setCodeHash(authorizationHelperService.getKeyHash(authCode)); transaction.setAcceptedClaims(acceptedClaims); transaction.setPermittedScopes(acceptedScopes); - consentHelperService.updateUserConsent(transaction, false, null); + consentHelperService.updateUserConsent(transaction, null); transaction = cacheUtilService.setAuthCodeGeneratedTransaction(authCodeRequest.getTransactionId(), transaction); auditWrapper.logAudit(Action.GET_AUTH_CODE, ActionStatus.SUCCESS, AuditHelper.buildAuditDto(authCodeRequest.getTransactionId(), transaction), null); diff --git a/oidc-service-impl/src/main/java/io/mosip/esignet/services/ConsentHelperService.java b/oidc-service-impl/src/main/java/io/mosip/esignet/services/ConsentHelperService.java index 0c2ea409a..f872803e5 100644 --- a/oidc-service-impl/src/main/java/io/mosip/esignet/services/ConsentHelperService.java +++ b/oidc-service-impl/src/main/java/io/mosip/esignet/services/ConsentHelperService.java @@ -58,6 +58,10 @@ public class ConsentHelperService { @Autowired private AuditPlugin auditWrapper; + private final String ACCEPTED_CLAIMS="accepted_claims"; + + private final String PERMITTED_AUTHORIZED_SCOPES="permitted_authorized_scopes"; + public void processConsent(OIDCTransaction transaction, boolean linked) { UserConsentRequest userConsentRequest = new UserConsentRequest(); userConsentRequest.setClientId(transaction.getClientId()); @@ -83,7 +87,7 @@ public void processConsent(OIDCTransaction transaction, boolean linked) { } - public void updateUserConsent(OIDCTransaction transaction, boolean linked, String signature) { + public void updateUserConsent(OIDCTransaction transaction, String signature) { if(ConsentAction.NOCAPTURE.equals(transaction.getConsentAction()) && transaction.getEssentialClaims().isEmpty() && transaction.getVoluntaryClaims().isEmpty() @@ -237,6 +241,7 @@ public boolean verifyConsentSignature (ConsentDetail consentDetail, OIDCTransact } return false; } catch (ParseException | JOSEException e) { + log.error("Failed to verify Signature ", e); throw new EsignetException(ErrorConstants.INVALID_AUTH_TOKEN); } } @@ -249,7 +254,6 @@ private String generateSignedObject (ConsentDetail consentDetail) throws ParseEx List acceptedClaims = consentDetail.getAcceptedClaims(); List permittedScopes = consentDetail.getPermittedScopes(); String jws = consentDetail.getSignature(); - if (!signatureFormatValidate(jws)) return null; String[] parts = jws.split("\\."); String header = parts[0]; @@ -260,8 +264,8 @@ private String generateSignedObject (ConsentDetail consentDetail) throws ParseEx Collections.sort(permittedScopes); Map payLoadMap = new TreeMap<>(); - payLoadMap.put("accepted_claims", acceptedClaims); - payLoadMap.put("permitted_authorized_scopes", permittedScopes); + payLoadMap.put(ACCEPTED_CLAIMS, acceptedClaims); + payLoadMap.put(PERMITTED_AUTHORIZED_SCOPES, permittedScopes); Payload payload = new Payload(new JSONObject(payLoadMap).toJSONString()); StringBuilder sb = new StringBuilder(); @@ -269,11 +273,4 @@ private String generateSignedObject (ConsentDetail consentDetail) throws ParseEx return sb.toString(); } - private boolean signatureFormatValidate (String signature) - { - if (signature == null || signature.isEmpty()) return false; - String jws[] = signature.split("\\."); - if (jws.length != 2) return false; - return true; - } } \ No newline at end of file diff --git a/oidc-service-impl/src/main/java/io/mosip/esignet/services/LinkedAuthorizationServiceImpl.java b/oidc-service-impl/src/main/java/io/mosip/esignet/services/LinkedAuthorizationServiceImpl.java index 6fa64154f..8b247bc16 100644 --- a/oidc-service-impl/src/main/java/io/mosip/esignet/services/LinkedAuthorizationServiceImpl.java +++ b/oidc-service-impl/src/main/java/io/mosip/esignet/services/LinkedAuthorizationServiceImpl.java @@ -24,8 +24,6 @@ import io.mosip.esignet.core.util.IdentityProviderUtil; import io.mosip.esignet.core.util.KafkaHelperService; import lombok.extern.slf4j.Slf4j; -import org.json.JSONException; -import org.json.JSONObject; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.data.util.Pair; @@ -241,7 +239,7 @@ public LinkedKycAuthResponseV2 authenticateUserV2(LinkedKycAuthRequest linkedKyc if(ConsentAction.NOCAPTURE.equals(transaction.getConsentAction())){ validateConsent(transaction, transaction.getAcceptedClaims(), transaction.getPermittedScopes()); cacheUtilService.setLinkedConsentedTransaction(transaction.getLinkedTransactionId(), transaction); - consentHelperService.updateUserConsent(transaction,true, ""); + consentHelperService.updateUserConsent(transaction, ""); kafkaHelperService.publish(linkedAuthCodeTopicName, transaction.getLinkedTransactionId()); } else { cacheUtilService.setLinkedAuthenticatedTransaction(linkedKycAuthRequest.getLinkedTransactionId(), transaction); @@ -286,7 +284,7 @@ public LinkedConsentResponse saveConsentV2(LinkedConsentRequestV2 linkedConsentR // cache consent only, auth-code will be generated on link-auth-code-status API call transaction.setAcceptedClaims(linkedConsentRequest.getAcceptedClaims()); transaction.setPermittedScopes(linkedConsentRequest.getPermittedAuthorizeScopes()); - consentHelperService.updateUserConsent(transaction, true, linkedConsentRequest.getSignature()); + consentHelperService.updateUserConsent(transaction, linkedConsentRequest.getSignature()); cacheUtilService.setLinkedConsentedTransaction(linkedConsentRequest.getLinkedTransactionId(), transaction); //Publish message after successfully saving the consent kafkaHelperService.publish(linkedAuthCodeTopicName, linkedConsentRequest.getLinkedTransactionId()); diff --git a/oidc-service-impl/src/test/java/io/mosip/esignet/services/ConsentHelperServiceTest.java b/oidc-service-impl/src/test/java/io/mosip/esignet/services/ConsentHelperServiceTest.java index b28d4f441..a9aca2d39 100644 --- a/oidc-service-impl/src/test/java/io/mosip/esignet/services/ConsentHelperServiceTest.java +++ b/oidc-service-impl/src/test/java/io/mosip/esignet/services/ConsentHelperServiceTest.java @@ -20,7 +20,6 @@ import io.mosip.esignet.core.dto.PublicKeyRegistry; import io.mosip.esignet.core.dto.UserConsent; import io.mosip.esignet.core.dto.UserConsentRequest; -import io.mosip.esignet.core.exception.EsignetException; import io.mosip.esignet.core.spi.ConsentService; import io.mosip.esignet.core.spi.PublicKeyRegistryService; import io.mosip.esignet.core.util.IdentityProviderUtil; @@ -61,6 +60,9 @@ public class ConsentHelperServiceTest { @Mock AuthorizationHelperService authorizationHelperService; + @Mock + AuditPlugin auditPlugin; + @InjectMocks ConsentHelperService consentHelperService; @@ -69,7 +71,7 @@ public class ConsentHelperServiceTest { private static final Certificate certificate; private static final PrivateKey privateKey; - private static String jwksString; + private static final String jwksString; static { try { @@ -104,9 +106,6 @@ public class ConsentHelperServiceTest { throw new RuntimeException(e); } } - @Mock - AuditPlugin auditHelper; - @Test public void addUserConsent_withValidLinkedTransaction_thenPass() throws Exception { @@ -140,7 +139,7 @@ public void addUserConsent_withValidLinkedTransaction_thenPass() throws Exceptio payLoadMap.put("accepted_claims",acceptedClaims); payLoadMap.put("permitted_authorized_scopes",permittedScopes); String signature = generateSignature(payLoadMap); - consentHelperService.updateUserConsent(oidcTransaction, true, signature); + consentHelperService.updateUserConsent(oidcTransaction, signature); UserConsent userConsent = new UserConsent(); userConsent.setAuthorizationScopes(Map.of("openid",false,"profile",false,"email",false)); userConsent.setHash("UrgNGrbWUB5v_oSvupBCqp7V31MJdE3nNqfGv9eazBc"); @@ -181,7 +180,7 @@ public void addUserConsent_withValidWebTransaction_thenPass() Mockito.when(consentService.saveUserConsent(Mockito.any())).thenReturn(new ConsentDetail()); - consentHelperService.updateUserConsent(oidcTransaction, false, ""); + consentHelperService.updateUserConsent(oidcTransaction, ""); UserConsent userConsent = new UserConsent(); userConsent.setHash("Cgh8oWpNM84WPYQVvluGj616_kd4z60elVXtc7R_lXw"); userConsent.setClaims(claims); @@ -207,7 +206,7 @@ public void addUserConsent_withValidWebTransactionNoClaimsAndScopes_thenPass() oidcTransaction.setPermittedScopes(List.of()); oidcTransaction.setClientId(clientId); oidcTransaction.setPartnerSpecificUserToken(psuToken); - consentHelperService.updateUserConsent(oidcTransaction, false, ""); + consentHelperService.updateUserConsent(oidcTransaction, ""); Mockito.verify(consentService).deleteUserConsent(clientId, psuToken); } @@ -256,14 +255,13 @@ public void processConsent_withWebFlowAndValidConsentAndGetConsentActionAsNoCapt List acceptedClaims = consentDetail.getAcceptedClaims(); List permittedScopes = consentDetail.getPermittedScopes(); - String jws = consentDetail.getSignature(); Collections.sort(acceptedClaims); Collections.sort(permittedScopes); Map payLoadMap = new TreeMap<>(); payLoadMap.put("accepted_claims",acceptedClaims); payLoadMap.put("permitted_authorized_scopes",permittedScopes); - String signature = generateSignature(payLoadMap);; + String signature = generateSignature(payLoadMap); consentDetail.setSignature(signature); consentDetail.setPsuToken("psutoken"); @@ -334,7 +332,6 @@ public void processConsent_withLinkedFlowAndValidConsentAndGetConsentActionAsCap List acceptedClaims = consentDetail.getAcceptedClaims(); List permittedScopes = consentDetail.getPermittedScopes(); - String jws = consentDetail.getSignature(); Collections.sort(acceptedClaims); Collections.sort(permittedScopes); Map payLoadMap = new TreeMap<>(); @@ -376,29 +373,6 @@ public void processConsent_withEmptyConsent_thenPass(){ Assert.assertEquals(oidcTransaction.getConsentAction(),ConsentAction.CAPTURE); } - @Test - public void processConsent_withInvalidConsent_thenPass(){ - - OIDCTransaction oidcTransaction=new OIDCTransaction(); - oidcTransaction.setClientId("abc"); - oidcTransaction.setPartnerSpecificUserToken("123"); - oidcTransaction.setVoluntaryClaims(List.of("email")); - oidcTransaction.setEssentialClaims(List.of()); - oidcTransaction.setRequestedAuthorizeScopes(List.of()); - UserConsentRequest userConsentRequest = new UserConsentRequest(); - userConsentRequest.setClientId(oidcTransaction.getClientId()); - userConsentRequest.setPsuToken(oidcTransaction.getPartnerSpecificUserToken()); - - ConsentDetail consentDetail=new ConsentDetail(); - consentDetail.setAcceptedClaims(Arrays.asList()); - consentDetail.setSignature("haa"); - - Mockito.when(consentService.getUserConsent(userConsentRequest)).thenReturn(Optional.of(consentDetail)); - - consentHelperService.processConsent(oidcTransaction,true); - Assert.assertEquals(oidcTransaction.getConsentAction(),ConsentAction.CAPTURE); - } - @Test public void processConsent_withInvalidIdHashOrThumbPrint_thenPass() throws Exception { @@ -444,14 +418,13 @@ public void processConsent_withInvalidIdHashOrThumbPrint_thenPass() throws Excep List acceptedClaims = consentDetail.getAcceptedClaims(); List permittedScopes = consentDetail.getPermittedScopes(); - String jws = consentDetail.getSignature(); Collections.sort(acceptedClaims); Collections.sort(permittedScopes); Map payLoadMap = new TreeMap<>(); payLoadMap.put("accepted_claims",acceptedClaims); payLoadMap.put("permitted_authorized_scopes",permittedScopes); - String signature = generateSignature(payLoadMap);; + String signature = generateSignature(payLoadMap); consentDetail.setSignature(signature); consentDetail.setPsuToken("psutoken");