On Tue, 2011-09-20 at 12:09 +1200, Amos Jeffries wrote:
> > Do you mean the -d option to the Squid binary? If so, this doesn't 
> > seem
> > to make any difference; it just prints all the log messages to the
> > display as well as the log file.
> 
>  -d parameter of the helper binary. stderr is piped to cache.log at 
>  level-0. Thus the need for a separate switch to enable lots of debug 
>  from the helper.

Sorry if I'm being stupid, but do you mean the actual session helper
binary or something else? If it's the former, then that obviously just
causes an exit due to an invalid option.

> > Done. Also, the following page should be updated:
> >
> > http://wiki.squid-cache.org/ConfigExamples/Portal/Splash
> >
> > I'm happy to do it myself, if you can give me wiki edit rights?
> 
>  Sure. Just need to know what username you login with.

Great stuff. AndrewBeverley

>  -h is reserved for display of "help" / usage() texts.
> 

Good point.

>  -T would probably be better for an absolute timeout.
> 

Changed as suggested.

>  ext_session_acl.8:
>   * " (unless -h is specified).". Please make a separate sentence. 
>  Something like "Also supports fixed length timeouts for regular splash 
>  page displays."

Done.

>    (if you can think of any other useful scenario than regular splash 
>  pages that can be mentioned too.)

Well I've mentioned the possibility of forcing a user to
re-authenticate, but I'm not sure that I'm correct in saying that. Do
you think that is possible?

>   * rather than calling the new mode "Hard timeout". It's a "Periodic" 
>  or "Fixed length" timeout ... something a bit more descriptive like 
>  that.

Done.

>   ** Needs to explain how it interacts with -a mode. ie I would expect 
>  LOGOUT or timeout to terminate the session. Explicit LOGIN required for 
>  a new one to start.

Done.

>  ext_session_acl.cc: (modulo the -h ==> -T change requested)
>   * in getopt() format syntax ':' indicates a value is received for the 
>  option. What you coded is "t:b:a?h?".
>   * I think you should make -t and -T both optional arguments which are 
>  interchangeable.   ("t:T:b:a?")

Done.

Please find updated patch attached.

Andy

This patch adds some additional error messages when concurrency has
not been specified for the session helper.

It also adds a -T timeout option, which gives a fixed-length timeout,
rather than one that times out on user inactivity.

=== modified file 'helpers/external_acl/session/ext_session_acl.8'
--- helpers/external_acl/session/ext_session_acl.8	2010-09-16 13:07:02 +0000
+++ helpers/external_acl/session/ext_session_acl.8	2011-09-20 21:22:00 +0000
@@ -1,11 +1,11 @@
-.if !'po4a'hide' .TH ext_session_acl 8 "19 March 2006"
+.if !'po4a'hide' .TH ext_session_acl 8 "19 September 2011"
 .
 .SH NAME
 .if !'po4a'hide' .B ext_session_acl
 .if !'po4a'hide' \-
 Squid session tracking external acl helper.
 .PP
-Version 1.0
+Version 1.1
 .
 .SH SYNOPSIS
 .if !'po4a'hide' .B ext_session_acl
@@ -18,23 +18,39 @@
 .SH DESCRIPTION
 .B ext_session_acl
 maintains a concept of sessions by monitoring requests
-and timing out sessions if no requests have been seen for the idle timeout
-timer.
-.PP
-Intended use is for displaying "terms of use" pages, ad popups etc.
+and timing out sessions. The timeout is based either on idle use (
+.B -t
+) or a fixed period of time (
+.B -T
+). The former is suitable for displaying terms and conditions to a user; the
+latter is suitable for the display of advertisments or other notices (both as a
+splash page - see config examples online). The session helper can also be used
+to force users to re-authenticate.
 .
 .SH OPTIONS
 .if !'po4a'hide' .TP 12
 .if !'po4a'hide' .B "\-t timeout"
-.B Timeout
-for any session. If not specified the default is 3600 seconds.
+Idle timeout for any session. The default if not specified (set to 3600 seconds).
+.
+.if !'po4a'hide' .TP
+.if !'po4a'hide' .B "\-T timeout"
+Fixed timeout for any session. This will end the session after the timeout regardless
+of a user's activity. If used with
+.B active
+mode, this will terminate the user's session after
+.B timeout
+, after which another
+.B LOGIN
+will be required.
+.B LOGOUT
+will reset the session and timeout.
 .
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B "\-b path"
 .B Path
 to persistent database. If not specified the session details
 will be kept in memory only and all sessions will reset each time
