Skip to content

Commit 1f3b8d6

Browse files
author
Vladimir Kotal
committed
make User field configurable for whitelist check
fixes #3288
1 parent d598003 commit 1f3b8d6

File tree

2 files changed

+93
-17
lines changed

2 files changed

+93
-17
lines changed

plugins/src/main/java/opengrok/auth/plugin/UserWhiteListPlugin.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2020, Chris Fraire <[email protected]>.
2323
*/
2424
package opengrok.auth.plugin;
@@ -43,18 +43,31 @@ public class UserWhiteListPlugin implements IAuthorizationPlugin {
4343
private static final String className = UserWhiteListPlugin.class.getName();
4444
private static final Logger LOGGER = Logger.getLogger(className);
4545

46+
// configuration parameters
4647
static final String FILE_PARAM = "file";
48+
static final String FIELD_PARAM = "fieldName";
49+
50+
// valid values for the FIELD_NAME parameter
51+
static final String USERNAME_FIELD = "username";
52+
static final String ID_FIELD = "id";
4753

4854
private final Set<String> whitelist = new TreeSet<>();
55+
private String fieldName;
4956

5057
@Override
5158
public void load(Map<String, Object> parameters) {
5259
String filePath;
53-
5460
if ((filePath = (String) parameters.get(FILE_PARAM)) == null) {
5561
throw new IllegalArgumentException("Missing parameter [" + FILE_PARAM + "] in the configuration");
5662
}
5763

64+
if ((fieldName = (String) parameters.get(FIELD_PARAM)) == null) {
65+
fieldName = USERNAME_FIELD;
66+
} else if (!fieldName.equals(USERNAME_FIELD) && !fieldName.equals(ID_FIELD)) {
67+
throw new IllegalArgumentException("Invalid value of parameter [" + FIELD_PARAM +
68+
"] in the configuration: only " + USERNAME_FIELD + " or " + ID_FIELD + " are supported");
69+
}
70+
5871
// Load whitelist from file to memory.
5972
try (Stream<String> stream = Files.lines(Paths.get(filePath))) {
6073
stream.forEach(whitelist::add);
@@ -76,7 +89,13 @@ private boolean checkWhitelist(HttpServletRequest request) {
7689
return false;
7790
}
7891

79-
return whitelist.contains(user.getUsername());
92+
if (fieldName.equals(USERNAME_FIELD)) {
93+
return user.getUsername() != null && whitelist.contains(user.getUsername());
94+
} else if (fieldName.equals(ID_FIELD)) {
95+
return user.getId() != null && whitelist.contains(user.getId());
96+
}
97+
98+
return false;
8099
}
81100

82101
@Override

plugins/src/test/java/opengrok/auth/plugin/UserWhiteListPluginTest.java

Lines changed: 71 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
/*
2121
* Copyright (c) 2020, Chris Fraire <[email protected]>.
22+
* Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved.
2223
*/
2324
package opengrok.auth.plugin;
2425

@@ -27,6 +28,8 @@
2728
import org.junit.Before;
2829
import org.junit.BeforeClass;
2930
import org.junit.Test;
31+
import org.junit.runner.RunWith;
32+
import org.junit.runners.Parameterized;
3033
import org.opengrok.indexer.configuration.Group;
3134
import org.opengrok.indexer.configuration.Project;
3235
import org.opengrok.indexer.util.RandomString;
@@ -37,6 +40,9 @@
3740
import java.io.FileOutputStream;
3841
import java.io.OutputStreamWriter;
3942
import java.nio.charset.StandardCharsets;
43+
import java.security.InvalidParameterException;
44+
import java.util.Arrays;
45+
import java.util.Collection;
4046
import java.util.HashMap;
4147

4248
import static org.junit.Assert.assertFalse;
@@ -47,38 +53,67 @@
4753
/**
4854
* Represents a container for tests of {@link UserWhiteListPlugin}.
4955
*/
56+
@RunWith(Parameterized.class)
5057
public class UserWhiteListPluginTest {
5158

5259
private static final String OK_USER = "user1321";
53-
private static File tempWhitelist;
60+
private static final String OK_ID = "id2178";
61+
private static File tempWhitelistUser;
62+
private static File tempWhitelistId;
5463
private static HashMap<String, Object> validPluginParameters;
5564

5665
private UserWhiteListPlugin plugin;
66+
private final String param;
67+
68+
@Parameterized.Parameters
69+
public static Collection<String> parameters() {
70+
return Arrays.asList(UserWhiteListPlugin.ID_FIELD, UserWhiteListPlugin.USERNAME_FIELD);
71+
}
72+
73+
public UserWhiteListPluginTest(String param) {
74+
this.param = param;
75+
}
5776

5877
@BeforeClass
5978
public static void beforeClass() throws Exception {
60-
tempWhitelist = File.createTempFile("UserWhiteListPluginTest", "txt");
79+
tempWhitelistUser = File.createTempFile("UserWhiteListPluginTestUser", "txt");
6180
try (BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(
62-
new FileOutputStream(tempWhitelist), StandardCharsets.UTF_8))) {
81+
new FileOutputStream(tempWhitelistUser), StandardCharsets.UTF_8))) {
6382
writer.write(OK_USER);
6483
// Don't bother with trailing LF.
6584
}
6685

86+
tempWhitelistId = File.createTempFile("UserWhiteListPluginTestId", "txt");
87+
try (BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(
88+
new FileOutputStream(tempWhitelistId), StandardCharsets.UTF_8))) {
89+
writer.write(OK_ID);
90+
// Don't bother with trailing LF.
91+
}
92+
6793
validPluginParameters = new HashMap<>();
68-
validPluginParameters.put(UserWhiteListPlugin.FILE_PARAM, tempWhitelist.getPath());
6994
}
7095

