Implement replicator session refresh based on cookie max age

Force couch_replicator_auth_session plugin to refresh the session periodically.
Normally it is not needed as the session would be refreshed when requests start
failing with a 401 (authentication) or 403 (authorization) errors. In some
cases when anonymous writes are allowed to the database and a VDU function is
used to forbid writes based on the authenticated in username, requests with an
expired session cookie will not fail with a 401 and the session will not be
refreshed.

To fix the issue using these two approaches:

 1. Use cookie's max-age expiry time to schedule a refresh. To ensure that time
 is provided in the cookie, switch the the option to enable it by default. This
 handles the issue for endpoints which are updated with this commit.

 2. For endpoints which do not put a max-age time in the cookie, use a value
 that's less than CouchDB's default auth timeout. If users changed their
 auth timeout value, and use VDUs in the pattern described above, and don't
 update their endpoints to version which sends max-age by default, they could
 adjust `[replicator] session_refresh_interval_sec` to their auth timeout minus
 some small delay.

Of course refresh based on auth/authz failures should still works as before.

Fixes #1607

3 files changed, 119 insertions, 26 deletions

diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini
index edaebf9e2..a77add4bd 100644
--- a/rel/overlay/etc/default.ini
+++ b/rel/overlay/etc/default.ini
@@ -211,7 +211,7 @@ authentication_redirect = /_utils/session.html
 require_valid_user = false
 timeout = 600 ; number of seconds before automatic logout
 auth_cache_size = 50 ; size is number of cache entries
-allow_persistent_cookies = false ; set to true to allow persistent cookies
+allow_persistent_cookies = true ; set to false to disallow persistent cookies
 iterations = 10 ; iterations for password hashing
 ; min_iterations = 1
 ; max_iterations = 1000000000
@@ -395,6 +395,15 @@ ssl_certificate_max_depth = 3
 ; To restore the old behaviour, use the following value:
 ;auth_plugins = couch_replicator_auth_noop
 
+; Force couch_replicator_auth_session plugin to refresh the session
+; periodically if max-age is not present in the cookie. This is mostly to
+; handle the case where anonymous writes are allowed to the database and a VDU
+; function is used to forbid writes based on the authenticated user name. In
+; that case this value should be adjusted based on the expected minimum session
+; expiry timeout on replication endpoints. If session expiry results in a 401
+; or 403 response this setting is not needed.
+;session_refresh_interval_sec = 550
+
 [compaction_daemon]
 ; The delay, in seconds, between each check for which database and view indexes
 ; need to be compacted.
