fix: Log ignored certificates during client authentication #4415

hrishikesh-nalawade · 2025-12-10T13:18:40Z

Description

API-Layer filters out its own certificates during client authentication but doesn't log which certificates were ignored, making troubleshooting difficult.

Created shared CertificateLoggingUtils utility class for certificate logging
Added logging to both CategorizeCertsFilter and CategorizeCertsWebFilter

Linked to #4164

Type of change

fix: Bug fix (non-breaking change which fixes an issue)

Checklist:

My code follows the style guidelines of this project
PR title conforms to commit message guideline ## Commit Message Structure Guideline
I have commented my code, particularly in hard-to-understand areas. In JS I did provide JSDoc
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Signed-off-by: hrishikesh-nalawade <[email protected]>

…ent-certificate-ino

pablocarle · 2025-12-10T14:16:59Z

apiml/src/main/java/org/zowe/apiml/filter/CategorizeCertsWebFilter.java

                    certificateForClientAuth
                );
+
+                // Log ignored certificates


Are the comments needed? The method name is self-explanatory

yes, will remove most of the comments after after review and before merging.

pablocarle · 2025-12-10T14:17:57Z

What about microservices' mode CategorizeCertsFilter? Do we want to print it also there? It's still the most used configuration mode and it may be helpful to have more diagnostics there

pablocarle · 2025-12-10T14:18:39Z

Does the PR fix #4164 fully? The issue also refers to documentation update

hrishikesh-nalawade · 2025-12-10T17:27:36Z

What about microservices' mode CategorizeCertsFilter? Do we want to print it also there? It's still the most used configuration mode and it may be helpful to have more diagnostics there

Thank You for pointing that out, I was not aware of that. I'll implement the changes in CategorizeCertsFilter as well.

…ld be done in CategorizeCertsFilter and CategorizeCertsWebFilter Signed-off-by: hrishikesh-nalawade <[email protected]>

Signed-off-by: hrishikesh-nalawade <[email protected]>

…ent-certificate-ino

pablocarle · 2026-01-06T11:38:46Z

@hrishikesh-nalawade can you take care of the 7 sonarqube issues reported?

pablocarle · 2026-01-06T11:44:53Z

apiml/src/main/java/org/zowe/apiml/filter/CategorizeCertsWebFilter.java

+            filteredCerts,
+            publicKeyCertificatesBase64,
+            log,
+            CategorizeCertsWebFilter::base64EncodePublicKey


This could be moved to a utility class, there's no reason to have it as a parameter everywhere

yes it is, I have created a separate utility class CertificateLoggingUtils to handle the certificate filtering logic

pablocarle · 2026-01-06T11:46:05Z

...curity-common/src/main/java/org/zowe/apiml/security/common/util/CertificateLoggingUtils.java

