Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cookie lifetime isn't extended on authenticated request #2

Open
sigio opened this issue Feb 26, 2015 · 3 comments
Open

Cookie lifetime isn't extended on authenticated request #2

sigio opened this issue Feb 26, 2015 · 3 comments

Comments

@sigio
Copy link

sigio commented Feb 26, 2015

I'ts my expectation from reading the source and documentation that upon authentication, any new web-request made with a valid authentication will re-new the lifetime of the authentication cookie.

So if you have a 10-minute cookie lifetime, making at least one request before the 10 minutes runs out will reset the counter to 10 minutes again.

Looking at the source (and my cookie-tracker), I;m not seeing the expire-time being reset when new cookies are sent.

@sigio
Copy link
Author

sigio commented Feb 26, 2015

I think the following block needs to be changed:

static authn_status
authn_linotp_check_password(request_rec r, const char *username, const char *otp_given)
{
[snip]
/
check for the existence of a cookie: do weak authentication if so /
if ((cookie = spot_cookie(r)) != NULL) {
ap_log_error(APLOG_MARK, APLOG_NOERRNO | APLOG_DEBUG, 0, r->server, "Found cookie=%s for user=%s : ", cookie, r->user);
/
valid username, passwd, and expiry date: don't do LinOTP auth */
if (valid_cookie(r, cookie, otp_given)) {
ap_log_error(APLOG_MARK, APLOG_NOERRNO | APLOG_DEBUG, 0, r->server,"cookie still valid. Serving page.");

We should re-set the cookie here

              expires = time(NULL) + conf->timeout;
              add_cookie(r, r->headers_out, cookie, expires);

Something along these lines...

      returnValue=AUTH_GRANTED;
      goto cleanup;
    } else {            /* the cookie has probably expired */

[snip]

@sigio
Copy link
Author

sigio commented Feb 26, 2015

This patch (tested) seems to do what I want / what the documentation seems to mean:
(Markdown seems to screw with the patch... I'll submit a pull-request)

diff --git a/mod_authn_linotp.c b/mod_authn_linotp.c
index 0934d2b..9c05b8d 100644
--- a/mod_authn_linotp.c
+++ b/mod_authn_linotp.c
@@ -639,6 +639,9 @@ authn_linotp_check_password(request_rec r, const char *username, const char *ot
/
valid username, passwd, and expiry date: don't do LinOTP auth */
if (valid_cookie(r, cookie, otp_given)) {
ap_log_error(APLOG_MARK, APLOG_NOERRNO | APLOG_DEBUG, 0, r->server,"cookie still valid. Serving page.");

  •             /\* Renew active cookie with new timeout */
    
  •             expires = time(NULL) + conf->timeout;
    
  •             add_cookie(r, r->headers_out, cookie, expires);
    
              returnValue=AUTH_GRANTED;
              goto cleanup;
            } else {                        /* the cookie has probably expired */
    

@sigio
Copy link
Author

sigio commented Jun 2, 2015

Any feedback please.... or at least a merge of this pull-request ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant