You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2015/04/15 11:13:54 UTC

svn commit: r1673687 - in /jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl: DefaultSyncConfig.java jmx/SyncMBeanImpl.java

Author: angela
Date: Wed Apr 15 09:13:53 2015
New Revision: 1673687

URL: http://svn.apache.org/r1673687
Log:
CQ-35565 : Fix FindBug Issues (null-deref in oak-auth-external)

Modified:
    jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfig.java
    jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/SyncMBeanImpl.java

Modified: jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfig.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfig.java?rev=1673687&r1=1673686&r2=1673687&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfig.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DefaultSyncConfig.java Wed Apr 15 09:13:53 2015
@@ -215,6 +215,10 @@ public class DefaultSyncConfig {
     )
     public static final String PARAM_GROUP_PATH_PREFIX = "group.pathPrefix";
 
+    private static final long MILLIS_PER_HOUR = 60 * 60 * 1000;
+    private static final ConfigurationParameters.Milliseconds ONE_HOUR = ConfigurationParameters.Milliseconds.of(MILLIS_PER_HOUR);
+    private static final ConfigurationParameters.Milliseconds ONE_DAY = ConfigurationParameters.Milliseconds.of(24 * MILLIS_PER_HOUR);
+
     /**
      * Base config class for users and groups
      */
