diff options
author | Nick Vatamaniuc <vatamane@apache.org> | 2018-09-21 13:12:03 -0400 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2018-11-26 17:19:32 -0500 |
commit | 1347806d2feebce53325070b475f9e211d240ddf (patch) | |
tree | 7cbe1581749adf8eb9ef58c20909476cd6ea155f | |
parent | d1c8a833055dd855bb57c7639f22f42216dde919 (diff) | |
download | couchdb-1347806d2feebce53325070b475f9e211d240ddf.tar.gz |
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
-rw-r--r-- | rel/overlay/etc/default.ini | 11 | ||||
-rw-r--r-- | src/couch/src/couch_httpd_auth.erl | 2 | ||||
-rw-r--r-- | src/couch_replicator/src/couch_replicator_auth_session.erl | 132 |
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. |