You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by KlausTrainer <gi...@git.apache.org> on 2014/03/11 00:45:39 UTC
[GitHub] couchdb pull request: Use cookie authentication during replication
GitHub user KlausTrainer opened a pull request:
https://github.com/apache/couchdb/pull/172
Use cookie authentication during replication
In `couch_replicator_httpc:setup/1`, if the URL contains basic auth
credentials, the replicator now removes those credentials from the URL
and inserts them into a new ets table, whose identifier is stored in the
returned `httpdb` record in a new field named `credentials`.
Then, if during replication there's a HTTP status code 401, the
replicator does a POST-request to the remote DB's `/_session` endpoint,
in which the request body contains the user name and password from the
basic auth credentials stored in the ets table. If that request is
successful, the session cookie is inserted into the ets table together
with the current timestamp returned by `os:timestamp/0`. From that
point on, `couch_replicator_httpc:send_req/3` always retrieves the
cookie from the ets table and sets it as value in the "Cookie" request
header. When a new session cookie is set, the cookie value, together
with a new timestamp, will be updated in the ets table accordingly.
Now, after having started a new session, the request that generated the
response status code 401 is repeated.
In order to prevent the replicator from wrongly starting a new session
after getting a 401 status code and then repeating the request again
(and thereby causing an infinite loop), it is checked whether the
difference between the current timestamp returned by `os:timestamp/0`
and the timestamp stored together with the session cookie is greater
than one minute. A new session is started and the request is repeated
only if the difference is greater than one minute. Otherwise it is
proceeded as ever, i.e., the response with the 401 status code is
returned.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/KlausTrainer/couchdb 2069-cookie-auth-replication
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb/pull/172.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #172
----
commit 97b7b4e4b2aa48f9f03fb781f0754cc1e2b14543
Author: Klaus Trainer <kl...@apache.org>
Date: 2014-03-09T19:10:53Z
Use cookie authentication during replication
In `couch_replicator_httpc:setup/1`, if the URL contains basic auth
credentials, the replicator now removes those credentials from the URL
and inserts them into a new ets table, whose identifier is stored in the
returned `httpdb` record in a new field named `credentials`.
Then, if during replication there's a HTTP status code 401, the
replicator does a POST-request to the remote DB's `/_session` endpoint,
in which the request body contains the user name and password from the
basic auth credentials stored in the ets table. If that request is
successful, the session cookie is inserted into the ets table together
with the current timestamp returned by `os:timestamp/0`. From that
point on, `couch_replicator_httpc:send_req/3` always retrieves the
cookie from the ets table and sets it as value in the "Cookie" request
header. When a new session cookie is set, the cookie value, together
with a new timestamp, will be updated in the ets table accordingly.
Now, after having started a new session, the request that generated the
response status code 401 is repeated.
In order to prevent the replicator from wrongly starting a new session
after getting a 401 status code and then repeating the request again
(and thereby causing an infinite loop), it is checked whether the
difference between the current timestamp returned by `os:timestamp/0`
and the timestamp stored together with the session cookie is greater
than one minute. A new session is started and the request is repeated
only if the difference is greater than one minute. Otherwise it is
proceeded as ever, i.e., the response with the 401 status code is
returned.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10467078
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
@KlausTrainer hm..wait a moment, what about expiration date then? Every cookie carries that information with.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10466339
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
@KlausTrainer How about to use [couch_httpd_auth/timeout](http://docs.couchdb.org/en/latest/config/auth.html#couch_httpd_auth/timeout) config value instead? It uses for the same case: [to check](https://github.com/apache/couchdb/blob/master/src/couchdb/couch_httpd_auth.erl#L195) the cookie for expiration.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by KlausTrainer <gi...@git.apache.org>.
Github user KlausTrainer commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10485167
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
It looks like an expiry time is only set if the
"allow_persistent_cookies" configuration is set to `true`, which is
`false` by default, however. See the `max_age/0` function in the
couch_httpd_auth module.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by KlausTrainer <gi...@git.apache.org>.
Github user KlausTrainer commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10465907
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
Yes, that's possible, but what advantage would it get us? I had previously implemented it, but then threw it away again. One disadvantage of that is that it makes us rely on clocks being synchronized, which is an assumption we probably like to avoid here.
As of now, we unfortunately don't have any knowledge about the expiry time of a cookie here (there's only have the timestamp). Choosing one minute seems like a good compromise to me, as it's unlikely that people will configuring a session timeout below that. However, it might be good to consequently limit the session timeout to a minimum of one minute and document it appropriately.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by KlausTrainer <gi...@git.apache.org>.
Github user KlausTrainer commented on the pull request:
https://github.com/apache/couchdb/pull/172#issuecomment-62225439
Closing in favour of a new pull request targeting the 1.x.x branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by benoitc <gi...@git.apache.org>.
Github user benoitc commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10467062
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
On Tue, Mar 11, 2014 at 10:56 AM, Klaus Trainer <no...@github.com>wrote:
> In src/couch_replicator/src/couch_replicator_httpc.erl:
>
> > + true -> start_new_session(HttpDb, Worker)
> > + end.
> > +
> > +
> > +need_new_session(#httpdb{credentials = undefined}) ->
> > + false;
> > +
> > +need_new_session(#httpdb{credentials = Credentials}) ->
> > + case ets:lookup(Credentials, cookie) of
> > + [] ->
> > + true;
> > + [{cookie, _, UpdatedAt}] ->
> > + %% As we don't know when the cookie will expire, we just decide
> > + %% that we want a new session if the current one is older than
> > + %% one minute.
> > + OneMinute = 60 * 1000000, % microseconds
>
> @benoitc <https://github.com/benoitc> I'm sorry, I don't see how TS - Nowcould relate to the expiry time that's configured on the remote server.
>
>
> Well when you get the expires times in the cookie and compare it to the
current hour on your machine you know the delay in which the cookie is
supposed to expire. So either you check against the cookie expires times
each time, or set a timer to remove it from the ets which will trigger the
401 the next time ou try to authenticate. In other cases you will have this
401 anyway.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by KlausTrainer <gi...@git.apache.org>.
Github user KlausTrainer commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10483984
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
> Well when you get the expires times in the cookie and compare it to
> the current hour on your machine you know the delay in which the
> cookie is supposed to expire.
Being able to do that would be nice, but the problem is that currently
there is no expiry time in the cookie. The timestamp contained in the
cookie is generated by `make_cookie_time/0`, which is the time in
seconds at the point in time when the cookie was created, and therefore
not really helpful. We can currently only make guesses about how long
a cookie is be valid.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by benoitc <gi...@git.apache.org>.
Github user benoitc commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10456813
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
can't it be read in the cookie header?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by benoitc <gi...@git.apache.org>.
Github user benoitc commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10485218
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
On Tue, Mar 11, 2014 at 6:28 PM, Klaus Trainer <no...@github.com>wrote:
> In src/couch_replicator/src/couch_replicator_httpc.erl:
>
> > + true -> start_new_session(HttpDb, Worker)
> > + end.
> > +
> > +
> > +need_new_session(#httpdb{credentials = undefined}) ->
> > + false;
> > +
> > +need_new_session(#httpdb{credentials = Credentials}) ->
> > + case ets:lookup(Credentials, cookie) of
> > + [] ->
> > + true;
> > + [{cookie, _, UpdatedAt}] ->
> > + %% As we don't know when the cookie will expire, we just decide
> > + %% that we want a new session if the current one is older than
> > + %% one minute.
> > + OneMinute = 60 * 1000000, % microseconds
>
> It looks like an expiry time is only set if the
> "allow_persistent_cookies" configuration is set to true, which is
> false by default, however. See the max_age/0 function in the
> couch_httpd_auth module.
>
yeah we are sending a wrong header imo.
- benoit
> --
> Reply to this email directly or view it on GitHub<https://github.com/apache/couchdb/pull/172/files#r10485167>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by benoitc <gi...@git.apache.org>.
Github user benoitc commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10484392
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
On Tue, Mar 11, 2014 at 6:06 PM, Klaus Trainer <no...@github.com>wrote:
> In src/couch_replicator/src/couch_replicator_httpc.erl:
>
> > + true -> start_new_session(HttpDb, Worker)
> > + end.
> > +
> > +
> > +need_new_session(#httpdb{credentials = undefined}) ->
> > + false;
> > +
> > +need_new_session(#httpdb{credentials = Credentials}) ->
> > + case ets:lookup(Credentials, cookie) of
> > + [] ->
> > + true;
> > + [{cookie, _, UpdatedAt}] ->
> > + %% As we don't know when the cookie will expire, we just decide
> > + %% that we want a new session if the current one is older than
> > + %% one minute.
> > + OneMinute = 60 * 1000000, % microseconds
>
> Well when you get the expires times in the cookie and compare it to
>
> the current hour on your machine you know the delay in which the
> cookie is supposed to expire.
>
> Being able to do that would be nice, but the problem is that currently
> there is no expiry time in the cookie. The timestamp contained in the
> cookie is generated by make_cookie_time/0, which is the time in
> seconds at the point in time when the cookie was created, and therefore
> not really helpful. We can currently only make guesses about how long
> a cookie is be valid.
>
If the expires time is not present, then why not adding it? But If I read
wel the code of `mochiweb_cookies:cookie/3` [1] you get the expires time
from the cookie header:
https://github.com/mochi/mochiweb/blob/master/src/mochiweb_cookies.erl#L63
and then it should be possible to do the above. Once I have access to a
machine i will do the test.
> --
> Reply to this email directly or view it on GitHub<https://github.com/apache/couchdb/pull/172/files#r10483984>
> .
>
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by KlausTrainer <gi...@git.apache.org>.
Github user KlausTrainer commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10466885
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
@benoitc I'm sorry, I don't see how `TS - Now` could relate to the expiry time that's configured on the remote server.
@kxepal Retrieving the timeout via `couch_config:get/3` will only tell us the timeout that's configured locally, which is unrelated to the timeout that's configured on the remote DB.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by benoitc <gi...@git.apache.org>.
Github user benoitc commented on a diff in the pull request:
https://github.com/apache/couchdb/pull/172#discussion_r10466390
--- Diff: src/couch_replicator/src/couch_replicator_httpc.erl ---
@@ -131,6 +171,90 @@ process_stream_response(ReqId, Worker, HttpDb, Params, Callback) ->
end.
+process_stream_response_headers(ReqId, Code, Headers, Worker, HttpDb, Params, Callback) ->
+ StreamDataFun = fun() ->
+ stream_data_self(HttpDb, Params, Worker, ReqId, Callback)
+ end,
+ ibrowse:stream_next(ReqId),
+ try
+ Ret = Callback(Code, Headers, StreamDataFun),
+ release_worker(Worker, HttpDb),
+ clean_mailbox_req(ReqId),
+ Ret
+ catch throw:{maybe_retry_req, Err} ->
+ clean_mailbox_req(ReqId),
+ maybe_retry(Err, Worker, HttpDb, Params, Callback)
+ end.
+
+
+maybe_start_new_session(HttpDb) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb)
+ end.
+
+
+maybe_start_new_session(HttpDb, Worker) ->
+ case need_new_session(HttpDb) of
+ false -> false;
+ true -> start_new_session(HttpDb, Worker)
+ end.
+
+
+need_new_session(#httpdb{credentials = undefined}) ->
+ false;
+
+need_new_session(#httpdb{credentials = Credentials}) ->
+ case ets:lookup(Credentials, cookie) of
+ [] ->
+ true;
+ [{cookie, _, UpdatedAt}] ->
+ %% As we don't know when the cookie will expire, we just decide
+ %% that we want a new session if the current one is older than
+ %% one minute.
+ OneMinute = 60 * 1000000, % microseconds
--- End diff --
On Mar 11, 2014 10:20 AM, "Klaus Trainer" <no...@github.com> wrote:
>
> In src/couch_replicator/src/couch_replicator_httpc.erl:
>
> > + true -> start_new_session(HttpDb, Worker)
> > + end.
> > +
> > +
> > +need_new_session(#httpdb{credentials = undefined}) ->
> > + false;
> > +
> > +need_new_session(#httpdb{credentials = Credentials}) ->
> > + case ets:lookup(Credentials, cookie) of
> > + [] ->
> > + true;
> > + [{cookie, _, UpdatedAt}] ->
> > + %% As we don't know when the cookie will expire, we just decide
> > + %% that we want a new session if the current one is older than
> > + %% one minute.
> > + OneMinute = 60 * 1000000, % microseconds
>
> Yes, that's possible, but what advantage would it get us? I had
previously implemented it, but then threw it away again. One disadvantage
of that is that it makes us rely on clocks being synchronized, which is an
assumption we probably like to avoid here.
>
> As of now, we unfortunately don't have any knowledge about the expiry
time of a cookie here (there's only have the timestamp). Choosing one
minute seems like a good compromise to me, as it's unlikely that people
will configuring a session timeout below that. However, it might be good to
consequently limit the session timeout to a minimum of one minute and
document it appropriately.
>
it would allows us to reduce the number of calls which is already pretty
high. what for example when the cookie never expires?
and well TS - Now = expiry time.
- benoit
> --
> Reply to this email directly or view it on GitHub.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb pull request: Use cookie authentication during replication
Posted by KlausTrainer <gi...@git.apache.org>.
Github user KlausTrainer closed the pull request at:
https://github.com/apache/couchdb/pull/172
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---