You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by lh...@apache.org on 2012/01/17 22:31:18 UTC

svn commit: r1232584 - in /shiro/trunk/core/src: main/java/org/apache/shiro/config/ main/java/org/apache/shiro/realm/text/ test/groovy/org/apache/shiro/config/ test/java/org/apache/shiro/config/

Author: lhazlewood
Date: Tue Jan 17 21:31:16 2012
New Revision: 1232584

URL: http://svn.apache.org/viewvc?rev=1232584&view=rev
Log:
SHIRO-322: Ensured IniSecurityManagerFactory does not initialize the implicit iniRealm instance before configured properties have the chance to be injected.  Added new test case in IniSecurityManagerFactoryTest to reflect this.  IniRealm now retains an 'ini' property, but constructors have not been changed to ensure backwards compatibility.

Added:
    shiro/trunk/core/src/test/groovy/org/apache/shiro/config/IniSecurityManagerFactoryTest.groovy
    shiro/trunk/core/src/test/groovy/org/apache/shiro/config/MockPermissionResolver.groovy
Removed:
    shiro/trunk/core/src/test/java/org/apache/shiro/config/IniSecurityManagerFactoryTest.java
Modified:
    shiro/trunk/core/src/main/java/org/apache/shiro/config/IniSecurityManagerFactory.java
    shiro/trunk/core/src/main/java/org/apache/shiro/realm/text/IniRealm.java

Modified: shiro/trunk/core/src/main/java/org/apache/shiro/config/IniSecurityManagerFactory.java
URL: http://svn.apache.org/viewvc/shiro/trunk/core/src/main/java/org/apache/shiro/config/IniSecurityManagerFactory.java?rev=1232584&r1=1232583&r2=1232584&view=diff
==============================================================================
--- shiro/trunk/core/src/main/java/org/apache/shiro/config/IniSecurityManagerFactory.java (original)
+++ shiro/trunk/core/src/main/java/org/apache/shiro/config/IniSecurityManagerFactory.java Tue Jan 17 21:31:16 2012
@@ -252,8 +252,10 @@ public class IniSecurityManagerFactory e
      * @return a new Realm instance reflecting the account data discovered in the {@code Ini}.
      */
     protected Realm createRealm(Ini ini) {
-        IniRealm realm = new IniRealm(ini);
+        //IniRealm realm = new IniRealm(ini); changed to support SHIRO-322
+        IniRealm realm = new IniRealm();
         realm.setName(INI_REALM_NAME);
+        realm.setIni(ini); //added for SHIRO-322
         return realm;
     }
 }
\ No newline at end of file

Modified: shiro/trunk/core/src/main/java/org/apache/shiro/realm/text/IniRealm.java
URL: http://svn.apache.org/viewvc/shiro/trunk/core/src/main/java/org/apache/shiro/realm/text/IniRealm.java?rev=1232584&r1=1232583&r2=1232584&view=diff
==============================================================================
--- shiro/trunk/core/src/main/java/org/apache/shiro/realm/text/IniRealm.java (original)
+++ shiro/trunk/core/src/main/java/org/apache/shiro/realm/text/IniRealm.java Tue Jan 17 21:31:16 2012
@@ -50,19 +50,48 @@ public class IniRealm extends TextConfig
     private static transient final Logger log = LoggerFactory.getLogger(IniRealm.class);
 
     private String resourcePath;
+    private Ini ini; //reference added in 1.2 for SHIRO-322
 
     public IniRealm() {
         super();
     }
 
+    /**
+     * This constructor will immediately process the definitions in the {@code Ini} argument.  If you need to perform
+     * additional configuration before processing (e.g. setting a permissionResolver, etc), do not call this
+     * constructor.  Instead, do the following:
+     * <ol>
+     * <li>Call the default no-arg constructor</li>
+     * <li>Set the Ini instance you wish to use via {@code #setIni}</li>
+     * <li>Set any other configuration properties</li>
+     * <li>Call {@link #init()}</li>
+     * </ol>
+     *
+     * @param ini the Ini instance which will be inspected to create accounts, groups and permissions for this realm.
+     */
     public IniRealm(Ini ini) {
         this();
         processDefinitions(ini);
     }
 
+    /**
+     * This constructor will immediately process the definitions in the {@code Ini} resolved from the specified
+     * {@code resourcePath}.  If you need to perform additional configuration before processing (e.g. setting a
+     * permissionResolver, etc), do not call this constructor.  Instead, do the following:
+     * <ol>
+     * <li>Call the default no-arg constructor</li>
+     * <li>Set the Ini instance you wish to use via {@code #setIni}</li>
+     * <li>Set any other configuration properties</li>
+     * <li>Call {@link #init()}</li>
+     * </ol>
+     *
+     * @param resourcePath the resource path of the Ini config which will be inspected to create accounts, groups and
+     *                     permissions for this realm.
+     */
     public IniRealm(String resourcePath) {
         this();
         Ini ini = Ini.fromResourcePath(resourcePath);
+        this.ini = ini;
         this.resourcePath = resourcePath;
         processDefinitions(ini);
     }
