From 0deb226953338eb7610ad4447b8e4a23a4d5ceb9 Mon Sep 17 00:00:00 2001 From: Lea Morschel Date: Wed, 13 Dec 2023 16:30:03 +0100 Subject: [PATCH] gplazma: configured banfile plugin should ignore non-existent ban file Motivation: When configured, gplazma requires a banfile to be present, even if it is empty. When one does not exist, gplazma will report the following on initialization, but continue to start: `failed to create banfile: config file doesn't exist or not a file`. All subsequent login requests will fail and result in a NullPointerException. Modification: As a workaround, the banfile plugin, even if configured, will now ignore an inexistent ban file and not fail with a NullPointerException on every subsequent login attempt. When a banfile is added or filled at some point, it will be used for subseqent login attempts. Eventually, failed plugins should be ignored by gplazma, or gplazma should fail when trying to load them. Result: Even if configured, an empty or inexistent banfile will be ignored and logins should succeed. Target: master, 9.2, 9.1, 9.0, 8.2 Fixes: #7465 Requires-notes: yes Requires-book: no Acked-by: Dmitry Litvintsev, Paul Millar, Tigran Mkrtchyan, Marina Sahakyan --- .../TheBook/src/main/markdown/config-gplazma.md | 2 +- docs/TheBook/src/main/markdown/install.md | 2 +- .../dcache/gplazma/plugins/BanFilePlugin.java | 17 +++++++++++++---- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/docs/TheBook/src/main/markdown/config-gplazma.md b/docs/TheBook/src/main/markdown/config-gplazma.md index ba21f5786f3..bd54ee25947 100644 --- a/docs/TheBook/src/main/markdown/config-gplazma.md +++ b/docs/TheBook/src/main/markdown/config-gplazma.md @@ -757,7 +757,7 @@ In this example the first line is a comment. Lines 2 to 5 define aliases for pri Please note that the plug-in only supports principals whose assiciated name is a single line of plain text. In programming terms this means the constructor of the principal class has to take exactly one single string parameter. -For the plugin to work, the configuration file has to exist even if it is empty. +The plugin assumes that a missing file is equivalent to a file with no contents; i.e., that no one has been banned. After modifying the banfile, you may want to update gplazma.conf to trigger a reload to activate the changes. You can test the result with the `explain login` command in the gPlazma cell. diff --git a/docs/TheBook/src/main/markdown/install.md b/docs/TheBook/src/main/markdown/install.md index d6a4d551952..8f8a57ef007 100644 --- a/docs/TheBook/src/main/markdown/install.md +++ b/docs/TheBook/src/main/markdown/install.md @@ -656,7 +656,7 @@ username:tester uid:1000 gid:1000,true username:admin uid:0 gid:0,true ``` -Additionally we create an empty banfile to make sure the ban file plugin works: +Additionally, we create an empty banfile to make sure the ban file plugin works: ```console-root touch /etc/dcache/ban.conf diff --git a/modules/gplazma2-banfile/src/main/java/org/dcache/gplazma/plugins/BanFilePlugin.java b/modules/gplazma2-banfile/src/main/java/org/dcache/gplazma/plugins/BanFilePlugin.java index 2abac1f7a4d..b9740e94e49 100644 --- a/modules/gplazma2-banfile/src/main/java/org/dcache/gplazma/plugins/BanFilePlugin.java +++ b/modules/gplazma2-banfile/src/main/java/org/dcache/gplazma/plugins/BanFilePlugin.java @@ -13,6 +13,7 @@ import java.time.Instant; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Properties; @@ -22,6 +23,8 @@ import java.util.stream.Collectors; import org.dcache.auth.Subjects; import org.dcache.gplazma.AuthenticationException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * The banfile plug-in bans users by their principal class and the associated name. It is configured @@ -40,6 +43,9 @@ */ public class BanFilePlugin implements GPlazmaAccountPlugin { + private final static Logger LOGGER = LoggerFactory.getLogger(BanFilePlugin.class); + + private final static Pattern BAN_PATTERN = Pattern.compile("^ban\\s+([^:]+):(.*)$"); private final static Pattern ALIAS_PATTERN = Pattern.compile("^alias\\s+([^:]+)=(.*)$"); @@ -50,14 +56,14 @@ public class BanFilePlugin implements GPlazmaAccountPlugin { private Instant lastFileRead; public BanFilePlugin(Properties properties) { - checkArgument(properties != null, "property is null"); String configFileName = properties.getProperty(BAN_FILE); checkArgument(configFileName != null, BAN_FILE + " property is not set"); configFile = Path.of(configFileName); - checkState(Files.exists(configFile), "config file doesn't exist or not a file"); - + if (!Files.exists(configFile)) { + LOGGER.error("Banfile plugin configured, but banfile doesn't exist or is not a file."); + } } List loadConfigLines() throws IOException { @@ -74,8 +80,11 @@ List loadConfigLines() throws IOException { * @return a set of banned principals */ private synchronized Set loadConfigIfNeeded() throws AuthenticationException { + if (!Files.exists(configFile)) { + LOGGER.debug("Banfile plugin configured, but banfile doesn't exist or is not a file."); + return new HashSet<>(); + } try { - if (bannedPrincipals == null || lastFileRead.isBefore( Files.readAttributes(configFile, BasicFileAttributes.class)