Simplify NullPermissionVerifier setup (#1890)

zeroc-ice · Mar 6, 2024 · a6fe742 · a6fe742
1 parent f983d5e
commit a6fe742
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 88 deletions.
diff --git a/cpp/src/Glacier2/Glacier2Router.cpp b/cpp/src/Glacier2/Glacier2Router.cpp
@@ -197,7 +197,16 @@ RouterService::start(int argc, char* argv[], int& status)
     verifierProperties.push_back("Glacier2.PermissionsVerifier");
     verifierProperties.push_back("Glacier2.SSLPermissionsVerifier");
 
-    Glacier2Internal::setupNullPermissionsVerifier(communicator(), instanceName, verifierProperties);
+    try
+    {
+        Glacier2Internal::setupNullPermissionsVerifier(communicator(), instanceName, verifierProperties);
+    }
+    catch(const std::exception& ex)
+    {
+        ServiceError err(this);
+        err << "unable to setup null permissions verifier:\n" << ex;
+        return false;
+    }
 
     const string verifierProperty = verifierProperties[0];
     optional<PermissionsVerifierPrx> verifier;

diff --git a/cpp/src/Glacier2Lib/NullPermissionsVerifier.cpp b/cpp/src/Glacier2Lib/NullPermissionsVerifier.cpp
@@ -12,11 +12,11 @@ using namespace std;
 namespace
 {
 
-class NullPermissionsVerifier : public Glacier2::PermissionsVerifier
+class NullPermissionsVerifier final : public Glacier2::PermissionsVerifier
 {
 public:
 
-    bool checkPermissions(string, string, string&, const Current&) const
+    bool checkPermissions(string, string, string&, const Current&) const final
     {
         return true;
     }
@@ -26,114 +26,62 @@ class NullSSLPermissionsVerifier : public Glacier2::SSLPermissionsVerifier
 {
 public:
 
-    virtual bool
-    authorize(Glacier2::SSLInfo, string&, const Ice::Current&) const
+    bool authorize(Glacier2::SSLInfo, string&, const Ice::Current&) const final
     {
         return true;
     }
 };
 
-class Init
-{
-public:
-
-    Init(const CommunicatorPtr&, const string&, const vector<string>&);
-
-private:
-
-    string checkPermissionVerifier(const string&);
-    void createObjects();
-
-    const CommunicatorPtr _communicator;
-    ObjectAdapterPtr _adapter;
+}
 
-    Identity _nullPVId;
-    Identity _nullSSLPVId;
-};
+namespace Glacier2Internal
+{
 
-Init::Init(const CommunicatorPtr& communicator, const string& category, const vector<string>& props) :
-    _communicator(communicator)
+void
+setupNullPermissionsVerifier(
+    const CommunicatorPtr& communicator,
+    const string& category,
+    const vector<string>& permissionsVerifierPropertyNames)
 {
-    _nullPVId.name = "NullPermissionsVerifier";
-    _nullPVId.category = category;
+    const Ice::Identity nullPermissionsVerifierId {"NullPermissionsVerifier", category};
+    const Ice::Identity nullSSLPermissionsVerifierId {"NullSSLPermissionsVerifier",  category};
+
+    const Ice::PropertiesPtr properties = communicator->getProperties();
 
-    _nullSSLPVId.name = "NullSSLPermissionsVerifier";
-    _nullSSLPVId.category = category;
+    shared_ptr<Glacier2::PermissionsVerifier> nullPermissionsVerifier;
+    shared_ptr<Glacier2::SSLPermissionsVerifier> nullSSLPermissionsVerifier;
 
-    Ice::PropertiesPtr properties = _communicator->getProperties();
-    for(vector<string>::const_iterator p = props.begin(); p != props.end(); ++p)
+    for (const auto& propertyName : permissionsVerifierPropertyNames)
     {
-        string val = properties->getProperty(*p);
-        if(!val.empty())
+        string propertyValue = properties->getProperty(propertyName);
+        if (!propertyValue.empty())
         {
-            //
-            // Check permission verifier proxy. It returns a non-empty
-            // value with the new stringified proxy if the property
-            // needs to be rewritten.
-            //
-            val = checkPermissionVerifier(val);
-            if(!val.empty())
+            ObjectPrx prx(communicator, propertyValue);
+            if (prx->ice_getIdentity() == nullPermissionsVerifierId && !nullPermissionsVerifier)
+            {
+                nullPermissionsVerifier = make_shared<NullPermissionsVerifier>();
+            }
+            else if (prx->ice_getIdentity() == nullSSLPermissionsVerifierId && !nullSSLPermissionsVerifier)
             {
-                properties->setProperty(*p, val);
+                nullSSLPermissionsVerifier = make_shared<NullSSLPermissionsVerifier>();
             }
         }
     }
-}
 
-string
-Init::checkPermissionVerifier(const string& val)
-{
-    // Check if it's in proxy format
-    try
+    if (nullPermissionsVerifier || nullSSLPermissionsVerifier)
     {
-        ObjectPrxPtr prx  = _communicator->stringToProxy(val);
-        if(prx->ice_getIdentity() == _nullPVId || prx->ice_getIdentity() == _nullSSLPVId)
+        // Create collocated object adapter for the null permissions verifier
+        Ice::ObjectAdapterPtr adapter = communicator->createObjectAdapter("");
+        if (nullPermissionsVerifier)
         {
-            createObjects();
+            adapter->add(std::move(nullPermissionsVerifier), nullPermissionsVerifierId);
         }
-    }
-    catch(const ProxyParseException&)
-    {
-        // check if it's actually a stringified identity
-        // (with typically missing " " because the category contains a space)
 
-        if(val == _communicator->identityToString(_nullPVId))
+        if (nullSSLPermissionsVerifier)
         {
-            createObjects();
-            return _adapter->createProxy(_nullPVId)->ice_toString(); // Return valid proxy to rewrite the property
+            adapter->add(std::move(nullSSLPermissionsVerifier), nullSSLPermissionsVerifierId);
         }
-        else if(val == _communicator->identityToString(_nullSSLPVId))
-        {
-            createObjects();
-            return _adapter->createProxy(_nullSSLPVId)->ice_toString(); // Return valid proxy to rewrite the property
-        }
-
-        // Otherwise let the service report this incorrectly formatted proxy
-    }
-    return string();
-}
-
-void
-Init::createObjects()
-{
-    if(!_adapter)
-    {
-        _adapter = _communicator->createObjectAdapter(""); // colloc-only adapter
-        _adapter->add(std::make_shared<NullPermissionsVerifier>(), _nullPVId);
-        _adapter->add(std::make_shared<NullSSLPermissionsVerifier>(), _nullSSLPVId);
-        _adapter->activate();
     }
 }
 
 }
-
-namespace Glacier2Internal
-{
-
-void
-setupNullPermissionsVerifier(const CommunicatorPtr& communicator, const string& category, const vector<string>& props)
-{
-    Init init(communicator, category, props);
-}
-
-}
diff --git a/cpp/src/IceGrid/RegistryI.cpp b/cpp/src/IceGrid/RegistryI.cpp
@@ -565,7 +565,16 @@ RegistryI::startImpl()
     verifierProperties.push_back("IceGrid.Registry.AdminPermissionsVerifier");
     verifierProperties.push_back("IceGrid.Registry.AdminSSLPermissionsVerifier");
 
-    Glacier2Internal::setupNullPermissionsVerifier(_communicator, _instanceName, verifierProperties);
+    try
+    {
+        Glacier2Internal::setupNullPermissionsVerifier(_communicator, _instanceName, verifierProperties);
+    }
+    catch(const std::exception& ex)
+    {
+        Error out(_communicator->getLogger());
+        out << "unable to setup null permissions verifier:\n" << ex;
+        return false;
+    }
 
     auto sessionAdpt = setupClientSessionFactory(internalLocator);
     auto admSessionAdpt = setupAdminSessionFactory(serverAdminRouter, nodeAdminRouter, replicaAdminRouter,