Re: [squid-users] external_acl_type requests authentication

On Friday 18 January 2008 00:51:40 ian j hart wrote:
> The external_acl_type requests authentication since bug 1278 was fixed.
>
> I have something like this (cut down and edited).
>
> external_acl_type logger ttl=0 negative_ttl=0 children=1 %LOGIN ...
>
> acl password proxy_auth REQUIRED
>
> acl proxylist dstdomain .some.site
> acl logproxy external logger 8
>
> http_access deny proxylist logproxy
> ###deny_info ERR_ACCESS_DENIED.proxy logproxy
>
> http_access allow password
> http_access deny all
>
> A hit on the proxylist causes authentication and the (bogus) error message
> only appears when the user selects cancel. Not intuitive.
>
> The bogus error message (you must authenticate) is easily fixed up with the
> deny_info line.
>
> Yes, I realise I could work around this with a dummy acl, but that's just
> nasty. In any case I'd rather add a feature than jump thru' hoops.
>
> Judging by the size of the patch to implement this is should be simple
> enough to fix up (famous last words).
>
> I had hoped I could just not set the flag, e.g.
>
> --- src/external_acl.c.orig Mon Jan 1 23:32:13 2007
> +++ src/external_acl.c Thu Jan 17 21:17:31 2008
> @@ -275,6 +275,8 @@
> format->type = EXT_ACL_LOGIN;
> a->require_auth = 1;
> }
> + else if (strcmp(token, "%NOAUTH") == 0)
> + format->type = EXT_ACL_LOGIN;
> #if USE_IDENT
> else if (strcmp(token, "%IDENT") == 0)
> format->type = EXT_ACL_IDENT;
>
> Unfortunately this breaks an assert in authenticate.c near line 648.
>
> At which point I need help.
>
> authenticateUserRequestUsername(auth_user_request_t * auth_user_request)
> {
> assert(auth_user_request != NULL);
>
> NULL seems to be a valid return value, that's one option. Dangerous?
>
> Fixing the call would be another. It appears to be called from
> external_acl.c makeExternalAclKey
>
> switch (format->type) {
> case EXT_ACL_LOGIN:
> str = authenticateUserRequestUsername(request->auth_user_request);
>
> Check the flag and set str=NULL?
>
> Maybe there's a patch for this already? Or a wish list where I could post
> it. Or is it near enough that someone could help me out?
>
> Thanks

This appears to work (tested for a whole 10 mins :)

--- src/external_acl.c.orig Mon Jan 1 23:32:13 2007
+++ src/external_acl.c Fri Jan 18 19:29:15 2008
@@ -275,6 +275,8 @@
            format->type = EXT_ACL_LOGIN;
            a->require_auth = 1;
        }
+ else if (strcmp(token, "%NOAUTH") == 0)
+ format->type = EXT_ACL_LOGIN;
 #if USE_IDENT
        else if (strcmp(token, "%IDENT") == 0)
            format->type = EXT_ACL_IDENT;
@@ -627,7 +629,8 @@
        const char *str = NULL;
        switch (format->type) {
        case EXT_ACL_LOGIN:
- str = authenticateUserRequestUsername(request->auth_user_request);
+ if (externalAclRequiresAuth(acl_data))
+ str = authenticateUserRequestUsername(request->auth_user_request);
            break;
 #if USE_IDENT
        case EXT_ACL_IDENT:

-- 
ian j hart