+     * @param logger The logger to use for output
+     * @param base64Encoder Function to encode certificate public key to Base64
+     */
+    public static void logIgnoredCertificates(


Try to refactor a bit this method, it's too long, you can extract smaller blocks into private methods

Done, Thank You for pointing that out.

pablocarle · 2026-01-06T11:46:49Z

...curity-common/src/main/java/org/zowe/apiml/security/common/util/CertificateLoggingUtils.java

+        Set<String> originalKeys = Arrays.stream(originalCerts)
+            .map(base64Encoder)
+            .collect(Collectors.toSet());
+
+        Set<String> filteredKeys = filteredCerts != null
+            ? Arrays.stream(filteredCerts)
+                .map(base64Encoder)
+                .collect(Collectors.toSet())
+            : new HashSet<>();
+
+        Set<String> ignoredKeys = new HashSet<>(originalKeys);
+        ignoredKeys.removeAll(filteredKeys);


This can be one method

Done, Thank You for pointing that out.

pablocarle · 2026-01-06T11:47:34Z

...curity-common/src/main/java/org/zowe/apiml/security/common/util/CertificateLoggingUtils.java

+        ignoredKeys.removeAll(filteredKeys);
+
+        if (!ignoredKeys.isEmpty()) {
+            List<X509Certificate> ignoredCerts = Arrays.stream(originalCerts)


Since it's java 17+, verbosity can be somewhat reduced using var, like:

var ignoredCerts = Arrays.stream(originalCerts)

pablocarle · 2026-01-06T11:48:35Z

...curity-common/src/main/java/org/zowe/apiml/security/common/util/CertificateLoggingUtils.java

+                ignoredCerts.stream()
+                    .map(cert -> {
+                        String subjectDN = cert.getSubjectX500Principal() != null
+                            ? cert.getSubjectX500Principal().getName()
+                            : "Unknown";
+                        String issuerDN = cert.getIssuerX500Principal() != null
+                            ? cert.getIssuerX500Principal().getName()
+                            : "Unknown";
+                        String publicKeyBase64 = base64Encoder.apply(cert);
+                        return String.format("[Subject: %s, Issuer: %s, Public Key (first 20 chars): %s...]",
+                            subjectDN, issuerDN, publicKeyBase64.substring(0, Math.min(20, publicKeyBase64.length())));
+                    })
+                    .collect(Collectors.joining(", ")));


This can be extracted to a private method as well

Done, Thank You for pointing that out.

pablocarle · 2026-01-06T12:00:47Z

apiml/src/test/java/org/zowe/apiml/filter/CategorizeCertsWebFilterTest.java

        }
    }

+    @Test


Move these tests into a nested class, if it's only verifying logging.

Have you checked if these same scenarios are already covered in previous tests? (so you can only add the assertion to the existing test about logging)

Thank You, Removed most of the tests, added assertions to the existing ones, and kept one test for a scenario that was not covered.

pablocarle · 2026-01-06T12:02:31Z

...ty-common/src/test/java/org/zowe/apiml/security/common/filter/CategorizeCertsFilterTest.java

    }
