While working on bug 17030 for integrating the REST API on the packages ot make it finally available, I found that the session is lost when Plack is used to access it. To reproduce: - Follow the steps for the Plack patch in bug 17030 => FAIL: Notice no matter what permissions your user has you get: { "error": "You don't have the required permission" } - On a separate tab, notice you've been kicked out from Koha. This doesn't happen with Plack disabled.
Just a sidenote: We experienced the same issue in our plack+REST setup, and found out it was due to a syspref: SessionRestrictionByIP When we disabled this (enabled by default) the problem went away. Don't know if it is related, though.
(In reply to Benjamin Rokseth from comment #1) > Just a sidenote: We experienced the same issue in our plack+REST setup, and > found out it was due to a syspref: SessionRestrictionByIP > > When we disabled this (enabled by default) the problem went away. > Don't know if it is related, though. Confirmed!
Created attachment 54054 [details] [review] Bug 17050: Do not kicks the session out when accessing the REST API Mojolicious does not set $ENV{REMOTE_ADDR} (and $ENV{HTTP_*} neither) as it may share ENV between different requests. Fortunately for us, Plack does not! This is a dirty patch to fix this issue but it seems that there is not lot of solutions. It adds a remote_addr parameter to C4::Auth::check_cookie_authin order to send it from Koha::Rest::V1::startup reading the headers sent by Mojolicious. Test plan: Hit /cgi-bin/koha/mainpage.pl Hit /api/v1/patrons/42 Hit /cgi-bin/koha/mainpage.pl With this patch applied, everything will be fine and you won't be logged out.
Amended test plan: 0/ Switch SessionRestrictionByIP on!
I tested this patch in our deployment setup (using docker containers), and experienced the following: Every second request invalidated/deleted the session. Seemingly the syspref cache was emptied also. Enabling/disabling the SessionRestrictedByIp had no effect, as it seemed the Context was gone. Really weird. I will have to see if I can reproduce this.
(In reply to Benjamin Rokseth from comment #5) > I tested this patch in our deployment setup (using docker containers), and > experienced the following: > > Every second request invalidated/deleted the session. Seemingly the syspref > cache was emptied also. Enabling/disabling the SessionRestrictedByIp had no > effect, as it seemed the Context was gone. Really weird. > > I will have to see if I can reproduce this. What's the value of SessionStorage?
I believe it was 'mysql'
(In reply to Benjamin Rokseth from comment #7) > I believe it was 'mysql' So should be ok. Let me know if you have steps to recreate the problem.
So I narrowed down the problem: session is created by a simple auth session module we wrote largely mimicking svc/authentication. Noticed that the $ip param is missing from the session, and neither ip nor _SESSION_REMOTE_ADDR is present in the mysql session storage. So I guess svc/authentication and other CGI scripts pass $ip somehow or generates by ENV{REMOTE_ADDR}, and we will have to adjust to that. Or revert to use svc/authentication. Or address some other API authentication bugs.
Jonathan, I inserted my infamous Plack::Middleware::Tomas that just prints env, and interestingly: { HTTP_ACCEPT "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", HTTP_ACCEPT_ENCODING "gzip, deflate", HTTP_ACCEPT_LANGUAGE "en-US,en;q=0.5", HTTP_COOKIE "KohaOpacLanguage=en; CGISESSID=59977e155eba3200f39ba4b1cb23efec", HTTP_DNT 1, HTTP_HOST "localhost:8081", HTTP_REFERER "http://localhost:8081/cgi-bin/koha/members/moremember.pl?borrowernumber=51", HTTP_UPGRADE_INSECURE_REQUESTS 1, HTTP_USER_AGENT "Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:49.0) Gecko/20100101 Firefox/49.0", HTTP_X_FORWARDED_FOR "10.0.2.2", HTTP_X_FORWARDED_HOST "localhost:8081", HTTP_X_FORWARDED_SERVER "kohadev-intra.myDNSname.org", PATH_INFO "/intranet/mainpage.pl", psgi.errors *main::STDERR (write-only, flags: append, layers: unix perlio), psgi.input *Starman::Server::$io (layers: scalar), psgi.multiprocess 1, psgi.multithread "", psgi.nonblocking "", psgi.run_once "", psgi.streaming 1, psgi.url_scheme "http", psgi.version [ [0] 1, [1] 1 ], psgix.harakiri 1, psgix.input.buffered 1, psgix.io *Symbol::GEN41 (read/write, layers: unix perlio), QUERY_STRING "", REMOTE_ADDR "10.0.2.2", REMOTE_HOST undef, REMOTE_PORT 0, REQUEST_METHOD "GET", REQUEST_URI "/intranet/mainpage.pl", SCRIPT_NAME "", SERVER_NAME 0, SERVER_PORT 8081, SERVER_PROTOCOL "HTTP/1.1" } so HTTP_X_FORWARDED_FOR and REMOTE_ADDR have the right values. Are you implying that Mojo is messing with ENV?
(In reply to Tomás Cohen Arazi from comment #10) > so HTTP_X_FORWARDED_FOR and REMOTE_ADDR have the right values. Are you > implying that Mojo is messing with ENV? ENV is correctly set if you hit mainpage, but not /api/*
Created attachment 54655 [details] [review] Bug 17050: Do not kick the session out when accessing the REST API Mojolicious does not set $ENV{REMOTE_ADDR} (neither $ENV{HTTP_*}) as it may share ENV between different requests. Fortunately for us, Plack does not! This is a dirty patch to fix this issue but it seems that there is not lot of solutions. It adds a remote_addr parameter to C4::Auth::check_cookie_authin order to send it from Koha::Rest::V1::startup reading the headers sent by Mojolicious. Test plan: Hit /cgi-bin/koha/mainpage.pl Hit /api/v1/patrons/42 Hit /cgi-bin/koha/mainpage.pl With this patch applied, everything will be fine and you won't be logged out. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 54656 [details] [review] Bug 17050: (QA followup) Use Mojo::Transaction to get the remote address While the original patch fixes the issue, reading at Mojolicious source code, revealed that Mojo::Transaction already has proper processing for detecting the remote address: sub remote_address { my $self = shift; return $self->original_remote_address(@_) if @_; return $self->original_remote_address unless $self->req->reverse_proxy; # Reverse proxy return ($self->req->headers->header('X-Forwarded-For') // '') =~ /([^,\s]+)$/ ? $1 : $self->original_remote_address; } Without this patch, if there's a request without HTTP_X_FORWARDED_FOR, then the remote address would be empty. Such would be the case of a dev/standard setup without Plack. This patch makes Koha::REST::V1::startup use tx->remote_address instead of dealing with the headers on its own. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 54661 [details] [review] Bug 17050: Do not kick the session out when accessing the REST API Mojolicious does not set $ENV{REMOTE_ADDR} (neither $ENV{HTTP_*}) as it may share ENV between different requests. Fortunately for us, Plack does not! This is a dirty patch to fix this issue but it seems that there is not lot of solutions. It adds a remote_addr parameter to C4::Auth::check_cookie_authin order to send it from Koha::Rest::V1::startup reading the headers sent by Mojolicious. Test plan: Hit /cgi-bin/koha/mainpage.pl Hit /api/v1/patrons/42 Hit /cgi-bin/koha/mainpage.pl With this patch applied, everything will be fine and you won't be logged out. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Created attachment 54662 [details] [review] Bug 17050: (QA followup) Use Mojo::Transaction to get the remote address While the original patch fixes the issue, reading at Mojolicious source code, revealed that Mojo::Transaction already has proper processing for detecting the remote address: sub remote_address { my $self = shift; return $self->original_remote_address(@_) if @_; return $self->original_remote_address unless $self->req->reverse_proxy; # Reverse proxy return ($self->req->headers->header('X-Forwarded-For') // '') =~ /([^,\s]+)$/ ? $1 : $self->original_remote_address; } Without this patch, if there's a request without HTTP_X_FORWARDED_FOR, then the remote address would be empty. Such would be the case of a dev/standard setup without Plack. This patch makes Koha::REST::V1::startup use tx->remote_address instead of dealing with the headers on its own. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Good Mojo, Jonathan & Tomas! Working now as intended.
Created attachment 54679 [details] [review] Bug 17050: Do not kick the session out when accessing the REST API Mojolicious does not set $ENV{REMOTE_ADDR} (neither $ENV{HTTP_*}) as it may share ENV between different requests. Fortunately for us, Plack does not! This is a dirty patch to fix this issue but it seems that there is not lot of solutions. It adds a remote_addr parameter to C4::Auth::check_cookie_authin order to send it from Koha::Rest::V1::startup reading the headers sent by Mojolicious. Test plan: Hit /cgi-bin/koha/mainpage.pl Hit /api/v1/patrons/42 Hit /cgi-bin/koha/mainpage.pl With this patch applied, everything will be fine and you won't be logged out. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 54680 [details] [review] Bug 17050: (QA followup) Use Mojo::Transaction to get the remote address While the original patch fixes the issue, reading at Mojolicious source code, revealed that Mojo::Transaction already has proper processing for detecting the remote address: sub remote_address { my $self = shift; return $self->original_remote_address(@_) if @_; return $self->original_remote_address unless $self->req->reverse_proxy; # Reverse proxy return ($self->req->headers->header('X-Forwarded-For') // '') =~ /([^,\s]+)$/ ? $1 : $self->original_remote_address; } Without this patch, if there's a request without HTTP_X_FORWARDED_FOR, then the remote address would be empty. Such would be the case of a dev/standard setup without Plack. This patch makes Koha::REST::V1::startup use tx->remote_address instead of dealing with the headers on its own. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 16.11, thanks Jonathan, Tomas!
I've been facing this problem, or something very similar, today. This is on 18.11.03.000 with Plack and memcached enabled and with the SessionRestrictionByIP syspref enabled. I am finding that API requests that require authorisation are causing the session to time out. This manifests itself as the API request returning a 401, with the body {"error":"Session has been expired."}. After doing some digging, the "timeouts" happen because the condition on this line fails: https://github.com/PTFS-Europe/koha/blob/18.11.03/C4/Auth.pm#L912 Specifically, $ip is coming back from the session store as 127.0.0.1 and $ENV{'REMOTE_ADDR'} is the true client IP, so they do not match and the condition fails. So, it seems to me that when the session is created, for some reason 127.0.0.1 is being stored as the remote IP. Doing some research, the "acknowledged" way of ensuring the remote IP passes through an Apache proxy cleanly is: ProxyPreserveHost On However, including this in the Apache config doesn't resolve the problem. Anyone got any ideas?
Andrew, please open a new bug report if the issue remains.
Just in case anybody in the future runs into this problem. I encountered it again today and determined that it was Postman, my REST client, that was at fault. I'm not sure what it was doing that was causing my IP to be set as 127.0.0.1, but switching to Insomnia as a client fixed it for me.