Develop mosi42630

[nagendra0721]

Summary by CodeRabbit

• New Features

  • Optional inclusion/exclusion of COSE Sign1 tag during signing; signing endpoint now follows COSE Sign1 semantics and exposes a flag to include the COSE tag.

• Bug Fixes

  • Support for parsing/verifying both tagged and untagged COSE Sign1 content.
  • Improved error handling and three new COSE-related error codes.

• Documentation

  • Payload encoding clarified to Base64URL.

• Tests

  • Updated tests to expect COSE verification failures consistently.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds support for COSE Sign1 tagged and untagged formats: new error codes, DTO flags to control COSE tag inclusion, controller method rename to coseSign1, parsing helpers for tagged/untagged Sign1, and signing/verification changes to handle optional COSE tags.

Changes

Cohort / File(s)	Summary
Error Codes kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/constant/SignatureErrorCode.java	Added UNTAGGED_COSE_SIGN1 (KER-SIG-120) and TAGGED_COSE_SIGN1 (KER-SIG-121) enum constants; adjusted trailing punctuation.
Request DTOs kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/dto/CoseSignRequestDto.java, kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/dto/CoseSignVerifyRequestDto.java	CoseSignRequestDto: payload note changed to "Base64URL encoded Data to sign" and added includeCOSETag (Boolean). CoseSignVerifyRequestDto: added isCOSETagIncluded (Boolean).
Controller kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/controller/CoseSignController.java	Renamed endpoint method from coseSign(...) to coseSign1(...) and delegates to service.coseSign1(...).
Service interface kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/service/CoseSignatureService.java	Javadoc updated to reference "COSE Sign1"/"COSE Verify1"; no signature changes.
Service implementation kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/service/impl/CoseSignatureServiceImpl.java	Switched payload handling to byte[]; added includeCoseTag option to encoding; added parseTaggedCoseSign1(...) and parseUntaggedCoseSign1(...); verification now selects parsing path based on isCOSETagIncluded; logging and error messages updated.
Tests kernel/kernel-keymanager-service/src/test/java/io/mosip/kernel/signature/test/service/CoseSignatureServiceTest.java	Formatting changes and updated several tests to expect SignatureFailureException instead of RequestException.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Controller as CoseSignController
    participant Service as CoseSignatureServiceImpl
    participant Parser as CBORDecoder

    rect rgba(100,150,200,0.5)
    Note over Client,Parser: COSE Sign1 sign/verify flow with optional tag
    Client->>Controller: POST /coseSign1 (includeCOSETag / isCOSETagIncluded)
    Controller->>Service: coseSign1 / coseVerify1(requestDto)
    alt Signing flow
        Service->>Service: decode Base64URL -> byte[]
        Service->>Service: signCose1(payloadBytes, includeCoseTag)
        Service->>Controller: ResponseWrapper(encoded COSE Sign1)
    else Verification flow
        Service->>Parser: decode CBOR(bytes)
        alt isCOSETagIncluded == true
            Service->>Service: parseTaggedCoseSign1(decoder)
        else isCOSETagIncluded == false
            Service->>Service: parseUntaggedCoseSign1(decoder)
        end
        Service->>Service: verify signature & payload bytes
        Service-->>Controller: ResponseWrapper(result)
    end
    Controller-->>Client: HTTP response
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

• mahammedtaheer

Poem

🐇 Hopping through bytes and COSE tags bright,
I sign and I parse both day and night,
Tagged or untagged, I check every part,
Bytes over strings — cryptographic art,
A joyful rabbit guarding each signature's heart 🥕✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Develop mosi42630' is vague and does not convey meaningful information about the changeset. It references a ticket/issue number but provides no context about what the PR actually implements.	Revise the title to clearly describe the main change, such as 'Add support for tagged and untagged COSE Sign1 signatures' or similar descriptive summary of the functionality.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/service/impl/CoseSignatureServiceImpl.java (1)

108-127: Guard Base64URL decoding failures.
Line 116 decodes Base64URL outside error handling; the preceding isDataValid() check only validates that the input is non-null and non-empty, not that it is valid Base64URL. Malformed input will throw IllegalArgumentException and surface as a 500 error instead of returning INVALID_INPUT. Wrap the decode in try-catch and map decoding failures to RequestException, consistent with patterns already used elsewhere in the codebase (e.g., SignatureUtil.java:411, CryptomanagerUtils.java:266).

🛠️ Suggested fix

-        byte[] payload = CryptoUtil.decodeURLSafeBase64(base64Payload);
+        byte[] payload;
+        try {
+            payload = CryptoUtil.decodeURLSafeBase64(base64Payload);
+        } catch (IllegalArgumentException e) {
+            LOGGER.error(SignatureConstant.SESSIONID, SignatureConstant.COSE_SIGN, SignatureConstant.BLANK,
+                    "Provided Data to sign is not valid Base64URL.", e);
+            throw new RequestException(SignatureErrorCode.INVALID_INPUT.getErrorCode(),
+                    SignatureErrorCode.INVALID_INPUT.getErrorMessage());
+        }

🤖 Fix all issues with AI agents

In
`@kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/service/impl/CoseSignatureServiceImpl.java`:
- Around line 637-669: The parseTaggedCoseSign1 method currently casts and
returns cborTaggedItem.getTagContent() outside the try which allows
ClassCastException to bypass intended error handling and the broad catch
swallows a previously thrown RequestException; move the cast-and-return of
COSESign1 into the try block, catch and rethrow RequestException separately
(i.e., add a catch (RequestException re) { throw re; }) and catch specific
exceptions (ClassCastException, IOException, COSEException, etc.) afterwards to
throw TAGGED_COSE_SIGN1 while logging the exception object (LOGGER.error(...,
e)). Do the same in parseUntaggedCoseSign1: include the exception object in
LOGGER.error calls and ensure you catch and rethrow RequestException if created,
and map other specific exceptions to UNTAGGED_COSE_SIGN1 so stack traces and
root causes are preserved.

🧹 Nitpick comments (1)

kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/controller/CoseSignController.java (1)

59-61: Consider backward-compatibility for the renamed endpoint.
Line 60 switches to coseSign1; if /coseSign had consumers, consider a deprecated alias or documented migration path.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

kernel/kernel-keymanager-service/src/main/java/io/mosip/kernel/signature/service/impl/CoseSignatureServiceImpl.java (1)

182-197: CWT tag is skipped when includeCOSETag=false.

With isCwt=true, the early return bypasses the outer CWT tag, yielding invalid CWT output when the caller opts out of the COSE tag.

🛠️ Proposed fix

 byte[] coseBytes = coseSign1.encode();
 CBORDecoder decoder = new CBORDecoder(coseBytes);
 CBORItem coseItem = decoder.next();
-
-if (!includeCoseTag)
-    return coseItem.encode();
-
-CBORTaggedItem sign1Tagged = new CBORTaggedItem(SignatureConstant.COSE_SIGN1_TAG, coseItem);
-
-if (isCwt) {
-    CBORTaggedItem cwtTagged = new CBORTaggedItem(SignatureConstant.CWT_SIGN_TAG, sign1Tagged);
-    return cwtTagged.encode();
-} else {
-    return sign1Tagged.encode();
-}
+CBORItem item = coseItem;
+if (includeCoseTag) {
+    item = new CBORTaggedItem(SignatureConstant.COSE_SIGN1_TAG, item);
+}
+if (isCwt) {
+    CBORTaggedItem cwtTagged = new CBORTaggedItem(SignatureConstant.CWT_SIGN_TAG, item);
+    return cwtTagged.encode();
+}
+return item.encode();