Mec dev #1
Mec dev #1
Conversation
Pull Request
📝 WalkthroughWalkthroughThis pull request establishes a new standalone print service module, migrating from kernel-based implementations to a self-contained Changes
Sequence DiagramssequenceDiagram
participant Client
participant Controller as Print Controller
participant Service as PrintServiceImpl
participant CryptoUtil as Crypto Utilities
participant BioAPI as BioAPI/CBEFF
participant PDFGen as PDF Generator
participant Repository as DB Repository
Client->>Controller: POST /callback/notifyPrint (EventModel)
Controller->>Service: generateCard(eventModel)
rect rgb(200, 220, 255)
Note over Service,CryptoUtil: Credential Decryption Phase
Service->>CryptoUtil: decrypt PIN-protected credential
CryptoUtil-->>Service: decrypted JSON payload
end
rect rgb(220, 255, 220)
Note over Service,BioAPI: Biometric Extraction Phase
Service->>BioAPI: extractTemplate(BIR)
BioAPI-->>Service: extracted biometric
Service->>BioAPI: checkQuality(sample)
BioAPI-->>Service: QualityScore
end
rect rgb(255, 240, 200)
Note over Service,PDFGen: Card Generation Phase
Service->>PDFGen: generate(template, cardData)
PDFGen-->>Service: PDF bytes
Service->>PDFGen: signAndEncryptPDF(pdf, cert, password)
PDFGen-->>Service: signed PDF bytes
end
rect rgb(255, 220, 220)
Note over Service,Repository: Persistence Phase
Service->>Repository: save(MspCardEntity)
Repository-->>Service: saved entity
end
Service-->>Controller: success
Controller-->>Client: 200 OK
sequenceDiagram
participant Service as PrintServiceImpl
participant TemplateManager as Template Manager
participant QRGen as QR Code Generator
participant PDFGen as PDF Generator
participant Crypto as CryptoCoreUtil
rect rgb(240, 255, 240)
Note over Service,TemplateManager: Template Rendering
Service->>TemplateManager: merge(templateName, cardAttributes)
TemplateManager-->>Service: rendered HTML bytes
end
rect rgb(255, 250, 220)
Note over Service,QRGen: QR Code Generation
Service->>QRGen: generateQrCode(cardData, QrVersion.V25)
QRGen-->>Service: QR code PNG bytes
end
rect rgb(240, 220, 255)
Note over Service,PDFGen: PDF Creation & Signing
Service->>PDFGen: generate(templateHTML)
PDFGen-->>Service: PDF OutputStream
Service->>Crypto: loadP12() + getCertificate()
Crypto-->>Service: PrivateKeyEntry + Certificate
Service->>PDFGen: signAndEncryptPDF(pdf, cert, rect, password)
PDFGen-->>Service: signed/encrypted PDF bytes
end
Service-->>Service: encodeQR & Photo to Base64
Service-->>Service: populate MSP card attributes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Rationale: This PR introduces a substantial, heterogeneous set of changes spanning new exception hierarchies, DTOs, service implementations, cryptographic utilities, configuration migration, and a complete service refactoring from kernel-based to print-module-based architecture. While many files follow repetitive patterns (constants, DTOs, exception types), the core service implementations (PrintServiceImpl, CbeffImpl, PDFGeneratorImpl, CryptoCoreUtil) contain dense logic with multiple integrations (encryption/decryption, PDF signing, biometric processing, template rendering). The dependency migration from kernel to print modules touches numerous files, and the new printService workflow orchestrates multiple sub-systems. The presence of dynamic encryption, certificate handling, and workflow state changes across decryption, biometric extraction, and PDF generation warrants careful verification of correctness and security. Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
pom.xml (1)
130-167: Remove duplicate dependencies.Both
json-simple(lines 130-134, 160-163) andgson(lines 136-138, 165-167) are declared twice. Maven will use one declaration and ignore the other, but this creates confusion and maintenance overhead.🔎 Proposed fix
</dependency> - <dependency> - <groupId>com.googlecode.json-simple</groupId> - <artifactId>json-simple</artifactId> - <version>1.1.1</version> - </dependency> - <dependency> - <groupId>com.google.code.gson</groupId> - <artifactId>gson</artifactId> - </dependency> <dependency> <groupId>org.json</groupId> <artifactId>json</artifactId>Remove lines 160-167 to eliminate the duplicates.
src/main/java/io/mosip/print/service/PrintService.java (1)
17-19: Remove commented-out method signature.The commented-out
getDocumentsmethod should be removed. Use version control to track historical changes.🔎 Proposed fix
public void generateCard(EventModel eventModel) throws Exception; - - // Map<String, byte[]> getDocuments(String credentialSubject, String sign, - // String cardType, - // boolean isPasswordProtected); - }src/main/java/io/mosip/print/util/PrintInstializer.java (1)
67-71: Off-by-one error in retry loop.Using
i <= retryCountmeans ifretryCount=3, there will be 4 attempts (i=0,1,2,3). This is likely unintended based on typical retry semantics.🔎 Proposed fix
private void retrySubscriptions() { // Call Init Subscriptions for the count until no error in the subscription - for (int i = 0; i <= retryCount; i++) { + for (int i = 0; i < retryCount; i++) { if (initSubsriptions()) { return; } } }src/main/java/io/mosip/print/service/impl/PrintServiceImpl.java (1)
308-378: Replacee.printStackTrace()with proper logging.Multiple exception handlers use
e.printStackTrace()(lines 309, 320, 330, 341, 351, 362, 372) before logging. This outputs to stderr and bypasses the configured logging framework. The logger is already being used on subsequent lines, so remove theprintStackTrace()calls.🔎 Proposed fix (example for one block)
} catch (VidCreationException e) { - e.printStackTrace(); description.setMessage(PlatformErrorMessages.PRT_PRT_VID_CREATION_ERROR.getMessage());Apply the same pattern to all other catch blocks.
🟠 Major comments (35)
src/main/java/io/mosip/print/constant/ApiName.java-97-97 (1)
97-97: Fix typo: DECRYPTPINBASSED should be DECRYPTPINBASED.The constant name contains "BASSED" which appears to be a typo for "BASED". This should be corrected before merge, as renaming enum constants is a breaking change.
🔎 Proposed fix
- DECRYPTPINBASSED, CREATEDATASHARE, + DECRYPTPINBASED, CREATEDATASHARE,src/main/java/io/mosip/print/util/Helpers.java-19-22 (1)
19-22: Major: Null return and undocumented resource ownership.This method has several concerns:
- Null return: Returns
nullwhen the resource is not found, requiring callers to perform null checks. This is error-prone and inconsistent withreadFileFromResources, which throws an exception.- Undocumented resource ownership: The returned
InputStreammust be closed by the caller, but this critical requirement is not documented. Missing cleanup will cause resource leaks.🔎 Proposed fix for consistent error handling
- public static InputStream readStreamFromResources(String filename) { - InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename); - return inputStream; + /** + * Reads a resource file and returns its InputStream. + * The caller is responsible for closing the returned InputStream. + * + * @param filename the resource filename + * @return the InputStream for the resource + * @throws IOException if the resource is not found + */ + public static InputStream readStreamFromResources(String filename) throws IOException { + InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename); + if (inputStream == null) { + throw new IOException("Resource not found: " + filename); + } + return inputStream; }src/main/java/io/mosip/print/dto/PrintMQDetails.java-7-9 (1)
7-9: Exclude password fromtoString()to prevent credential exposure.The
@Dataannotation generates atoString()method that includes all fields, including thepasswordfield at line 17. This creates a security risk where credentials can be accidentally exposed through logging, debugging, or error messages.🔎 Proposed fix to exclude password from toString()
+import lombok.ToString; + @Data @NoArgsConstructor @AllArgsConstructor +@ToString(exclude = "password") public class PrintMQDetails {src/main/java/io/mosip/print/util/DateUtils.java-166-173 (1)
166-173: Wrong exception type being caught -java.lang.NullPointerExceptionwill not be caught.The catch clause catches
io.mosip.print.exception.NullPointerException(imported at line 35), butDateFormatUtils.format()throwsjava.lang.NullPointerException. The custom exception will never be thrown here, so null arguments will propagate as uncaughtjava.lang.NullPointerException.This same issue exists in all
formatDateandformatCalendarmethods (lines 189-196, 213-221, 234-241, 255-262, 276-283, 298-306).🔎 Proposed fix
public static String formatDate(final Date date, final String pattern) { try { return DateFormatUtils.format(date, pattern, null, null); - } catch (java.lang.IllegalArgumentException | NullPointerException e) { + } catch (java.lang.IllegalArgumentException | java.lang.NullPointerException e) { throw new IllegalArgumentException(DateUtilConstants.ILLEGALARGUMENT_ERROR_CODE.getErrorCode(), - DateUtilConstants.ILLEGALARGUMENT_ERROR_CODE.getEexceptionMessage(), e.getCause()); + DateUtilConstants.ILLEGALARGUMENT_ERROR_CODE.getExceptionMessage(), e.getCause()); } }src/main/java/io/mosip/print/util/TokenHandlerUtil.java-52-53 (1)
52-53: PotentialNullPointerExceptionifclientIdclaim is missing.If the token doesn't contain a
clientIdclaim,claims.get("clientId")returnsnull, and calling.asString()will throw NPE. Add a null check.🔎 Proposed fix
- } else if (!claims.get("clientId").asString().equals(clientId)) { + Claim clientIdClaim = claims.get("clientId"); + if (clientIdClaim == null || !clientId.equals(clientIdClaim.asString())) { return false; }src/main/java/io/mosip/print/dto/CredentialRequestDto.java-25-25 (1)
25-25: Fix the spelling of "recepiant" to "recipient".The field name
recepiantis misspelled and should berecipient. Since this DTO is likely serialized and exposed via API contracts, fixing this typo now avoids introducing a breaking change later.🔎 Proposed fix
- private String recepiant; + private String recipient;src/main/java/io/mosip/print/service/impl/UinCardGeneratorImpl.java-58-71 (1)
58-71: Update PrintServiceImpl to use the new configuration property key.The migration from
mosip.registration.processor.print.service.uincard.*tomosip.print.service.uincard.*is incomplete. PrintServiceImpl.java (line 109) still references the old property keymosip.registration.processor.print.service.uincard.password, which no longer exists in the configuration file. This will cause a NullPointerException at runtime whengetPassword()is called (line 737). Update the constant to use the new key:mosip.print.service.uincard.password.pom.xml-8-8 (1)
8-8: Downgrading Spring Boot from 2.0.9 to 2.0.2 introduces known security vulnerabilities.Spring Boot 2.0.2 (May 2018) is vulnerable to CVE-2022-27772 (temporary-directory hijacking via AbstractConfigurableWebServerFactory.createTempDir), which affects all spring-boot versions prior to 2.2.11.RELEASE. Additionally, 2.0.9 contains security fixes that were released after 2.0.2. If 2.0.x must be used, upgrade to at least 2.0.9 or preferably 2.0.14 for all 2.0.x fixes. Consider upgrading to a supported Spring Boot version, as the 2.0 branch is end-of-life.
src/main/java/io/mosip/print/init/SetupPrint.java-26-26 (1)
26-26: Removal of default value creates a required property that may break deployments.The default value of
60000was removed from the@Valueannotation. Ifmosip.event.delayis not configured in the environment, the application will fail to start with a property resolution error.Consider either:
- Restoring the default value:
@Value("${mosip.event.delay:60000}")- Documenting this as a required property in deployment guides
🔎 Proposed fix to restore the default value
- @Value("${mosip.event.delay}") + @Value("${mosip.event.delay:60000}") private int taskSubsctiptionDelay;src/main/java/io/mosip/print/exception/NullPointerException.java-12-37 (1)
12-37: Rename to avoid shadowingjava.lang.NullPointerException.This class name shadows the standard Java
NullPointerException, which will cause confusion and potential bugs. Any code in theio.mosip.print.exceptionpackage (or that imports this class) that referencesNullPointerExceptionwill resolve to this custom exception instead of the JDK class, leading to unexpected behavior.Consider renaming to
NullArgumentException,InvalidArgumentException, orArgumentNullExceptionto clearly differentiate it from the standard Java exception.🔎 Example rename to NullArgumentException
-public class NullPointerException extends BaseUncheckedException { +public class NullArgumentException extends BaseUncheckedException { /** Serializable version Id. */ private static final long serialVersionUID = 784321102100630614L; /** * Constructor with errorCode, and rootCause * * @param errorCode The error code for this exception * @param errorMessage The error message for this exception */ - public NullPointerException(String errorCode, String errorMessage) { + public NullArgumentException(String errorCode, String errorMessage) { super(errorCode, errorMessage); } /** * @param arg0 Error Code Corresponds to Particular Exception * @param arg1 Message providing the specific context of the error. * @param arg2 Cause of exception */ - public NullPointerException(String arg0, String arg1, Throwable arg2) { + public NullArgumentException(String arg0, String arg1, Throwable arg2) { super(arg0, arg1, arg2); } }src/main/java/io/mosip/print/service/CbeffContainerI.java-16-20 (1)
16-20: Replace generic Exception with specific exception types.The methods declare
throws Exception, which is overly broad and forces callers to handle all exception types indiscriminately. Consider defining specific checked exceptions (e.g.,CbeffExceptionorBIRProcessingException) to provide clearer error semantics and enable more precise exception handling.💡 Recommended approach
Define a specific exception type and update the method signatures:
public abstract U createBIRType(List<T> bir) throws CbeffException; public abstract U updateBIRType(List<T> bir, byte[] fileBytes) throws CbeffException; public abstract boolean validateXML(byte[] fileBytes, byte[] xsdBytes) throws CbeffException;src/main/java/io/mosip/print/exception/IllegalArgumentException.java-13-13 (1)
13-13: Rename to avoid name collision with java.lang.IllegalArgumentException.This class name shadows
java.lang.IllegalArgumentException, which can cause:
- Import ambiguity requiring fully-qualified names
- Unintended exception type usage
- Confusion for developers maintaining the code
- Potential for catching the wrong exception type
Rename to a domain-specific name like
InvalidArgumentException,PrintIllegalArgumentException, orMosipIllegalArgumentException.🔎 Proposed fix
-public class IllegalArgumentException extends BaseUncheckedException { +public class InvalidArgumentException extends BaseUncheckedException { /** Serializable version Id. */ private static final long serialVersionUID = 924722202110630628L; - public IllegalArgumentException(String errorCode, String errorMessage) { + public InvalidArgumentException(String errorCode, String errorMessage) { super(errorCode, errorMessage); } - public IllegalArgumentException(String errorCode, String errorMessage, Throwable cause) { + public InvalidArgumentException(String errorCode, String errorMessage, Throwable cause) { super(errorCode, errorMessage, cause); } }src/main/java/io/mosip/print/exception/InfoItem.java-23-29 (1)
23-29: Remove redundant Lombok annotations or make fields private.The fields are declared
publicwhile also annotated with@Getterand@Setter. Public fields are directly accessible, making the Lombok-generated accessors redundant. For proper encapsulation, make the fieldsprivateand retain the Lombok annotations.🔎 Proposed fix
@Getter @Setter - public String errorCode = null; + private String errorCode = null; @Getter @Setter - public String errorText = null; + private String errorText = null;src/test/java/io/mosip/print/test/controller/PrintControllerTest.java-56-62 (1)
56-62: Uncomment and properly configure the mock to validate service behavior.The mock setup on line 58 is commented out, which means this test only verifies that the endpoint returns 200 OK but doesn't validate that the
PrintService.generateCard()method is actually called or that it processes the request correctly. This significantly reduces the test's effectiveness.🔎 Proposed fix
@Test public void testHandleSubscribeEventSuccess() throws Exception { byte[] pdfbytes = "pdf".getBytes(); -// Mockito.when(printService.generateCard(Mockito.any())).thenReturn(pdfbytes); + Mockito.when(printService.generateCard(Mockito.any())).thenReturn(pdfbytes); mockMvc.perform(MockMvcRequestBuilders.post("/print/callback/notifyPrint") .contentType(MediaType.APPLICATION_JSON_VALUE).content(reqCredentialEventJson.getBytes())) .andExpect(status().isOk()); + Mockito.verify(printService, Mockito.times(1)).generateCard(Mockito.any()); }src/main/java/io/mosip/print/service/PrintDBService.java-25-39 (1)
25-39: Remove commented-out code.Large blocks of commented-out code (lines 25-39) reduce readability and should be removed. Use version control to track historical changes instead.
🔎 Proposed fix
PrintcardMCE card = new PrintcardMCE(); card.setId(obj.getId()); card.setName(obj.getName()); -// card.setProvince(obj.getProvince()); -// card.setCity(obj.getCity()); -// card.setZip(obj.getZip()); -// card.setZone(obj.getZone()); card.setRequestId(obj.getRequestId()); -// card.setRequestId1(obj.getRequestId1()); -// card.setDownloadDate(new Date()); -// card.setRegistrationDate(obj.getRegistrationDate()); -// card.setRegistrationCenterId(obj.getRegistrationCenterId()); -// card.setResident(obj.getResident()); -// card.setIntroducer(obj.getIntroducer()); -// card.setRid(obj.getRid()); - -// card.setId(obj.getId()); -// card.setBirthdate(obj.getBirthdate()); entityManager.persist(card);src/main/java/io/mosip/print/service/PrintDBService.java-21-21 (1)
21-21: Remove unused variable.The
taskStartvariable is assigned but never used, suggesting incomplete timing/logging logic.🔎 Proposed fix
try { - long taskStart = System.currentTimeMillis(); PrintcardMCE card = new PrintcardMCE();src/main/java/io/mosip/print/service/PrintDBService.java-50-52 (1)
50-52: Remove commented-out method signature.The commented-out method signature should be removed to improve code clarity.
🔎 Proposed fix
} - - // Map<String, byte[]> getDocuments(String credentialSubject, String sign, - // String cardType, - // boolean isPasswordProtected); - }src/main/java/io/mosip/print/util/CryptoUtil.java-167-223 (1)
167-223: Remove large block of commented-out code.This commented-out PDF signing implementation adds significant noise. If this code is intended for future use, track it in version control or a separate branch instead.
src/main/java/io/mosip/print/util/CryptoUtil.java-50-59 (1)
50-59: Remove commented-out debug code.This commented-out
main()method with hardcoded test data should be removed. It adds noise and could accidentally expose test credentials if uncommented.🔎 Proposed fix
- /* - * public static void main(String[] args) throws NoSuchAlgorithmException, - * InvalidKeySpecException, InvalidKeyException, - * InvalidAlgorithmParameterException, IllegalBlockSizeException, - * BadPaddingException { CryptoUtil cu = new CryptoUtil(); - * CryptoWithPinRequestDto cp = new CryptoWithPinRequestDto(); cp.setData( - * "sH0LIHnUee6wGni1stdDrO2GS_MzjwchlO6-0rmWbrduPwO4BE95xEyCDnRzy3joKz_l4UNRa64t15jWcT3dW2vI3w" - * ); cp.setUserPin("abc123"); CryptoWithPinResponseDto result = - * cu.decryptWithPin(cp); System.out.println("result: " + result); } - */src/main/java/io/mosip/print/exception/PlatformErrorMessages.java-69-72 (1)
69-72: Duplicate error code "026" used for two different errors.Both
API_NOT_ACCESSIBLE_EXCEPTIONandCERTIFICATE_THUMBPRINT_ERRORuse error codePRT_PRINT_PREFIX + "026". This will cause confusion when debugging issues. Assign a unique code toCERTIFICATE_THUMBPRINT_ERROR.🔎 Proposed fix
DATASHARE_EXCEPTION(PlatformConstants.PRT_PRINT_PREFIX + "025", "Data share api failure"), API_NOT_ACCESSIBLE_EXCEPTION(PlatformConstants.PRT_PRINT_PREFIX + "026", "Api not accessible failure"), -CERTIFICATE_THUMBPRINT_ERROR(PlatformConstants.PRT_PRINT_PREFIX + "026", "certificate thumbprint failure"), +CERTIFICATE_THUMBPRINT_ERROR(PlatformConstants.PRT_PRINT_PREFIX + "028", "certificate thumbprint failure"), PRT_INVALID_KEY_EXCEPTION(PlatformConstants.PRT_PRINT_PREFIX + "027", "invalid key");src/main/java/io/mosip/print/util/DigitalSignatureUtility.java-64-76 (1)
64-76: Potential NullPointerException after logging errors.When
response.getErrors()is non-empty, errors are logged but execution continues to line 71 whereresponse.getResponse()may be null, causing an NPE atsignResponseDto.getSignature(). Consider throwing an exception when errors are present.🔎 Proposed fix
if (response.getErrors() != null && response.getErrors().size() > 0) { response.getErrors().stream().forEach(r -> { printLogger.error(LoggerFileConstant.SESSIONID.toString(), LoggerFileConstant.UIN.toString(), "", "DigitalSignatureUtility::getDigitalSignature():: error with error message " + r.getMessage()); }); + throw new DigitalSignatureException("Digital signature API returned errors: " + + response.getErrors().stream().map(e -> e.getMessage()).collect(Collectors.joining(", "))); } +if (response.getResponse() == null) { + throw new DigitalSignatureException("Digital signature API returned null response"); +} + SignResponseDto signResponseDto = mapper.readValue(mapper.writeValueAsString(response.getResponse()), SignResponseDto.class);src/main/java/io/mosip/print/util/CryptoUtil.java-18-18 (1)
18-18:DatatypeConverteris deprecated since Java 9 and removed from the JDK in Java 11.While the project explicitly includes JAXB dependencies, use Apache Commons Codec's
Hexclass instead, which is already a project dependency. Replace withorg.apache.commons.codec.binary.Hex.encodeHexString()for hex encoding/decoding to avoid reliance on the deprecated JAXB module.src/main/java/io/mosip/print/service/impl/QrcodeGeneratorImpl.java-81-81 (1)
81-81: Verify binary string parsing handles edge cases correctly.The regex
"(?<=\\G.{8})"splits the binary string into 8-bit chunks, but there's no validation that:
- The input contains only '0' and '1' characters
- The length is a multiple of 8
Invalid input could produce incorrect characters or throw uncaught
NumberFormatException.🔎 Proposed fix: Add validation
@Override public byte[] generateQrCodeFromBinaryData(String data, QrVersion version) throws QrcodeGenerationException, IOException { QrcodegeneratorUtils.verifyInput(data, version); + if (!data.matches("[01]+")) { + throw new QrcodeGenerationException( + QrcodeExceptionConstants.INVALID_INPUT_DATA_EMPTY.getErrorCode(), + "Binary data must contain only 0 and 1 characters", + null); + } + if (data.length() % 8 != 0) { + throw new QrcodeGenerationException( + QrcodeExceptionConstants.INVALID_INPUT_DATA_EMPTY.getErrorCode(), + "Binary data length must be a multiple of 8", + null); + } StringBuilder stringBuilder = new StringBuilder(); Arrays.stream(data.split("(?<=\\G.{8})")).forEach(s -> stringBuilder.append((char) Integer.parseInt(s, 2))); return generateQrCode(stringBuilder.toString(), version); }src/main/java/io/mosip/print/util/WebSubSubscriptionHelper.java-39-39 (1)
39-39: Type safety weakened: PublisherClient now accepts Object.The generic type parameter is declared as
Objectinstead ofCredentialStatusEvent, removing compile-time type safety. Currently, onlyCredentialStatusEventis published viapb.publishUpdate(), making this over-generalization unnecessary and allowing arbitrary objects to be published at runtime.If multiple event types need to be supported in the future, consider:
- Using a base interface/class for all event types
- Creating separate publisher clients for different event types
- Using bounded wildcards if flexibility is required
Otherwise, revert the generic parameter to
CredentialStatusEventto restore type safety.src/main/java/io/mosip/print/service/impl/TemplateManagerBuilderImpl.java-17-36 (1)
17-36: Singleton@Componentwith mutable state causes shared state pollution.The class is marked as
@Component(default singleton scope) but the fluent builder methods mutate instance fields. Concurrent or repeated use of the same injected builder will share/overwrite state unexpectedly.Either:
- Use
@Scope("prototype")to get a new instance per injection, or- Have fluent methods return a new builder instance (immutable builder), or
- Remove
@Componentand instantiate manually where needed.🔎 Proposed fix (Option 1: Prototype scope)
+import org.springframework.context.annotation.Scope; + @Getter @Component +@Scope("prototype") public class TemplateManagerBuilderImpl implements TemplateManagerBuilder {src/main/java/io/mosip/print/service/impl/CbeffContainerImpl.java-22-43 (1)
22-43: Instance fieldbirTypeis not thread-safe.The
birTypefield is modified bycreateBIRTypeviaload()and used across method calls. If this class is used concurrently, this could lead to race conditions. Consider makingbirTypea local variable withincreateBIRType.🔎 Proposed fix
-public class CbeffContainerImpl extends CbeffContainerI<BIR, BIRType> { - - private BIRType birType; +public class CbeffContainerImpl extends CbeffContainerI<BIR, BIRType> { @Override public BIRType createBIRType(List<BIR> birList) { - load(); + BIRType birType = createBaseBIRType(); List<BIRType> birTypeList = new ArrayList<>(); if (birList != null && birList.size() > 0) { for (BIR bir : birList) { birTypeList.add(bir.toBIRType(bir)); } } birType.setBir(birTypeList); return birType; } - private void load() { + private BIRType createBaseBIRType() { + BIRType birType = new BIRType(); - birType = new BIRType(); BIRInfoType birInfo = new BIRInfoType(); birInfo.setIntegrity(false); birType.setBIRInfo(birInfo); + return birType; }Committable suggestion skipped: line range outside the PR's diff.
src/main/java/io/mosip/print/service/impl/BioApiImpl.java-86-97 (1)
86-97: PotentialNullPointerExceptiondue to array gaps.When
max.isPresent()returnsfalse,countis not incremented but the loop continues, leavingnullelements inmatchingScore. The subsequentArrays.stream(matchingScore).mapToDouble(Score::getInternalScore)on line 97 will throw an NPE when accessing anullelement.🔎 Proposed fix
public CompositeScore compositeMatch(BIR[] sampleList, BIR[] recordList, KeyValuePair[] flags) { - Score matchingScore[] = new Score[sampleList.length]; - int count = 0; + List<Score> matchingScores = new ArrayList<>(); for (BIR sampleValue : sampleList) { Score[] match = match(sampleValue, recordList, flags); Optional<Score> max = Arrays.stream(match).max(Comparator.comparing(Score::getInternalScore)); - if (max.isPresent()) { - matchingScore[count] = max.get(); - count++; - } + max.ifPresent(matchingScores::add); } + Score[] matchingScore = matchingScores.toArray(new Score[0]); + if (matchingScore.length == 0) { + CompositeScore compositeScore = new CompositeScore(); + compositeScore.setIndividualScores(matchingScore); + compositeScore.setInternalScore(0L); + compositeScore.setScaledScore(0L); + return compositeScore; + } double sum = Arrays.stream(matchingScore).mapToDouble(Score::getInternalScore).sum(); CompositeScore compositeScore = new CompositeScore(); compositeScore.setIndividualScores(matchingScore);Also add import:
import java.util.ArrayList; import java.util.List;src/main/java/io/mosip/print/service/impl/TemplateManagerImpl.java-77-77 (1)
77-77: Returningnullon failure allowsNullPointerExceptionto occur in callers.The
merge(InputStream, Map)method returnsnullwhencontextis null (line 60) or when the merge operation fails (line 62). The caller inTemplateGenerator.getTemplate()assigns this result directly tofileTextStreamwithout null validation (line 106) and returns it unconditionally (line 110), violating the interface contract that specifies returning "merged template content asInputStream". Either throw an exception for failure cases or return an emptyInputStream.src/main/java/io/mosip/print/util/CryptoCoreUtil.java-183-200 (1)
183-200: Silent exception swallowing insymmetricDecrypt.The catch block at lines 196-198 silently swallows exceptions and returns null, which will cause NPE or unexpected behavior upstream. This makes debugging cryptographic failures extremely difficult.
🔎 Proposed fix
private static byte[] symmetricDecrypt(SecretKey key, byte[] data, byte[] aad) { byte[] output = null; try { Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5Padding"); byte[] randomIV = Arrays.copyOfRange(data, data.length - cipher.getBlockSize(), data.length); SecretKeySpec keySpec = new SecretKeySpec(key.getEncoded(), "AES"); GCMParameterSpec gcmParameterSpec = new GCMParameterSpec(128, randomIV); cipher.init(Cipher.DECRYPT_MODE, keySpec, gcmParameterSpec); if (aad != null && aad.length != 0) { cipher.updateAAD(aad); } output = cipher.doFinal(Arrays.copyOf(data, data.length - cipher.getBlockSize())); } catch (Exception e) { - + throw new CryptoManagerException("Symmetric decryption failed", e.getMessage(), e); } return output; }Committable suggestion skipped: line range outside the PR's diff.
src/main/java/io/mosip/print/util/CryptoCoreUtil.java-108-111 (1)
108-111: Avoide.printStackTrace()- use proper logging.Using
printStackTrace()writes to stderr and is not captured by logging frameworks. Log the exception properly and don't throw a generic exception with a vague message.🔎 Proposed fix
+ private static final Logger logger = LoggerFactory.getLogger(CryptoCoreUtil.class); + // In decryptData method: } catch (Exception e) { - e.printStackTrace(); + logger.error("Decryption failed", e); + throw new CryptoManagerException("Decryption failed", e.getMessage(), e); } - throw new Exception("Not able to decrypt the data.");Committable suggestion skipped: line range outside the PR's diff.
src/test/java/io/mosip/print/test/util/UinCardGeneratorImplTest.java-69-101 (1)
69-101: Multiple issues: resource leaks and incorrect mock setup.
FileInputStreamat line 72 is never closed and is consumed before being passed to the mock (line 81).buffer.toString()(line 84) produces[B@hashcodeinstead of actual data - usenew String(buffer)or Base64 encoding.- The stream is consumed in the while loop before being used in the mock, so
pdfGenerator.generate(is)will receive an exhausted stream.🔎 Proposed fix
@Test public void testCardGenerationSuccess() throws IOException, ApisResourceAccessException { ClassLoader classLoader = getClass().getClassLoader(); String inputFile = classLoader.getResource("csshtml.html").getFile(); - InputStream is = new FileInputStream(inputFile); - - byte[] buffer = new byte[8192]; - int bytesRead; - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - while ((bytesRead = is.read(buffer)) != -1) { - outputStream.write(buffer, 0, bytesRead); - } + byte[] fileContent; + try (InputStream is = new FileInputStream(inputFile)) { + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + byte[] buffer = new byte[8192]; + int bytesRead; + while ((bytesRead = is.read(buffer)) != -1) { + outputStream.write(buffer, 0, bytesRead); + } + fileContent = outputStream.toByteArray(); + } - Mockito.when(pdfGenerator.generate(is)).thenReturn(outputStream); + ByteArrayOutputStream pdfOutput = new ByteArrayOutputStream(); + pdfOutput.write(fileContent); + Mockito.when(pdfGenerator.generate(any(InputStream.class))).thenReturn(pdfOutput); ResponseWrapper<SignatureResponseDto> responseWrapper = new ResponseWrapper<>(); SignatureResponseDto signatureResponseDto = new SignatureResponseDto(); - signatureResponseDto.setData(buffer.toString()); + signatureResponseDto.setData(java.util.Base64.getEncoder().encodeToString(fileContent)); responseWrapper.setResponse(signatureResponseDto);Committable suggestion skipped: line range outside the PR's diff.
src/main/java/io/mosip/print/entity/MspCardEntity.java-23-29 (1)
23-29: Remove@Componentfrom JPA entity and addserialVersionUID.The
@Componentannotation is inappropriate for a JPA entity. Entities are managed by the persistence context, not as Spring beans. Additionally,Serializablerequires aserialVersionUIDconstant.Note: There is a
PrintcardMCEentity insrc/main/java/io/mosip/print/model/PrintcardMCE.javaalso mapped tomsp_card. WhileMspCardEntityspecifies schema as"print"andPrintcardMCEdoes not, having two entity classes for the same logical table introduces unnecessary complexity and field inconsistencies (e.g.,jsonDatavsname, type differences fordownloadDateandregistrationDate). Consider consolidating them into a single entity.src/main/java/io/mosip/print/service/impl/CbeffImpl.java-195-206 (1)
195-206: Remove commented-out test code before merging.This commented-out
mainmethod with a hardcoded local file path (C:\\Users\\M1046464\\...) should be removed. Debug/test artifacts shouldn't be committed.🔎 Proposed fix
- //TODO for testing, will be removed later -// public static void main(String arg[]) throws Exception -// { -// Map<String,String> test= new CbeffImpl().getBDBBasedOnType(readCreatedXML(),"Iris",null); -// System.out.println(test.size()); -// -// } -// -// private static byte[] readCreatedXML() throws IOException { -// byte[] fileContent = Files.readAllBytes(Paths.get("C:\\Users\\M1046464\\Downloads\\cbeff1.xml")); -// return fileContent; -// } }src/main/java/io/mosip/print/service/impl/CbeffImpl.java-52-57 (1)
52-57:loadXSDmay block indefinitely and lacks error handling.The URL connection has no timeout configured. If the config server is unresponsive, this
@PostConstructmethod will block application startup indefinitely. Consider adding connection/read timeouts and logging the failure.🔎 Proposed fix with timeout and error handling
@PostConstruct public void loadXSD() throws IOException { - try (InputStream xsdBytes = new URL(configServerFileStorageURL + schemaName).openStream()) { + URL url = new URL(configServerFileStorageURL + schemaName); + java.net.URLConnection connection = url.openConnection(); + connection.setConnectTimeout(10000); // 10 seconds + connection.setReadTimeout(30000); // 30 seconds + try (InputStream xsdBytes = connection.getInputStream()) { xsd = IOUtils.toByteArray(xsdBytes); + } catch (IOException e) { + // Log and rethrow or handle gracefully + throw new IOException("Failed to load XSD from " + url, e); } }src/main/java/io/mosip/print/service/impl/PrintServiceImpl.java-236-241 (1)
236-241:getSignaturemay throwArrayIndexOutOfBoundsExceptionif input format is unexpected.The method splits on
"."and accesses indices[0]and[2]without validating the array length. If the signature doesn't contain at least 3 parts, this will throw an exception.🔎 Proposed fix with validation
private String getSignature(String sign, String crdential) { + if (sign == null) { + throw new IllegalArgumentException("Signature cannot be null"); + } String[] parts = sign.split("\\."); + if (parts.length < 3) { + throw new IllegalArgumentException("Invalid signature format: expected at least 3 parts separated by '.'"); + } - String signHeader = sign.split("\\.")[0]; - String signData = sign.split("\\.")[2]; + String signHeader = parts[0]; + String signData = parts[2]; String signature = signHeader + "." + crdential + "." + signData; return signature; }
🟡 Minor comments (30)
src/main/java/io/mosip/print/constant/DateUtilConstants.java-28-30 (1)
28-30: Fix typo in getter method name.
getEexceptionMessagehas a duplicate 'e' and should begetExceptionMessage. This typo will propagate to all callers and create an inconsistent API.🔎 Proposed fix
- public String getEexceptionMessage() { + public String getExceptionMessage() { return exceptionMessage; }src/main/java/io/mosip/print/util/DateUtils.java-78-79 (1)
78-79: Javadoc references non-existent packageio.mosip.kernel.core.exception.Multiple
@throwsclauses referenceio.mosip.kernel.core.exception.IllegalArgumentExceptionandio.mosip.kernel.core.exception.ParseException, but this module usesio.mosip.print.exception. Update the Javadoc to reference the correct package.Also applies to: 99-100, 120-121, 141-142, 162-164, 185-187, 209-211, 230-232, 251-253, 272-274, 294-296, 320-326, 349-355, 379-385, 409-415, 441-447, 471-477, 501-507, 531-537, 710-714, 753-760, 784-788, 813-817, 840-845
src/main/java/io/mosip/print/constant/DateUtilConstants.java-6-6 (1)
6-6: Fix spelling error in message string."occours" should be "occurs".
🔎 Proposed fix
- PARSE_EXCEPTION_ERROR_CODE("KER-UTL-103", "Parsing error occours"); + PARSE_EXCEPTION_ERROR_CODE("KER-UTL-103", "Parsing error occurs");src/main/java/io/mosip/print/util/DateUtils.java-866-870 (1)
866-870: Passinge.getCause()loses the original exception context.When wrapping exceptions, passing
e.getCause()instead ofeloses the immediate exception's message and stack trace. This makes debugging harder. Useedirectly as the cause.🔎 Proposed fix
} catch (Exception e) { throw new io.mosip.print.exception.ParseException( DateUtilConstants.PARSE_EXCEPTION_ERROR_CODE.getErrorCode(), - DateUtilConstants.PARSE_EXCEPTION_ERROR_CODE.getEexceptionMessage(), e.getCause()); + DateUtilConstants.PARSE_EXCEPTION_ERROR_CODE.getExceptionMessage(), e); }src/main/java/io/mosip/print/util/TokenHandlerUtil.java-58-62 (1)
58-62: PotentialNullPointerExceptionin logging and consider using parameterized logging.
e.getMessage()can returnnull, causing.concat()to throw NPE. Also, string concatenation in log statements is inefficient. Use parameterized logging instead.🔎 Proposed fix
} catch (JWTDecodeException e) { - LOGGER.error("JWT DECODE EXCEPTION ::" .concat(e.getMessage()).concat(ExceptionUtils.getStackTrace(e))); + LOGGER.error("JWT DECODE EXCEPTION :: {}", e.getMessage(), e); return false; } catch (Exception e) { - LOGGER.error(e.getMessage().concat(ExceptionUtils.getStackTrace(e))); + LOGGER.error("Token validation failed: {}", e.getMessage(), e); return false; }src/main/java/io/mosip/print/dto/CryptoWithPinRequestDto.java-1-6 (1)
1-6: Remove or populate the empty comment block.The file header contains an empty comment block. Either remove it or add appropriate copyright/license information.
src/main/java/io/mosip/print/service/impl/PrintImpl.java-16-22 (1)
16-22: Remove trailing empty lines.Multiple blank lines at the end of the file are unnecessary and should be removed to maintain code cleanliness.
.github/workflows/manual_push_trigger.yml-15-15 (1)
15-15: Update GitHub Actions to current versions.The actions
actions/checkout@v2,actions/setup-java@v1, andactions/cache@v1use deprecated runners that may not execute on current GitHub Actions infrastructure.🔎 Proposed fix
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up JDK 11 - uses: actions/setup-java@v1 + uses: actions/setup-java@v4 with: ref: ${{ github.ref }} java-version: 11 server-id: ossrh # Value of the distributionManagement/repository/id field of the pom.xml settings-path: ${{ github.workspace }} # location for the settings.xml file - name: Setup branch and env run: | # Strip git ref prefix from version echo "BRANCH_NAME=$(echo ${{ github.ref }} | sed -e 's,.*/\(.*\),\1,')" >> $GITHUB_ENV echo "GPG_TTY=$(tty)" >> $GITHUB_ENV - name: Setup branch and GPG public key run: | # Strip git ref prefix from version echo ${{ env.BRANCH_NAME }} echo ${{ env.GPG_TTY }} sudo apt-get --yes install gnupg2 # gpg2 --import ./.github/keys/mosipgpgkey_pub.gpg # gpg2 --quiet --batch --passphrase=${{secrets.gpg_secret}} --allow-secret-key-import --import ./.github/keys/mosipgpgkey_sec.gpg - - uses: actions/cache@v1 + - uses: actions/cache@v4 with: path: ~/.m2/repositoryBased on learnings, GitHub Actions should use up-to-date action versions to ensure compatibility with current runners.
Also applies to: 17-17, 38-38
src/main/java/io/mosip/print/service/impl/UinCardGeneratorImpl.java-134-140 (1)
134-140: Replace printStackTrace() with logger.Calling
e.printStackTrace()on Line 135 bypasses the logging framework and writes directly to stderr, which is not captured by centralized logging systems. This is inconsistent with the structured logging approach used elsewhere in this class.🔎 Proposed fix
} - catch (ApisResourceAccessException e) { - e.printStackTrace(); - printLogger.error(LoggerFileConstant.SESSIONID.toString(), + catch (ApisResourceAccessException e) { + printLogger.error(LoggerFileConstant.SESSIONID.toString(), LoggerFileConstant.REGISTRATIONID.toString(), "", PlatformErrorMessages.PRT_PRT_PDF_SIGNATURE_EXCEPTION.name() + e.getMessage() + ExceptionUtils.getStackTrace(e)); throw new PDFSignatureException( e.getMessage() + ExceptionUtils.getStackTrace(e)); }src/main/java/io/mosip/print/model/MatchDecision.java-5-7 (1)
5-7: Fix the incorrect Javadoc comment.The Javadoc comment says "The Class Score" but the actual class name is
MatchDecision. This appears to be a copy-paste error.🔎 Proposed fix
/** - * The Class Score. + * The Class MatchDecision. * * @author Manoj SP */src/main/java/io/mosip/print/dto/BaseResponseDTO.java-29-35 (1)
29-35: Incomplete null handling insetErrorsmethod.When
errorsListisnull, theerrorsfield is not updated and retains its previous value. This could lead to stale error data remaining in the response object when the intent is to clear errors.🔎 Proposed fix
public void setErrors(List<Errors> errorsList) { if (errorsList != null) { errors = new Errors[errorsList.size()]; for (int i = 0; i < errorsList.size(); i++) errors[i] = errorsList.get(i); + } else { + errors = null; } }src/main/java/io/mosip/print/exception/PDFGeneratorException.java-15-21 (1)
15-21: Fix Javadoc inconsistency.The Javadoc references "PDFGeneratorGenericException" but the actual class name is "PDFGeneratorException". Update the documentation to match the class name.
🔎 Proposed fix
/** - * Constructor for PDFGeneratorGenericException + * Constructor for PDFGeneratorException * * @param errorCode The errorcode * @param errorMessage The errormessage * @param cause The cause *//** - * Constructor for PDFGeneratorGenericException + * Constructor for PDFGeneratorException * * @param errorCode The errorcode * @param errorMessage The errormessage */Also applies to: 26-31
src/main/java/io/mosip/print/constant/TemplateManagerConstant.java-13-13 (1)
13-13: Fix typo in enum constant name.The constant
TEMPATE_NAME_NULLhas a typo—"TEMPATE" should be "TEMPLATE" for consistency with the other constants and the error message.🔎 Proposed fix
WRITER_NULL("Writer cannot be null"), TEMPLATE_VALUES_NULL("Values cannot be null, it requires process template"), TEMPLATE_INPUT_STREAM_NULL("Template cannot be null"), ENCODING_TYPE_NULL("Encoding type cannot be null"), - TEMPATE_NAME_NULL("Template name cannot be null"); + TEMPLATE_NAME_NULL("Template name cannot be null");src/main/java/io/mosip/print/constant/HubMode.java-39-41 (1)
39-41: Fix getter method naming to follow JavaBeans conventions.The getter method name
gethubModeValue()violates Java naming conventions. The 'M' in 'Mode' should be capitalized.🔎 Proposed fix
- public String gethubModeValue() { + public String getHubModeValue() { return this.hubModeValue; }src/test/java/io/mosip/print/test/controller/PrintControllerTest.java-49-49 (1)
49-49: Replace deprecated MockitoAnnotations.initMocks() API.
MockitoAnnotations.initMocks(this)is deprecated. UseMockitoAnnotations.openMocks(this)instead, which returns anAutoCloseablethat should be closed after tests.🔎 Proposed fix
+ private AutoCloseable closeable; + @Before public void setup() throws Exception { - MockitoAnnotations.initMocks(this); + closeable = MockitoAnnotations.openMocks(this); this.mockMvc = MockMvcBuilders.standaloneSetup(printController).build(); EventModel credentialEvent = new EventModel(); reqCredentialEventJson = gson.toJson(credentialEvent); }Add an
@Aftermethod to close the resources:@After public void tearDown() throws Exception { if (closeable != null) { closeable.close(); } }src/main/java/io/mosip/print/constant/TemplateManagerExceptionCodeConstant.java-12-12 (1)
12-12: Fix typo in user-facing error message."occured" should be spelled "occurred".
🔎 Proposed fix
- TEMPLATE_PARSING("KER-TEM-003", "Exception occured during template processing"), + TEMPLATE_PARSING("KER-TEM-003", "Exception occurred during template processing"),src/main/java/io/mosip/print/exception/CryptoManagerException.java-30-38 (1)
30-38: Fix misleading Javadoc and remove unused parameter.The Javadoc refers to "access denied exception" but this is
CryptoManagerException. Additionally, themessage1parameter is unused.🔎 Proposed fix
/** - * Instantiates a new access denied exception. + * Instantiates a new crypto manager exception. * * @param message the message * @param cause the cause */ - public CryptoManagerException(String message, String message1, Throwable cause) { + public CryptoManagerException(String message, Throwable cause) { super(PlatformErrorMessages.PRT_AUT_ACCESS_DENIED.getCode(), message, cause); }src/main/java/io/mosip/print/exception/CryptoManagerException.java-26-28 (1)
26-28: Remove unused parametermessage1.The
message1parameter is declared but never used in the constructor body. This creates confusion and should either be used or removed.🔎 Proposed fix
- public CryptoManagerException(String message, String message1) { + public CryptoManagerException(String message) { super(PlatformErrorMessages.PRT_AUT_ACCESS_DENIED.getCode(), message); }src/main/java/io/mosip/print/util/Utilities.java-4-7 (1)
4-7: Remove unused imports.The imports
InputStream,MalformedURLException,URL,StandardCharsets, andIOUtilsare not used in the code and should be removed.src/test/java/io/mosip/print/test/service/impl/PrintServiceTest.java-118-125 (1)
118-125: Test is ignored, providing no coverage.The only test method
testQrcodegeneration()is annotated with@Ignore, meaning no tests will actually execute. This defeats the purpose of having test scaffolding. Either implement the test properly or add a TODO comment explaining when it will be enabled.src/main/java/io/mosip/print/constant/PDFGeneratorExceptionCodeConstant.java-26-35 (1)
26-35: Fix copy-pasted Javadoc.The constructor Javadoc references
UINErrorConstants Enum(line 27), but this isPDFGeneratorExceptionCodeConstant.🔎 Proposed fix
/** - * Constructor for UINErrorConstants Enum. + * Constructor for PDFGeneratorExceptionCodeConstant Enum. * * @param errorCode the error code. * @param errorMessage the error message. */src/main/java/io/mosip/print/constant/WebSubClientErrorCode.java-65-65 (1)
65-65: Fix typo in Javadoc.The Javadoc has an extra character:
@return {@link #errorMessage}rshould be@return {@link #errorMessage}.🔎 Proposed fix
/** * Function to get the error message * - * @return {@link #errorMessage}r + * @return {@link #errorMessage} */ public String getErrorMessage() {src/main/java/io/mosip/print/constant/WebSubClientErrorCode.java-21-25 (1)
21-25: Fix typos in enum constant names and messages.Three issues:
- Lines 21, 23, 25:
AUTHENTTICATEDshould beAUTHENTICATED(remove extra 'T')- Line 25:
"maching"should be"matching"These typos reduce code clarity and may hinder searchability.
🔎 Proposed fix
- AUTHENTTICATED_CONTENT_VERIFICATION_HEADER_ERROR("KER-WSC-106", "Error occured while verifing authenticated content :- header for signature is empty or null"), + AUTHENTICATED_CONTENT_VERIFICATION_HEADER_ERROR("KER-WSC-106", "Error occured while verifying authenticated content :- header for signature is empty or null"), - AUTHENTTICATED_CONTENT_ANNOTATION_SECRET_ERROR("KER-WSC-107", "Error occured while verifing authenticated content :- secret parameter for annotation preauthenticatecontent is empty"), + AUTHENTICATED_CONTENT_ANNOTATION_SECRET_ERROR("KER-WSC-107", "Error occured while verifying authenticated content :- secret parameter for annotation preauthenticatecontent is empty"), - AUTHENTTICATED_CONTENT_ERROR("KER-WSC-108", "Error occured while verifing authenticated content :- content signature is not maching"), + AUTHENTICATED_CONTENT_ERROR("KER-WSC-108", "Error occured while verifying authenticated content :- content signature is not matching"),Note: Also consider fixing
"Error occured"→"Error occurred"and"verifing"→"verifying"throughout.src/main/java/io/mosip/print/exception/ExceptionUtils.java-70-75 (1)
70-75: Remove unused private methodgetJsonValue.The method has no references in the codebase and is dead code.
- /** - * This method provide jsonvalue based on propname mention. - * - * @param node the jsonnode. - * @param propName the property name. - * @return the property value. - */ - private static String getJsonValue(JsonNode node, String propName) { - if (node.get(propName) != null) { - return node.get(propName).asText(); - } - return null; - } -Committable suggestion skipped: line range outside the PR's diff.
src/main/java/io/mosip/print/constant/QrcodeExceptionConstants.java-12-12 (1)
12-12: Typo in error message: "occured" should be "occurred".🔎 Proposed fix
- QRCODE_GENERATION_EXCEPTION("KER-QRG-001", "exception occured while writing QR code "), + QRCODE_GENERATION_EXCEPTION("KER-QRG-001", "exception occurred while writing QR code"),src/main/java/io/mosip/print/entity/MspCardEntity.java-67-71 (1)
67-71: Type mismatch with existing entity for date fields.The
registrationDateanddownloadDatefields useLocalDateTime, whilePrintcardMCEusesTimestampandDaterespectively for the same columns. This inconsistency could cause data conversion issues.src/test/java/io/mosip/print/test/util/UinCardGeneratorImplTest.java-93-100 (1)
93-100: Resource leak in FileOutputStream.The
FileOutputStreamis not properly closed if an exception occurs between lines 93-96. Use try-with-resources.🔎 Proposed fix
- FileOutputStream op = new FileOutputStream(OutPutPdfFile); - op.write(bos); - op.flush(); - assertTrue(OutPutPdfFile.exists()); - if (op != null) { - op.close(); - } - OutPutPdfFile.delete(); + try (FileOutputStream op = new FileOutputStream(OutPutPdfFile)) { + op.write(bos); + op.flush(); + } + assertTrue(OutPutPdfFile.exists()); + OutPutPdfFile.delete();src/main/java/io/mosip/print/util/CbeffToBiometricUtil.java-71-72 (1)
71-72: SLF4J logger doesn't support this multi-argument pattern.The logging calls use a pattern like
logger.debug(arg1, arg2, arg3, message)which doesn't work with SLF4J's standard parameterized logging. The extra arguments are likely being ignored.🔎 Proposed fix using standard SLF4J pattern
- printLogger.debug(LoggerFileConstant.SESSIONID.toString(), LoggerFileConstant.USERID.toString(), "", - "CbeffToBiometricUtil::getImageBytes()::entry"); + printLogger.debug("CbeffToBiometricUtil::getImageBytes()::entry - sessionId={}, userId={}", + LoggerFileConstant.SESSIONID.toString(), LoggerFileConstant.USERID.toString());src/main/java/io/mosip/print/util/PrintInstializer.java-81-86 (1)
81-86: Exception not logged in error handler.The caught exception
eis not included in the log, making debugging difficult.🔎 Proposed fix
} catch (Exception e) { - logger.error(LoggerFileConstant.SESSIONID.toString(), PRINTINSTIALIZER, ONAPPLICATIONEVENT, - "Initializing subscribptions failed: "); + logger.error("Initializing subscriptions failed", e); return false; }Committable suggestion skipped: line range outside the PR's diff.
src/main/java/io/mosip/print/service/impl/PrintServiceImpl.java-530-537 (1)
530-537: Remove the unusedgenerateQrCodeprivate method.This method is never called anywhere in the codebase and simply duplicates the logic from
setQrCode. It should be deleted.
| run: | | ||
| IMAGE_ID=$NAMESPACE/$SERVICE_NAME | ||
| # Change all uppercase to lowercase | ||
| IMAGE_ID=$(echo $IMAGE_ID | tr '[A-Z]' '[a-z]') | ||
| echo "push version ${{steps.getPomVersion.outputs.info}}" | ||
| VERSION=$BRANCH_NAME |
There was a problem hiding this comment.
Fix undefined property reference.
Line 72 references ${{steps.getPomVersion.outputs.info}}, but there is no step with id getPomVersion defined in this workflow. This will cause the workflow to fail or use an empty string.
🔎 Proposed fix
Either remove the unused echo statement referencing the undefined property:
- name: Push image
run: |
IMAGE_ID=$NAMESPACE/$SERVICE_NAME
# Change all uppercase to lowercase
IMAGE_ID=$(echo $IMAGE_ID | tr '[A-Z]' '[a-z]')
- echo "push version ${{steps.getPomVersion.outputs.info}}"
VERSION=$BRANCH_NAMEOr add the missing step to extract version from pom.xml if needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| run: | | |
| IMAGE_ID=$NAMESPACE/$SERVICE_NAME | |
| # Change all uppercase to lowercase | |
| IMAGE_ID=$(echo $IMAGE_ID | tr '[A-Z]' '[a-z]') | |
| echo "push version ${{steps.getPomVersion.outputs.info}}" | |
| VERSION=$BRANCH_NAME | |
| run: | | |
| IMAGE_ID=$NAMESPACE/$SERVICE_NAME | |
| # Change all uppercase to lowercase | |
| IMAGE_ID=$(echo $IMAGE_ID | tr '[A-Z]' '[a-z]') | |
| VERSION=$BRANCH_NAME |
🧰 Tools
🪛 actionlint (1.7.9)
67-67: property "getpomversion" is not defined in object type {}
(expression)
🤖 Prompt for AI Agents
In .github/workflows/manual_push_trigger.yml around lines 67 to 73, the echo
uses an undefined step id `${{steps.getPomVersion.outputs.info}}` which will be
empty/fail; remove or replace that reference — either delete the echo line
entirely or change it to reference an existing step output/env var that actually
provides the POM version (or add a step with id `getPomVersion` that extracts
the version into `outputs.info`) so the workflow no longer references a
nonexistent output.
| <groupId>org.apache.httpcomponents</groupId> | ||
| <artifactId>httpclient</artifactId> | ||
| <version>4.5.8</version> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Apache HttpClient 4.5.8 security vulnerabilities CVE
💡 Result:
Apache HttpClient 4.5.8 — known/security CVEs
- CVE-2020-13956: improper handling of malformed authority in request URIs (can cause requests to be sent to the wrong host). Affects HttpClient < 4.5.13 — fixed in 4.5.13. [1][2]
Notes:
- Public vulnerability listings for httpclient 4.5.8 show CVE-2020-13956 as the direct CVE; some sources also list additional older TLS/hostname-check issues in related components but those are not specific to 4.5.8 itself. [2][3]
Recommendation:
- Upgrade to at least httpclient 4.5.13 (or the latest 4.5.x / 5.x release) to address CVE-2020-13956. Verify transitive dependencies for other CVEs. [1][2][3]
Sources:
[1] NVD entry — CVE-2020-13956.
[2] CVE Details — CVE-2020-13956 / Apache HttpClient 4.5.8.
[3] Maven Central / MVNRepository listing for httpclient 4.5.8 (vulnerabilities note).
Upgrade httpclient to 4.5.13 or later to address CVE-2020-13956.
Apache HttpClient 4.5.8 contains CVE-2020-13956, which allows improper handling of malformed authority in request URIs, potentially causing requests to be sent to the wrong host. Upgrade to version 4.5.13 or later to resolve this vulnerability.
🤖 Prompt for AI Agents
In pom.xml around lines 256 to 259, the project currently depends on
org.apache.httpcomponents:httpclient:4.5.8 which is vulnerable (CVE-2020-13956);
update the version to 4.5.13 or later in the dependency entry, ensure any
dependencyManagement or parent BOM entries are aligned (or override them) so the
upgraded version is used, then run mvn -U clean install (and mvn
dependency:tree) to confirm the newer httpclient is resolved and run tests to
verify no regressions.
| public static final int FINGER_FORMAT_IDENTIFIER = 0x46495200; | ||
|
|
||
| public static final int IRIS_FORMAT_IDENTIFIER = 0x49495200; | ||
|
|
||
| public static final int FACE_FORMAT_IDENTIFIER = 0x46495200; | ||
|
|
||
| // TODO Actual face identifier waiting for Face ISO image | ||
| // public static final int FACE_FORMAT_IDENTIFIER = 0x46414300; |
There was a problem hiding this comment.
Fix duplicate face format identifier.
Lines 27 and 31 both define 0x46495200 (Finger Image Record format). The commented alternative on line 34 (0x46414300, Face format) should be used for FACE_FORMAT_IDENTIFIER instead, as CBEFF face and finger identifiers must be distinct. This duplicate will cause incorrect behavior when processing face biometric data.
🔎 Proposed fix
public static final int FINGER_FORMAT_IDENTIFIER = 0x46495200;
public static final int IRIS_FORMAT_IDENTIFIER = 0x49495200;
- public static final int FACE_FORMAT_IDENTIFIER = 0x46495200;
+ public static final int FACE_FORMAT_IDENTIFIER = 0x46414300;
-
- // TODO Actual face identifier waiting for Face ISO image
- // public static final int FACE_FORMAT_IDENTIFIER = 0x46414300;🤖 Prompt for AI Agents
In src/main/java/io/mosip/print/constant/CbeffConstant.java around lines 27 to
34, the FACE_FORMAT_IDENTIFIER is incorrectly set to the same value as
FINGER_FORMAT_IDENTIFIER (0x46495200); replace the duplicate with the correct
face identifier value 0x46414300 (as shown in the commented TODO) so that face
and finger CBEFF identifiers are distinct, and remove or update the TODO/comment
accordingly to reflect the change.
| public String getErrorCode() { | ||
| return infoItems.get(0).errorCode; | ||
| } | ||
|
|
||
| /** | ||
| * Return the last exception message. | ||
| * | ||
| * @return the last exception message | ||
| */ | ||
| public String getErrorText() { | ||
| return infoItems.get(0).errorText; | ||
| } |
There was a problem hiding this comment.
getErrorCode() and getErrorText() will throw IndexOutOfBoundsException when infoItems is empty.
When the exception is constructed using the no-arg or single-string constructors (lines 36-47), infoItems remains empty, causing these methods to fail.
🔎 Proposed fix
public String getErrorCode() {
+ if (infoItems.isEmpty()) {
+ return null;
+ }
return infoItems.get(0).errorCode;
}
/**
* Return the last exception message.
*
* @return the last exception message
*/
public String getErrorText() {
+ if (infoItems.isEmpty()) {
+ return getMessage();
+ }
return infoItems.get(0).errorText;
}🤖 Prompt for AI Agents
In src/main/java/io/mosip/print/exception/BaseCheckedException.java around lines
131-142, getErrorCode() and getErrorText() currently index infoItems.get(0)
which throws IndexOutOfBoundsException when infoItems is empty; update both
methods to first check if infoItems is non-empty and return a safe default
(e.g., null or empty string) when empty, or alternatively ensure the
no-arg/single-string constructors populate infoItems with a default InfoItem;
implement the simpler guard: if infoItems == null || infoItems.isEmpty() return
null (or ""), else return infoItems.get(0).errorCode / .errorText respectively.
| public String getErrorCode() { | ||
| return infoItems.get(0).errorCode; | ||
| } | ||
|
|
||
| /** | ||
| * Return the last exception message. | ||
| * | ||
| * @return the last exception message | ||
| */ | ||
| public String getErrorText() { | ||
| return infoItems.get(0).errorText; | ||
| } |
There was a problem hiding this comment.
getErrorCode() and getErrorText() will throw IndexOutOfBoundsException when infoItems is empty.
Same issue as in BaseCheckedException: when using no-arg or single-string constructors, infoItems remains empty, causing these methods to fail.
🔎 Proposed fix
public String getErrorCode() {
+ if (infoItems.isEmpty()) {
+ return null;
+ }
return infoItems.get(0).errorCode;
}
/**
* Return the last exception message.
*
* @return the last exception message
*/
public String getErrorText() {
+ if (infoItems.isEmpty()) {
+ return getMessage();
+ }
return infoItems.get(0).errorText;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public String getErrorCode() { | |
| return infoItems.get(0).errorCode; | |
| } | |
| /** | |
| * Return the last exception message. | |
| * | |
| * @return the last exception message | |
| */ | |
| public String getErrorText() { | |
| return infoItems.get(0).errorText; | |
| } | |
| public String getErrorCode() { | |
| if (infoItems.isEmpty()) { | |
| return null; | |
| } | |
| return infoItems.get(0).errorCode; | |
| } | |
| /** | |
| * Return the last exception message. | |
| * | |
| * @return the last exception message | |
| */ | |
| public String getErrorText() { | |
| if (infoItems.isEmpty()) { | |
| return getMessage(); | |
| } | |
| return infoItems.get(0).errorText; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/io/mosip/print/exception/BaseUncheckedException.java around
lines 136-147, getErrorCode() and getErrorText() directly access
infoItems.get(0) which will throw IndexOutOfBoundsException when infoItems is
empty; change each method to first check if infoItems is null or empty and
return null (or an empty string if project convention prefers) when empty,
otherwise return infoItems.get(0).errorCode / .errorText; keep the existing
return type and do not alter other logic.
| @Query("SELECT p.id FROM PrintcardMCE p where p.requestId1= ?1 ") | ||
| public Integer findIdByRequestId1(@Param("requestId1") String rid); |
There was a problem hiding this comment.
Return type mismatch: p.id is String, not Integer.
Same issue as sno() - the id field is a String but the method returns Integer.
🔎 Proposed fix
@Query("SELECT p.id FROM PrintcardMCE p where p.requestId1= ?1 ")
- public Integer findIdByRequestId1(@Param("requestId1") String rid);
+ public String findIdByRequestId1(@Param("requestId1") String rid);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Query("SELECT p.id FROM PrintcardMCE p where p.requestId1= ?1 ") | |
| public Integer findIdByRequestId1(@Param("requestId1") String rid); | |
| @Query("SELECT p.id FROM PrintcardMCE p where p.requestId1= ?1 ") | |
| public String findIdByRequestId1(@Param("requestId1") String rid); |
🤖 Prompt for AI Agents
In src/main/java/io/mosip/print/service/PrintRepository.java around lines 20 to
21, the repository method findIdByRequestId1 is declared to return Integer but
the JPQL selects p.id which is a String; change the method signature to return
String (and adjust any callers if necessary) so the return type matches the
entity field type; ensure @Query and @Param remain the same and update
imports/usages accordingly.
| @Modifying | ||
| @Query("update PrintcardMCE p set p.status=?3 where p.requestId1= ?1 and p.status=?2 ") | ||
| public Integer updateExistingId(@Param("requestId1") String rid,@Param("status") int status,@Param("status") int newstatus); |
There was a problem hiding this comment.
Duplicate @Param("status") will cause parameter binding failure.
Both status and newstatus parameters use @Param("status"), which will cause incorrect binding. The second parameter should have a distinct name.
🔎 Proposed fix
@Modifying
- @Query("update PrintcardMCE p set p.status=?3 where p.requestId1= ?1 and p.status=?2 ")
- public Integer updateExistingId(@Param("requestId1") String rid,@Param("status") int status,@Param("status") int newstatus);
+ @Query("update PrintcardMCE p set p.status=:newStatus where p.requestId1=:requestId1 and p.status=:status")
+ public Integer updateExistingId(@Param("requestId1") String rid, @Param("status") int status, @Param("newStatus") int newstatus);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Modifying | |
| @Query("update PrintcardMCE p set p.status=?3 where p.requestId1= ?1 and p.status=?2 ") | |
| public Integer updateExistingId(@Param("requestId1") String rid,@Param("status") int status,@Param("status") int newstatus); | |
| @Modifying | |
| @Query("update PrintcardMCE p set p.status=:newStatus where p.requestId1=:requestId1 and p.status=:status") | |
| public Integer updateExistingId(@Param("requestId1") String rid, @Param("status") int status, @Param("newStatus") int newstatus); |
🤖 Prompt for AI Agents
In src/main/java/io/mosip/print/service/PrintRepository.java around lines 22-24,
the repository method uses duplicate @Param("status") annotations causing
binding failure; change the third parameter annotation to a distinct name (e.g.,
@Param("newstatus") int newstatus) and update the @Query to reference named
parameters (or matching positional indices) accordingly (for example use
p.requestId1 = :requestId1 and p.status = :status and set p.status = :newstatus)
so each method parameter has a unique @Param name that matches the query.
| public PrivateKeyEntry loadP12() throws KeyStoreException, NoSuchAlgorithmException, CertificateException, | ||
| IOException, UnrecoverableEntryException { | ||
| KeyStore mosipKeyStore = KeyStore.getInstance("PKCS12"); | ||
| InputStream in = getClass().getClassLoader().getResourceAsStream("partner.p12"); | ||
| mosipKeyStore.load(in, "qwerty@123".toCharArray()); | ||
| ProtectionParameter password = new PasswordProtection("qwerty@123".toCharArray()); | ||
| PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) mosipKeyStore.getEntry("partner", password); | ||
| return privateKeyEntry; | ||
| } |
There was a problem hiding this comment.
Critical security issue: Hardcoded credentials and keystore path.
The password qwerty@123 and keystore file partner.p12 are hardcoded. This is a severe security vulnerability. Credentials should be externalized to environment variables, a secrets manager, or secure configuration.
🔎 Proposed fix
+ @Value("${mosip.print.partner.keystore.path}")
+ private String keystorePath;
+
+ @Value("${mosip.print.partner.keystore.password}")
+ private String keystorePassword;
+
+ @Value("${mosip.print.partner.key.alias}")
+ private String keyAlias;
+
public PrivateKeyEntry loadP12() throws KeyStoreException, NoSuchAlgorithmException, CertificateException,
IOException, UnrecoverableEntryException {
KeyStore mosipKeyStore = KeyStore.getInstance("PKCS12");
- InputStream in = getClass().getClassLoader().getResourceAsStream("partner.p12");
- mosipKeyStore.load(in, "qwerty@123".toCharArray());
- ProtectionParameter password = new PasswordProtection("qwerty@123".toCharArray());
- PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) mosipKeyStore.getEntry("partner", password);
+ try (InputStream in = getClass().getClassLoader().getResourceAsStream(keystorePath)) {
+ mosipKeyStore.load(in, keystorePassword.toCharArray());
+ }
+ ProtectionParameter password = new PasswordProtection(keystorePassword.toCharArray());
+ PrivateKeyEntry privateKeyEntry = (PrivateKeyEntry) mosipKeyStore.getEntry(keyAlias, password);
return privateKeyEntry;
}🤖 Prompt for AI Agents
src/main/java/io/mosip/print/util/CryptoCoreUtil.java around lines 64 to 72, the
method currently hardcodes the keystore filename and password which is a
security risk; change it to read the keystore path/name and password from a
secure external source (e.g., environment variables, application config, or a
secrets manager) instead of literals, validate that the values exist and fail
fast with a clear error if not provided, obtain the password as a char[] (not a
String) from the secure source, open the keystore InputStream via a
try-with-resources so it is always closed, and use the externally provided
path/stream and password when loading the KeyStore and retrieving the
PrivateKeyEntry.
| public static String readFileFromResources(String filename) throws URISyntaxException, IOException { | ||
| InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename); | ||
| StringWriter writer = new StringWriter(); | ||
| IOUtils.copy(inputStream, writer, "UTF-8"); | ||
| return writer.toString(); | ||
| } |
There was a problem hiding this comment.
Critical: Null pointer exception and resource leak.
This method has multiple critical issues:
- NPE risk: Line 13's
getResourceAsStreamreturnsnullif the resource is not found, but line 15 usesinputStreamwithout a null check, causing aNullPointerException. - Resource leak: The
InputStreamis never closed, violating proper resource management. - Misleading signature:
URISyntaxExceptionis declared but never thrown by any code in this method.
🔎 Proposed fix using try-with-resources
- public static String readFileFromResources(String filename) throws URISyntaxException, IOException {
- InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename);
- StringWriter writer = new StringWriter();
- IOUtils.copy(inputStream, writer, "UTF-8");
- return writer.toString();
+ public static String readFileFromResources(String filename) throws IOException {
+ try (InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename)) {
+ if (inputStream == null) {
+ throw new IOException("Resource not found: " + filename);
+ }
+ StringWriter writer = new StringWriter();
+ IOUtils.copy(inputStream, writer, "UTF-8");
+ return writer.toString();
+ }
}Alternatively, use IOUtils.toString() for a more concise implementation:
- public static String readFileFromResources(String filename) throws URISyntaxException, IOException {
- InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename);
- StringWriter writer = new StringWriter();
- IOUtils.copy(inputStream, writer, "UTF-8");
- return writer.toString();
+ public static String readFileFromResources(String filename) throws IOException {
+ try (InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename)) {
+ if (inputStream == null) {
+ throw new IOException("Resource not found: " + filename);
+ }
+ return IOUtils.toString(inputStream, "UTF-8");
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static String readFileFromResources(String filename) throws URISyntaxException, IOException { | |
| InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename); | |
| StringWriter writer = new StringWriter(); | |
| IOUtils.copy(inputStream, writer, "UTF-8"); | |
| return writer.toString(); | |
| } | |
| public static String readFileFromResources(String filename) throws IOException { | |
| try (InputStream inputStream = Helpers.class.getClassLoader().getResourceAsStream(filename)) { | |
| if (inputStream == null) { | |
| throw new IOException("Resource not found: " + filename); | |
| } | |
| return IOUtils.toString(inputStream, "UTF-8"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/io/mosip/print/util/Helpers.java around lines 12 to 17, the
method risks an NPE because getResourceAsStream may return null, leaks the
InputStream because it is never closed, and declares URISyntaxException which is
not thrown; fix by removing the unused URISyntaxException from the signature,
obtain the InputStream and immediately check for null and throw a clear
IOException (or FileNotFoundException) if the resource isn't found, then read
the stream inside a try-with-resources block (or use
IOUtils.toString(inputStream, "UTF-8") inside the try-with-resources) so the
stream is always closed; ensure any thrown exceptions are appropriate
(IOException) and update callers if the signature changes.
| public static boolean isValidBearerToken(String accessToken, String issuerUrl, String clientId) { | ||
|
|
||
| try { | ||
| DecodedJWT decodedJWT = JWT.decode(accessToken); | ||
| Map<String, Claim> claims = decodedJWT.getClaims(); | ||
| LocalDateTime expiryTime = DateUtils | ||
| .convertUTCToLocalDateTime(DateUtils.getUTCTimeFromDate(decodedJWT.getExpiresAt())); | ||
|
|
||
| if (!decodedJWT.getIssuer().equals(issuerUrl)) { | ||
| return false; | ||
| } else if (!DateUtils.before(DateUtils.getUTCCurrentDateTime(), expiryTime)) { | ||
| return false; | ||
| } else if (!claims.get("clientId").asString().equals(clientId)) { | ||
| return false; | ||
| } else { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Security: JWT signature is not verified.
JWT.decode() only decodes the token without verifying its cryptographic signature. An attacker could forge a token with arbitrary claims. Use JWT.require(...).build().verify(accessToken) to validate the signature with the issuer's public key.
🔎 Proposed fix outline
+import com.auth0.jwt.algorithms.Algorithm;
+import com.auth0.jwt.JWTVerifier;
+
public static boolean isValidBearerToken(String accessToken, String issuerUrl, String clientId) {
try {
- DecodedJWT decodedJWT = JWT.decode(accessToken);
+ // Fetch public key from issuer's JWKS endpoint and create Algorithm
+ Algorithm algorithm = Algorithm.RSA256(rsaPublicKey, null);
+ JWTVerifier verifier = JWT.require(algorithm)
+ .withIssuer(issuerUrl)
+ .withClaim("clientId", clientId)
+ .build();
+ DecodedJWT decodedJWT = verifier.verify(accessToken);
+ return true;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/io/mosip/print/util/TokenHandlerUtil.java around lines 40 to
56, the code uses JWT.decode(accessToken) which only decodes JWTs and does not
verify their signatures; replace the decode call with a verified decode using
JWT.require(...).build().verify(accessToken) configured with the correct
Algorithm (public key or shared secret) for the issuer, validate the returned
DecodedJWT only after verification, and catch JWTVerificationException to return
false on verification failure; also keep the existing issuer, expiry and
clientId checks but ensure null-safety for claims and decodedJWT before
accessing fields.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.