Separate AddCert and AddCertAndOverride for client side cert managers

Summary: Mirroring the change from the server side diff D57737100, to refactor `addcert` function from client cert managers, so no functions with bool default param are exposed as public function.

Differential Revision: D57939731

fbshipit-source-id: 532753592b13a4ea116c6bb015591d48878ace4c

fizz/client/CertManager.cpp

@@ -31,6 +31,14 @@ CertManager::CertMatch CertManager::getCert(
   return none;
 }
 
+void CertManager::addCert(std::shared_ptr<SelfCert> cert) {
+  addCert(cert, false);
+}
+
+void CertManager::addCertAndOverride(std::shared_ptr<SelfCert> cert) {
+  addCert(cert, true);
+}
+
 void CertManager::addCert(
     std::shared_ptr<SelfCert> cert,
     bool overrideExistingEntry) {

fizz/client/CertManager.h

@@ -28,15 +28,20 @@ class CertManager : public CertManagerBase {
       const std::vector<Extension>& peerExtensions) const override;
 
   /*
-   * This will add the cert to the internal map, note we expect only a single
-   * cert for a particular signature scheme, by default we will override any
-   * existing entry. The caller may choose to not do so.
+   * This will add the cert to the internal map, and will not override existing
+   * entry.
    */
-  virtual void addCert(
-      std::shared_ptr<SelfCert> cert,
-      bool overrideExistingEntry = true);
+  void addCert(std::shared_ptr<SelfCert> cert);
+
+  /*
+   * This will add the cert to the internal map, and override any existing
+   * entry.
+   */
+  void addCertAndOverride(std::shared_ptr<SelfCert> cert);
 
  protected:
+  void addCert(std::shared_ptr<SelfCert> cert, bool overrideExistingEntry);
+
   SigSchemeMap certs_;
 };
 } // namespace client

fizz/client/FizzClientContext.h

