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);

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?

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

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()).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;
    }

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 ;)

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.

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/mocks/).
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

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

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/b8ef96bd09a684651cd4164c1fe6abf6c64ef419/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...

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("language");
 
    protected static Listener axSchemaExtension = new
AxSchemaExtension().addExchange("email").addExchange("country").addExchange("language");
 
    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);
    }
}

The test class attached

TestLoginServlet.java 41.2 KB   Download