@@ -75,27 +104,68 @@ public class IniRealm extends TextConfig
         this.resourcePath = resourcePath;
     }
 
+    /**
+     * Returns the Ini instance used to configure this realm.  Provided for JavaBeans-style configuration of this
+     * realm, particularly useful in Dependency Injection environments.
+     * 
+     * @return the Ini instance which will be inspected to create accounts, groups and permissions for this realm.
+     */
+    public Ini getIni() {
+        return ini;
+    }
+
+    /**
+     * Sets the Ini instance used to configure this realm.  Provided for JavaBeans-style configuration of this
+     * realm, particularly useful in Dependency Injection environments.
+     * 
+     * @param ini the Ini instance which will be inspected to create accounts, groups and permissions for this realm.
+     */
+    public void setIni(Ini ini) {
+        this.ini = ini;
+    }
+
     @Override
     protected void onInit() {
         super.onInit();
+
         // This is an in-memory realm only - no need for an additional cache when we're already
         // as memory-efficient as we can be.
+        
+        Ini ini = getIni();
         String resourcePath = getResourcePath();
-
-        if (CollectionUtils.isEmpty(this.users) && CollectionUtils.isEmpty(this.roles)) {
-            //no account data manually populated - try the resource path:
+                
+        if (!CollectionUtils.isEmpty(this.users) || !CollectionUtils.isEmpty(this.roles)) {
+            if (!CollectionUtils.isEmpty(ini)) {
+                log.warn("Users or Roles are already populated.  Configured Ini instance will be ignored.");
+            }
             if (StringUtils.hasText(resourcePath)) {
-                log.debug("Resource path {} defined.  Creating INI instance.", resourcePath);
-                Ini ini = Ini.fromResourcePath(resourcePath);
-                processDefinitions(ini);
-            } else {
-                throw new IllegalStateException("No resource path was specified.  Cannot load account data.");
+                log.warn("Users or Roles are already populated.  resourcePath '{}' will be ignored.", resourcePath);
             }
-        } else {
+            
+            log.debug("Instance is already populated with users or roles.  No additional user/role population " +
+                    "will be performed.");
+            return;
+        }
+        
+        if (CollectionUtils.isEmpty(ini)) {
+            log.debug("No INI instance configuration present.  Checking resourcePath...");
+            
             if (StringUtils.hasText(resourcePath)) {
-                log.warn("Users or Roles are already populated.  Resource path property will be ignored.");
+                log.debug("Resource path {} defined.  Creating INI instance.", resourcePath);
+                ini = Ini.fromResourcePath(resourcePath);
+                if (!CollectionUtils.isEmpty(ini)) {
+                    setIni(ini);
+                }
             }
         }
+        
+        if (CollectionUtils.isEmpty(ini)) {
+            String msg = "Ini instance and/or resourcePath resulted in null or empty Ini configuration.  Cannot " +
+                    "load account data.";
+            throw new IllegalStateException(msg);
+        }
+
+        processDefinitions(ini);
     }
 
     private void processDefinitions(Ini ini) {

Added: shiro/trunk/core/src/test/groovy/org/apache/shiro/config/IniSecurityManagerFactoryTest.groovy
URL: http://svn.apache.org/viewvc/shiro/trunk/core/src/test/groovy/org/apache/shiro/config/IniSecurityManagerFactoryTest.groovy?rev=1232584&view=auto
==============================================================================
--- shiro/trunk/core/src/test/groovy/org/apache/shiro/config/IniSecurityManagerFactoryTest.groovy (added)
+++ shiro/trunk/core/src/test/groovy/org/apache/shiro/config/IniSecurityManagerFactoryTest.groovy Tue Jan 17 21:31:16 2012
@@ -0,0 +1,222 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.shiro.config
+
+import org.apache.shiro.SecurityUtils
+import org.apache.shiro.authc.UsernamePasswordToken
+import org.apache.shiro.cache.Cache
+import org.apache.shiro.cache.MapCache
+import org.apache.shiro.crypto.hash.Sha256Hash
+import org.apache.shiro.mgt.DefaultSecurityManager
+import org.apache.shiro.mgt.SecurityManager
+import org.apache.shiro.realm.Realm
+import org.apache.shiro.realm.text.IniRealm
+import org.apache.shiro.realm.text.PropertiesRealm
+import org.apache.shiro.session.Session
+import org.apache.shiro.session.mgt.AbstractSessionManager
+import org.apache.shiro.session.mgt.DefaultSessionManager
+import org.apache.shiro.session.mgt.eis.CachingSessionDAO
+import org.apache.shiro.session.mgt.eis.EnterpriseCacheSessionDAO
+import org.apache.shiro.session.mgt.eis.SessionDAO
+import org.apache.shiro.subject.Subject
+
+/**
+ * Unit tests for the {@link IniSecurityManagerFactory} implementation.
+ *
+ * @since 1.0
+ */
+class IniSecurityManagerFactoryTest extends GroovyTestCase {
+
+    void testGetInstanceWithoutIni() {
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory();
+        SecurityManager sm = factory.getInstance();
+        assertNotNull(sm);
+        assertTrue(sm instanceof DefaultSecurityManager);
+    }
+
+    void testGetInstanceWithResourcePath() {
+        String path = "classpath:org/apache/shiro/config/IniSecurityManagerFactoryTest.ini";
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(path);
+        SecurityManager sm = factory.getInstance();
+        assertNotNull(sm);
+        assertTrue(sm instanceof DefaultSecurityManager);
+    }
+
+    void testGetInstanceWithEmptyIni() {
+        Ini ini = new Ini();
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
+        SecurityManager sm = factory.getInstance();
+        assertNotNull(sm);
+        assertTrue(sm instanceof DefaultSecurityManager);
+    }
+
+    void testGetInstanceWithSimpleIni() {
+        Ini ini = new Ini();
+        ini.setSectionProperty(IniSecurityManagerFactory.MAIN_SECTION_NAME,
+                "securityManager.sessionManager.globalSessionTimeout", "5000");
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
+        SecurityManager sm = factory.getInstance();
+        assertNotNull(sm);
+        assertTrue(sm instanceof DefaultSecurityManager);
+        assertEquals(5000, ((AbstractSessionManager) ((DefaultSecurityManager) sm).getSessionManager()).getGlobalSessionTimeout());
+    }
+
+    void testGetInstanceWithConfiguredRealm() {
+        Ini ini = new Ini();
+        Ini.Section section = ini.addSection(IniSecurityManagerFactory.MAIN_SECTION_NAME);
+        section.put("propsRealm", PropertiesRealm.class.getName());
+        section.put("propsRealm.resourcePath",
+                "classpath:org/apache/shiro/config/IniSecurityManagerFactoryTest.propsRealm.properties");
+
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
+        SecurityManager sm = factory.getInstance();
+        assertNotNull(sm);
+        assertTrue(sm instanceof DefaultSecurityManager);
+        Collection<Realm> realms = ((DefaultSecurityManager) sm).getRealms();
+        assertEquals(1, realms.size());
+        Realm realm = realms.iterator().next();
+        assertTrue(realm instanceof PropertiesRealm);
+    }
+
+    void testGetInstanceWithAutomaticallyCreatedIniRealm() {
+        Ini ini = new Ini();
+        Ini.Section section = ini.addSection(IniRealm.USERS_SECTION_NAME);
+        section.put("admin", "admin");
+
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
+        SecurityManager sm = factory.getInstance();
+        assertNotNull(sm);
+        assertTrue(sm instanceof DefaultSecurityManager);
+        Collection<Realm> realms = ((DefaultSecurityManager) sm).getRealms();
+        assertEquals(1, realms.size());
+        Realm realm = realms.iterator().next();
+        assertTrue(realm instanceof IniRealm);
+        assertTrue(((IniRealm) realm).accountExists("admin"));
+    }
+
+    /**
+     * Test for issue <a href="https://issues.apache.org/jira/browse/SHIRO-125">SHIRO-125</a>.
+     */
+    void testImplicitIniRealmWithAdditionalRealmConfiguration() {
+
+        Ini ini = new Ini();
+
+        //The users section below should create an implicit 'iniRealm' instance in the
+        //main configuration.  So we should be able to set properties on it immediately
+        //such as the Sha256 credentials matcher:
+        Ini.Section main = ini.addSection("main");
+        main.put("credentialsMatcher", "org.apache.shiro.authc.credential.Sha256CredentialsMatcher");
+        main.put("iniRealm.credentialsMatcher", '$credentialsMatcher');
+
+        //create a users section - user 'admin', with a Sha256-hashed 'admin' password (hex encoded):
+        Ini.Section users = ini.addSection(IniRealm.USERS_SECTION_NAME);
+        users.put("admin", new Sha256Hash("secret").toString());
+
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
+        SecurityManager sm = factory.getInstance();
+
+        //go ahead and try to log in with the admin user, ensuring the
+        //iniRealm has a Sha256CredentialsMatcher enabled:
+
+        //try to log-in:
+        Subject subject = new Subject.Builder(sm).buildSubject();
+        //ensure thread clean-up after the login method returns.  Test cases only:
+        subject.execute(new Runnable() {
+            public void run() {
+                //the plain-text 'secret' should be converted to an Sha256 hash first
+                //by the CredentialsMatcher.  This should return quietly if
+                //this test case is valid:
+                SecurityUtils.getSubject().login(new UsernamePasswordToken("admin", "secret"));
+            }
+        });
+        assertTrue(subject.getPrincipal().equals("admin"));
+    }
+
+    /**
+     * Test for issue <a href="https://issues.apache.org/jira/browse/SHIRO-322">SHIRO-322</a>.
+     */
+    void testImplicitIniRealmWithConfiguredPermissionResolver() {
+        def ini = new Ini();
+        ini.load('''
+            [main]
+            # The MockPermissionResolver is a peer class to this test class.
+            permissionResolver = org.apache.shiro.config.MockPermissionResolver
+            iniRealm.permissionResolver = $permissionResolver
+
+            [users]
+            jsmith = secret, author
+
+            [roles]
+            author = book:write
+        ''');
+
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
+        SecurityManager sm = factory.instance
+        
+        def realm = sm.realms[0]
+        assertNotNull realm
+        def permResolver = realm.permissionResolver
+        assertTrue permResolver instanceof MockPermissionResolver
+        assertTrue permResolver.invoked
+    }
+
+    /**
+     * Test case for issue <a href="https://issues.apache.org/jira/browse/SHIRO-95">SHIRO-95</a>.
+     */
+    void testCacheManagerConfigOrderOfOperations() {
+
+        Ini ini = new Ini();
+        Ini.Section main = ini.addSection(IniSecurityManagerFactory.MAIN_SECTION_NAME);
+        //create a non-default CacheManager:
+        main.put("cacheManager", "org.apache.shiro.config.HashMapCacheManager");
+
+        //now add a session DAO after the cache manager has been set - this is what tests the user-reported issue
+        main.put("sessionDAO", "org.apache.shiro.session.mgt.eis.EnterpriseCacheSessionDAO");
+        main.put("securityManager.sessionManager.sessionDAO", '$sessionDAO');
+
+        //add the cache manager after the sessionDAO has been set:
+        main.put("securityManager.cacheManager", '$cacheManager');
+
+        //add a test user:
+        ini.setSectionProperty(IniRealm.USERS_SECTION_NAME, "admin", "admin");
+
+        IniSecurityManagerFactory factory = new IniSecurityManagerFactory(ini);
+        SecurityManager sm = factory.getInstance();
+
+        //try to log-in:
+        Subject subject = new Subject.Builder(sm).buildSubject();
+        subject.login(new UsernamePasswordToken("admin", "admin"));
+        Session session = subject.getSession();
+        session.setAttribute("hello", "world");
+        //session should have been started, and a cache is in use.  Assert that the SessionDAO is still using
+        //the cache instances provided by our custom CacheManager and not the Default MemoryConstrainedCacheManager
+
+        SessionDAO sessionDAO = ((DefaultSessionManager) ((DefaultSecurityManager) sm).getSessionManager()).getSessionDAO();
+        assertTrue(sessionDAO instanceof EnterpriseCacheSessionDAO);
+        CachingSessionDAO cachingSessionDAO = (CachingSessionDAO) sessionDAO;
+        Cache activeSessionsCache = cachingSessionDAO.getActiveSessionsCache();
+        assertTrue(activeSessionsCache instanceof MapCache);
+        MapCache mapCache = (MapCache) activeSessionsCache;
+
+        //this is the line that verifies Caches created by our specific CacheManager are not overwritten by the
+        //default cache manager's caches:
+        assertTrue(mapCache instanceof HashMapCacheManager.HashMapCache);
+    }
+
+}

Added: shiro/trunk/core/src/test/groovy/org/apache/shiro/config/MockPermissionResolver.groovy
URL: http://svn.apache.org/viewvc/shiro/trunk/core/src/test/groovy/org/apache/shiro/config/MockPermissionResolver.groovy?rev=1232584&view=auto
==============================================================================
--- shiro/trunk/core/src/test/groovy/org/apache/shiro/config/MockPermissionResolver.groovy (added)
+++ shiro/trunk/core/src/test/groovy/org/apache/shiro/config/MockPermissionResolver.groovy Tue Jan 17 21:31:16 2012
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.shiro.config
+
+import org.apache.shiro.authz.Permission
+import org.apache.shiro.authz.permission.PermissionResolver
+import org.apache.shiro.authz.permission.WildcardPermission
+
+/**
+ * Test {@code PermissionResolver} implementation used in the {@link IniSecurityManagerFactoryTest}.
+ */
+class MockPermissionResolver implements PermissionResolver {
+
+    boolean invoked = false
+
+    Permission resolvePermission(String permissionString) {
+        invoked = true
+        return new WildcardPermission(permissionString)
+    }
+}