@@ -409,22 +413,16 @@ public class DefaultSyncConfig {
                 .setName(params.getConfigValue(PARAM_NAME, PARAM_NAME_DEFAULT));
 
         cfg.user()
-                .setMembershipExpirationTime(
-                        ConfigurationParameters.Milliseconds.of(params.getConfigValue(PARAM_USER_MEMBERSHIP_EXPIRATION_TIME, PARAM_USER_MEMBERSHIP_EXPIRATION_TIME_DEFAULT)).value
-                )
+                .setMembershipExpirationTime(getMilliSeconds(params, PARAM_USER_MEMBERSHIP_EXPIRATION_TIME, PARAM_USER_MEMBERSHIP_EXPIRATION_TIME_DEFAULT, ONE_HOUR))
                 .setMembershipNestingDepth(params.getConfigValue(PARAM_USER_MEMBERSHIP_NESTING_DEPTH, PARAM_USER_MEMBERSHIP_NESTING_DEPTH_DEFAULT))
-                .setExpirationTime(
-                        ConfigurationParameters.Milliseconds.of(params.getConfigValue(PARAM_USER_EXPIRATION_TIME, PARAM_USER_EXPIRATION_TIME_DEFAULT)).value
-                )
+                .setExpirationTime(getMilliSeconds(params, PARAM_USER_EXPIRATION_TIME, PARAM_USER_EXPIRATION_TIME_DEFAULT, ONE_HOUR))
                 .setPathPrefix(params.getConfigValue(PARAM_USER_PATH_PREFIX, PARAM_USER_PATH_PREFIX_DEFAULT))
                 .setAutoMembership(params.getConfigValue(PARAM_USER_AUTO_MEMBERSHIP, PARAM_USER_AUTO_MEMBERSHIP_DEFAULT))
                 .setPropertyMapping(createMapping(
                         params.getConfigValue(PARAM_USER_PROPERTY_MAPPING, PARAM_USER_PROPERTY_MAPPING_DEFAULT)));
 
         cfg.group()
-                .setExpirationTime(
-                        ConfigurationParameters.Milliseconds.of(params.getConfigValue(PARAM_GROUP_EXPIRATION_TIME, PARAM_GROUP_EXPIRATION_TIME_DEFAULT)).value
-                )
+                .setExpirationTime(getMilliSeconds(params, PARAM_GROUP_EXPIRATION_TIME, PARAM_GROUP_EXPIRATION_TIME_DEFAULT, ONE_DAY))
                 .setPathPrefix(params.getConfigValue(PARAM_GROUP_PATH_PREFIX, PARAM_GROUP_PATH_PREFIX_DEFAULT))
                 .setAutoMembership(params.getConfigValue(PARAM_GROUP_AUTO_MEMBERSHIP, PARAM_GROUP_AUTO_MEMBERSHIP_DEFAULT))
                 .setPropertyMapping(createMapping(
@@ -433,12 +431,18 @@ public class DefaultSyncConfig {
         return cfg;
     }
 
+    private static long getMilliSeconds(@Nonnull ConfigurationParameters params, @Nonnull String paramName,
+                                        @Nonnull String defaultParamValue,
+                                        @Nonnull ConfigurationParameters.Milliseconds defaultMillis) {
+        return ConfigurationParameters.Milliseconds.of(params.getConfigValue(paramName, defaultParamValue), defaultMillis).value;
+    }
+
     /**
      * Creates a new property mapping map from a list of patterns.
      * @param patterns the patterns
      * @return the mapping map
      */
-    private static Map<String, String> createMapping(String[] patterns) {
+    private static Map<String, String> createMapping(@Nonnull String[] patterns) {
         Map<String, String> mapping = new HashMap<String, String>();
         for (String pattern: patterns) {
             int idx = pattern.indexOf('=');

Modified: jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/SyncMBeanImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/SyncMBeanImpl.java?rev=1673687&r1=1673686&r2=1673687&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/SyncMBeanImpl.java (original)
+++ jackrabbit/oak/trunk/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/jmx/SyncMBeanImpl.java Wed Apr 15 09:13:53 2015
@@ -27,7 +27,6 @@ import javax.jcr.Repository;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.security.auth.Subject;
-import javax.security.auth.login.LoginException;
 
 import org.apache.jackrabbit.api.JackrabbitRepository;
 import org.apache.jackrabbit.api.JackrabbitSession;
@@ -111,13 +110,13 @@ public class SyncMBeanImpl implements Sy
 
         private Session systemSession;
 
-        private Delegatee(SyncHandler handler, ExternalIdentityProvider idp) throws SyncException {
+        private Delegatee(@Nonnull SyncHandler handler, @Nonnull ExternalIdentityProvider idp) throws SyncException {
             this.handler = handler;
             this.idp = idp;
             try {
                 systemSession = Subject.doAs(SystemSubject.INSTANCE, new PrivilegedExceptionAction<Session>() {
                     @Override
-                    public Session run() throws LoginException, RepositoryException {
+                    public Session run() throws RepositoryException {
                         if (repository instanceof JackrabbitRepository) {
                             // this is to bypass GuestCredentials injection in the "AbstractSlingRepository2"
                             return ((JackrabbitRepository) repository).login(null, null, null);
@@ -275,7 +274,8 @@ public class SyncMBeanImpl implements Sy
                 while (iter.hasNext()) {
                     SyncedIdentity id = iter.next();
                     if (isMyIDP(id)) {
-                        ExternalIdentity extId = idp.getIdentity(id.getExternalIdRef());
+                        ExternalIdentityRef exIdRef = id.getExternalIdRef();
+                        ExternalIdentity extId = (exIdRef == null) ? null : idp.getIdentity(exIdRef);
                         if (extId == null) {
                             list.add(id.getId());
                         }
@@ -310,16 +310,16 @@ public class SyncMBeanImpl implements Sy
             return result.toArray(new String[result.size()]);
         }
 
-        private boolean isMyIDP(SyncedIdentity id) {
+        private boolean isMyIDP(@Nonnull SyncedIdentity id) {
             String idpName = id.getExternalIdRef() == null
                     ? null
                     : id.getExternalIdRef().getProviderName();
-            return (idpName == null || idpName.length() ==0 || idpName.equals(idp.getName()));
+            return (idpName == null || idpName.isEmpty() || idpName.equals(idp.getName()));
         }
     }
 
-    private static String getJSONString(SyncResult r) {
-        String op = "";
+    private static String getJSONString(@Nonnull SyncResult r) {
+        String op;
         switch (r.getStatus()) {
             case NOP:
                 op = "nop";
@@ -345,19 +345,19 @@ public class SyncMBeanImpl implements Sy
             case FOREIGN:
                 op = "for";
                 break;
+            default:
+                op = "";
         }
-        String uid = JsonUtil.getJsonString(r.getIdentity().getId());
-        String eid = r.getIdentity().getExternalIdRef() == null
-                ? "\"\""
-                : JsonUtil.getJsonString(r.getIdentity().getExternalIdRef().getString());
+        SyncedIdentity syncedIdentity = r.getIdentity();
+        String uid = JsonUtil.getJsonString((syncedIdentity == null ? null : syncedIdentity.getId()));
+        ExternalIdentityRef externalIdentityRef = (syncedIdentity == null) ? null : syncedIdentity.getExternalIdRef();
+        String eid = (externalIdentityRef == null) ? "\"\"" : JsonUtil.getJsonString(externalIdentityRef.getString());
         return String.format("{op:\"%s\",uid:%s,eid:%s}", op, uid, eid);
     }
 
-    private static String getJSONString(SyncedIdentity id, Exception e) {
+    private static String getJSONString(@Nonnull SyncedIdentity id, @Nonnull Exception e) {
         String uid = JsonUtil.getJsonString(id.getId());
-        String eid = id.getExternalIdRef() == null
-                ? "\"\""
-                : JsonUtil.getJsonString(id.getExternalIdRef().getString());
+        String eid = (id.getExternalIdRef() == null) ? "\"\"" : JsonUtil.getJsonString(id.getExternalIdRef().getString());
         String msg = JsonUtil.getJsonString(e.toString());
         return String.format("{op:\"ERR\",uid:%s,eid:%s,msg:%s}", uid, eid, msg);
     }