Skip to content

Commit 5a9b9d9

Browse files
arjantijmsarjantijms
committed
Added extra comments and failure explanations
1 parent 71a68f4 commit 5a9b9d9

File tree

3 files changed

+148
-53
lines changed

3 files changed

+148
-53
lines changed

‎jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationProtectedTest.java

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
import org.xml.sax.SAXException;
1515

1616
/**
17-
* This tests that we can login from a protected resource (a resource for which security constraints have been set) and then
18-
* access it.
17+
* This tests that we can login from a protected resource (a resource for which
18+
* security constraints have been set) and then access it.
1919
*
2020
* @author Arjan Tijms
2121
*
@@ -34,7 +34,10 @@ public void testProtectedPageNotLoggedin() throws IOException, SAXException {
3434
String response = getFromServerPath("protected/servlet");
3535

3636
// Not logged-in thus should not be accessible.
37-
assertFalse(response.contains("This is a protected servlet"));
37+
assertFalse(
38+
"Not authenticated, so should not have been able to access protected resource",
39+
response.contains("This is a protected servlet")
40+
);
3841
}
3942

4043
@Test
@@ -43,7 +46,40 @@ public void testProtectedPageLoggedin() throws IOException, SAXException {
4346
String response = getFromServerPath("protected/servlet?doLogin=true");
4447

4548
// Now has to be logged-in so page is accessible
46-
assertTrue(response.contains("This is a protected servlet"));
49+
assertTrue(
50+
"Should have been authenticated, but could not access protected resource",
51+
response.contains("This is a protected servlet")
52+
);
53+
54+
// Not only does the page needs to be accessible, the caller should have
55+
// the correct
56+
// name and roles as well
57+
58+
// Being able to access a page protected by a role but then seeing the un-authenticated
59+
// (anonymous) user would normally be impossible, but could happen if the authorization
60+
// system checks roles on the authenticated subject, but does not correctly expose
61+
// or propagate these to the HttpServletRequest
62+
assertFalse(
63+
"Protected resource could be accessed, but the user appears to be the unauthenticated user. " +
64+
"This should not be possible",
65+
response.contains("web username: null")
66+
);
67+
68+
// An authenticated user should have the exact name "test" and nothing else.
69+
assertTrue(
70+
"Protected resource could be accessed, but the username is not correct.",
71+
response.contains("web username: test")
72+
);
73+
74+
// Being able to access a page protected by role "architect" but failing
75+
// the test for this role would normally be impossible, but could happen if the
76+
// authorization system checks roles on the authenticated subject, but does not
77+
// correctly expose or propagate these to the HttpServletRequest
78+
assertTrue(
79+
"Resource protected by role \"architect\" could be accessed, but user fails test for this role." +
80+
"This should not be possible",
81+
response.contains("web user has role \"architect\": true")
82+
);
4783
}
4884

4985
}

‎jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationPublicTest.java

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,16 @@ public void testPublicPageNotLoggedin() throws IOException, SAXException {
3232
String response = getFromServerPath("public/servlet");
3333

3434
// Not logged-in
35-
assertTrue(response.contains("web username: null"));
36-
assertTrue(response.contains("web user has role \"architect\": false"));
35+
assertTrue(
36+
"Not authenticated, but a username other than null was encountered. " +
37+
"This is not correct.",
38+
response.contains("web username: null")
39+
);
40+
assertTrue(
41+
"Not authenticated, but the user seems to have the role \"architect\". " +
42+
"This is not correct.",
43+
response.contains("web user has role \"architect\": false")
44+
);
3745
}
3846

3947
@Test
@@ -44,36 +52,16 @@ public void testPublicPageLoggedin() throws IOException, SAXException {
4452
String response = getFromServerPath("public/servlet?doLogin");
4553

4654
// Now has to be logged-in
47-
assertTrue(response.contains("web username: test"));
48-
assertTrue(response.contains("web user has role \"architect\": true"));
49-
}
50-
51-
@Test
52-
public void testPublicPageNotRememberLogin() throws IOException, SAXException {
53-
54-
// -------------------- Request 1 ---------------------------
55-
56-
String response = getFromServerPath("public/servlet");
57-
58-
// Not logged-in
59-
assertTrue(response.contains("web username: null"));
60-
assertTrue(response.contains("web user has role \"architect\": false"));
61-
62-
// -------------------- Request 2 ---------------------------
63-
64-
response = getFromServerPath("public/servlet?doLogin");
65-
66-
// Now has to be logged-in
67-
assertTrue(response.contains("web username: test"));
68-
assertTrue(response.contains("web user has role \"architect\": true"));
69-
70-
// -------------------- Request 3 ---------------------------
71-
72-
response = getFromServerPath("public/servlet");
73-
74-
// Not logged-in
75-
assertTrue(response.contains("web username: null"));
76-
assertTrue(response.contains("web user has role \"architect\": false"));
55+
assertTrue(
56+
"User should have been authenticated and given name \"test\", " +
57+
" but does not appear to have this name",
58+
response.contains("web username: test")
59+
);
60+
assertTrue(
61+
"User should have been authenticated and given role \"architect\", " +
62+
" but does not appear to have this role",
63+
response.contains("web user has role \"architect\": true")
64+
);
7765
}
7866

7967
}

‎jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java

Lines changed: 88 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public void testProtectedAccessIsStateless() throws IOException, SAXException {
4141
// Not logged-in thus should not be accessible.
4242
assertFalse(response.contains("This is a protected servlet"));
4343

44+
4445
// -------------------- Request 2 ---------------------------
4546

4647
// JASPIC is stateless and login (re-authenticate) has to happen for every request
@@ -53,10 +54,13 @@ public void testProtectedAccessIsStateless() throws IOException, SAXException {
5354
response = getFromServerPath("protected/servlet?doLogin");
5455

5556
// Now has to be logged-in so page is accessible
56-
assertTrue("Could not access protected page, but should be able to. "
57-
+ "Did the container remember the previously set 'unauthenticated identity'?",
58-
response.contains("This is a protected servlet"));
57+
assertTrue(
58+
"Could not access protected page, but should be able to. " +
59+
"Did the container remember the previously set 'unauthenticated identity'?",
60+
response.contains("This is a protected servlet")
61+
);
5962

63+
6064
// -------------------- Request 3 ---------------------------
6165

6266
// JASPIC is stateless and login (re-authenticate) has to happen for every request
@@ -66,9 +70,11 @@ public void testProtectedAccessIsStateless() throws IOException, SAXException {
6670
response = getFromServerPath("protected/servlet");
6771

6872
// Not logged-in thus should not be accessible.
69-
assertFalse("Could access protected page, but should not be able to. "
70-
+ "Did the container remember the authenticated identity that was set in previous request?",
71-
response.contains("This is a protected servlet"));
73+
assertFalse(
74+
"Could access protected page, but should not be able to. " +
75+
"Did the container remember the authenticated identity that was set in previous request?",
76+
response.contains("This is a protected servlet")
77+
);
7278
}
7379

7480
/**
@@ -83,6 +89,7 @@ public void testProtectedAccessIsStateless2() throws IOException, SAXException {
8389
// Start with doing a login
8490
String response = getFromServerPath("protected/servlet?doLogin");
8591

92+
8693
// -------------------- Request 2 ---------------------------
8794

8895
// JASPIC is stateless and login (re-authenticate) has to happen for every request
@@ -94,9 +101,66 @@ public void testProtectedAccessIsStateless2() throws IOException, SAXException {
94101
response = getFromServerPath("protected/servlet");
95102

96103
// Not logged-in thus should not be accessible.
97-
assertFalse("Could access protected page, but should not be able to. "
98-
+ "Did the container remember the authenticated identity that was set in previous request?",
99-
response.contains("This is a protected servlet"));
104+
assertFalse(
105+
"Could access protected page, but should not be able to. " +
106+
"Did the container remember the authenticated identity that was set in the previous request?",
107+
response.contains("This is a protected servlet")
108+
);
109+
}
110+
111+
/**
112+
* Tests that access to a public page does not depend on the authenticated identity that was established in a previous
113+
* request.
114+
*/
115+
@Test
116+
public void testPublicAccessIsStateless() throws IOException, SAXException {
117+
118+
// -------------------- Request 1 ---------------------------
119+
120+
String response = getFromServerPath("public/servlet");
121+
122+
// Establish that we're initially not logged-in
123+
assertTrue(
124+
"Not authenticated, but a username other than null was encountered. " +
125+
"This is not correct.",
126+
response.contains("web username: null")
127+
);
128+
assertTrue(
129+
"Not authenticated, but the user seems to have the role \"architect\". " +
130+
"This is not correct.",
131+
response.contains("web user has role \"architect\": false")
132+
);
133+
134+
135+
// -------------------- Request 2 ---------------------------
136+
137+
response = getFromServerPath("public/servlet?doLogin");
138+
139+
// Now has to be logged-in
140+
assertTrue(
141+
"User should have been authenticated and given name \"test\", " +
142+
" but does not appear to have this name",
143+
response.contains("web username: test")
144+
);
145+
assertTrue(response.contains("web user has role \"architect\": true"));
146+
147+
148+
// -------------------- Request 3 ---------------------------
149+
150+
// Accessing public page without login
151+
response = getFromServerPath("public/servlet");
152+
153+
// No details should linger around
154+
assertTrue(
155+
"Should not be authenticated, but a username other than null was encountered. " +
156+
"Did the container remember the authenticated identity that was set in the previous request?",
157+
response.contains("web username: null")
158+
);
159+
assertTrue(
160+
"The unauthenticated user has the role 'architect', which should not be the case. " +
161+
"The container seemed to have remembered it from the previous request.",
162+
response.contains("web user has role \"architect\": false")
163+
);
100164
}
101165