diff --git a/src/couch/src/couch_httpd_auth.erl b/src/couch/src/couch_httpd_auth.erl
index 660727195..b5195349b 100644
--- a/src/couch/src/couch_httpd_auth.erl
+++ b/src/couch/src/couch_httpd_auth.erl
@@ -436,7 +436,7 @@ cookie_scheme(#httpd{mochi_req=MochiReq}) ->
     end.
 
 max_age() ->
-    case config:get("couch_httpd_auth", "allow_persistent_cookies", "false") of
+    case config:get("couch_httpd_auth", "allow_persistent_cookies", "true") of
         "false" ->
             [];
         "true" ->
diff --git a/src/couch_replicator/src/couch_replicator_auth_session.erl b/src/couch_replicator/src/couch_replicator_auth_session.erl
index 24f7c8e3c..51efd2af8 100644
--- a/src/couch_replicator/src/couch_replicator_auth_session.erl
+++ b/src/couch_replicator/src/couch_replicator_auth_session.erl
@@ -79,9 +79,11 @@
 -type headers() :: [{string(), string()}].
 -type code() :: non_neg_integer().
 -type creds() :: {string() | undefined, string() | undefined}.
+-type time_sec() :: non_neg_integer().
+-type age() :: time_sec() | undefined.
 
-
--define(MIN_UPDATE_INTERVAL, 5).
+-define(MIN_UPDATE_INTERVAL_SEC, 5).
+-define(DEFAULT_REFRESH_INTERVAL_SEC, 550).
 
 
 -record(state, {
@@ -448,7 +450,7 @@ http_response({error, Error}, #state{session_url = Url, user = User}) ->
     {error, {session_request_failed, Url, User, Error}}.
 
 
--spec parse_cookie(list()) -> {ok, string()} | {error, term()}.
+-spec parse_cookie(list()) -> {ok, age(), string()} | {error, term()}.
 parse_cookie(Headers0) ->
     Headers = mochiweb_headers:make(Headers0),
     case mochiweb_headers:get_value("Set-Cookie", Headers) of
@@ -461,48 +463,88 @@ parse_cookie(Headers0) ->
                 undefined ->
                     {error, cookie_format_invalid};
                 Cookie ->
-                    {ok, Cookie}
+                    MaxAge = parse_max_age(CaseInsKVs),
+                    {ok, MaxAge, Cookie}
             end
     end.
 
 
+-spec parse_max_age(list()) -> age().
+parse_max_age(CaseInsKVs) ->
+    case mochiweb_headers:get_value("Max-Age", CaseInsKVs) of
+        String when is_list(String) ->
+            try
+                list_to_integer(String)
+            of
+                MaxAge when MaxAge >= 0 ->
+                    MaxAge;
+                _ ->
+                    undefined
+            catch
+                error:badarg ->
+                    undefined
+            end;
+        _ ->
+            undefined
+    end.
+
+
 -spec maybe_update_cookie(headers(), #state{}) ->
     {ok, string()} | {error, term()}.
 maybe_update_cookie(ResponseHeaders, State) ->
     case parse_cookie(ResponseHeaders) of
-        {ok, Cookie} ->
-            {ok, update_cookie(State, Cookie, now_sec())};
+        {ok, MaxAge, Cookie} ->
+            {ok, update_cookie(State, Cookie, now_sec(), MaxAge)};
         {error, Error} ->
             {error, Error}
     end.
 
 
--spec update_cookie(#state{}, string(), non_neg_integer()) -> #state{}.
-update_cookie(#state{cookie = Cookie} = State, Cookie, _) ->
+-spec update_cookie(#state{}, string(), time_sec(), age()) -> #state{}.
+update_cookie(#state{cookie = Cookie} = State, Cookie, _, _) ->
     State;
-update_cookie(#state{epoch = Epoch} = State, Cookie, NowSec) ->
-    State#state{
+update_cookie(#state{epoch = Epoch} = State, Cookie, NowSec, MaxAge) ->
+    NextRefresh = next_refresh(NowSec, MaxAge, refresh_interval()),
+    NewState = State#state{
         epoch = Epoch + 1,
         cookie = Cookie,
         refresh_tstamp = NowSec
-    }.
+    },
+    schedule_refresh(NextRefresh, NewState).
+
+
+-spec next_refresh(time_sec(), age(), time_sec()) -> time_sec().
+next_refresh(NowSec, undefined, RefreshInterval) ->
+    NowSec + RefreshInterval;
 
+next_refresh(NowSec, MaxAge, _) when is_integer(MaxAge) ->
+    % Apply a fudge factor to account for delays in receving the cookie
+    % and / or time adjustments happening over a longer period of time
+    NowSec + trunc(MaxAge * 0.9).
 
--spec cookie_age_sec(#state{}, non_neg_integer()) -> non_neg_integer().
+
+-spec cookie_age_sec(#state{}, time_sec()) -> time_sec().
 cookie_age_sec(#state{refresh_tstamp = RefreshTs}, Now) ->
     max(0, Now - RefreshTs).
 
 
--spec now_sec() -> non_neg_integer().
+-spec now_sec() -> time_sec().
 now_sec() ->
     {Mega, Sec, _Micro} = os:timestamp(),
     Mega * 1000000 + Sec.
 
 
--spec min_update_interval() -> non_neg_integer().
+-spec min_update_interval() -> time_sec().
 min_update_interval() ->
     config:get_integer("replicator", "session_min_update_interval",
-        ?MIN_UPDATE_INTERVAL).
+        ?MIN_UPDATE_INTERVAL_SEC).
+
+
+-spec refresh_interval() -> integer().
+refresh_interval() ->
+    config:get_integer("replicator", "session_refresh_interval_sec",
+        ?DEFAULT_REFRESH_INTERVAL_SEC).
+
 
 
 -spec b64creds(string(), string()) -> string().
@@ -593,7 +635,8 @@ cookie_update_test_() ->
         fun setup/0,
         fun teardown/1,
         [
-            t_do_refresh(),
+            t_do_refresh_without_max_age(),
+            t_do_refresh_with_max_age(),
             t_dont_refresh(),
             t_process_auth_failure(),
             t_process_auth_failure_stale_epoch(),
@@ -609,24 +652,41 @@ cookie_update_test_() ->
     }.
 
 
-t_do_refresh() ->
+t_do_refresh_without_max_age() ->
     ?_test(begin
         State = #state{next_refresh = 0},
         {ok, State1} = maybe_refresh(State),
-        ?assertMatch(#state{
-            next_refresh = infinity,
-            epoch = 1,
-            cookie = "Abc"
-        }, State1)
+        ?assertMatch(#state{epoch = 1, cookie = "Abc"}, State1),
+        #state{next_refresh = NextRefresh} = State1,
+        RefreshInterval = NextRefresh - now_sec(),
+        ?assert(540 < RefreshInterval andalso RefreshInterval =< 550)
+    end).
+
+
+t_do_refresh_with_max_age() ->
+    ?_test(begin
+        State = #state{next_refresh = 0},
+        mock_http_cookie_response_with_age("Zig", "100"),
+        {ok, State1} = maybe_refresh(State),
+        ?assertMatch(#state{epoch = 1, cookie = "Zig"}, State1),
+        #state{next_refresh = NextRefresh} = State1,
+        RefreshInterval = NextRefresh - now_sec(),
+        ?assert(80 < RefreshInterval andalso RefreshInterval =< 90)
     end).
 
 
 t_dont_refresh() ->
     ?_test(begin
-        State = #state{next_refresh = now_sec() + 100},
+        State = #state{
+            next_refresh = now_sec() + 100,
+            refresh_tstamp = now_sec()
+        },
         {ok, State1} = maybe_refresh(State),
         ?assertMatch(State, State1),
-        State2 = #state{next_refresh = infinity},
+        State2 = #state{
+            next_refresh = infinity,
+            refresh_tstamp = now_sec()
+        },
         {ok, State3} = maybe_refresh(State2),
         ?assertMatch(State2, State3)
     end).
@@ -731,6 +791,13 @@ mock_http_cookie_response(Cookie) ->
     meck:expect(ibrowse, send_req_direct, 7, Resp).
 
 
+mock_http_cookie_response_with_age(Cookie, Age) ->
+    AgeKV = "Max-Age=" ++ Age,
+    CookieKV = "AuthSession=" ++ Cookie,
+    Resp = {ok, "200", [{"Set-Cookie", CookieKV ++ ";" ++ AgeKV}], []},
+    meck:expect(ibrowse, send_req_direct, 7, Resp).
+
+
 mock_http_401_response() ->
     meck:expect(ibrowse, send_req_direct, 7, {ok, "401", [], []}).
 
@@ -756,4 +823,21 @@ extract_creds_error_test_() ->
         {#httpdb{url = "http://h/db"}, missing_credentials}
     ]].
 
+
+parse_max_age_test_() ->
+    [?_assertEqual(R, parse_max_age(mochiweb_headers:make([{"Max-Age", A}])))
+        ||  {A, R} <- [
+            {"-10", undefined},
+            {"\ufeff", undefined},
+            {"*", undefined},
+            {"\n1", undefined},
+            {"1", 1},
+            {"1 1", undefined},
+            {"2", 2},
+            {"100", 100},
+            {"1234567890", 1234567890}
+        ]
+    ].
+
+
 -endif.