From f2de60ae684ad4bf9391997e8b59226f29c66b31 Mon Sep 17 00:00:00 2001 From: "Endi S. Dewata" Date: Tue, 1 Oct 2024 17:53:04 -0500 Subject: [PATCH] Update pki ca-cert-find for API v2 The CertServlet has been modified to use the same path (i.e. /ca/v2/certs) for list and search operations, but list will will use a GET method and search will use a POST method. The CACertClient has been updated to use the proper path based on the API version. The search operation has also been modified to no longer return the total certs found to allow future performance optimization. Calculating the total certs found with Simple Paged Results requires retrieving the full search results from the database so it should be avoided. The basic CA test has been updated to test pki ca-cert-find with the default API and API v2 then verify the access logs generated by these commands. The test-ca-certs.sh script is no longer used so it has been removed. --- .github/workflows/ca-basic-test.yml | 66 ++++++++++++++++++- .../server/ca/rest/v2/CertServlet.java | 5 +- .../netscape/certsrv/base/DataCollection.java | 24 ++----- .../com/netscape/certsrv/ca/CACertClient.java | 7 +- .../netscape/cmstools/ca/CACertFindCLI.java | 7 +- tests/ca/bin/test-ca-certs.sh | 11 ---- 6 files changed, 86 insertions(+), 34 deletions(-) delete mode 100755 tests/ca/bin/test-ca-certs.sh diff --git a/.github/workflows/ca-basic-test.yml b/.github/workflows/ca-basic-test.yml index e6716213fe3..5e02e849394 100644 --- a/.github/workflows/ca-basic-test.yml +++ b/.github/workflows/ca-basic-test.yml @@ -365,7 +365,71 @@ jobs: run: | docker exec pki /usr/share/pki/tests/ca/bin/test-ca-signing-cert.sh docker exec pki /usr/share/pki/tests/ca/bin/test-subsystem-cert.sh - docker exec pki /usr/share/pki/tests/ca/bin/test-ca-certs.sh + + - name: Check pki ca-cert-find with default API + run: | + docker exec pki pki ca-cert-find | tee output + + # get certs returned + grep "Serial Number:" output | wc -l > actual + + # there should be 6 certs returned + echo "6" > expected + diff expected actual + + # get total certs found + sed -n "s/^\(\S*\) entries found$/\1/p" output > actual + + # there should be 6 certs found + echo "6" > expected + diff expected actual + + # check HTTP methods, paths, protocols, status, and authenticated users + docker exec pki find /var/log/pki/pki-tomcat \ + -name "localhost_access_log.*" \ + -exec cat {} \; \ + | tail -2 \ + | sed -e 's/^.* .* \(.*\) \[.*\] "\(.*\)" \(.*\) .*$/\2 \3 \1/' \ + | tee output + + cat > expected << EOF + GET /pki/v1/info HTTP/1.1 200 - + POST /ca/v1/certs/search HTTP/1.1 200 - + EOF + + diff expected output + + - name: Check pki ca-cert-find with API v2 + run: | + docker exec pki pki --api v2 ca-cert-find | tee output + + # get certs returned + grep "Serial Number:" output | wc -l > actual + + # there should be 6 certs returned + echo "6" > expected + diff expected actual + + # get total certs found + sed -n "s/^\(\S*\) entries found$/\1/p" output > actual + + # there should be no total certs found + diff /dev/null actual + + # check HTTP methods, paths, protocols, status, and authenticated users + docker exec pki find /var/log/pki/pki-tomcat \ + -name "localhost_access_log.*" \ + -exec cat {} \; \ + | tail -2 \ + | sed -e 's/^.* .* \(.*\) \[.*\] "\(.*\)" \(.*\) .*$/\2 \3 \1/' \ + | tee output + + cat > expected << EOF + GET /pki/v2/info HTTP/1.1 200 - + POST /ca/v2/certs HTTP/1.1 200 - + EOF + + diff expected output - name: Test CA admin run: | diff --git a/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java b/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java index fb70dd36698..104012d3bad 100644 --- a/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java +++ b/base/ca/src/main/java/org/dogtagpki/server/ca/rest/v2/CertServlet.java @@ -107,7 +107,7 @@ public void getCert(HttpServletRequest request, HttpServletResponse response) th } } - @WebAction(method = HttpMethod.POST, paths = {"search"}) + @WebAction(method = HttpMethod.POST, paths = {""}) public void searchCerts(HttpServletRequest request, HttpServletResponse response) throws Exception { HttpSession session = request.getSession(); logger.debug("CertServlet.searchCerts(): session: {}", session.getId()); @@ -220,7 +220,8 @@ private CertDataInfos listCerts(CertSearchRequest searchReq, int maxTime, int st results.add(createCertDataInfo(rec)); } - infos.setTotal(results.size()); + // do not call infos.setTotal() in API v2 + logger.info("Search results: {}", results.size()); infos.setEntries(results); } catch (Exception e) { diff --git a/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java b/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java index eeea62b78fd..63f5e08148b 100644 --- a/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java +++ b/base/common/src/main/java/com/netscape/certsrv/base/DataCollection.java @@ -20,20 +20,21 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Objects; /** * @author Endi S. Dewata */ public class DataCollection { - protected int total; + protected Integer total; protected Collection entries = new ArrayList<>(); - public int getTotal() { + public Integer getTotal() { return total; } - public void setTotal(int total) { + public void setTotal(Integer total) { this.total = total; } @@ -57,11 +58,7 @@ public void removeEntry(E entry) { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((entries == null) ? 0 : entries.hashCode()); - result = prime * result + total; - return result; + return Objects.hash(entries, total); } @Override @@ -72,14 +69,7 @@ public boolean equals(Object obj) { return false; if (getClass() != obj.getClass()) return false; - DataCollection other = (DataCollection) obj; - if (entries == null) { - if (other.entries != null) - return false; - } else if (!entries.equals(other.entries)) - return false; - if (total != other.total) - return false; - return true; + DataCollection other = (DataCollection) obj; + return Objects.equals(entries, other.entries) && Objects.equals(total, other.total); } } diff --git a/base/common/src/main/java/com/netscape/certsrv/ca/CACertClient.java b/base/common/src/main/java/com/netscape/certsrv/ca/CACertClient.java index d1a857d63b6..4bd6609c5ef 100644 --- a/base/common/src/main/java/com/netscape/certsrv/ca/CACertClient.java +++ b/base/common/src/main/java/com/netscape/certsrv/ca/CACertClient.java @@ -99,7 +99,12 @@ public CertDataInfos findCerts(CertSearchRequest data, Integer start, Integer si if (size != null) params.put("size", size); String searchRequest = (String) client.marshall(data); Entity entity = client.entity(searchRequest); - return post("search", params, entity, CertDataInfos.class); + + if ("v2".equals(prefix)) { + return post(null, params, entity, CertDataInfos.class); + } else { + return post("search", params, entity, CertDataInfos.class); + } } public CertRequestInfo revokeCert(CertId id, CertRevokeRequest request) throws Exception { diff --git a/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java b/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java index d77a419a203..ce44843799f 100644 --- a/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java +++ b/base/tools/src/main/java/com/netscape/cmstools/ca/CACertFindCLI.java @@ -227,8 +227,11 @@ public void execute(CommandLine cmd) throws Exception { CACertClient certClient = certCLI.getCertClient(); CertDataInfos certs = certClient.findCerts(searchData, start, size); - MainCLI.printMessage(certs.getTotal() + " entries found"); - if (certs.getTotal() == 0) return; + Integer total = certs.getTotal(); + if (total != null) { + MainCLI.printMessage(total + " entries found"); + if (total == 0) return; + } boolean first = true; diff --git a/tests/ca/bin/test-ca-certs.sh b/tests/ca/bin/test-ca-certs.sh deleted file mode 100755 index bed4516d948..00000000000 --- a/tests/ca/bin/test-ca-certs.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/bash -e - -# list certs in CA -pki ca-cert-find | tee /tmp/certs.txt - -# get the number of certs returned -sed -n "s/^\(\S*\) entries found$/\1/p" /tmp/certs.txt > /tmp/certs.count - -# by default there should be 6 certs initially -echo 6 > /tmp/expected -diff /tmp/certs.count /tmp/expected