7196
@AfterClass
7297
public static void afterClass() {
73-
if (tempWhitelist != null) {
98+
if (tempWhitelistUser != null) {
7499
//noinspection ResultOfMethodCallIgnored
75-
tempWhitelist.delete();
100+
tempWhitelistUser.delete();
101+
}
102+
if (tempWhitelistId != null) {
103+
//noinspection ResultOfMethodCallIgnored
104+
tempWhitelistId.delete();
76105
}
77106
}
78107

79108
@Before
80109
public void setUp() {
81110
plugin = new UserWhiteListPlugin();
111+
validPluginParameters.put(UserWhiteListPlugin.FIELD_PARAM, this.param);
112+
if (this.param.equals(UserWhiteListPlugin.USERNAME_FIELD)) {
113+
validPluginParameters.put(UserWhiteListPlugin.FILE_PARAM, tempWhitelistUser.getPath());
114+
} else {
115+
validPluginParameters.put(UserWhiteListPlugin.FILE_PARAM, tempWhitelistId.getPath());
116+
}
82117
}
83118

84119
@Test
@@ -89,6 +124,16 @@ public void shouldThrowOnLoadIfNullArgument() {
89124
}, "plugin.load(null)");
90125
}
91126

127+
@Test
128+
public void shouldThrowOnLoadIfInvalidFieldName() {
129+
assertThrows(IllegalArgumentException.class, () -> {
130+
HashMap<String, Object> map = new HashMap<>();
131+
map.put(UserWhiteListPlugin.FILE_PARAM, tempWhitelistUser.getPath());
132+
map.put(UserWhiteListPlugin.FIELD_PARAM, "huh");
133+
plugin.load(map);
134+
}, "plugin.load(null)");
135+
}
136+
92137
@Test
93138
public void shouldThrowOnLoadIfUnreadableFileSpecified() {
94139
HashMap<String, Object> unreadablePluginParameters = new HashMap<>();
@@ -131,15 +176,21 @@ public void shouldAllowWhitelistedUserForAnyProject() {
131176
plugin.load(validPluginParameters);
132177

133178
DummyHttpServletRequest req = new DummyHttpServletRequest();
134-
req.setAttribute(UserPlugin.REQUEST_ATTR, new User(OK_USER));
179+
User user;
180+
if (this.param.equals(UserWhiteListPlugin.USERNAME_FIELD)) {
181+
user = new User(OK_USER);
182+
} else {
183+
user = new User("blurb", OK_ID);
184+
}
185+
req.setAttribute(UserPlugin.REQUEST_ATTR, user);
135186

136187
Project randomProject = new Project(RandomString.generateUpper(10));
137188
boolean projectAllowed = plugin.isAllowed(req, randomProject);
138-
assertTrue("should allow OK_USER for random project 1", projectAllowed);
189+
assertTrue("should allow OK entity for random project 1", projectAllowed);
139190

140191
randomProject = new Project(RandomString.generateUpper(10));
141192
projectAllowed = plugin.isAllowed(req, randomProject);
142-
assertTrue("should allow OK_USER for random project 2", projectAllowed);
193+
assertTrue("should allow OK entity for random project 2", projectAllowed);
143194
}
144195

145196
@Test
@@ -151,19 +202,25 @@ public void shouldNotAllowRandomUserForAnyProject() {
151202

152203
Project randomProject = new Project(RandomString.generateUpper(10));
153204
boolean projectAllowed = plugin.isAllowed(req, randomProject);
154-
assertFalse("should not allow rando for random project 1", projectAllowed);
205+
assertFalse("should not allow random user for random project 1", projectAllowed);
155206

156207
randomProject = new Project(RandomString.generateUpper(10));
157208
projectAllowed = plugin.isAllowed(req, randomProject);
158-
assertFalse("should not allow rando for random project 2", projectAllowed);
209+
assertFalse("should not allow random user for random project 2", projectAllowed);
159210
}
160211

161212
@Test
162213
public void shouldAllowWhitelistedUserForAnyGroup() {
163214
plugin.load(validPluginParameters);
164215

165216
DummyHttpServletRequest req = new DummyHttpServletRequest();
166-
req.setAttribute(UserPlugin.REQUEST_ATTR, new User(OK_USER));
217+
User user;
218+
if (this.param.equals(UserWhiteListPlugin.USERNAME_FIELD)) {
219+
user = new User(OK_USER);
220+
} else {
221+
user = new User("blurb", OK_ID);
222+
}
223+
req.setAttribute(UserPlugin.REQUEST_ATTR, user);
167224

168225
Group randomGroup = new Group(RandomString.generateUpper(10));
169226
boolean groupAllowed = plugin.isAllowed(req, randomGroup);
@@ -183,10 +240,10 @@ public void shouldNotAllowRandomUserForAnyGroup() {
183240

184241
Group randomGroup = new Group(RandomString.generateUpper(10));
185242
boolean projectAllowed = plugin.isAllowed(req, randomGroup);
186-
assertFalse("should not allow rando for random group 1", projectAllowed);
243+
assertFalse("should not allow random random group 1", projectAllowed);
187244

188245
randomGroup = new Group(RandomString.generateUpper(10));
189246
projectAllowed = plugin.isAllowed(req, randomGroup);
190-
assertFalse("should not allow rando for random group 2", projectAllowed);
247+
assertFalse("should not allow random for random group 2", projectAllowed);
191248
}
192249
}

0 commit comments

Comments
 (0)