From b59d8305130e81d3e00240b5612a327c9dfc7d12 Mon Sep 17 00:00:00 2001
From: Ade Lee <alee@redhat.com>
Date: Tue, 19 Apr 2016 14:52:40 -0400
Subject: Realms - Address comments from review

Review comments addressed:
1. when archiving or generating keys, realm is checked
2. when no plugin is found for a realm, access is denied.
3. rename mFoo to foo for new variables.
4. add chaining of exceptions
5. remove attributes from KeyArchivalRequest etc. when realm is null
6. Add more detail to denial in BasicGroupAuthz

Part of Trac Ticket 2041
---
 .../netscape/cms/authorization/BasicGroupAuthz.java | 21 +++++++++++----------
 .../com/netscape/cms/servlet/key/KeyRequestDAO.java | 15 ++++++++++-----
 2 files changed, 21 insertions(+), 15 deletions(-)

(limited to 'base/server/cms')

diff --git a/base/server/cms/src/com/netscape/cms/authorization/BasicGroupAuthz.java b/base/server/cms/src/com/netscape/cms/authorization/BasicGroupAuthz.java
index 1908e3c69..0bf24311f 100644
--- a/base/server/cms/src/com/netscape/cms/authorization/BasicGroupAuthz.java
+++ b/base/server/cms/src/com/netscape/cms/authorization/BasicGroupAuthz.java
@@ -44,35 +44,35 @@ public class BasicGroupAuthz implements IAuthzManager, IExtendedPluginInfo {
     private static final String GROUP = "group";
 
     /* name of this authorization manager instance */
-    private String name = null;
+    private String name;
 
     /* name of the authorization manager plugin */
-    private String implName = null;
+    private String implName;
 
     /* configuration store */
     private IConfigStore config;
 
     /* group that is allowed to access resources */
-    private String groupName = null;
+    private String groupName;
 
     /* Vector of extendedPluginInfo strings */
-    protected static Vector<String> mExtendedPluginInfo = null;
+    protected static Vector<String> extendedPluginInfo;
 
-    protected static String[] mConfigParams = null;
+    protected static String[] configParams;
 
     static {
-        mExtendedPluginInfo = new Vector<String>();
-        mExtendedPluginInfo.add("group;string,required;" +
+        extendedPluginInfo = new Vector<String>();
+        extendedPluginInfo.add("group;string,required;" +
                 "Group to permit access");
     }
 
     public BasicGroupAuthz() {
-        mConfigParams = new String[] {"group"};
+        configParams = new String[] {"group"};
     }
 
     @Override
     public String[] getExtendedPluginInfo(Locale locale) {
-        String[] s = Utils.getStringArrayFromVector(mExtendedPluginInfo);
+        String[] s = Utils.getStringArrayFromVector(extendedPluginInfo);
         return s;
     }
 
@@ -103,6 +103,7 @@ public class BasicGroupAuthz implements IAuthzManager, IExtendedPluginInfo {
         IUGSubsystem ug = (IUGSubsystem) CMS.getSubsystem(CMS.SUBSYSTEM_UG);
         IGroup group = ug.getGroupFromName(groupName);
         if (!group.isMember(user)) {
+            CMS.debug("BasicGroupAuthz: access denied. User: " + user + " is not a member of group: " + groupName);
             throw new EAuthzAccessDenied("Access denied");
         }
 
@@ -139,7 +140,7 @@ public class BasicGroupAuthz implements IAuthzManager, IExtendedPluginInfo {
 
     @Override
     public String[] getConfigParams() throws EBaseException {
-        return mConfigParams;
+        return configParams;
     }
 
     @Override
diff --git a/base/server/cms/src/com/netscape/cms/servlet/key/KeyRequestDAO.java b/base/server/cms/src/com/netscape/cms/servlet/key/KeyRequestDAO.java
index 8aa0d21ee..04bb6f2ec 100644
--- a/base/server/cms/src/com/netscape/cms/servlet/key/KeyRequestDAO.java
+++ b/base/server/cms/src/com/netscape/cms/servlet/key/KeyRequestDAO.java
@@ -37,6 +37,7 @@ import org.mozilla.jss.crypto.KeyPairAlgorithm;
 
 import com.netscape.certsrv.apps.CMS;
 import com.netscape.certsrv.authentication.IAuthToken;
+import com.netscape.certsrv.authorization.EAuthzUnknownRealm;
 import com.netscape.certsrv.base.BadRequestException;
 import com.netscape.certsrv.base.EBaseException;
 import com.netscape.certsrv.base.PKIException;
@@ -259,13 +260,15 @@ public class KeyRequestDAO extends CMSRequestDAO {
         try {
             rec = repo.readKeyRecord(keyId.toBigInteger());
         } catch (EDBRecordNotFoundException e) {
-            throw new KeyNotFoundException(keyId);
+            throw new KeyNotFoundException(keyId, "key not found to recover", e);
         }
 
         try {
             authz.checkRealm(rec.getRealm(), authToken, rec.getOwnerName(), "key", "recover");
+        } catch (EAuthzUnknownRealm e) {
+            throw new UnauthorizedException("Invalid realm", e);
         } catch (EBaseException e) {
-            throw new UnauthorizedException("Agent not authorized by realm");
+            throw new UnauthorizedException("Agent not authorized by realm", e);
         }
 
         Hashtable<String, Object> requestParams;
@@ -315,13 +318,15 @@ public class KeyRequestDAO extends CMSRequestDAO {
         try {
             rec = repo.readKeyRecord(keyId.toBigInteger());
         } catch (EDBRecordNotFoundException e) {
-            throw new KeyNotFoundException(keyId);
+            throw new KeyNotFoundException(keyId, "key not found to recover", e);
         }
 
         try {
             authz.checkRealm(rec.getRealm(), authToken, rec.getOwnerName(), "key", "recover");
+        } catch (EAuthzUnknownRealm e) {
+            throw new UnauthorizedException("Invalid realm", e);
         } catch (EBaseException e) {
-            throw new UnauthorizedException("Agent not authorized by realm");
+            throw new UnauthorizedException("Agent not authorized by realm", e);
         }
 
         String b64Certificate = data.getCertificate();
@@ -332,7 +337,7 @@ public class KeyRequestDAO extends CMSRequestDAO {
             // TODO - update request with realm
         } catch (EBaseException | CertificateException e) {
             e.printStackTrace();
-            throw new PKIException(e.toString());
+            throw new PKIException(e.toString(), e);
         }
         IRequest request = null;
         try {
-- 
cgit