Skip to content

Commit 3c20e27

Browse files
stataru8stataru8
committed
KARAF-5014: consider first group role in users.properties and ignore empty roles - unit tests
1 parent 2e57742 commit 3c20e27

6 files changed

Lines changed: 242 additions & 133 deletions

File tree

‎jaas/modules/src/main/java/org/apache/karaf/jaas/modules/AbstractPropertiesBackingEngine.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public void deleteRole(String username, String role) {
199199
}
200200
for (int i = firstRoleIndex; i < infos.length; i++) {
201201
if (infos[i] != null && !infos[i].equals(role)) {
202-
if(userInfoBuffer.length() > 0) {
202+
if(firstRoleIndex == 1 || userInfoBuffer.length() > 0) {
203203
userInfoBuffer.append(",");
204204
}
205205
userInfoBuffer.append(infos[i]);

‎jaas/modules/src/test/java/org/apache/karaf/jaas/modules/AbstractPropertiesBackingEngineTest.java‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
* under the License.
14+
*/
15+
package org.apache.karaf.jaas.modules;
16+
17+
import static org.apache.karaf.jaas.modules.PrincipalHelper.names;
18+
import static org.hamcrest.MatcherAssert.assertThat;
19+
import static org.hamcrest.Matchers.contains;
20+
import static org.hamcrest.Matchers.containsInAnyOrder;
21+
import static org.junit.Assert.assertEquals;
22+
import static org.junit.Assert.assertNull;
23+
import static org.junit.Assert.assertTrue;
24+
import static org.junit.Assert.fail;
25+
26+
import org.apache.felix.utils.properties.Properties;
27+
import org.apache.karaf.jaas.boot.principal.GroupPrincipal;
28+
import org.apache.karaf.jaas.boot.principal.RolePrincipal;
29+
import org.apache.karaf.jaas.boot.principal.UserPrincipal;
30+
import org.apache.karaf.jaas.modules.properties.PropertiesBackingEngine;
31+
import org.junit.After;
32+
import org.junit.Before;
33+
import org.junit.Test;
34+
import java.io.File;
35+
import java.io.IOException;
36+
import java.util.List;
37+
38+
public class AbstractPropertiesBackingEngineTest {
39+
40+
private File f;
41+
private Properties p;
42+
private PropertiesBackingEngine engine;
43+
44+
@Before
45+
public void start() throws IOException {
46+
f = File.createTempFile(getClass().getName(), ".tmp");
47+
p = new Properties(f);
48+
engine = new PropertiesBackingEngine(p);
49+
}
50+
51+
@Test
52+
public void addUserInternalTest() {
53+
// non-existing user
54+
engine.addUserInternal("a", "");
55+
56+
// update empty password on existing user with no roles
57+
engine.addUserInternal("a", "pass1");
58+
assertThat(List.of(p.get("a").split(",")), contains("pass1"));
59+
UserPrincipal upa = PrincipalHelper.getUser(engine, "a");
60+
assertTrue(engine.listGroups(upa).isEmpty());
61+
assertTrue(engine.listRoles(upa).isEmpty());
62+
63+
// update password on existing user with no roles
64+
engine.addUserInternal("a", "pass2");
65+
assertThat(List.of(p.get("a").split(",")), contains("pass2"));
66+
upa = PrincipalHelper.getUser(engine, "a");
67+
assertTrue(engine.listGroups(upa).isEmpty());
68+
assertTrue(engine.listRoles(upa).isEmpty());
69+
70+
// update password on existing user with roles
71+
engine.addRole("a", "role1");
72+
engine.addGroup("a", "g1");
73+
engine.addGroupRole("g1", "role2");
74+
engine.addUserInternal("a", "pass3");
75+
assertThat(List.of(p.get("a").split(",")),
76+
contains("pass3", "role1", PropertiesBackingEngine.GROUP_PREFIX + "g1"));
77+
upa = PrincipalHelper.getUser(engine, "a");
78+
assertThat(names(engine.listGroups(upa)), containsInAnyOrder("g1"));
79+
assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2"));
80+
}
81+
82+
@Test
83+
public void lookupUserTest() {
84+
engine.addUser("a", "pass1");
85+
engine.addGroup("a", "g1");
86+
87+
assertEquals("a", engine.lookupUser("a").getName());
88+
assertNull(engine.lookupUser("g1"));
89+
assertNull(engine.lookupUser("test"));
90+
}
91+
92+
@Test
93+
public void listRolesTest() {
94+
// empty roles
95+
p.put("a", "pass1,,,"); // simulate manual editing of the properties file
96+
assertTrue(engine.listRoles("a").isEmpty());
97+
98+
// duplicate role
99+
p.put("a", "pass1,role1,role1");
100+
UserPrincipal upa = PrincipalHelper.getUser(engine, "a");
101+
List<RolePrincipal> roles = engine.listRoles(upa);
102+
assertEquals(1, roles.size());
103+
assertEquals("role1", roles.get(0).getName());
104+
}
105+
106+
@Test
107+
public void addRoleTest() {
108+
// empty info in groups
109+
engine.createGroup("g1");
110+
engine.addGroupRole("g1", "role1");
111+
GroupPrincipal gpg1 = PrincipalHelper.getGroup(engine, "g1");
112+
assertThat(names(engine.listRoles(gpg1)), contains("role1"));
113+
114+
// empty info in users
115+
engine.addUser("a", "");
116+
engine.addRole("a", "role2");
117+
UserPrincipal upa = PrincipalHelper.getUser(engine, "a");
118+
assertThat(names(engine.listRoles(upa)), contains("role2"));
119+
// user empty password is preserved
120+
assertThat(List.of(p.get("a").split(",")), containsInAnyOrder("", "role2"));
121+
122+
// duplicate role
123+
engine.addRole("a", "role2");
124+
upa = PrincipalHelper.getUser(engine, "a");
125+
assertThat(names(engine.listRoles(upa)), contains("role2"));
126+
assertThat(List.of(p.get("a").split(",")), containsInAnyOrder("", "role2"));
127+
128+
// duplicate group
129+
engine.addUser("b", "");
130+
engine.addGroup("b", "g2");
131+
engine.addGroup("b", "g2");
132+
UserPrincipal upb = PrincipalHelper.getUser(engine, "b");
133+
assertThat(names(engine.listGroups(upb)), contains("g2"));
134+
assertThat(List.of(p.get("b").split(",")),
135+
containsInAnyOrder("", PropertiesBackingEngine.GROUP_PREFIX + "g2"));
136+
}
137+
138+
@Test
139+
public void deleteRoleTest() {
140+
// delete in group
141+
engine.createGroup("g1");
142+
engine.addGroupRole("g1", "role1");
143+
engine.addGroupRole("g1", "role2");
144+
engine.addGroupRole("g1", "role3");
145+
engine.deleteGroupRole("g1", "role1");
146+
GroupPrincipal gpg1 = PrincipalHelper.getGroup(engine, "g1");
147+
assertThat(names(engine.listRoles(gpg1)), containsInAnyOrder("role2", "role3"));
148+
149+
// delete in user
150+
engine.addUser("a", "");
151+
engine.addRole("a", "role4");
152+
engine.addRole("a", "role5");
153+
engine.addRole("a", "role6");
154+
engine.deleteRole("a", "role4");
155+
UserPrincipal upa = PrincipalHelper.getUser(engine, "a");
156+
assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role5", "role6"));
157+
// user empty password is preserved
158+
assertThat(List.of(p.get("a").split(",")), containsInAnyOrder("", "role5", "role6"));
159+
}
160+
161+
@After
162+
public void cleanup() {
163+
if (!f.delete()) {
164+
fail("Could not delete temporary file: " + f);
165+
}
166+
}
167+
168+
}

‎jaas/modules/src/test/java/org/apache/karaf/jaas/modules/PrincipalHelper.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,32 @@
1616

1717
import static java.util.stream.Collectors.toList;
1818

19+
import org.apache.karaf.jaas.boot.principal.GroupPrincipal;
20+
import org.apache.karaf.jaas.boot.principal.UserPrincipal;
21+
import org.junit.Assert;
1922
import java.security.Principal;
2023
import java.util.Collection;
2124
import java.util.List;
25+
import java.util.stream.Collectors;
2226

2327
public class PrincipalHelper {
2428

2529
public static List<String> names(Collection<? extends Principal> principals) {
2630
return principals.stream().map(Principal::getName).collect(toList());
2731
}
32+
33+
public static UserPrincipal getUser(AbstractPropertiesBackingEngine engine, String name) {
34+
List<UserPrincipal> matchingUsers = engine.listUsers().stream()
35+
.filter(user -> name.equals(user.getName())).collect(Collectors.toList());
36+
Assert.assertFalse("User with name " + name + " was not found", matchingUsers.isEmpty());
37+
return matchingUsers.iterator().next();
38+
}
39+
40+
public static GroupPrincipal getGroup(AbstractPropertiesBackingEngine engine, String name) {
41+
List<GroupPrincipal> matchingGroups = engine.listGroups().keySet().stream()
42+
.filter(group -> name.equals(group.getName())).collect(Collectors.toList());
43+
Assert.assertFalse("Group with name " + name + " was not found", matchingGroups.isEmpty());
44+
return matchingGroups.iterator().next();
45+
}
2846

29-
}
47+
}

‎jaas/modules/src/test/java/org/apache/karaf/jaas/modules/properties/PropertiesBackingEngineTest.java‎

Lines changed: 13 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,17 @@
1818

1919
import static org.apache.karaf.jaas.modules.PrincipalHelper.names;
2020
import static org.hamcrest.Matchers.containsInAnyOrder;
21-
import static org.hamcrest.Matchers.contains;
22-
import static org.hamcrest.MatcherAssert.assertThat;
2321
import static org.junit.Assert.assertEquals;
2422
import static org.junit.Assert.assertNotNull;
25-
import static org.junit.Assert.assertTrue;
2623
import static org.junit.Assert.fail;
2724

2825
import java.io.File;
2926
import java.io.IOException;
30-
import java.util.Arrays;
31-
import java.util.List;
32-
import java.util.stream.Collectors;
3327

3428
import org.apache.felix.utils.properties.Properties;
3529
import org.apache.karaf.jaas.boot.principal.GroupPrincipal;
3630
import org.apache.karaf.jaas.boot.principal.UserPrincipal;
31+
import org.apache.karaf.jaas.modules.PrincipalHelper;
3732
import org.junit.After;
3833
import org.junit.Assert;
3934
import org.junit.Before;
@@ -58,8 +53,8 @@ public void testUserRoles() throws IOException {
5853
engine.addRole("a", "role1");
5954
engine.addRole("a", "role2");
6055

61-
UserPrincipal upa = getUser(engine, "a");
62-
assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2"));
56+
UserPrincipal upa = PrincipalHelper.getUser(engine, "a");
57+
Assert.assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2"));
6358

6459
engine.addGroup("a", "g");
6560
engine.addGroupRole("g", "role2");
@@ -68,26 +63,26 @@ public void testUserRoles() throws IOException {
6863
engine.addGroup("b", "g2");
6964
engine.addGroupRole("g2", "role4");
7065

71-
assertThat(names(engine.listUsers()), containsInAnyOrder("a", "b"));
72-
assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2", "role3"));
66+
Assert.assertThat(names(engine.listUsers()), containsInAnyOrder("a", "b"));
67+
Assert.assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2", "role3"));
7368

7469
checkLoading();
7570

7671
assertNotNull(engine.lookupUser("a"));
7772
assertEquals("a", engine.lookupUser("a").getName());
7873

7974
// removing some stuff
80-
UserPrincipal upb = getUser(engine, "b");
75+
UserPrincipal upb = PrincipalHelper.getUser(engine, "b");
8176
assertEquals(1, engine.listGroups(upa).size());
8277
assertEquals(2, engine.listGroups(upb).size());
8378

8479
GroupPrincipal gp = engine.listGroups(upa).iterator().next();
8580
engine.deleteGroupRole("g", "role2");
86-
assertThat(names(engine.listRoles(gp)), containsInAnyOrder("role3"));
81+
Assert.assertThat(names(engine.listRoles(gp)), containsInAnyOrder("role3"));
8782

8883
// role2 should still be there as it was added to the user directly too
89-
assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2", "role3"));
90-
assertThat(names(engine.listRoles(upb)), containsInAnyOrder("role3", "role4"));
84+
Assert.assertThat(names(engine.listRoles(upa)), containsInAnyOrder("role1", "role2", "role3"));
85+
Assert.assertThat(names(engine.listRoles(upb)), containsInAnyOrder("role3", "role4"));
9186

9287
engine.deleteGroup("b", "g");
9388
engine.deleteGroup("b", "g2");
@@ -101,65 +96,14 @@ public void testUserRoles() throws IOException {
10196
private void checkLoading() throws IOException {
10297
PropertiesBackingEngine engine = new PropertiesBackingEngine(new Properties(f));
10398
assertEquals(2, engine.listUsers().size());
104-
UserPrincipal upa_2 = getUser(engine, "a");
105-
UserPrincipal upb_2 = getUser(engine, "b");
99+
UserPrincipal upa_2 = PrincipalHelper.getUser(engine, "a");
100+
UserPrincipal upb_2 = PrincipalHelper.getUser(engine, "b");
106101

107102
assertEquals(3, engine.listRoles(upa_2).size());
108-
assertThat(names(engine.listRoles(upa_2)), containsInAnyOrder("role1", "role2", "role3"));
103+
Assert.assertThat(names(engine.listRoles(upa_2)), containsInAnyOrder("role1", "role2", "role3"));
109104

110105
assertEquals(3, engine.listRoles(upb_2).size());
111-
assertThat(names(engine.listRoles(upb_2)), containsInAnyOrder("role2", "role3", "role4"));
112-
}
113-
114-
private UserPrincipal getUser(PropertiesBackingEngine engine, String name) {
115-
List<UserPrincipal> matchingUsers = engine.listUsers().stream()
116-
.filter(user->name.equals(user.getName())).collect(Collectors.toList());
117-
Assert.assertFalse("User with name " + name + " was not found", matchingUsers.isEmpty());
118-
return matchingUsers.iterator().next();
119-
}
120-
121-
@Test
122-
public void testUserPassword() throws IOException {
123-
Properties p = new Properties(f);
124-
PropertiesBackingEngine engine = new PropertiesBackingEngine(p);
125-
126-
// update password when user has no roles
127-
engine.addUser("a", "pass1");
128-
engine.addUser("a", "pass2");
129-
assertThat(Arrays.asList(p.get("a").split(",")), contains("pass2"));
130-
UserPrincipal upa = getUser(engine, "a");
131-
assertTrue(engine.listRoles(upa).isEmpty());
132-
133-
// update empty password when user has no roles
134-
engine.addUser("b", "");
135-
engine.addUser("b", "pass3");
136-
assertThat(Arrays.asList(p.get("b").split(",")), contains("pass3"));
137-
UserPrincipal upb = getUser(engine, "b");
138-
assertTrue(engine.listRoles(upb).isEmpty());
139-
140-
// update password when user has roles
141-
engine.addUser("c", "pass4");
142-
engine.addRole("c", "role1");
143-
engine.addGroup("c", "g1");
144-
engine.addGroupRole("g1", "role2");
145-
engine.addUser("c", "pass5");
146-
assertThat(Arrays.asList(p.get("c").split(",")),
147-
contains("pass5", "role1", PropertiesBackingEngine.GROUP_PREFIX + "g1"));
148-
UserPrincipal upc = getUser(engine, "c");
149-
assertThat(names(engine.listRoles(upc)), containsInAnyOrder("role1", "role2"));
150-
assertThat(names(engine.listGroups(upc)), containsInAnyOrder("g1"));
151-
152-
// update empty password when user has roles
153-
engine.addUser("d", "");
154-
engine.addRole("d", "role3");
155-
engine.addGroup("d", "g2");
156-
engine.addGroupRole("g2", "role4");
157-
engine.addUser("d", "pass6");
158-
assertThat(Arrays.asList(p.get("d").split(",")),
159-
contains("pass6", "role3", PropertiesBackingEngine.GROUP_PREFIX + "g2"));
160-
UserPrincipal upd = getUser(engine, "d");
161-
assertThat(names(engine.listRoles(upd)), containsInAnyOrder("role3", "role4"));
162-
assertThat(names(engine.listGroups(upd)), containsInAnyOrder("g2"));
106+
Assert.assertThat(names(engine.listRoles(upb_2)), containsInAnyOrder("role2", "role3", "role4"));
163107
}
164108

165109
@After

0 commit comments

Comments
 (0)