@@ -123,7 +123,7 @@ class FizzClientContext {
     if (cert != nullptr) {
       auto certMgr = std::make_shared<CertManager>();
       clientCert_ = cert;
-      certMgr->addCert(std::move(cert));
+      certMgr->addCertAndOverride(std::move(cert));
       certManager_ = std::move(certMgr);
     } else {
       certManager_ = nullptr;

fizz/client/test/CertManagerTest.cpp

@@ -33,7 +33,7 @@ class CertManagerTest : public Test {
 
 TEST_F(CertManagerTest, TestBasicMatch) {
   auto cert = getCert(kRsa);
-  manager_.addCert(cert);
+  manager_.addCertAndOverride(cert);
   auto res = manager_.getCert(folly::none, kRsa, kRsa, {});
   EXPECT_EQ(res->cert, cert);
 }
@@ -45,7 +45,7 @@ TEST_F(CertManagerTest, TestNoCertsInMgr) {
 TEST_F(CertManagerTest, TestSigSchemesPref) {
   auto cert = getCert(
       {SignatureScheme::rsa_pss_sha256, SignatureScheme::rsa_pss_sha512});
-  manager_.addCert(cert);
+  manager_.addCertAndOverride(cert);
 
   auto res = manager_.getCert(
       folly::none,
@@ -67,8 +67,8 @@ TEST_F(CertManagerTest, TestSigSchemesPref) {
 TEST_F(CertManagerTest, TestServerSigScheme) {
   auto cert1 = getCert({SignatureScheme::rsa_pss_sha256});
   auto cert2 = getCert({SignatureScheme::rsa_pss_sha512});
-  manager_.addCert(cert1);
-  manager_.addCert(cert2);
+  manager_.addCertAndOverride(cert1);
+  manager_.addCertAndOverride(cert2);
 
   auto res = manager_.getCert(
       folly::none,
@@ -81,7 +81,7 @@ TEST_F(CertManagerTest, TestServerSigScheme) {
 
 TEST_F(CertManagerTest, TestNoSigSchemeMatch) {
   auto cert = getCert({SignatureScheme::rsa_pss_sha256});
-  manager_.addCert(cert);
+  manager_.addCertAndOverride(cert);
   auto res = manager_.getCert(
       folly::none,
       {SignatureScheme::rsa_pss_sha256},

fizz/extensions/delegatedcred/DelegatedCredentialClientCertManager.cpp

@@ -34,10 +34,16 @@ DelegatedCredentialClientCertManager::getCert(
 }
 
 void DelegatedCredentialClientCertManager::addDelegatedCredential(
-    std::shared_ptr<SelfCert> cert,
-    bool overrideExistingEntry) {
+    std::shared_ptr<SelfCert> cert) {
   VLOG(8) << "Adding delegated credential";
-  dcMgr_.addCert(std::move(cert), overrideExistingEntry);
+  dcMgr_.addCert(std::move(cert));
 }
+
+void DelegatedCredentialClientCertManager::addDelegatedCredentialAndOverride(
+    std::shared_ptr<SelfCert> cert) {
+  VLOG(8) << "Adding delegated credential";
+  dcMgr_.addCertAndOverride(std::move(cert));
+}
+
 } // namespace extensions
 } // namespace fizz

fizz/extensions/delegatedcred/DelegatedCredentialClientCertManager.h

@@ -31,12 +31,17 @@ class DelegatedCredentialClientCertManager : public fizz::client::CertManager {
 
   /*
    * It is the callers responsibility to call this with an actual delegated
-   * cred. The override flag here has the same meaning as above, if there is
-   * already a dc for the signature scheme we will replace the existing entry.
+   * cred. If there is already a dc for the signature scheme we will not
+   * override the existing entry.
    */
-  void addDelegatedCredential(
-      std::shared_ptr<SelfCert> cert,
-      bool overrideExistingEntry = true);
+  void addDelegatedCredential(std::shared_ptr<SelfCert> cert);
+
+  /*
+   * It is the callers responsibility to call this with an actual delegated
+   * cred. If there is already a dc for the signature scheme we will override
+   * the existing entry.
+   */
+  void addDelegatedCredentialAndOverride(std::shared_ptr<SelfCert> cert);
 
  protected:
   client::CertManager dcMgr_;

fizz/extensions/delegatedcred/test/DelegatedCredentialClientCertManagerTest.cpp

@@ -49,17 +49,17 @@ TEST_F(DelegatedClientCertManagerTest, TestNoCert) {
 TEST_F(DelegatedClientCertManagerTest, TestBasicMatch) {
   // Only non dc cert must be returned
   auto cert = getCert(kRsa);
-  manager_.addCert(cert);
+  manager_.addCertAndOverride(cert);
   auto res = manager_.getCert(folly::none, kRsa, kRsa, {});
   EXPECT_EQ(res->cert, cert);
 }
 
 TEST_F(DelegatedClientCertManagerTest, TestBasicMatchNoExt) {
   auto cert1 = getCert(kRsa);
   auto cert2 = getCert(kRsa);
-  manager_.addCert(cert1);
+  manager_.addCertAndOverride(cert1);
   // Let's just pretend here the impl of the self cert doesnt really matter
-  manager_.addDelegatedCredential(cert2);
+  manager_.addDelegatedCredentialAndOverride(cert2);
   {
     auto res = manager_.getCert(folly::none, kRsa, kRsa, {});
     EXPECT_EQ(res->cert, cert1);
@@ -72,9 +72,9 @@ TEST_F(DelegatedClientCertManagerTest, TestBasicMatchNoExt) {
 
 TEST_F(DelegatedClientCertManagerTest, TestSigSchemes) {
   auto cert = getCert({SignatureScheme::rsa_pss_sha256});
-  manager_.addCert(cert);
+  manager_.addCertAndOverride(cert);
   auto dcCert = getCert({SignatureScheme::rsa_pss_sha512});
-  manager_.addDelegatedCredential(dcCert);
+  manager_.addDelegatedCredentialAndOverride(dcCert);
 
   {
     auto res = manager_.getCert(
@@ -128,7 +128,7 @@ TEST_F(DelegatedClientCertManagerTest, TestSigSchemes) {
 
 TEST_F(DelegatedClientCertManagerTest, TestNoSigSchemeMatch) {
   auto cert = getCert({SignatureScheme::rsa_pss_sha256});
-  manager_.addCert(cert);
+  manager_.addCertAndOverride(cert);
   auto res = manager_.getCert(
       folly::none,
       {SignatureScheme::rsa_pss_sha256},