+
+    @Nested
+    class LogIgnoredCertificatesTests {


Same question, have you checked if these test conditions are covered in the existing tests?
Maybe it's easier to add assertions to the existing tests rather than writing all new ones
If the conditions were not covered previously, then let's add new tests, but the focus should be on the main validations rather than whether they are logged or not. Logging should be in addition to.

Thank You, Removed most of the tests, added assertions to the existing ones, and kept one test for a scenario that was not covered.

…ent-certificate-ino

…eCertsWebFilterTest and CategorizeCertsFilterTest by adding logging assertions into existing tests Signed-off-by: hrishikesh-nalawade <[email protected]>

Signed-off-by: hrishikesh-nalawade <[email protected]>

…ent-certificate-ino

pablocarle · 2026-01-13T12:47:43Z

...curity-common/src/main/java/org/zowe/apiml/security/common/filter/CategorizeCertsFilter.java

+            filteredCerts,
+            publicKeyCertificatesBase64,
+            log,
+            CategorizeCertsFilter::base64EncodePublicKey


please remove this as an argument, simply call the method or if not visible, move to a utility class that's visible in apiml-common

pablocarle · 2026-01-14T12:37:27Z

...curity-common/src/main/java/org/zowe/apiml/security/common/filter/CategorizeCertsFilter.java

+            filteredCerts,
+            publicKeyCertificatesBase64,
+            log,
+            CategorizeCertsFilter::base64EncodePublicKey


@hrishikesh-nalawade can you confirm if you see this comment? There's some weird behaviour in the github view

pablocarle · 2026-01-14T12:38:34Z

...curity-common/src/main/java/org/zowe/apiml/security/common/util/CertificateLoggingUtils.java

+ * Utility class for logging certificate-related operations, particularly for logging
+ * certificates that were ignored/filtered during client authentication.
+ */
+@UtilityClass


This annotation doesn't provide much value here honestly, it's preferable to remove it and add a private constructor

…ent-certificate-ino

sonarqubecloud · 2026-01-14T14:00:31Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
90.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

hrishikesh-nalawade added 2 commits September 10, 2025 16:51

Logs for Ignored Client Certificates

970bf43

Signed-off-by: hrishikesh-nalawade <[email protected]>

Optimizing Code and adding Tests

9d14b4c

Signed-off-by: hrishikesh-nalawade <[email protected]>

github-project-automation bot added this to API Mediation Layer Backlog Management Dec 10, 2025

github-project-automation bot moved this to New in API Mediation Layer Backlog Management Dec 10, 2025

pull-request-size bot added the size/L label Dec 10, 2025

Merge branch 'v3.x.x' into hrishikesh-nalawade/GH4164/log-ignored-cli…

cf8e372

…ent-certificate-ino

hrishikesh-nalawade requested review from achmelo, pablocarle and taban03 December 10, 2025 13:21

EvaJavornicka moved this from New to In Progress in API Mediation Layer Backlog Management Dec 10, 2025

pablocarle reviewed Dec 10, 2025

View reviewed changes

Implemented CertificateLoggingUtils utility class so that logging cou…

9258c5e

…ld be done in CategorizeCertsFilter and CategorizeCertsWebFilter Signed-off-by: hrishikesh-nalawade <[email protected]>

hrishikesh-nalawade changed the title ~~Fix: Log ignored certificates during client authentication~~ fix: Log ignored certificates during client authentication Dec 10, 2025

github-actions bot added the Sensitive Sensitive change that requires peer review label Dec 10, 2025

Removing Comments

5dd882c

Signed-off-by: hrishikesh-nalawade <[email protected]>

hrishikesh-nalawade requested a review from pablocarle December 11, 2025 08:46

hrishikesh-nalawade added 2 commits December 22, 2025 18:28

Merge branch 'v3.x.x' into hrishikesh-nalawade/GH4164/log-ignored-cli…

c8bda34

…ent-certificate-ino

Merge branch 'v3.x.x' into hrishikesh-nalawade/GH4164/log-ignored-cli…

7ee33f4

…ent-certificate-ino

pablocarle reviewed Jan 6, 2026

View reviewed changes

Merge branch 'v3.x.x' into hrishikesh-nalawade/GH4164/log-ignored-cli…

b006e54

…ent-certificate-ino

hrishikesh-nalawade added 2 commits January 7, 2026 18:42

Restructured the CertificateLoggingUtils class and enhanced Categoriz…

d7cfaa8

…eCertsWebFilterTest and CategorizeCertsFilterTest by adding logging assertions into existing tests Signed-off-by: hrishikesh-nalawade <[email protected]>

removing unused Imports

6d5b5d0

Signed-off-by: hrishikesh-nalawade <[email protected]>

hrishikesh-nalawade requested a review from pablocarle January 7, 2026 13:33

Merge branch 'v3.x.x' into hrishikesh-nalawade/GH4164/log-ignored-cli…

6e5656f

…ent-certificate-ino

pablocarle reviewed Jan 13, 2026

View reviewed changes

pablocarle reviewed Jan 14, 2026

View reviewed changes

pablocarle added 2 commits January 14, 2026 13:39

Merge branch 'v3.x.x' into hrishikesh-nalawade/GH4164/log-ignored-cli…

7571743

…ent-certificate-ino

Merge branch 'v3.x.x' into hrishikesh-nalawade/GH4164/log-ignored-cli…

b69e310

…ent-certificate-ino

                       }
                   }
+                  @Test

fix: Log ignored certificates during client authentication #4415

Are you sure you want to change the base?

fix: Log ignored certificates during client authentication #4415

Uh oh!

Conversation

hrishikesh-nalawade commented Dec 10, 2025 • edited by pablocarle Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pablocarle commented Dec 10, 2025

Uh oh!

pablocarle commented Dec 10, 2025

Uh oh!

hrishikesh-nalawade commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pablocarle commented Jan 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jan 14, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hrishikesh-nalawade commented Dec 10, 2025 •

edited by pablocarle

Loading

hrishikesh-nalawade commented Dec 10, 2025 •

edited

Loading