Skip to content

Commit

Permalink
gplazma: configured banfile plugin should ignore non-existent ban file
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lemora committed Dec 20, 2023
1 parent 46f5a61 commit 0deb226
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
2 changes: 1 addition & 1 deletion docs/TheBook/src/main/markdown/config-gplazma.md
Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion docs/TheBook/src/main/markdown/install.md
Expand Up @@ -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
Expand Down
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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+([^:]+)=(.*)$");

Expand All @@ -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<String> loadConfigLines() throws IOException {
Expand All @@ -74,8 +80,11 @@ List<String> loadConfigLines() throws IOException {
* @return a set of banned principals
*/
private synchronized Set<Principal> 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)
Expand Down

0 comments on commit 0deb226

Please sign in to comment.