This is an automated email from the ASF dual-hosted git repository. lprimak pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/shiro.git
commit 337b4f194931dfaa9e559cc8b69cc9c61b446ebf Author: lprimak <[email protected]> AuthorDate: Wed Oct 21 14:01:40 2026 -0500 bugfix: improve resourcePath validation and clear loaded state on restart --- .../apache/shiro/realm/text/PropertiesRealm.java | 34 +++- .../shiro/realm/text/PropertiesRealmTest.java | 190 +++++++++++++++++++++ 2 files changed, 220 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/shiro/realm/text/PropertiesRealm.java b/core/src/main/java/org/apache/shiro/realm/text/PropertiesRealm.java index 067dcb785..cbb681ab3 100644 --- a/core/src/main/java/org/apache/shiro/realm/text/PropertiesRealm.java +++ b/core/src/main/java/org/apache/shiro/realm/text/PropertiesRealm.java @@ -18,6 +18,7 @@ */ package org.apache.shiro.realm.text; +import org.apache.shiro.cache.Cache; import org.apache.shiro.lang.ShiroException; import org.apache.shiro.lang.io.ResourceUtils; import org.apache.shiro.lang.util.Destroyable; @@ -28,11 +29,13 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.util.Enumeration; +import java.util.Objects; import java.util.Properties; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; /** * A {@link TextConfigurationRealm} that defers all logic to the parent class, but just enables @@ -205,7 +208,7 @@ public class PropertiesRealm extends TextConfigurationRealm implements Destroyab } private void loadProperties() { - if (resourcePath == null || resourcePath.length() == 0) { + if (resourcePath == null || resourcePath.isEmpty()) { throw new IllegalStateException("The resourcePath property is not set. " + "It must be set prior to this realm being initialized."); } @@ -280,9 +283,15 @@ public class PropertiesRealm extends TextConfigurationRealm implements Destroyab } } - @SuppressWarnings("unchecked") - private void restart() { - if (resourcePath == null || resourcePath.length() == 0) { + /** + * Reloads the realm from the properties source. + * <p/> + * The accounts and roles loaded so far are dropped first, so entries that are no longer present in the source + * (or whose roles and permissions were narrowed) are removed. If the source can't be loaded, the realm is left + * without the entries that were dropped. + */ + void restart() { + if (resourcePath == null || resourcePath.isEmpty()) { throw new IllegalStateException("The resourcePath property is not set. " + "It must be set prior to this realm being initialized."); } @@ -296,9 +305,26 @@ public class PropertiesRealm extends TextConfigurationRealm implements Destroyab } catch (Exception e) { //ignored } + clearLoadedState(); init(); } + private void clearLoadedState() { + usersLock.writeLock().lock(); + rolesLock.writeLock().lock(); + try { + users.clear(); + roles.clear(); + } finally { + rolesLock.writeLock().unlock(); + usersLock.writeLock().unlock(); + } + // init() would process the previous definitions again, which would bring the dropped entries back + setUserDefinitions(null); + setRoleDefinitions(null); + Stream.of(getAuthenticationCache(), getAuthorizationCache()).filter(Objects::nonNull).forEach(Cache::clear); + } + @SuppressWarnings("unchecked") private void createRealmEntitiesFromProperties(Properties properties) { diff --git a/core/src/test/java/org/apache/shiro/realm/text/PropertiesRealmTest.java b/core/src/test/java/org/apache/shiro/realm/text/PropertiesRealmTest.java new file mode 100644 index 000000000..c06baff74 --- /dev/null +++ b/core/src/test/java/org/apache/shiro/realm/text/PropertiesRealmTest.java @@ -0,0 +1,190 @@ +/* + * 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.realm.text; + +import org.apache.shiro.authc.UsernamePasswordToken; +import org.apache.shiro.cache.MemoryConstrainedCacheManager; +import org.apache.shiro.subject.ImmutablePrincipalCollection; +import org.apache.shiro.subject.PrincipalCollection; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests that reloading a {@link PropertiesRealm} replaces its content: access can be revoked, not only granted. + */ +class PropertiesRealmTest { + private static final String INITIAL = """ + user.alice = pass1, admin + user.bob = pass2, admin + role.admin = * + """; + + @TempDir + Path tempDir; + + private Path file; + private PropertiesRealm realm; + + @BeforeEach + void setUp() throws IOException { + file = tempDir.resolve("users.properties"); + write(INITIAL); + realm = new PropertiesRealm(); + realm.setResourcePath("file:" + file.toAbsolutePath()); + // reloads are triggered explicitly, no background thread + realm.setReloadIntervalSeconds(0); + realm.init(); + } + + @AfterEach + void tearDown() { + realm.destroy(); + } + + private void write(String content) throws IOException { + Files.writeString(file, content); + } + + private boolean authenticates(String user, String password) { + return realm.getAuthenticationInfo(new UsernamePasswordToken(user, password)) != null; + } + + private boolean permitted(String user, String permission) { + PrincipalCollection principals = ImmutablePrincipalCollection.ofSinglePrincipal(user, realm.getName()); + return realm.isPermitted(principals, permission); + } + + @Test + void initialStateIsLoaded() { + assertThat(authenticates("alice", "pass1")).isTrue(); + assertThat(authenticates("bob", "pass2")).isTrue(); + assertThat(permitted("bob", "anything:anything")).isTrue(); + } + + @Test + void reloadRemovesDeletedUser() throws IOException { + write(""" + user.bob = pass2, admin + role.admin = * + """); + realm.restart(); + + assertThat(authenticates("alice", "pass1")).isFalse(); + assertThat(permitted("alice", "anything:anything")).isFalse(); + assertThat(authenticates("bob", "pass2")).isTrue(); + } + + @Test + void reloadRemovesNarrowedPermissions() throws IOException { + write(""" + user.alice = pass1, admin + user.bob = pass2, admin + role.admin = app:read + """); + realm.restart(); + + assertThat(permitted("bob", "app:read")).isTrue(); + assertThat(permitted("bob", "anything:anything")).isFalse(); + } + + @Test + void reloadRemovesDroppedRole() throws IOException { + write(""" + user.alice = pass1, admin, auditor + user.bob = pass2, auditor + role.admin = * + role.auditor = audit:read + """); + realm.restart(); + assertThat(permitted("bob", "audit:read")).isTrue(); + assertThat(permitted("bob", "anything:anything")).isFalse(); + + write(""" + user.alice = pass1, auditor + user.bob = pass2 + role.auditor = audit:read + """); + realm.restart(); + assertThat(permitted("alice", "anything:anything")).isFalse(); + assertThat(permitted("alice", "audit:read")).isTrue(); + assertThat(permitted("bob", "audit:read")).isFalse(); + } + + @Test + void reloadAddsAndUpdatesUsers() throws IOException { + write(""" + user.alice = changed, admin + user.carol = pass3, admin + role.admin = * + """); + realm.restart(); + + assertThat(authenticates("carol", "pass3")).isTrue(); + assertThat(authenticates("alice", "changed")).isTrue(); + assertThat(authenticates("bob", "pass2")).isFalse(); + } + + @Test + void reloadClearsCachedInfo() throws IOException { + realm.setCacheManager(new MemoryConstrainedCacheManager()); + realm.setAuthenticationCachingEnabled(true); + realm.setAuthorizationCachingEnabled(true); + assertThat(authenticates("alice", "pass1")).isTrue(); + assertThat(permitted("alice", "anything:anything")).isTrue(); + + write(""" + user.bob = pass2, admin + role.admin = * + """); + realm.restart(); + + assertThat(authenticates("alice", "pass1")).isFalse(); + assertThat(permitted("alice", "anything:anything")).isFalse(); + } + + @Test + void reloadRunsInitializationHooks() throws IOException { + AtomicInteger initCalls = new AtomicInteger(); + PropertiesRealm hooked = new PropertiesRealm() { + @Override + public void onInit() { + super.onInit(); + initCalls.incrementAndGet(); + } + }; + hooked.setResourcePath("file:" + file.toAbsolutePath()); + hooked.setReloadIntervalSeconds(0); + hooked.init(); + assertThat(initCalls).hasValue(1); + + hooked.restart(); + + assertThat(initCalls).hasValue(2); + hooked.destroy(); + } +}