-Squid restarts it's helpers (Squid restart or rotation of logs).
+Squid restarts its helpers (Squid restart or rotation of logs).
 .
 .if !'po4a'hide' .TP
 .if !'po4a'hide' .B \-a
@@ -43,12 +59,16 @@
 .B LOGIN
 , or terminated by the argument
 .B LOGOUT
-.PP
 Without this flag the helper automatically starts the session after
 the first request.
-.
 .SH CONFIGURATION
 .PP
+The
+.B ext_session_acl
+helper is a concurrent helper; therefore, the concurrency= option
+.B must
+be specified in the configuration.
+.PP
 Configuration example using the default automatic mode
 .if !'po4a'hide' .RS
 .if !'po4a'hide' .B external_acl_type session ttl=300 negative_ttl=0 children=1 concurrency=200 %LOGIN /usr/local/squid/libexec/ext_session_acl

=== modified file 'helpers/external_acl/session/ext_session_acl.cc'
--- helpers/external_acl/session/ext_session_acl.cc	2010-07-29 14:23:35 +0000
+++ helpers/external_acl/session/ext_session_acl.cc	2011-09-20 21:22:07 +0000
@@ -52,6 +52,7 @@
 #endif
 
 static int session_ttl = 3600;
+static int fixed_timeout = 0;
 char *db_path = NULL;
 const char *program_name;
 
@@ -101,6 +102,7 @@
     data.data = &now;
     data.size = sizeof(now);
     db->put(db, &key, &data, 0);
+    db->sync(db, 0);
 }
 
 static void session_logout(const char *details, size_t len)
@@ -113,8 +115,9 @@
 
 static void usage(void)
 {
-    fprintf(stderr, "Usage: %s [-t session_timeout] [-b dbpath] [-a]\n", program_name);
-    fprintf(stderr, "	-t sessiontimeout	Idle timeout after which sessions will be forgotten\n");
+    fprintf(stderr, "Usage: %s [-t|-T session_timeout] [-b dbpath] [-a]\n", program_name);
+    fprintf(stderr, "	-t sessiontimeout	Idle timeout after which sessions will be forgotten (user activity will reset)\n");
+    fprintf(stderr, "	-T sessiontimeout	Fixed timeout after which sessions will be forgotten (regardless of user activity)\n");
     fprintf(stderr, "	-b dbpath		Path where persistent session database will be kept\n");
     fprintf(stderr, "	-a			Active mode requiring LOGIN argument to start a session\n");
 }
@@ -126,8 +129,10 @@
 
     program_name = argv[0];
 
-    while ((opt = getopt(argc, argv, "t:b:a?")) != -1) {
+    while ((opt = getopt(argc, argv, "t:T:b:a?")) != -1) {
         switch (opt) {
+        case 'T':
+            fixed_timeout = 1;
         case 't':
             session_ttl = strtol(optarg, NULL, 0);
             break;
@@ -150,8 +155,13 @@
 
     while (fgets(request, HELPER_INPUT_BUFFER, stdin)) {
         int action = 0;
-        const char *user_key = strtok(request, " \n");
+        const char *channel_id = strtok(request, " ");
         const char *detail = strtok(NULL, "\n");
+        if (detail == NULL) {
+            // Only 1 paramater supplied. We are expecting at least 2 (including the channel ID)
+            fprintf(stderr, "FATAL: %s is concurrent and requires the concurrency option to be specified.\n", program_name);
+            exit(1);
+        }
         const char *lastdetail = strrchr(detail, ' ');
         size_t detail_len = strlen(detail);
         if (lastdetail) {
@@ -165,18 +175,20 @@
         }
         if (action == -1) {
             session_logout(detail, detail_len);
-            printf("%s OK message=\"Bye\"\n", user_key);
+            printf("%s OK message=\"Bye\"\n", channel_id);
         } else if (action == 1) {
             session_login(detail, detail_len);
-            printf("%s OK message=\"Welcome\"\n", user_key);
+            printf("%s OK message=\"Welcome\"\n", channel_id);
         } else if (session_active(detail, detail_len)) {
-            session_login(detail, detail_len);
-            printf("%s OK\n", user_key);
+            if (fixed_timeout == 0) {
+                session_login(detail, detail_len);
+            }
+            printf("%s OK\n", channel_id);
         } else if (default_action == 1) {
             session_login(detail, detail_len);
-            printf("%s ERR message=\"Welcome\"\n", user_key);
+            printf("%s ERR message=\"Welcome\"\n", channel_id);
         } else {
-            printf("%s ERR message=\"No session available\"\n", user_key);
+            printf("%s ERR message=\"No session available\"\n", channel_id);
         }
     }
     shutdown_db();

Reply via email to