You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2018/11/26 22:19:38 UTC
[couchdb] branch master updated: Implement replicator session
refresh based on cookie max age
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/couchdb.git
The following commit(s) were added to refs/heads/master by this push:
new 1347806 Implement replicator session refresh based on cookie max age
1347806 is described below
commit 1347806d2feebce53325070b475f9e211d240ddf
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Fri Sep 21 13:12:03 2018 -0400
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
---
rel/overlay/etc/default.ini | 11 +-
src/couch/src/couch_httpd_auth.erl | 2 +-
.../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 edaebf9..a77add4 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 6607271..b519534 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 24f7c8e..51efd2a 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.