LUI-170 - Not redirecting to requested page after redirected to login… #130
Conversation
… page This adds a new request filter that sets the redirect url on the session to the currently requested url if the user is not authenticated and it is not already set.
| if (log.isDebugEnabled()) { | ||
| log.debug("Set " + OPENMRS_LOGIN_REDIRECT_HTTPSESSION_ATTR + " = " + redirectUrl); | ||
| } |
There was a problem hiding this comment.
| if (log.isDebugEnabled()) { | |
| log.debug("Set " + OPENMRS_LOGIN_REDIRECT_HTTPSESSION_ATTR + " = " + redirectUrl); | |
| } | |
| log.debug("Set {} = {}", OPENMRS_LOGIN_REDIRECT_HTTPSESSION_ATTR, redirectUrl); |
| } | ||
| } | ||
| catch (Exception e) { | ||
| // Ignore errors here to prevent any failures from disrupting service |
There was a problem hiding this comment.
Should we maybe log exceptions, at least initially?
| String requestURI = httpReq.getRequestURI(); | ||
| if (!Context.isAuthenticated()) { | ||
| if ("GET".equalsIgnoreCase(httpReq.getMethod()) && !requestURI.contains("login.")) { | ||
| if (req.getAttribute(OPENMRS_LOGIN_REDIRECT_HTTPSESSION_ATTR) == null) { |
There was a problem hiding this comment.
Does this check actually work? I can see we use it elsewhere, but it seems like this should be something like:
req.getSession(false).getAttribute(OPENMRS_LOGIN_REDIRECT_HTTPSESSION_ATTR) == nullWhen I try naïvely to call:
req.getSession().setAttribute("MyAttribute", "SomeValue");And then do:
req.getAttribute("MyAttribute")The response is still null.
There was a problem hiding this comment.
Oops, sorry, yeah, good catch. I was initially setting a request attribute and then changed it when I recognized it was supposed to be a session attribute, but didn't change this.
… page Follow-up tweaks following code review.
| String requestURI = httpReq.getRequestURI(); | ||
| if (!Context.isAuthenticated()) { | ||
| if ("GET".equalsIgnoreCase(httpReq.getMethod()) && !requestURI.contains("login.")) { | ||
| if (req.getAttribute(OPENMRS_LOGIN_REDIRECT_HTTPSESSION_ATTR) == null) { |
There was a problem hiding this comment.
Oops, sorry, yeah, good catch. I was initially setting a request attribute and then changed it when I recognized it was supposed to be a session attribute, but didn't change this.
| if (log.isDebugEnabled()) { | ||
| log.debug("Set " + OPENMRS_LOGIN_REDIRECT_HTTPSESSION_ATTR + " = " + redirectUrl); | ||
| } |
| } | ||
| } | ||
| catch (Exception e) { | ||
| // Ignore errors here to prevent any failures from disrupting service |
… page
This adds a new request filter that sets the redirect url on the session to the currently requested url if the user is not authenticated and it is not already set.