102166
/**
@@ -111,20 +175,27 @@ public void testUserIdentityIsStateless() throws IOException, SAXException {
111175
// Accessing protected page with login
112176
String response = getFromServerPath("protected/servlet?doLogin");
113177

178+
114179
// -------------------- Request 2 ---------------------------
115180

116181
// Accessing public page without login
117182
response = getFromServerPath("public/servlet");
118183

119184
// No details should linger around
120-
assertFalse("User principal was 'test', but it should be null here. "
121-
+ "The container seemed to have remembered it from the previous request.",
122-
response.contains("web username: test"));
123-
assertTrue("User principal was not null, but it should be null here. ",
124-
response.contains("web username: null"));
125-
assertTrue("The unauthenticated user has the role 'architect', which should not be the case. "
126-
+ "The container seemed to have remembered it from the previous request.",
127-
response.contains("web user has role \"architect\": false"));
185+
assertFalse(
186+
"User principal was 'test', but it should be null here. " +
187+
"The container seemed to have remembered it from the previous request.",
188+
response.contains("web username: test")
189+
);
190+
assertTrue(
191+
"User principal was not null, but it should be null here. ",
192+
response.contains("web username: null")
193+
);
194+
assertTrue(
195+
"The unauthenticated user has the role 'architect', which should not be the case. " +
196+
"The container seemed to have remembered it from the previous request.",
197+
response.contains("web user has role \"architect\": false")
198+
);
128199
}
129200

130201
}

0 commit comments

Comments
 (0)