Skip to content

Commit 35d4965

Browse files
DmitryLitvintsevmksahakyan
authored andcommitted
gplazma: fix banfile plugin
Motivation: ----------- Issue #7947 reports that banfile only bans user if there is only one 'ban foo:bar' line is present (one line per alias). Further investigation showed that actually only last line is taken. Modification: ------------- Modified BanFilePlugin to use List<String> containing elements already suitable for Pincipal coversion instead of Map<String, String> that naturally only kept the last instance of ban statement. Result: ------- Banfile plugin works as expected. Ticket: #7947 Acked-by: Paul Millar Target: trunk Request: 11.1, 11.0, 10.2 Require-book: no Require-notes: yes
1 parent 1367d50 commit 35d4965

File tree

2 files changed

+17
-11
lines changed

2 files changed

+17
-11
lines changed

modules/gplazma2-banfile/src/main/java/org/dcache/gplazma/plugins/BanFilePlugin.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import java.nio.file.attribute.BasicFileAttributes;
1212
import java.security.Principal;
1313
import java.time.Instant;
14+
import java.util.ArrayList;
1415
import java.util.Collections;
1516
import java.util.HashMap;
1617
import java.util.HashSet;
@@ -93,8 +94,8 @@ private synchronized Set<Principal> loadConfigIfNeeded() throws AuthenticationEx
9394
// alias -> value, like uid=org.dcache.auth.UidPrincipal
9495
Map<String, String> aliases = new HashMap<>();
9596

96-
// class/alias -> value, like uid:123 or org.dcache.auth.UidPrincipal:123
97-
Map<String, String> bans = new HashMap<>();
97+
// List of banned names like "uid:123" or "org.dcache.auth.UidPrincipal:123"
98+
List<String> bannedNames = new ArrayList<>();
9899

99100
// group all 'alias' and all 'ban' records, skip comments and empty lines
100101
Map<String, List<String>> config = loadConfigLines().stream()
@@ -129,7 +130,7 @@ private synchronized Set<Principal> loadConfigIfNeeded() throws AuthenticationEx
129130
}
130131
String clazz = m.group(1);
131132
String value = m.group(2);
132-
bans.put(aliases.getOrDefault(clazz, clazz), value);
133+
bannedNames.add(aliases.getOrDefault(clazz, clazz)+":"+value);
133134
});
134135
}
135136

@@ -140,13 +141,6 @@ private synchronized Set<Principal> loadConfigIfNeeded() throws AuthenticationEx
140141
throw new IllegalArgumentException("Line has bad format: '" + badLines
141142
+ "', expected '[alias|ban] <key>:<value>'");
142143
}
143-
144-
// construct lines suitable for Subjects.principalsFromArgs
145-
// class:value or shortname:value
146-
List<String> bannedNames = bans.entrySet().stream()
147-
.map(e -> e.getKey() + ":" + e.getValue())
148-
.collect(Collectors.toList());
149-
150144
bannedPrincipals = Subjects.principalsFromArgs(bannedNames);
151145
lastFileRead = Instant.now();
152146
}

modules/gplazma2-banfile/src/test/java/org/dcache/gplazma/plugins/BanFilePluginTest.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,22 @@ public void shouldWorkMultipleInvocations() throws IOException, AuthenticationEx
9898

9999
@Test(expected = AuthenticationException.class)
100100
public void shouldFailForBannedUserByAlias() throws IOException, AuthenticationException {
101-
givenConfig("alias foo=org.dcache.auth.UserNamePrincipal\nban foo:bert");
101+
givenConfig("alias foo=org.dcache.auth.UserNamePrincipal\nban foo:bert\n");
102102
plugin.account(Set.of(new UserNamePrincipal("bert")));
103103
}
104104

105+
@Test(expected = AuthenticationException.class)
106+
public void shouldFailForBannedUserByAliasFirstInList() throws IOException, AuthenticationException {
107+
givenConfig("alias foo=org.dcache.auth.UserNamePrincipal\nban foo:bert\nban foo:bart\n");
108+
plugin.account(Set.of(new UserNamePrincipal("bert")));
109+
}
110+
111+
@Test(expected = AuthenticationException.class)
112+
public void shouldFailForBannedUserByAliasLastInList() throws IOException, AuthenticationException {
113+
givenConfig("alias foo=org.dcache.auth.UserNamePrincipal\nban foo:bert\nban foo:bart\n");
114+
plugin.account(Set.of(new UserNamePrincipal("bart")));
115+
}
116+
105117
@Test(expected = AuthenticationException.class)
106118
public void shouldFailForBannedUserByStandardAlias()
107119
throws IOException, AuthenticationException {

0 commit comments

Comments
 (0)