Client cookies are not getting removed after server session invalidated

Hi @ibacher,

To invalidate session from server side you added below commit with webservices version 2.36.0, RESTWS-887: Session endpoint should expire the cookie on delete (#543) · openmrs/openmrs-module-webservices.rest@cc6c1ab · GitHub

We wanted to address you the issue which we are facing because this commit is not able to remove cookies from client side.

Below is the list of steps to reproduce the issue which we faced,

Below is login curl request 

curl -v -k --request GET 'https://localhost/openmrs/ws/rest/v1/session' \
--header 'Authorization: Basic c3VwZXJtYW46QWRtaW4xMjM=' 

which will return jsessionId for eg. B0A59487946C81A63CCCA4EDEB128D0F
after that when we are making delete call with same jsession_id

curl -v -k --request DELETE 'https://localhost/openmrs/ws/rest/v1/session' \
--header 'Cookie: JSESSIONID=B0A59487946C81A63CCCA4EDEB128D0F'

will invalidate session from server side, and server will set the same jsession_id B0A59487946C81A63CCCA4EDEB128D0F as cookie, which is already expired
post delete call to ensure that prev session was destroyed, when we are making another login request (which will send the prev jsession_id thats is expired on server side) 

curl -v -k --request GET 'https://localhost/openmrs/ws/rest/v1/session' \
--header 'Authorization: Basic c3VwZXJtYW46QWRtaW4xMjM=' \
--header 'Cookie: JSESSIONID=B0A59487946C81A63CCCA4EDEB128D0F'

it returns 401 UNAUTHORIZED, because we passed expired jsession_id and it is returning new jsession_id which was created as part of delete call
and as jsession_id is a HttpOnly cookie, browser will take care of setting it, we cannot set it from JS
now when we make get request with valid jsession_id 

curl -v -k --request GET 'https://localhost/openmrs/ws/rest/v1/session' \
--header 'Cookie: JSESSIONID=0B8222111FB8B81D12A096046648B4D8'

it will return the required result

But as we are making curls we are modifying requests as per valid jsession_id, but as jsession_id is a HttpOnly cookie, browser will take care of setting it, we cannot set it from JS

so we made below changes,

in SessionController1_9.java, when we are invalidating the session, we are setting jsession_id with new_session_id and attaching to response

public void delete(HttpServletRequest request, HttpServletResponse response) {
	Context.logout();
	request.getSession().invalidate();
	Cookie[] cookies = request.getCookies();
    StringBuilder customHeader = new StringBuilder("");
	if (cookies != null) {
		for (Cookie cookie : cookies) {
			if(cookie.getName().equals("JSESSIONID") || cookie.getName().equals("reporting_session")) {
				customHeader.append(cookie.getName()).append("=").append(request.getSession().getId()).append(";");					
                response.setHeader("Set-Cookie", customHeader + "Path=/; Secure; HttpOnly;");
			}
		}
	}
}

And on client end(bahmniapps), after we call delete method(to make server session invalidate) then we are calling a dummy get call, which will make the request with valid jsession_id

@ibacher, what is your opinion on this issue? will you suggest any change to make?

cc: @angshuonline @gsluthra @n0man @binduak

1 Like

So, it seems to me inappropriate for the DELETE operation to create a new session, when it’s whole purpose is to end the session. The appropriate point to create a session is on the start of the next request. Ultimately, though, managing the JSESSIONID cookie should be the responsibility of the servlet container and not the application (since the Servlet API theoretically supports different mechanisms for tracking sessions).

This statement is slightly inaccurate, at least with the latest version of the REST module. For me, the sequence looks something like this (I’ve removed some irrelevant headers):

GET /openmrs/ws/rest/v1/session HTTP/1.1
Authorization: Basic abba123abba=
Host: dev3.openmrs.org
HTTP/1.1 200 
Connection: close
Set-Cookie: JSESSIONID=F6ED129C61AEC4AE769D4E452305ADA7; Path=/openmrs; HttpOnly; Secure; SameSite=Strict
DELETE /openmrs/ws/rest/v1/session HTTP/1.1
Cookie: JSESSIONID=F6ED129C61AEC4AE769D4E452305ADA7
Host: dev3.openmrs.org
HTTP/1.1 204 
Connection: close

Note that there’s no repeat of the session id from the server, although the client will still hold onto the cookie.

Now, I’m actually going to suggest that looking at the DELETE method here is the wrong place to look. The DELETE method just exists to expire the session, but you can get a 401 response without having called the DELETE method first, namely because the session timed out. I’d suggest that the right place to deal with this issue is where the 401 response gets returned (in the AuthenticationFilter).

Probably ideally the flow looks something like this:

  • Check if the session is expired
    • If not, process the request
    • If so, does the header contain Basic auth?
      • If not, return a 401 error and unset the session cookie
      • If so, start a new session and proceed as normal
1 Like

@Ian, shouldn’t DELETE expire the cookie (specifically the httponly ones) and communicate so to the client?

Typically in spring application, we do something like (in a WebSecurityConfigurerAdapter) below to configure:

httpSecurity.logout().deleteCookies(“JSESSIONID”)

otherwise, you run the risk of a valid authentication request being rejected - client just wants to create a new login, but since the httponly cookie (invalidated during delete) is accompanied, it will throw an error.

Sure, and I’m fine that being implemented, but even if we implement that, there will still be scenarios where the session becomes invalid before the cookie has expired. E.g., Tomcat has a standard session timeout of 30 minutes, i.e., if a request isn’t submitted in that time frame, the session will be invalidated without any explicit logout action being taken. I was trying to propose something that would work with both the user explicitly logging out and the user implicitly being logged out via session expiration.

Actually did something about this with TRUNK-6151, which basically adds a filter that will clear a configurable list of cookies if the session is invalidated during a request. This will be available in 2.6.0 and I backported the code to the 2.5.x branch, but it isn’t yet part of a 2.5.x release.

By default, it will clear the JSESSIONID cookie, though you can have it clear a configurable list of cookies by setting the cookieClearingFilter.toClear property to a comma-separated list of cookies you want to clear (if set, JSESSIONID should be in this list).

1 Like

Hi @ibacher, Thanks for the change … :slightly_smiling_face:

Hi @ibacher,

As you have added CookieClearFilter to remove the httpOnly(JSESSIONID) cookie, We were trying things on our end and we noticed that, after delete call, subsequent get call is not returning any JSESSIONID (as we want it to return valid JSESSIONID)

While debugging we got to know that it only returns valid JSESSIONID when clearedCookie.path is set to “/openmrs”

Please add your inputs on this, we have two questions,

  1. do we have any openmrs env where we have deployed latest openmrs 2.5.x(with filter changes) and webservices 2.38.0?
  2. setting the path for cookie, should be done in openmrs-core filter or any configuration file (nginx/httpd)?

We were going through OpenMRS, and after delete(logout) call, when the very next get call is made, it is returning /openmrs path, from where are we adding path field?

Set-Cookie: JSESSIONID=DA29378784C9CED4D4D46E8B5F71CBC2; Path=/openmrs; HttpOnly; Secure; SameSite=Strict

dev3.openmrs.org or qa-refapp.openmrs.org should both have both of those.

We don’t manually create this cookie. The JSESSIONID cookie is part of the servlet spec and is created by the servlet container (normally Tomcat).

Could you give me an example of this? I can’t reproduce it. In general, the JSESSIONID cookie will only be created if the endpoint actually requests a session…

Hi @ibacher,

Below are steps to reproduce the issue which we are facing, we are using openmrs-core 2.5.x latest nightly build image with openmrs-core filter changes and webservices 2.38.0 module

When we are making login call with authorization, we are getting JSESSIONID in response

curl 'https://localhost/openmrs/ws/rest/v1/session?v=custom:(uuid)' \
  -H 'Authorization: Basic c3VwZXJtYW46QWRtaW4xMjM=' \
  
getting below cookie in response
Set-Cookie: JSESSIONID=E4ECA938359B7BA225900623EAB897A6; Path=/openmrs; HttpOnly
After that when we are making delete call and same older JSESSIONID get passed automatically, and as per openmrs filter we are getting expired JSESSIONID in response

curl 'https://localhost/openmrs/ws/rest/v1/session?v=custom:(uuid)' \
  -X 'DELETE' \
  -H 'Cookie: JSESSIONID=E4ECA938359B7BA225900623EAB897A6; bahmni.user=%22superman%22; bahmni.user.location=%7B%22name%22%3A%22Unknown%20Location%22%2C%22uuid%22%3A%228d6c993e-c2cc-11de-8d13-0010c6dffd0f%22%7D; reporting_session=E4ECA938359B7BA225900623EAB897A6' \

getting below cookie in response
Set-Cookie: JSESSIONID=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:10 GMT
After that when we are making get call to get valid JSESSIONID, older JSESSIONID got passed and we are not able to get valid JSESSIONID as response

curl 'https://localhost/openmrs/ws/rest/v1/session?v=custom:(uuid)' \
  -H 'Cookie: JSESSIONID=E4ECA938359B7BA225900623EAB897A6; bahmni.user.location=%7B%22name%22%3A%22Unknown%20Location%22%2C%22uuid%22%3A%228d6c993e-c2cc-11de-8d13-0010c6dffd0f%22%7D; bahmni.user=null; reporting_session=null; bahmni.clinical.retrospectiveEncounterDate=null; app.clinical.grantProviderAccessData=null' \

not getting JSESSIONID in response, and now every next get call will use older JSESSIONID and will receive 401 Unknown Error issue

This issue is getting resolved, only when we add clearedCookie.setPath(“/openmrs”) in below filter. And after that delete call will return response as below

Set-Cookie: JSESSIONID=; Max-Age=0; Expires=Thu, 01 Jan 1970 00:00:10 GMT; Path=/openmrs

and then next get call will return the valid JSESSIONID as response

please go through the steps, and add your thoughts, Thanks

Fair… my only real comment is that we should use something like request.getContextPath() rather than hard-coding /openmrs, since in general, we support OpenMRS being installed in any servlet context.

sure @ibacher, Thanks

Wanted to add one more thing like, openmrs-core’s ClearCookieFilter will only clear cookies when we are calling delete(logout) after successful login.

As part of Bahmni, we are making delete call before login just for safety purpose, in that case ClearCookieFilter will not clear the cookies because initial call(delete) would not be having any cookie attached in request header.

Please go through the steps, to replicate

delete call will be made with no cookies attached, as application will be loaded for the first time

curl 'https://localhost/openmrs/ws/rest/v1/session?v=custom:(uuid)' -k -v \
  -X 'DELETE'

Will set invalidated session's id as JSESSIONID as part of response:

Set-Cookie: JSESSIONID=C439BCA08D5F5C1134D004EC471B6295; Path=/openmrs; HttpOnly
Then if we make get call with same older JSESSIONID

curl 'https://localhost/openmrs/ws/rest/v1/session?v=custom:(uuid)' -v -k \
  -H 'Cookie: JSESSIONID=E4ECA938359B7BA225900623EAB897A6;'

not getting valid JSESSIONID in response and will return 401 Unknown error

Ian, Is this a fair scenario? or we should not call delete before login? please add your thoughts

Well, more exactly, it will only clear the cookie if the session is invalid and the initial request had a session cookie. There doesn’t seem much point in clearing the cookie if the client didn’t send one (this is where the curl requests are a little weird).

The real issue here seems to be that the DELETE endpoint itself is creating a session, which it absolutely should not be doing.

The fact that we return an error if a request has been made with an invalid session ID has never been my favourite thing, but I also dislike changing behaviour that’s over a decade old unless it’s an obvious bug. Fixing the DELETE call above should make this a non-issue though, i.e., you call DELETE and that will clear the cookie if the user’s browser has one stored.

Hi @ibacher,

We tried few ways to resolve delete call before login call issue, in finally block, we added below code in openmrs-core, and is fixing delete call before login issue.

if(request.getRequestURI().endsWith("/session") && request.getMethod().equals("DELETE") && !response.isCommitted()){
	HttpSession newSession = request.getSession(true);
	newSession.setAttribute("username", "-anonymous user-");
	UserContext userContext = new UserContext(Context.getAuthenticationScheme());
    newSession.setAttribute(WebConstants.OPENMRS_USER_CONTEXT_HTTPSESSION_ATTR, userContext);
	newSession.setAttribute("locale", userContext.getLocale());
	Context.setUserContext(userContext);
}

but we are still not able to resolve session timeout issue,

Steps: As server session is timed out, browser still have invalidated session. When we are making a get locations call and then delete call (even if we make delete call directly we will face the same issue), making a delete call with invalidated cookie, when server session is invalidated

curl 'https://localhost/openmrs/ws/rest/v1/session?v=custom:(uuid)' \
  -X 'DELETE' \
  -H 'Cookie: JSESSIONID=1B92ED8A8BA5A3225D91DF3A5D27AF99;'

will not return and update the JSESSIONID and that is causing the problem, and after that further calls are returning 401 Unknown error

We even tried adding clear cookie code before httpResponse.sendError(“Session Timeout”) in webservices

It is adding expired JSESSIONID cookie on response object, but response status is getting updated from somewhere and 401 Unknown Error is returned

@ibacher, please add your thoughts on this

Hi @ibacher @dkayiwa @rafael,

With respect to Bahmni, we are working with openmrs 2.5.9 version and webservices-rest 2.39.0-SNAPSHOT omod

We got an issue with respect to session timeout, we can consider below case:

If user is logged in and after some time server restarts or session gets timed out, in such scenario browser is still holding the JSESSIONID cookie which is invalidated now, and now whenever some get request is made, invalid JSESSIONID cookie will get attached to call’s request header and it returns 401 Unknown Reason as response, this will be similar for all subsequent request calls, unless user clears the application storage manually or server sets the expired JSESSIONID as part of response header, so that it will not get attached in further requests. We need some help in mitigating this issue.

e.g. curl request with invalid JSESSIONID attached in request header

curl 'https://localhost/openmrs/ws/rest/v1/location?operator=ALL&s=byTags&tags=Login+Location&v=default' -k -v \
  -H 'Cookie: JSESSIONID=79DCA2591575ED99B2310ACD724A3762;'

is returning, 
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>401 Unknown Reason</title>
</head><body>
<h1>Unknown Reason</h1>
<p>This server could not verify that you
are authorized to access the document
requested.  Either you supplied the wrong
credentials (e.g., bad password), or your
browser doesn't understand how to supply
the credentials required.</p>
</body></html>

not returning valid/any JSESSIONID cookie in response

and one more thing to add like, from where it is sending error response in html format. Please add you thoughts

So, bahmni-core overrides the location search endpoint with it’s own implementation (the base version doesn’t support the operator, or byTags parameters). When I do this against a base OpenMRS instance with an invalid session cookie, I get a 401 Unauthorized exception with a new session cookie. Using that session cookie with the same parameters, I get a 400 Bad Response error.

@ibacher, We even tried some other urls, please see below curl request,

curl 'https://localhost/openmrs/ws/rest/v1/encountertype/Consultation' -v -k \
  -H 'Cookie: JSESSIONID=E9CF5B4E9349B327DA539B8D23D65053;'

is also returning same, and not returning valid JSESSIONID cookie in response.

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>401 Unknown Reason</title>
</head><body>
<h1>Unknown Reason</h1>
<p>This server could not verify that you
are authorized to access the document
requested.  Either you supplied the wrong
credentials (e.g., bad password), or your
browser doesn't understand how to supply
the credentials required.</p>
</body></html>

@ibacher, can you please let us know, on which environment you are testing on?

A standard version of the O3 RefApp I have running locally on my machine… But even if I point it at one of our reference servers, e.g. qa-refapp.openmrs.org, I don’t get the same response you’re showing… For this one, I get a 401 again with a new JSESSIONID cookie.

@ibacher, I also tried using curl with qa-refapp,

curl 'https://qa-refapp.openmrs.org/openmrs/ws/rest/v1/encountertype/Consultation' -v -k  -H 'Cookie: JSESSIONID=E9CF5B4E9349B327DA539B8D23D65033;'

its returning 401 with valid JSESSIONID
< HTTP/1.1 401
< Server: nginx/1.18.0 (Ubuntu)
< Date: Mon, 23 Jan 2023 09:56:06 GMT
< Content-Type: text/html;charset=utf-8
< Content-Length: 708
< Connection: keep-alive
< Set-Cookie: JSESSIONID=087CEABB91AE0C846A5474A93DDD90AA; Path=/openmrs; HttpOnly
< Content-Language: en

@ibacher, but the thing is with respect to http://qa-refapp.openmrs.org, it doesn’t interact with webservices, like with login/logout it is not calling /session get or delete from webservices (verified from network tab), in our case we do call.

The point here isn’t how the app works, the point is how the REST API works, right? So it doesn’t really matter that the logout functionality doesn’t call the REST API, what matters is that the REST API does the expected thing.

The proper sequence here is something like:

  1. DELETE (clear the client cookie)
  2. Request /session with credentials (receive new session cookie)
  3. Continue to use the session cookie to access resources

The request to /session should really be able to be to any REST endpoint (or really anywhere in the app). Apart from DELETE clearing the cookie, that seems to work…

@ibacher yes, the requirements are simple really

  1. When called DELETE /session - it should do the following - expire the older cookie, and if at all send a new JSESSIONID cookie. We found some fixes - we will raise PRs for that in openmrs-core (cookie clearing filter).

  2. When any REST API call happens - if it accompanied a JSESSIONID cookie that the server deems invalid, it should throw 401 error - which is whats happening now. (Note, in older version of webservices rest the code used to throw 403 forbidden). However, as Rohit explained

  • since in such case (401), the server does not expire the older JSESSIONID or send back a new JSESSIONID cookie, the browser will keep sending the older JSESSIONID for any subsequent call, and it will be rejected all the time. JSESSIONID is a httpOnly cookie, which means you can’t clear that from javascript. The only way to proceed in this case, is to clear the cache - and now the flow works. This is not particular to any app. If you try with openmrs-core (2.5.x) and latest webservices-rest, this can happen to any app. One can wait till browser determines the older JSESSIONID has expired, but this isn’t really acceptable UX.

This is the scenario we are stuck with the latest webservices code. Hope this is clear, we can get in a call if thats more helpful, we can show in postman API calls

1 Like