RelyingParty and unit testing

[GoogleCodeExporter]

David,

First, thanks for having made the code available to all of us. That's really 
cool and help saving a pretty 
good amount of work.

I want to write unit tests for the LogoutServlet class:

    public class LogoutServlet extends HttpServlet {
        //...
        public void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException, 
ServletException {
            RelyingParty relyingParty = getInstance(); // Externalize to allow mock injection
            relyingParty.invalidate(request, response);
            response.sendRedirect(ApplicationSettings.get().getMainPageURL());
        }
        //...
        protected RelyingParty getInstance() {
            return RelyingParty.getInstance();
        }
    }

One goal is to ensure the "invalidate()" method is called.

I've written the following code but it does not work because RelyingParty is a 
final class...

    @Test
    @SuppressWarnings("serial")
    public void testInvalidate() throws IOException, ServletException {
        HttpServletRequest request = new MockHttpServletRequest();
        HttpServletResponse response = new MockHttpServletResponse() {
            @Override
            public void sendRedirect(String url) throws IOException {
                assertEquals(ApplicationSettings.DEFAULT_MAIN_PAGE_URL, url);
            }
        };

        final RelyingParty relyingParty = EasyMock.createMock(RelyingParty.class);
        EasyMock.expect(relyingParty.invalidate(request, response));
        EasyMock.replay();

        new LogoutServlet() {
            @Override
            public RelyingParty getInstance() {
                return new RelyingParty() {

                };
            }
        }.doGet(request, response);
    }

Do you have a suggestion?

For now, I just rely on the RelyingParty.getInstance() method. The test passes 
but I've no way to ensure that 
future refactoring won't remove the call to "invalidate()" by mistake. This is 
not a safe situation, 
especially in my Agile environment ;)

A+, Dom

Original issue reported on code.google.com by dominiqu...@gmail.com on 3 Nov 2009 at 10:59

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

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

[GoogleCodeExporter]

The test class attached

Original comment by dominiqu...@gmail.com on 21 Nov 2009 at 12:26

Attachments:

• TestLoginServlet.java