Open GoogleCodeExporter opened 8 years ago
The piece of code in the unit test method should be read as:
new LogoutServlet() {
@Override
public RelyingParty getInstance() {
return relyingParty; // The EasyMock instance
}
}.doGet(request, response);
Original comment by dominiqu...@gmail.com
on 3 Nov 2009 at 11:00
This is somewhat the negative effect for marking it final.
The solution is not to mock the RelyingParty but instead mock the components
that the
RelyingParty is using.
- OpenIdUserManager, UserCache, AuthRedirection
The OpenIdContext is also marked final but its wrapped components can be mocked.
- Association, Discovery, HttpConnector
You can then instantiate the RelyingParty with the mocked components in the
constructor.
Would that work for you?
Original comment by david.yu...@gmail.com
on 4 Nov 2009 at 7:50
This is the avenue I've started to use ;) It does require the creation of a lot
of mock classes and a good
understanding of your code. Without it, this solution would not be possible and
you would to code
them for us.
Once I've finished the coding, I'm going to propose you to review it.
Stay tuned ;)
A+, Dom
Original comment by dominiqu...@gmail.com
on 4 Nov 2009 at 12:21
David,
Just a weird path I've identified while unit testing your HomeServlet code. It's
related to the following piece of code:
if (user.isAssociated() && RelyingParty.isAuthResponse(request)) {
// verify authentication
if (relyingParty.verifyAuth(user, request, response)) {
// authenticated redirect to home to remove the query params instead of
doing:
// request.setAttribute("user", user);
request.getRequestDispatcher("/home.jsp").forward(request, response);
response.sendRedirect(ApplicationSettings.get().getMainPageURL());
}
else {
// failed verification
request.getRequestDispatcher(ApplicationSettings.get().getLoginPageURL()).forwar
d(request,
response);
}
return;
}
// associate and authenticate user
StringBuffer url = request.getRequestURL();
String trustRoot = url.substring(0, url.indexOf("/", 9));
String realm = url.substring(0, url.lastIndexOf("/"));
String returnTo = url.toString();
if (relyingParty.associateAndAuthenticate(user, request, response, trustRoot,
realm, returnTo)) {
// successful association
return;
}
The first "if(){}" block is skipped if the user is not associated (some
attributes
missing or if the request comes back from the authentication service. With this
block
skipped, there's a call to RelyingParty.associateAndAuthenticate() which goes
up to
RelyingParty.getAuthUrlMap() which contains this test:
if(!user.isAssociated())
throw new IllegalArgumentException("claimed_id of user has not been verified.");
If my understanding is correct, it seems that RelyingParty.discover() can only
deliver null or an already associated user. It seems your code would work
without the
test on user.isAssociated() in the first "if(){}" block.
Maybe the test on the association should be done explicitly after the call to
discover() to be more obvious (instead of embedding it in getAuthUrlMap()...
A+, Dom
Note: I'm not that far to provide good code coverage for your code with the
help of
many mock classes. For now, only one have been instrumented to fake successful
authentication ;)
Original comment by dominiqu...@gmail.com
on 5 Nov 2009 at 1:39
RelyingParty.discovery(request) can either return null, an associated user, an
authenticated user or a newly discovered (unassociated) user.
Removing the if(user.isAssociated()) check would not work.
The RelyingParty.isAuthResponse(request) can be mocked by simply appending
"?openid.mode=id_res" to the url.
So instead of associating the newly discovered user, he would instead get
redirected
back to the login page because the inner
if(RelyingParty.verifyAuth(user,request,response) would fail.
PS. At first I agreed with your approach ... you almost convinced me :-)
Had to double-check.
Original comment by david.yu...@gmail.com
on 5 Nov 2009 at 4:33
FYI, here is the test verifying the invalidation, in the LogoutServlet, has
been done
as expected:
@Test
@SuppressWarnings("serial")
public void testInvalidate() throws IOException, ServletException {
MockHttpServletRequest request = new MockHttpServletRequest();
MockHttpServletResponse response = new MockHttpServletResponse() {
@Override
public void sendRedirect(String url) throws IOException {
assertEquals(ApplicationSettings.DEFAULT_MAIN_PAGE_URL, url);
}
};
Properties propertiesForInjection = new Properties() {
propertiesForInjection.setProperty("openid.discovery",
"com.dyuproject.openid.MockDiscovery");
propertiesForInjection.setProperty("openid.association",
"com.dyuproject.openid.MockAssociation");
propertiesForInjection.setProperty("openid.httpconnector",
"com.dyuproject.util.http.MockHttpConnector");
propertiesForInjection.setProperty("openid.user.manager",
"com.dyuproject.openid.MockOpenIdUserManager");
propertiesForInjection.setProperty("openid.user.cache",
"com.dyuproject.openid.MockUserCache");
propertiesForInjection.setProperty("openid.authredirection",
"com.dyuproject.openid.MockAuthRedirection");
};
final RelyingParty relyingParty =
RelyingParty.newInstance(propertiesForInjection);;
new LogoutServlet() {
@Override
protected RelyingParty getRelyingParty() {
return relyingParty;
}
}.doGet(request, response);
assertTrue(((MockOpenIdUserManager)
relyingParty.getOpenIdUserManager()).isInvalidated());
}
I had to create a bunch of mock classes but that works fine (the ones I use a
lot are
on
http://github.com/DomDerrien/two-tiers-utils/tree/master/src/Java/domderrien/moc
ks/).
I'm going to post the mock files, my two servlet files, and my test files on
GitHub
in few days. You'll be able to package them with the rest of your files ;)
As far as this defect entry is concerned, there's no more issue.
You can close it.
A+, Dom
Original comment by dominiqu...@gmail.com
on 5 Nov 2009 at 5:03
Thanks for the investigation around the OpenIdUser.isAssociated(). I think I
understand your point but I cannot create the corresponding test case... You'll
probably be able to set one up quickly with the code I'm going to publish in
few days.
Thank you very much,
A+, Dom
Original comment by dominiqu...@gmail.com
on 5 Nov 2009 at 5:09
Hi David,
Here are the code I would like you to validate ;) Note that I cannot exercise
the
path you mentioned. If you don't mind it will be great to produce the exact
test case
for it.
First, there is the Login servlet code, and then its corresponding unit test
suite.
The mock class I created (most of them are simple implementation of your
interfaces)
are available on my open source project on
http://github.com/DomDerrien/two-tiers-utils/tree/b8ef96bd09a684651cd4164c1fe6ab
f6c64ef419/src/Java/com/dyuproject.
Feel free to reuse these classes into your project. IMO, it's always nice to
offer
mock classes along with the code, so third parties can write accurate tests ;)
A+, Dom
Note: because one post with the code is too long, I'm going to submit it in
many parts...
Original comment by dominiqu...@gmail.com
on 21 Nov 2009 at 12:21
The login servlet code
--****************************************************************--
package domderrien.j2ee;
// Adaptation of David Yu's code for dyuproject, made available under
// the Apache licence 2.0. The adaptation mostly introduce more flexibility
// regarding the workflow and to make it unit-test-able
package domderrien.j2ee;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.UnknownHostException;
import java.util.Map;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import com.dyuproject.openid.OpenIdServletFilter;
import com.dyuproject.openid.OpenIdUser;
import com.dyuproject.openid.RelyingParty;
import com.dyuproject.openid.YadisDiscovery;
import com.dyuproject.openid.RelyingParty.Listener;
import com.dyuproject.openid.ext.AxSchemaExtension;
import com.dyuproject.openid.ext.SRegExtension;
import com.dyuproject.util.http.UrlEncodedParameterMap;
/**
* Home Servlet. If authenticated, goes to the home page. If not, goes to the
login page.
*
* @author David Yu
* @maintainer Dom Derrien
*/
@SuppressWarnings("serial")
public class LoginServlet extends HttpServlet {
protected static Listener sregExtension = new
SRegExtension().addExchange("email").addExchange("country").addExchange("languag
e");
protected static Listener axSchemaExtension = new
AxSchemaExtension().addExchange("email").addExchange("country").addExchange("lan
guage");
protected static Listener relyingPartyListener = new RelyingParty.Listener() {
public void onDiscovery(OpenIdUser user, HttpServletRequest request) {
System.err.println("******** discovered user: " + user.getClaimedId());
}
public void onPreAuthenticate(OpenIdUser user, HttpServletRequest request,
UrlEncodedParameterMap params) {
System.err.println("******** pre-authenticate user: " + user.getClaimedId());
}
public void onAuthenticate(OpenIdUser user, HttpServletRequest request) {
System.err.println("******** newly authenticated user: " +
user.getIdentity());
Map<String, String> sreg = SRegExtension.remove(user);
Map<String, String> axschema = AxSchemaExtension.remove(user);
if (sreg != null && !sreg.isEmpty()) {
user.setAttribute("info", sreg);
}
else if (axschema != null && !axschema.isEmpty()) {
user.setAttribute("info", axschema);
}
}
public void onAccess(OpenIdUser user, HttpServletRequest request) {
System.err.println("******** user access: " + user.getIdentity());
System.err.println("******** info: " + user.getAttribute("info"));
}
};
protected static RelyingParty _relyingParty;
static {
_relyingParty = RelyingParty.getInstance().
addListener(sregExtension).
addListener(axSchemaExtension).
addListener(relyingPartyListener);
}
// To allow injection of a mock instance
protected RelyingParty getRelyingParty() {
return _relyingParty;
}
@Override
public void doGet(HttpServletRequest request, HttpServletResponse response)
throws IOException, ServletException {
doPost(request, response);
}
public final static String LOGIN_WITH_PARAMETER_KEY = "loginWith";
public final static String GOOGLE_OPENID_SERVER_URL =
"https://www.google.com/accounts/o8/ud";
public final static String YAHOO_OPENID_SERVER_URL =
"https://open.login.yahooapis.com/openid/op/auth";
protected void preselectOpendIdServer(HttpServletRequest request) {
String identifier = null;
String openIdServer = null;
// If the ui supplies a LoginWith=google or LoginWith=yahoo link/button,
// this will speed up the openid process by skipping discovery.
// The override is done by adding the OpenIdUser to the request attribute.
String loginWith = request.getParameter(LOGIN_WITH_PARAMETER_KEY);
String openidIdentifier =
request.getParameter(RelyingParty.DEFAULT_IDENTIFIER_PARAMETER);
if (loginWith != null) {
if (loginWith.equals("google")) {
identifier = "https://www.google.com/accounts/o8/id";
openIdServer = GOOGLE_OPENID_SERVER_URL;
}
else if (loginWith.equals("yahoo")) {
identifier = "http://yahoo.com/";
openIdServer = YAHOO_OPENID_SERVER_URL;
}
}
else if (openidIdentifier != null) {
if (openidIdentifier.contains("@gmail")) {
identifier = "https://www.google.com/accounts/o8/id";
openIdServer = GOOGLE_OPENID_SERVER_URL;
}
if (openidIdentifier.contains("@yahoo")) {
identifier = "http://yahoo.com/";
openIdServer = YAHOO_OPENID_SERVER_URL;
}
}
if (identifier != null) { // && openIdServer != null) {
request.setAttribute(
OpenIdUser.ATTR_NAME,
OpenIdUser.populate(
identifier,
YadisDiscovery.IDENTIFIER_SELECT, // claimedId,
openIdServer
)
);
}
}
@Override
public void doPost(HttpServletRequest request, HttpServletResponse response)
throws IOException, ServletException {
preselectOpendIdServer(request);
RelyingParty relyingParty = getRelyingParty();
String errorMsg = OpenIdServletFilter.DEFAULT_ERROR_MSG;
try {
OpenIdUser user = relyingParty.discover(request);
if (user == null) {
if (RelyingParty.isAuthResponse(request)) {
// authentication timeout
response.sendRedirect(request.getRequestURI());
}
else {
// set error msg if the openid_identifier is not resolved.
if (request.getParameter(relyingParty.getIdentifierParameter())
!= null) {
request.setAttribute(OpenIdServletFilter.ERROR_MSG_ATTR,
errorMsg);
}
// new user
request.getRequestDispatcher("/login").forward(request, response);
}
return;
}
if (user.isAuthenticated()) {
// user already authenticated
request.getRequestDispatcher("/login").forward(request, response);
return;
}
// if (user.isAssociated() && RelyingParty.isAuthResponse(request)) { //
Temporary shortcut, waiting for David Yu's response
if (RelyingParty.isAuthResponse(request)) {
// verify authentication
if (relyingParty.verifyAuth(user, request, response)) {
// authenticated redirect to home to remove the query params
instead of doing:
// request.setAttribute("user", user);
request.getRequestDispatcher("/home.jsp").forward(request, response);
response.sendRedirect("/login");
}
else {
// failed verification
request.getRequestDispatcher("/login").forward(request, response);
}
return;
}
// associate and authenticate user
StringBuffer url = request.getRequestURL();
String trustRoot = url.substring(0, url.indexOf("/", 9));
String realm = url.substring(0, url.lastIndexOf("/"));
String returnTo = url.toString();
if (relyingParty.associateAndAuthenticate(user, request, response,
trustRoot, realm, returnTo)) {
// successful association
return;
}
}
catch (UnknownHostException uhe) {
errorMsg = OpenIdServletFilter.ID_NOT_FOUND_MSG;
}
catch (FileNotFoundException fnfe) {
errorMsg = OpenIdServletFilter.DEFAULT_ERROR_MSG;
}
catch (Exception e) {
// e.printStackTrace();
errorMsg = OpenIdServletFilter.DEFAULT_ERROR_MSG;
}
request.setAttribute(OpenIdServletFilter.ERROR_MSG_ATTR, errorMsg);
request.getRequestDispatcher("/login").forward(request, response);
}
}
Original comment by dominiqu...@gmail.com
on 21 Nov 2009 at 12:22
The test class attached
Original comment by dominiqu...@gmail.com
on 21 Nov 2009 at 12:26
Attachments:
Original issue reported on code.google.com by
dominiqu...@gmail.com
on 3 Nov 2009 at 10:59