| 2014-11-06 20:59:55 |
Mike Rylander |
description |
Because the recent security fix to forcibly delete the auth session there is an opportunity for an internal server error to be thrown during auto-logout in the OPAC.
Specifically, when the refresh-based auto-logout triggers in the browser, the cookie will not be there anymore because it expired at or just before that time. Thus, the new code in the security patch does not have an auth token value to send to the delete method. As it turns out, that is a worse offense that I'd anticipated.
The proximate cause of the internal server error is the, um, severity with which the auth service responds to a request to delete a session but not being given a session to delete. Instead of just returning a simple failure code, or even a "successfully did nothing, like you asked" response, it throws an OpenSRF 404 error. That, in turn, is raised via perl's die().
While that is arguably far too strong a reaction to a missing or empty parameter, there may be code out there depending on that behavior. So, changing that is not a good solution.
There are two other options:
1) redirect to ctx.home_page instead of ctx.logout_page when performing a timeout-based auto-logout
2) wrap the code in the security patch in a try block, so that we needn't care that it died -- this, as it turns out, is exactly what Jeff Godin from TADL suggested to me privately, and I waived him off of that, not imagining that the lack of a cookie value would provoke such a reaction from auth. Sorry, Jeff ...
(1) solves the problem handily, and has the benefit of being very easy to deploy quickly. Just replace one word and save the file -- no restarts of anything are required. (2) has the benefit of protecting older custom templates without having to edit them for this issue in particular.
I can't think of a good reason not to do both of those, so I'll be following up with a branch containing a commit for each. Folks can then easily hot-patch their stock template, and get the second half with the next upgrade that contains the perl change. |
Because of the recent security fix to forcibly delete the auth session there is an opportunity for an internal server error to be thrown during auto-logout in the OPAC.
Specifically, when the refresh-based auto-logout triggers in the browser, the cookie will not be there anymore because it expired at or just before that time. Thus, the new code in the security patch does not have an auth token value to send to the delete method. As it turns out, that is a worse offense that I'd anticipated.
The proximate cause of the internal server error is the, um, severity with which the auth service responds to a request to delete a session but not being given a session to delete. Instead of just returning a simple failure code, or even a "successfully did nothing, like you asked" response, it throws an OpenSRF 404 error. That, in turn, is raised via perl's die().
While that is arguably far too strong a reaction to a missing or empty parameter, there may be code out there depending on that behavior. So, changing that is not a good solution.
There are two other options:
1) redirect to ctx.home_page instead of ctx.logout_page when performing a timeout-based auto-logout
2) wrap the code in the security patch in a try block, so that we needn't care that it died -- this, as it turns out, is exactly what Jeff Godin from TADL suggested to me privately, and I waived him off of that, not imagining that the lack of a cookie value would provoke such a reaction from auth. Sorry, Jeff ...
(1) solves the problem handily, and has the benefit of being very easy to deploy quickly. Just replace one word and save the file -- no restarts of anything are required. (2) has the benefit of protecting older custom templates without having to edit them for this issue in particular.
I can't think of a good reason not to do both of those, so I'll be following up with a branch containing a commit for each. Folks can then easily hot-patch their stock template, and get the second half with the next upgrade that contains the perl change. |
|
