Loss of last visited page

[ricsxn]

By default Liferay installation (Liferay Community Edition Portal 7.0.5 GA6) saves the last visited page once the signIn procedure is successfully accomplished.
Enabling the liferay-oidc-plugin, the automated redirect to the last visited page is lost and the signIn procedure always lands in the Login portlet.

http:///web/guest/home?p_p_id=com_liferay_login_web_portlet_LoginPortlet&p_p_lifecycle=0&p_p_state=maximized&p_p_mode=view&saveLastPath=false&_com_liferay_login_web_portlet_LoginPortlet_mvcRenderCommandName=%2Flogin%2Flogin

I think this is the main consequence of Issues 34, 33 and 31.

[ricsxn]

Adding the following code in OpenIDConnectAutoLogin.java, the user will be redirected to its last visited page once successful logged in.
Don't know if this solution is acceptable from a pure Liferay architecture point of view; but it seems to work fine and provides an acceptable solution for my specific case.
The tip is to ever store last visited URL when the user is not yet signed in (URL does not contain 'login' and no credentials are available).
The saved URL will be placed in redirection when the user has just signed in (URL contains login and credentials are avaiable).

// Stored last visited URL, when not yet signed in
    protected String redirect = "";

    @Override
    protected String[] doLogin(HttpServletRequest request, HttpServletResponse response) throws Exception {
        String currentURL = _portal.getCurrentURL(request);
        LOG.debug("[doLogin] currentURL: " + currentURL);
	String[] credentials = new String[3];
        credentials = libAutologin.doLogin(request, response);
	for(int i=0; i<3; i++) {
	    LOG.debug("[doLogin] credentials[" + i + "] = '" + credentials[i] + "'");
	}
	if(!currentURL.contains("login") && credentials[0].length() == 0) {
		// Store last URL when not yet signed in
		redirect = currentURL;
	} else if(currentURL.contains("login") && credentials[0].length() != 0) {
		// When login just accomplished, recall the stored URL
                if (Validator.isNotNull(redirect)) {
                        redirect = _portal.escapeRedirect(redirect);
                }
                else {
                        redirect = _portal.getPathMain();
                }
                request.setAttribute(AutoLogin.AUTO_LOGIN_REDIRECT, redirect);
                LOG.debug("[doLogin] redirect: " + redirect);
        } else {
                LOG.debug("[doLogin] Signed or URL contains login, redirect: " + redirect);
        }
	LOG.debug("[doLogin] Leaving credentials");
	return credentials;
    }

As written when I opened this issue, this change fixes (at least I suppose it should fix) issues: #34 #33 and #31

My forked code contains this change at commit: ricsxn@3c9209d

[gvanderploeg]

I'm sorry that I cannot review and discuss the issue in global. But a quick glance shows a complete no-go:

protected String redirect = "";

This makes this solution thread unsafe: it will probably work locally, but as soon as you deploy in a multi user environment it will fail.
Read about this in general here: https://stackoverflow.com/questions/3106452/how-do-servlets-work-instantiation-sessions-shared-variables-and-multithreadi

[ricsxn]

Making further tests, I discovered that stored path was not used as I was expecting. Simplifying then the code as follows:

@Override
    protected String[] doLogin(HttpServletRequest request, HttpServletResponse response) throws Exception {
        String currentURL = _portal.getCurrentURL(request);
        LOG.debug("[doLogin] currentURL: " + currentURL);
	String[] credentials = { "", "", "" };
        credentials = libAutologin.doLogin(request, response);
	// Take care of redirection
	if(currentURL.contains("login") && credentials[0].length() != 0) {
		// User just SignedIn
                String redirect = _portal.getPathMain();
                request.setAttribute(AutoLogin.AUTO_LOGIN_REDIRECT, redirect);
                LOG.debug("[doLogin] redirect: " + redirect);
        }
	LOG.debug("[doLogin] Leaving credentials");
	return credentials;
    }

The plugin works and no thread safety issues are present this time.
Honestly speaking I don't know why this is working. What I'm understanding is that I am placing path: '/c' (return value of _portal.getPathMain()), as soon as the user completed successfully the login procedure.

Forked code containing this change is available at: ricsxn@665c988