From a33a428a9d803640055f16e791f449dda8ea33b6 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Thu, 17 Dec 2009 17:48:51 +1300 Subject: [PATCH] Upgrade AuthUserRequest ref-counting TODO: Test auth still runs. Some problems were noted in conversion which affect digest, NTLM, Negotiate AuthUser objects. Notably that the virtual functions setting a custom field instead of the parent objects _auth_user woudl result in teh *::Pointer having a NULL user() Convert AuthUser, AuthScheme, AuthConfig to ref-counted as well. --- src/CompositePoolNode.h | 5 +- src/HttpRequest.cc | 7 +- src/HttpRequest.h | 3 +- src/acl/FilledChecklist.cc | 38 ++------ src/acl/FilledChecklist.h | 4 +- src/adaptation/icap/ModXact.cc | 11 +-- src/auth/Acl.cc | 16 --- src/auth/AclMaxUserIp.cc | 6 +- src/auth/AclMaxUserIp.h | 5 +- src/auth/AclProxyAuth.cc | 13 ++- src/auth/Config.cc | 11 +-- src/auth/Config.h | 9 +- src/auth/Makefile.am | 1 + src/auth/User.cc | 11 +-- src/auth/User.cci | 8 +- src/auth/User.h | 5 +- src/auth/UserRequest.cc | 139 +++++++++------------------ src/auth/UserRequest.h | 61 +++++------- src/auth/basic/auth_basic.cc | 16 +-- src/auth/basic/auth_basic.h | 20 ++-- src/auth/digest/auth_digest.cc | 35 +++---- src/auth/digest/auth_digest.h | 6 +- src/auth/negotiate/auth_negotiate.cc | 31 +++--- src/auth/negotiate/auth_negotiate.h | 6 +- src/auth/ntlm/auth_ntlm.cc | 23 ++--- src/auth/ntlm/auth_ntlm.h | 6 +- src/client_side.cc | 9 +- src/client_side.h | 9 +- src/client_side_reply.cc | 15 +-- src/client_side_reply.h | 2 +- src/client_side_request.cc | 10 +- src/enums.h | 16 --- src/errorpage.cc | 4 +- src/errorpage.h | 4 +- src/external_acl.cc | 5 +- src/http.cc | 2 +- src/peer_userhash.cc | 2 +- src/redirect.cc | 2 +- src/stat.cc | 2 +- src/tests/testAuth.cc | 62 ++++++------ 40 files changed, 232 insertions(+), 408 deletions(-) diff --git a/src/CompositePoolNode.h b/src/CompositePoolNode.h index ccecf4a671..6318c8233b 100644 --- a/src/CompositePoolNode.h +++ b/src/CompositePoolNode.h @@ -41,6 +41,7 @@ #if DELAY_POOLS #include "squid.h" +#include "auth/UserRequest.h" #include "DelayPools.h" #include "DelayIdComposite.h" #include "CommRead.h" @@ -48,8 +49,6 @@ class StoreEntry; -class AuthUserRequest; - /// \ingroup DelayPoolsAPI class CompositePoolNode : public RefCountable, public Updateable { @@ -77,7 +76,7 @@ public: CompositeSelectionDetails() {} IpAddress src_addr; - AuthUserRequest *user; + AuthUserRequest::Pointer user; String tag; }; diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 0ce9786b0b..c003ccfa02 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -123,7 +123,7 @@ HttpRequest::clean() // points to a pipe that is owned and initiated by another object. body_pipe = NULL; - AUTHUSERREQUESTUNLOCK(auth_user_request, "request"); + auth_user_request = NULL; safe_free(canonical); @@ -589,10 +589,7 @@ bool HttpRequest::inheritProperties(const HttpMsg *aMsg) // may eventually need cloneNullAdaptationImmune() for that. flags = aReq->flags.cloneAdaptationImmune(); - if (aReq->auth_user_request) { - auth_user_request = aReq->auth_user_request; - AUTHUSERREQUESTLOCK(auth_user_request, "inheritProperties"); - } + auth_user_request = aReq->auth_user_request; if (aReq->pinned_connection) { pinned_connection = cbdataReference(aReq->pinned_connection); diff --git a/src/HttpRequest.h b/src/HttpRequest.h index 5c935e9440..4f8558dc55 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -1,4 +1,3 @@ - /* * $Id$ * @@ -151,7 +150,7 @@ private: public: IpAddress host_addr; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; u_short port; diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc index 9b2c500549..7cfd5306b3 100644 --- a/src/acl/FilledChecklist.cc +++ b/src/acl/FilledChecklist.cc @@ -30,23 +30,7 @@ ACLFilledChecklist::authenticated() /* get authed here */ /* Note: this fills in auth_user_request when applicable */ - /* - * DPW 2007-05-08 - * tryToAuthenticateAndSetAuthUser used to try to lock and - * unlock auth_user_request on our behalf, but it was too - * ugly and hard to follow. Now we do our own locking here. - * - * I'm not sure what tryToAuthenticateAndSetAuthUser does when - * auth_user_request is set before calling. I'm tempted to - * unlock and set it to NULL, but it seems safer to save the - * pointer before calling and unlock it afterwards. If the - * pointer doesn't change then its a no-op. - */ - AuthUserRequest *old_auth_user_request = auth_user_request; - auth_acl_t result = AuthUserRequest::tryToAuthenticateAndSetAuthUser (&auth_user_request, headertype, request, conn(), src_addr); - if (auth_user_request) - AUTHUSERREQUESTLOCK(auth_user_request, "ACLFilledChecklist"); - AUTHUSERREQUESTUNLOCK(old_auth_user_request, "old ACLFilledChecklist"); + auth_acl_t result = AuthUserRequest::tryToAuthenticateAndSetAuthUser(&auth_user_request, headertype, request, conn(), src_addr); switch (result) { case AUTH_ACL_CANNOT_AUTHENTICATE: @@ -83,18 +67,14 @@ ACLFilledChecklist::checkCallback(allow_t answer) /* During reconfigure, we can end up not finishing call * sequences into the auth code */ - if (auth_user_request) { + if (auth_user_request != NULL) { /* the filled_checklist lock */ - AUTHUSERREQUESTUNLOCK(auth_user_request, "ACLFilledChecklist"); + auth_user_request = NULL; /* it might have been connection based */ - assert(conn() != NULL); - /* - * DPW 2007-05-08 - * yuck, this make me uncomfortable. why do this here? - * ConnStateData will do its own unlocking. - */ - AUTHUSERREQUESTUNLOCK(conn()->auth_user_request, "conn via ACLFilledChecklist"); - conn()->auth_type = AUTH_BROKEN; + if(conn()) { + conn()->auth_user_request = NULL; + conn()->auth_type = AUTH_BROKEN; + } } ACLChecklist::checkCallback(answer); // may delete us @@ -156,10 +136,6 @@ ACLFilledChecklist::~ACLFilledChecklist() HTTPMSGUNLOCK(reply); - // no auth_user_request in builds without any Authentication configured - if (auth_user_request) - AUTHUSERREQUESTUNLOCK(auth_user_request, "ACLFilledChecklist destructor"); - cbdataReferenceDone(conn_); debugs(28, 4, HERE << "ACLFilledChecklist destroyed " << this); diff --git a/src/acl/FilledChecklist.h b/src/acl/FilledChecklist.h index dd9572499b..80ecd02ab0 100644 --- a/src/acl/FilledChecklist.h +++ b/src/acl/FilledChecklist.h @@ -2,8 +2,8 @@ #define SQUID_ACLFILLED_CHECKLIST_H #include "acl/Checklist.h" +#include "auth/UserRequest.h" -class AuthUserRequest; class ExternalACLEntry; class ConnStateData; @@ -53,7 +53,7 @@ public: HttpReply *reply; char rfc931[USER_IDENT_SZ]; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; #if SQUID_SNMP char *snmp_community; diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index 42f05709fa..9c4f237d67 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -1271,12 +1271,11 @@ void Adaptation::Icap::ModXact::makeRequestHeaders(MemBuf &buf) void Adaptation::Icap::ModXact::makeUsernameHeader(const HttpRequest *request, MemBuf &buf) { - if (const AuthUserRequest *auth = request->auth_user_request) { - if (char const *name = auth->username()) { - const char *value = TheConfig.client_username_encode ? - base64_encode(name) : name; - buf.Printf("%s: %s\r\n", TheConfig.client_username_header, - value); + if (request->auth_user_request != NULL) { + char const *name = (request->auth_user_request)->username(); + if (name) { + const char *value = TheConfig.client_username_encode ? base64_encode(name) : name; + buf.Printf("%s: %s\r\n", TheConfig.client_username_header, value); } } } diff --git a/src/auth/Acl.cc b/src/auth/Acl.cc index dddea7d0d4..6952fbc76d 100644 --- a/src/auth/Acl.cc +++ b/src/auth/Acl.cc @@ -32,25 +32,9 @@ AuthenticateAcl(ACLChecklist *ch) /* get authed here */ /* Note: this fills in auth_user_request when applicable */ - /* - * DPW 2007-05-08 - * tryToAuthenticateAndSetAuthUser used to try to lock and - * unlock auth_user_request on our behalf, but it was too - * ugly and hard to follow. Now we do our own locking here. - * - * I'm not sure what tryToAuthenticateAndSetAuthUser does when - * auth_user_request is set before calling. I'm tempted to - * unlock and set it to NULL, but it seems safer to save the - * pointer before calling and unlock it afterwards. If the - * pointer doesn't change then its a no-op. - */ - AuthUserRequest *old_auth_user_request = checklist->auth_user_request; const auth_acl_t result = AuthUserRequest::tryToAuthenticateAndSetAuthUser( &checklist->auth_user_request, headertype, request, checklist->conn(), checklist->src_addr); - if (checklist->auth_user_request) - AUTHUSERREQUESTLOCK(checklist->auth_user_request, "ACLAuth::authenticated"); - AUTHUSERREQUESTUNLOCK(old_auth_user_request, "old ACLAuth"); switch (result) { case AUTH_ACL_CANNOT_AUTHENTICATE: diff --git a/src/auth/AclMaxUserIp.cc b/src/auth/AclMaxUserIp.cc index 881ea82fce..571268726a 100644 --- a/src/auth/AclMaxUserIp.cc +++ b/src/auth/AclMaxUserIp.cc @@ -112,9 +112,7 @@ ACLMaxUserIP::parse() * 1 : Match */ int -ACLMaxUserIP::match(AuthUserRequest * auth_user_request, - - IpAddress const &src_addr) +ACLMaxUserIP::match(AuthUserRequest::Pointer auth_user_request, IpAddress const &src_addr) { /* * the logic for flush the ip list when the limit is hit vs keep @@ -159,7 +157,7 @@ ACLMaxUserIP::match(ACLChecklist *cl) ti = match(checklist->auth_user_request, checklist->src_addr); - AUTHUSERREQUESTUNLOCK(checklist->auth_user_request, "ACLChecklist via ACLMaxUserIP"); + checklist->auth_user_request = NULL; return ti; } diff --git a/src/auth/AclMaxUserIp.h b/src/auth/AclMaxUserIp.h index 9b9bcf0239..feb45428c9 100644 --- a/src/auth/AclMaxUserIp.h +++ b/src/auth/AclMaxUserIp.h @@ -37,8 +37,7 @@ #include "acl/Acl.h" #include "acl/Checklist.h" - -class AuthUserRequest; +#include "auth/UserRequest.h" /// \ingroup ACLAPI class ACLMaxUserIP : public ACL @@ -69,7 +68,7 @@ private: static Prototype RegistryProtoype; static ACLMaxUserIP RegistryEntry_; - int match(AuthUserRequest *, IpAddress const &); + int match(AuthUserRequest::Pointer, IpAddress const &); char const *class_; int maximum; diff --git a/src/auth/AclProxyAuth.cc b/src/auth/AclProxyAuth.cc index b676957f88..0eed9c8b75 100644 --- a/src/auth/AclProxyAuth.cc +++ b/src/auth/AclProxyAuth.cc @@ -141,7 +141,7 @@ ProxyAuthLookup::checkForAsync(ACLChecklist *cl)const checklist->asyncInProgress(true); debugs(28, 3, "ACLChecklist::checkForAsync: checking password via authenticator"); - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; /* make sure someone created auth_user_request for us */ assert(checklist->auth_user_request != NULL); auth_user_request = checklist->auth_user_request; @@ -165,10 +165,10 @@ ProxyAuthLookup::LookupDone(void *data, char *result) /* credentials could not be checked either way * restart the whole process */ /* OR the connection was closed, there's no way to continue */ - AUTHUSERREQUESTUNLOCK(checklist->auth_user_request, "ProxyAuthLookup"); + checklist->auth_user_request = NULL; if (checklist->conn() != NULL) { - AUTHUSERREQUESTUNLOCK(checklist->conn()->auth_user_request, "conn via ProxyAuthLookup"); // DPW discomfort + checklist->conn()->auth_user_request = NULL; checklist->conn()->auth_type = AUTH_BROKEN; } } @@ -201,7 +201,7 @@ int ACLProxyAuth::matchForCache(ACLChecklist *cl) { ACLFilledChecklist *checklist = Filled(cl); - assert (checklist->auth_user_request); + assert (checklist->auth_user_request != NULL); return data->match(checklist->auth_user_request->username()); } @@ -215,9 +215,8 @@ ACLProxyAuth::matchProxyAuth(ACLChecklist *cl) ACLFilledChecklist *checklist = Filled(cl); checkAuthForCaching(checklist); /* check to see if we have matched the user-acl before */ - int result = cacheMatchAcl(&checklist->auth_user_request->user()-> - proxy_match_cache, checklist); - AUTHUSERREQUESTUNLOCK(checklist->auth_user_request, "ACLChecklist via ACLProxyAuth"); + int result = cacheMatchAcl(&checklist->auth_user_request->user()->proxy_match_cache, checklist); + checklist->auth_user_request = NULL; return result; } diff --git a/src/auth/Config.cc b/src/auth/Config.cc index 0b4beb0862..c6557bba85 100644 --- a/src/auth/Config.cc +++ b/src/auth/Config.cc @@ -42,7 +42,7 @@ * Unauthenticated structure. The structure is given an inital lock here. * It may also be NULL reflecting that no user could be created. */ -AuthUserRequest * +AuthUserRequest::Pointer AuthConfig::CreateAuthUser(const char *proxy_auth) { assert(proxy_auth != NULL); @@ -55,14 +55,7 @@ AuthConfig::CreateAuthUser(const char *proxy_auth) return NULL; } - AuthUserRequest *result = config->decode (proxy_auth); - - /* - * DPW 2007-05-08 - * Do not lock the AuthUserRequest on the caller's behalf. - * Callers must manage their own locks. - */ - return result; + return config->decode(proxy_auth); } AuthConfig * diff --git a/src/auth/Config.h b/src/auth/Config.h index bdb553c57d..db83cfa8c1 100644 --- a/src/auth/Config.h +++ b/src/auth/Config.h @@ -32,7 +32,8 @@ #ifndef SQUID_AUTHCONFIG_H #define SQUID_AUTHCONFIG_H -class AuthUserRequest; +#include "auth/UserRequest.h" + class StoreEntry; class HttpReply; class HttpRequest; @@ -56,7 +57,7 @@ class AuthConfig { public: - static AuthUserRequest *CreateAuthUser (const char *proxy_auth); + static AuthUserRequest::Pointer CreateAuthUser(const char *proxy_auth); static AuthConfig *Find(const char *proxy_auth); AuthConfig() {} @@ -81,7 +82,7 @@ public: \param proxy_auth Login Pattern to parse. \retval * Details needed to authenticate. */ - virtual AuthUserRequest *decode(char const *proxy_auth) = 0; + virtual AuthUserRequest::Pointer decode(char const *proxy_auth) = 0; /** * squid is finished with this config, release any unneeded resources. @@ -109,7 +110,7 @@ public: virtual void dump(StoreEntry *, const char *, AuthConfig *) = 0; /** add headers as needed when challenging for auth */ - virtual void fixHeader(AuthUserRequest *, HttpReply *, http_hdr_type, HttpRequest *) = 0; + virtual void fixHeader(AuthUserRequest::Pointer, HttpReply *, http_hdr_type, HttpRequest *) = 0; /** prepare to handle requests */ virtual void init(AuthConfig *) = 0; /** expose any/all statistics to a CacheManager */ diff --git a/src/auth/Makefile.am b/src/auth/Makefile.am index 9912440df3..1abd128080 100644 --- a/src/auth/Makefile.am +++ b/src/auth/Makefile.am @@ -10,6 +10,7 @@ EXTRA_LTLIBRARIES = libbasic.la libdigest.la libntlm.la libnegotiate.la libauth_la_SOURCES = \ Config.cc \ Config.h \ + enums.h \ Scheme.cc \ Scheme.h \ User.h \ diff --git a/src/auth/User.cc b/src/auth/User.cc index 71094e1c06..5c242c72e4 100644 --- a/src/auth/User.cc +++ b/src/auth/User.cc @@ -68,9 +68,8 @@ AuthUser::AuthUser (AuthConfig *aConfig) : * related scheme data itself. */ void -AuthUser::absorb (AuthUser *from) +AuthUser::absorb(AuthUser *from) { - AuthUserRequest *auth_user_request; /* * XXX combine two authuser structs. Incomplete: it should merge * in hash references too and ask the module to merge in scheme @@ -80,12 +79,13 @@ AuthUser::absorb (AuthUser *from) dlink_node *link = from->requests.head; while (link) { - auth_user_request = static_cast(link->data); + AuthUserRequest::Pointer *auth_user_request = static_cast(link->data); dlink_node *tmplink = link; link = link->next; dlinkDelete(tmplink, &from->requests); dlinkAddTail(auth_user_request, tmplink, &requests); - auth_user_request->user(this); + AuthUserRequest::Pointer aur = *(auth_user_request); + aur->user(this); } references += from->references; @@ -95,7 +95,6 @@ AuthUser::absorb (AuthUser *from) AuthUser::~AuthUser() { - AuthUserRequest *auth_user_request; dlink_node *link, *tmplink; debugs(29, 5, "AuthUser::~AuthUser: Freeing auth_user '" << this << "' with refcount '" << references << "'."); assert(references == 0); @@ -116,7 +115,7 @@ AuthUser::~AuthUser() while (link) { debugs(29, 5, "AuthUser::~AuthUser: removing request entry '" << link->data << "'"); - auth_user_request = static_cast(link->data); + AuthUserRequest::Pointer *auth_user_request = static_cast(link->data); tmplink = link; link = link->next; dlinkDelete(tmplink, &requests); diff --git a/src/auth/User.cci b/src/auth/User.cci index 38ce96511f..511a8af746 100644 --- a/src/auth/User.cci +++ b/src/auth/User.cci @@ -58,12 +58,14 @@ AuthUser::username(char const*aString) } void -AuthUser::addRequest(AuthUserRequest *request) +AuthUser::addRequest(AuthUserRequest::Pointer request) { /* lock for the request link */ - + /* AYJ: 2009-12-12: WTF? locking the AuthUser because it has a reference to a request? */ + /* seems to me like this once-upon-a-time may have been intended to lock the AuthUserRequest */ lock(); + dlink_node *node = dlinkNodeNew(); - dlinkAdd(request, node, &requests); + dlinkAdd(&request, node, &requests); } diff --git a/src/auth/User.h b/src/auth/User.h index 47a99337e9..3bccee2b60 100644 --- a/src/auth/User.h +++ b/src/auth/User.h @@ -34,7 +34,8 @@ #ifndef SQUID_AUTHUSER_H #define SQUID_AUTHUSER_H -class AuthUserRequest; +#include "auth/UserRequest.h" + class AuthConfig; class AuthUserHashPointer; @@ -87,7 +88,7 @@ public: void clearIp(); void removeIp(IpAddress); void addIp(IpAddress); - _SQUID_INLINE_ void addRequest(AuthUserRequest *); + _SQUID_INLINE_ void addRequest(AuthUserRequest::Pointer); void lock(); void unlock(); diff --git a/src/auth/UserRequest.cc b/src/auth/UserRequest.cc index 285e19c706..417f381263 100644 --- a/src/auth/UserRequest.cc +++ b/src/auth/UserRequest.cc @@ -1,4 +1,3 @@ - /* * $Id$ * @@ -54,12 +53,6 @@ /* Generic Functions */ -size_t -AuthUserRequest::refCount () const -{ - return references; -} - char const * AuthUserRequest::username() const { @@ -69,12 +62,6 @@ AuthUserRequest::username() const return NULL; } -size_t -authenticateRequestRefCount (AuthUserRequest *aRequest) -{ - return aRequest->refCount(); -} - /**** PUBLIC FUNCTIONS (ALL GENERIC!) ****/ /* send the initial data to an authenticator module */ @@ -92,11 +79,11 @@ AuthUserRequest::start(RH * handler, void *data) */ int -authenticateValidateUser(AuthUserRequest * auth_user_request) +authenticateValidateUser(AuthUserRequest::Pointer auth_user_request) { debugs(29, 9, "authenticateValidateUser: Validating Auth_user request '" << auth_user_request << "'."); - if (auth_user_request == NULL) { + if (auth_user_request.getRaw() == NULL) { debugs(29, 4, "authenticateValidateUser: Auth_user_request was NULL!"); return 0; } @@ -141,18 +128,15 @@ AuthUserRequest::operator delete (void *address) } AuthUserRequest::AuthUserRequest():_auth_user(NULL), message(NULL), - references (0), lastReply (AUTH_ACL_CANNOT_AUTHENTICATE) + lastReply (AUTH_ACL_CANNOT_AUTHENTICATE) { - debugs(29, 5, "AuthUserRequest::AuthUserRequest: initialised request " << - this); + debugs(29, 5, "AuthUserRequest::AuthUserRequest: initialised request " << this); } AuthUserRequest::~AuthUserRequest() { dlink_node *link; - debugs(29, 5, "AuthUserRequest::~AuthUserRequest: freeing request " << - this); - assert(references == 0); + debugs(29, 5, "AuthUserRequest::~AuthUserRequest: freeing request " << this); if (user()) { /* unlink from the auth_user struct */ @@ -200,7 +184,7 @@ AuthUserRequest::denyMessage(char const * const default_message) } static void -authenticateAuthUserRequestSetIp(AuthUserRequest * auth_user_request, IpAddress &ipaddr) +authenticateAuthUserRequestSetIp(AuthUserRequest::Pointer auth_user_request, IpAddress &ipaddr) { AuthUser *auth_user = auth_user_request->user(); @@ -211,7 +195,7 @@ authenticateAuthUserRequestSetIp(AuthUserRequest * auth_user_request, IpAddress } void -authenticateAuthUserRequestRemoveIp(AuthUserRequest * auth_user_request, IpAddress const &ipaddr) +authenticateAuthUserRequestRemoveIp(AuthUserRequest::Pointer auth_user_request, IpAddress const &ipaddr) { AuthUser *auth_user = auth_user_request->user(); @@ -222,16 +206,16 @@ authenticateAuthUserRequestRemoveIp(AuthUserRequest * auth_user_request, IpAddre } void -authenticateAuthUserRequestClearIp(AuthUserRequest * auth_user_request) +authenticateAuthUserRequestClearIp(AuthUserRequest::Pointer auth_user_request) { - if (auth_user_request) + if (auth_user_request != NULL) auth_user_request->user()->clearIp(); } int -authenticateAuthUserRequestIPCount(AuthUserRequest * auth_user_request) +authenticateAuthUserRequestIPCount(AuthUserRequest::Pointer auth_user_request) { - assert(auth_user_request); + assert(auth_user_request != NULL); assert(auth_user_request->user()); return auth_user_request->user()->ipcount; } @@ -241,7 +225,7 @@ authenticateAuthUserRequestIPCount(AuthUserRequest * auth_user_request) * authenticateUserAuthenticated: is this auth_user structure logged in ? */ int -authenticateUserAuthenticated(AuthUserRequest * auth_user_request) +authenticateUserAuthenticated(AuthUserRequest::Pointer auth_user_request) { if (!authenticateValidateUser(auth_user_request)) return 0; @@ -286,19 +270,19 @@ AuthUserRequest::connLastHeader() * This is basically a handle approach. */ static void -authenticateAuthenticateUser(AuthUserRequest * auth_user_request, HttpRequest * request, ConnStateData * conn, http_hdr_type type) +authenticateAuthenticateUser(AuthUserRequest::Pointer auth_user_request, HttpRequest * request, ConnStateData * conn, http_hdr_type type) { - assert(auth_user_request != NULL); + assert(auth_user_request.getRaw() != NULL); auth_user_request->authenticate(request, conn, type); } -static AuthUserRequest * -authTryGetUser (AuthUserRequest **auth_user_request, ConnStateData * conn, HttpRequest * request) +static AuthUserRequest::Pointer +authTryGetUser(AuthUserRequest::Pointer auth_user_request, ConnStateData * conn, HttpRequest * request) { - if (*auth_user_request) - return *auth_user_request; - else if (request != NULL && request->auth_user_request) + if (auth_user_request != NULL) + return auth_user_request; + else if (request != NULL && request->auth_user_request != NULL) return request->auth_user_request; else if (conn != NULL) return conn->auth_user_request; @@ -328,7 +312,7 @@ authTryGetUser (AuthUserRequest **auth_user_request, ConnStateData * conn, HttpR * Caller is responsible for locking and unlocking their *auth_user_request! */ auth_acl_t -AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_type headertype, HttpRequest * request, ConnStateData * conn, IpAddress &src_addr) +AuthUserRequest::authenticate(AuthUserRequest::Pointer * auth_user_request, http_hdr_type headertype, HttpRequest * request, ConnStateData * conn, IpAddress &src_addr) { const char *proxy_auth; assert(headertype != 0); @@ -342,7 +326,7 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ * connection when we recieve no authentication header. */ - if (((proxy_auth == NULL) && (!authenticateUserAuthenticated(authTryGetUser(auth_user_request,conn,request)))) + if (((proxy_auth == NULL) && (!authenticateUserAuthenticated(authTryGetUser(*auth_user_request,conn,request)))) || (conn != NULL && conn->auth_type == AUTH_BROKEN)) { /* no header or authentication failed/got corrupted - restart */ debugs(29, 4, "authenticateAuthenticate: broken auth or no proxy_auth header. Requesting auth header."); @@ -350,7 +334,7 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ if (conn != NULL) { conn->auth_type = AUTH_UNKNOWN; - AUTHUSERREQUESTUNLOCK(conn->auth_user_request, "conn"); + conn->auth_user_request = NULL; } *auth_user_request = NULL; @@ -362,7 +346,7 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ * No check for function required in the if: its compulsory for conn based * auth modules */ - if (proxy_auth && conn != NULL && conn->auth_user_request && + if (proxy_auth && conn != NULL && conn->auth_user_request != NULL && authenticateUserAuthenticated(conn->auth_user_request) && conn->auth_user_request->connLastHeader() != NULL && strcmp(proxy_auth, conn->auth_user_request->connLastHeader())) { @@ -379,7 +363,7 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ * authenticateAuthenticate */ assert(*auth_user_request == NULL); - AUTHUSERREQUESTUNLOCK(conn->auth_user_request, "conn"); + conn->auth_user_request = NULL; /* Set the connection auth type */ conn->auth_type = AUTH_UNKNOWN; } @@ -391,7 +375,7 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ if (*auth_user_request == NULL) { debugs(29, 9, "authenticateAuthenticate: This is a new checklist test on FD:" << (conn != NULL ? conn->fd : -1) ); - if (proxy_auth && !request->auth_user_request && conn != NULL && conn->auth_user_request) { + if (proxy_auth && request->auth_user_request == NULL && conn != NULL && conn->auth_user_request != NULL) { AuthConfig * scheme = AuthConfig::Find(proxy_auth); if (!conn->auth_user_request->user() || conn->auth_user_request->user()->config != scheme) { @@ -400,7 +384,7 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ "' to '" << proxy_auth << "' (client " << src_addr << ")"); - AUTHUSERREQUESTUNLOCK(conn->auth_user_request, "conn"); + conn->auth_user_request = NULL; conn->auth_type = AUTH_UNKNOWN; } } @@ -420,7 +404,6 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ if ((*auth_user_request)->username()) { request->auth_user_request = *auth_user_request; - AUTHUSERREQUESTLOCK(request->auth_user_request, "request"); } *auth_user_request = NULL; @@ -428,11 +411,11 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ } /* the user_request comes prelocked for the caller to createAuthUser (us) */ - } else if (request->auth_user_request) { + } else if (request->auth_user_request != NULL) { *auth_user_request = request->auth_user_request; } else { assert (conn != NULL); - if (conn->auth_user_request) { + if (conn->auth_user_request != NULL) { *auth_user_request = conn->auth_user_request; } else { /* failed connection based authentication */ @@ -449,16 +432,14 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ if (!authenticateUserAuthenticated(*auth_user_request)) { /* User not logged in. Log them in */ - authenticateAuthenticateUser(*auth_user_request, request, - conn, headertype); + authenticateAuthenticateUser(*auth_user_request, request, conn, headertype); switch (authenticateDirection(*auth_user_request)) { case 1: - if (NULL == request->auth_user_request) { + if (request->auth_user_request == NULL) { request->auth_user_request = *auth_user_request; - AUTHUSERREQUESTLOCK(request->auth_user_request, "request"); } /* fallthrough to -2 */ @@ -481,7 +462,6 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ if ((*auth_user_request)->username()) { if (!request->auth_user_request) { request->auth_user_request = *auth_user_request; - AUTHUSERREQUESTLOCK(request->auth_user_request, "request"); } } @@ -492,9 +472,8 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ /* copy username to request for logging on client-side */ /* the credentials are correct at this point */ - if (NULL == request->auth_user_request) { + if (request->auth_user_request == NULL) { request->auth_user_request = *auth_user_request; - AUTHUSERREQUESTLOCK(request->auth_user_request, "request"); authenticateAuthUserRequestSetIp(*auth_user_request, src_addr); } @@ -502,20 +481,17 @@ AuthUserRequest::authenticate(AuthUserRequest ** auth_user_request, http_hdr_typ } auth_acl_t - -AuthUserRequest::tryToAuthenticateAndSetAuthUser(AuthUserRequest ** auth_user_request, http_hdr_type headertype, HttpRequest * request, ConnStateData * conn, IpAddress &src_addr) +AuthUserRequest::tryToAuthenticateAndSetAuthUser(AuthUserRequest::Pointer * auth_user_request, http_hdr_type headertype, HttpRequest * request, ConnStateData * conn, IpAddress &src_addr) { /* If we have already been called, return the cached value */ - AuthUserRequest *t = authTryGetUser (auth_user_request, conn, request); + AuthUserRequest::Pointer t = authTryGetUser(*auth_user_request, conn, request); - if (t && t->lastReply != AUTH_ACL_CANNOT_AUTHENTICATE - && t->lastReply != AUTH_ACL_HELPER) { - if (!*auth_user_request) + if (t != NULL && t->lastReply != AUTH_ACL_CANNOT_AUTHENTICATE && t->lastReply != AUTH_ACL_HELPER) { + if (*auth_user_request == NULL) *auth_user_request = t; - if (!request->auth_user_request && t->lastReply == AUTH_AUTHENTICATED) { + if (request->auth_user_request == NULL && t->lastReply == AUTH_AUTHENTICATED) { request->auth_user_request = t; - AUTHUSERREQUESTLOCK(request->auth_user_request, "request"); } return t->lastReply; } @@ -523,10 +499,9 @@ AuthUserRequest::tryToAuthenticateAndSetAuthUser(AuthUserRequest ** auth_user_re /* ok, call the actual authenticator routine. */ auth_acl_t result = authenticate(auth_user_request, headertype, request, conn, src_addr); - t = authTryGetUser (auth_user_request, conn, request); + t = authTryGetUser(*auth_user_request, conn, request); - if (t && result != AUTH_ACL_CANNOT_AUTHENTICATE && - result != AUTH_ACL_HELPER) + if (t != NULL && result != AUTH_ACL_CANNOT_AUTHENTICATE && result != AUTH_ACL_HELPER) t->lastReply = result; return result; @@ -539,16 +514,16 @@ AuthUserRequest::tryToAuthenticateAndSetAuthUser(AuthUserRequest ** auth_user_re * -2: authenticate broken in some fashion */ int -authenticateDirection(AuthUserRequest * auth_user_request) +authenticateDirection(AuthUserRequest::Pointer auth_user_request) { - if (!auth_user_request) + if (auth_user_request == NULL) return -2; return auth_user_request->direction(); } void -AuthUserRequest::addReplyAuthHeader(HttpReply * rep, AuthUserRequest * auth_user_request, HttpRequest * request, int accelerated, int internal) +AuthUserRequest::addReplyAuthHeader(HttpReply * rep, AuthUserRequest::Pointer auth_user_request, HttpRequest * request, int accelerated, int internal) /* send the auth types we are configured to support (and have compiled in!) */ { http_hdr_type type; @@ -609,7 +584,7 @@ AuthUserRequest::addReplyAuthHeader(HttpReply * rep, AuthUserRequest * auth_user } void -authenticateFixHeader(HttpReply * rep, AuthUserRequest * auth_user_request, HttpRequest * request, int accelerated, int internal) +authenticateFixHeader(HttpReply * rep, AuthUserRequest::Pointer auth_user_request, HttpRequest * request, int accelerated, int internal) { AuthUserRequest::addReplyAuthHeader(rep, auth_user_request, request, accelerated, internal); } @@ -617,40 +592,12 @@ authenticateFixHeader(HttpReply * rep, AuthUserRequest * auth_user_request, Http /* call the active auth module and allow it to add a trailer to the request */ void -authenticateAddTrailer(HttpReply * rep, AuthUserRequest * auth_user_request, HttpRequest * request, int accelerated) +authenticateAddTrailer(HttpReply * rep, AuthUserRequest::Pointer auth_user_request, HttpRequest * request, int accelerated) { if (auth_user_request != NULL) auth_user_request->addTrailer(rep, accelerated); } -void - -AuthUserRequest::_lock() -{ - assert(this); - debugs(29, 9, "AuthUserRequest::lock: auth_user request '" << this << " " << references << "->" << references+1); - ++references; -} - -void -AuthUserRequest::_unlock() -{ - assert(this != NULL); - - if (references > 0) { - debugs(29, 9, "AuthUserRequest::unlock: auth_user request '" << this << " " << references << "->" << references-1); - --references; - } else { - debugs(29, 1, "Attempt to lower Auth User request " << this << " refcount below 0!"); - } - - if (references == 0) { - debugs(29, 9, "AuthUserRequest::unlock: deleting auth_user_request '" << this << "'."); - /* not locked anymore */ - delete this; - } -} - AuthScheme * AuthUserRequest::scheme() const { diff --git a/src/auth/UserRequest.h b/src/auth/UserRequest.h index 611917629f..75cd7bfa1a 100644 --- a/src/auth/UserRequest.h +++ b/src/auth/UserRequest.h @@ -1,4 +1,3 @@ - /* * $Id$ * @@ -36,13 +35,18 @@ #ifndef SQUID_AUTHUSERREQUEST_H #define SQUID_AUTHUSERREQUEST_H -#include "client_side.h" +#include "auth/enums.h" +#include "dlink.h" +#include "ip/IpAddress.h" +#include "RefCount.h" +#include "typedefs.h" +#include "HttpHeader.h" +class AuthScheme; class AuthUser; - class ConnStateData; - -class AuthScheme; +class HttpReply; +class HttpRequest; struct AuthUserIP { dlink_node node; @@ -56,8 +60,10 @@ struct AuthUserIP { \ingroup AuthAPI * This is a short lived structure is the visible aspect of the authentication framework. */ -class AuthUserRequest +class AuthUserRequest : public RefCountable { +public: + typedef RefCount Pointer; public: /** @@ -108,8 +114,8 @@ public: virtual void user(AuthUser *aUser) {_auth_user=aUser;} - static auth_acl_t tryToAuthenticateAndSetAuthUser(AuthUserRequest **, http_hdr_type, HttpRequest *, ConnStateData *, IpAddress &); - static void addReplyAuthHeader(HttpReply * rep, AuthUserRequest * auth_user_request, HttpRequest * request, int accelerated, int internal); + static auth_acl_t tryToAuthenticateAndSetAuthUser(AuthUserRequest::Pointer *, http_hdr_type, HttpRequest *, ConnStateData *, IpAddress &); + static void addReplyAuthHeader(HttpReply * rep, AuthUserRequest::Pointer auth_user_request, HttpRequest * request, int accelerated, int internal); AuthUserRequest(); @@ -126,10 +132,6 @@ public: /** Possibly overrideable in future */ char const * getDenyMessage(); - size_t refCount() const; - void _lock(); /**< \note please use AUTHUSERREQUESTLOCK() */ - void _unlock(); /**< \note please use AUTHUSERREQUESTUNLOCK() */ - /** * Squid does not make assumptions about where the username is stored. * This function must return a pointer to a NULL terminated string to be used in logging the request. @@ -146,14 +148,11 @@ public: private: - static auth_acl_t authenticate(AuthUserRequest ** auth_user_request, http_hdr_type headertype, HttpRequest * request, ConnStateData * conn, IpAddress &src_addr); + static auth_acl_t authenticate(AuthUserRequest::Pointer * auth_user_request, http_hdr_type headertype, HttpRequest * request, ConnStateData * conn, IpAddress &src_addr); /** return a message on the 407 error pages */ char *message; - /** how many 'processes' are working on this data */ - size_t references; - /** * We only attempt authentication once per http request. This * is to allow multiple auth acl references from different _access areas @@ -164,40 +163,26 @@ private: /* AuthUserRequest */ -/** - \ingroup AuthAPI - \deprecated Use AuthUserRequest::refCount() instead. - */ -extern size_t authenticateRequestRefCount (AuthUserRequest *); - /// \ingroup AuthAPI -extern void authenticateFixHeader(HttpReply *, AuthUserRequest *, HttpRequest *, int, int); +extern void authenticateFixHeader(HttpReply *, AuthUserRequest::Pointer, HttpRequest *, int, int); /// \ingroup AuthAPI -extern void authenticateAddTrailer(HttpReply *, AuthUserRequest *, HttpRequest *, int); +extern void authenticateAddTrailer(HttpReply *, AuthUserRequest::Pointer, HttpRequest *, int); /// \ingroup AuthAPI -extern void authenticateAuthUserRequestRemoveIp(AuthUserRequest *, IpAddress const &); +extern void authenticateAuthUserRequestRemoveIp(AuthUserRequest::Pointer, IpAddress const &); /// \ingroup AuthAPI -extern void authenticateAuthUserRequestClearIp(AuthUserRequest *); +extern void authenticateAuthUserRequestClearIp(AuthUserRequest::Pointer); /// \ingroup AuthAPI -extern int authenticateAuthUserRequestIPCount(AuthUserRequest *); +extern int authenticateAuthUserRequestIPCount(AuthUserRequest::Pointer); /// \ingroup AuthAPI /// \deprecated Use AuthUserRequest::direction() instead. -extern int authenticateDirection(AuthUserRequest *); +extern int authenticateDirection(AuthUserRequest::Pointer); /// \ingroup AuthAPI /// See AuthUserRequest::authenticated() -extern int authenticateUserAuthenticated(AuthUserRequest *); +extern int authenticateUserAuthenticated(AuthUserRequest::Pointer); /// \ingroup AuthAPI -extern int authenticateValidateUser(AuthUserRequest *); - -/// \todo Drop dead code? or make a debugging option. -#if 0 -#define AUTHUSERREQUESTUNLOCK(a,b) if(a){(a)->_unlock();debugs(0,0,HERE << "auth_user_request " << a << " was unlocked for " << b); (a)=NULL;} -#define AUTHUSERREQUESTLOCK(a,b) { (a)->_lock(); debugs(0,0,HERE << "auth_user_request " << a << " was locked for " << b); } -#endif -#define AUTHUSERREQUESTUNLOCK(a,b) if(a){(a)->_unlock();(a)=NULL;} -#define AUTHUSERREQUESTLOCK(a,b) (a)->_lock() +extern int authenticateValidateUser(AuthUserRequest::Pointer); #endif /* SQUID_AUTHUSERREQUEST_H */ diff --git a/src/auth/basic/auth_basic.cc b/src/auth/basic/auth_basic.cc index bc67847427..3e30f6345d 100644 --- a/src/auth/basic/auth_basic.cc +++ b/src/auth/basic/auth_basic.cc @@ -211,7 +211,7 @@ AuthBasicUserRequest::module_direction() } void -AuthBasicConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, http_hdr_type hdrType, HttpRequest * request) +AuthBasicConfig::fixHeader(AuthUserRequest::Pointer auth_user_request, HttpReply *rep, http_hdr_type hdrType, HttpRequest * request) { if (authenticate) { debugs(29, 9, HERE << "Sending type:" << hdrType << " header: 'Basic realm=\"" << basicAuthRealm << "\"'"); @@ -462,11 +462,11 @@ BasicUser::extractPassword() } void -BasicUser::decode(char const *proxy_auth, AuthUserRequest *auth_user_request) +BasicUser::decode(char const *proxy_auth, AuthUserRequest::Pointer auth_user_request) { currentRequest = auth_user_request; httpAuthHeader = proxy_auth; - if (decodeCleartext ()) { + if (decodeCleartext()) { extractUsername(); extractPassword(); } @@ -485,7 +485,7 @@ BasicUser::valid() const } void -BasicUser::makeLoggingInstance(AuthBasicUserRequest *auth_user_request) +BasicUser::makeLoggingInstance(AuthUserRequest::Pointer auth_user_request) { if (username()) { /* log the username */ @@ -552,10 +552,10 @@ BasicUser::updateCached(BasicUser *from) * "cannot decode credentials". Use the message field to return a * descriptive message to the user. */ -AuthUserRequest * +AuthUserRequest::Pointer AuthBasicConfig::decode(char const *proxy_auth) { - AuthBasicUserRequest *auth_user_request = new AuthBasicUserRequest(); + AuthUserRequest::Pointer auth_user_request = new AuthBasicUserRequest(); /* decode the username */ /* trim BASIC from string */ @@ -633,7 +633,7 @@ AuthBasicConfig::registerWithCacheManager(void) } void -BasicUser::queueRequest(AuthUserRequest * auth_user_request, RH * handler, void *data) +BasicUser::queueRequest(AuthUserRequest::Pointer auth_user_request, RH * handler, void *data) { BasicAuthQueueNode *node; node = static_cast(xmalloc(sizeof(BasicAuthQueueNode))); @@ -672,7 +672,7 @@ AuthBasicUserRequest::module_start(RH * handler, void *data) } void -BasicUser::submitRequest(AuthUserRequest * auth_user_request, RH * handler, void *data) +BasicUser::submitRequest(AuthUserRequest::Pointer auth_user_request, RH * handler, void *data) { /* mark the user as haveing verification in progress */ flags.credentials_ok = 2; diff --git a/src/auth/basic/auth_basic.h b/src/auth/basic/auth_basic.h index 26001b57a6..4f4df1f9bd 100644 --- a/src/auth/basic/auth_basic.h +++ b/src/auth/basic/auth_basic.h @@ -20,7 +20,7 @@ class AuthenticateStateData public: void *data; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; RH *handler; }; @@ -31,13 +31,11 @@ class BasicAuthQueueNode public: BasicAuthQueueNode *next; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; RH *handler; void *data; }; -class AuthBasicUserRequest; - class BasicUser : public AuthUser { @@ -48,13 +46,13 @@ public: BasicUser(AuthConfig *); ~BasicUser(); bool authenticated() const; - void queueRequest(AuthUserRequest * auth_user_request, RH * handler, void *data); - void submitRequest (AuthUserRequest * auth_user_request, RH * handler, void *data); - void decode(char const *credentials, AuthUserRequest *); + void queueRequest(AuthUserRequest::Pointer auth_user_request, RH * handler, void *data); + void submitRequest(AuthUserRequest::Pointer auth_user_request, RH * handler, void *data); + void decode(char const *credentials, AuthUserRequest::Pointer); char *getCleartext() {return cleartext;} bool valid() const; - void makeLoggingInstance(AuthBasicUserRequest *auth_user_request); + void makeLoggingInstance(AuthUserRequest::Pointer auth_user_request); AuthUser * makeCachedFrom(); void updateCached(BasicUser *from); char *passwd; @@ -72,7 +70,7 @@ private: void extractUsername(); void extractPassword(); char *cleartext; - AuthUserRequest *currentRequest; + AuthUserRequest::Pointer currentRequest; char const *httpAuthHeader; }; @@ -117,10 +115,10 @@ public: ~AuthBasicConfig(); virtual bool active() const; virtual bool configured() const; - virtual AuthUserRequest *decode(char const *proxy_auth); + virtual AuthUserRequest::Pointer decode(char const *proxy_auth); virtual void done(); virtual void dump(StoreEntry *, const char *, AuthConfig *); - virtual void fixHeader(AuthUserRequest *, HttpReply *, http_hdr_type, HttpRequest *); + virtual void fixHeader(AuthUserRequest::Pointer, HttpReply *, http_hdr_type, HttpRequest *); virtual void init(AuthConfig *); virtual void parse(AuthConfig *, int, char *); virtual void registerWithCacheManager(void); diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc index bf0add9313..daf9a008fa 100644 --- a/src/auth/digest/auth_digest.cc +++ b/src/auth/digest/auth_digest.cc @@ -773,16 +773,16 @@ AuthDigestUserRequest::addTrailer(HttpReply * rep, int accel) /* add the [www-|Proxy-]authenticate header on a 407 or 401 reply */ void -AuthDigestConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, http_hdr_type hdrType, HttpRequest * request) +AuthDigestConfig::fixHeader(AuthUserRequest::Pointer auth_user_request, HttpReply *rep, http_hdr_type hdrType, HttpRequest * request) { if (!authenticate) return; int stale = 0; - if (auth_user_request) { + if (auth_user_request != NULL) { AuthDigestUserRequest *digest_request; - digest_request = dynamic_cast < AuthDigestUserRequest * >(auth_user_request); + digest_request = dynamic_cast(auth_user_request.getRaw()); assert (digest_request != NULL); stale = !digest_request->flags.invalid_password; @@ -820,9 +820,6 @@ static void authenticateDigestHandleReply(void *data, char *reply) { DigestAuthenticateStateData *replyData = static_cast < DigestAuthenticateStateData * >(data); - AuthUserRequest *auth_user_request; - AuthDigestUserRequest *digest_request; - digest_user_h *digest_user; char *t = NULL; void *cbdata; debugs(29, 9, "authenticateDigestHandleReply: {" << (reply ? reply : "") << "}"); @@ -836,11 +833,13 @@ authenticateDigestHandleReply(void *data, char *reply) } assert(replyData->auth_user_request != NULL); - auth_user_request = replyData->auth_user_request; - digest_request = dynamic_cast < AuthDigestUserRequest * >(auth_user_request); + AuthUserRequest::Pointer auth_user_request = replyData->auth_user_request; + + /* AYJ: 2009-12-12: allow this because the digest_request pointer is purely local */ + AuthDigestUserRequest *digest_request = dynamic_cast < AuthDigestUserRequest * >(auth_user_request.getRaw()); assert(digest_request); - digest_user = dynamic_cast < digest_user_h * >(auth_user_request->user()); + digest_user_h *digest_user = dynamic_cast < digest_user_h * >(auth_user_request->user()); assert(digest_user != NULL); if (reply && (strncasecmp(reply, "ERR", 3) == 0)) { @@ -857,8 +856,7 @@ authenticateDigestHandleReply(void *data, char *reply) if (cbdataReferenceValidDone(replyData->data, &cbdata)) replyData->handler(cbdata, NULL); - //we know replyData->auth_user_request != NULL, or we'd have asserted - AUTHUSERREQUESTUNLOCK(replyData->auth_user_request, "replyData"); + replyData->auth_user_request = NULL; cbdataFree(replyData); } @@ -1045,8 +1043,8 @@ authDigestUserLinkNonce(DigestUser * user, digest_nonce_h * nonce) } /* setup the necessary info to log the username */ -static AuthUserRequest * -authDigestLogUsername(char *username, AuthDigestUserRequest *auth_user_request) +static AuthUserRequest::Pointer +authDigestLogUsername(char *username, AuthUserRequest::Pointer auth_user_request) { assert(auth_user_request != NULL); @@ -1058,9 +1056,9 @@ authDigestLogUsername(char *username, AuthDigestUserRequest *auth_user_request) /* set the auth_user type */ digest_user->auth_type = AUTH_BROKEN; /* link the request to the user */ - auth_user_request->authUser(digest_user); auth_user_request->user(digest_user); - digest_user->addRequest (auth_user_request); + digest_user->lock(); + digest_user->addRequest(auth_user_request); return auth_user_request; } @@ -1068,7 +1066,7 @@ authDigestLogUsername(char *username, AuthDigestUserRequest *auth_user_request) * Decode a Digest [Proxy-]Auth string, placing the results in the passed * Auth_user structure. */ -AuthUserRequest * +AuthUserRequest::Pointer AuthDigestConfig::decode(char const *proxy_auth) { const char *item; @@ -1360,7 +1358,7 @@ AuthDigestUserRequest::module_start(RH * handler, void *data) char buf[8192]; digest_user_h *digest_user; assert(user()->auth_type == AUTH_DIGEST); - digest_user = dynamic_cast < digest_user_h * >(user()); + digest_user = dynamic_cast(user()); assert(digest_user != NULL); debugs(29, 9, "authenticateStart: '\"" << digest_user->username() << "\":\"" << realm << "\"'"); @@ -1372,8 +1370,7 @@ AuthDigestUserRequest::module_start(RH * handler, void *data) r = cbdataAlloc(DigestAuthenticateStateData); r->handler = handler; r->data = cbdataReference(data); - r->auth_user_request = this; - AUTHUSERREQUESTLOCK(r->auth_user_request, "r"); + r->auth_user_request = static_cast(this); if (digestConfig.utf8) { char userstr[1024]; latin1_to_utf8(userstr, sizeof(userstr), digest_user->username()); diff --git a/src/auth/digest/auth_digest.h b/src/auth/digest/auth_digest.h index 02c69773c0..3bf9db8707 100644 --- a/src/auth/digest/auth_digest.h +++ b/src/auth/digest/auth_digest.h @@ -19,7 +19,7 @@ class DigestAuthenticateStateData public: void *data; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; RH *handler; }; @@ -143,10 +143,10 @@ public: AuthDigestConfig(); virtual bool active() const; virtual bool configured() const; - virtual AuthUserRequest *decode(char const *proxy_auth); + virtual AuthUserRequest::Pointer decode(char const *proxy_auth); virtual void done(); virtual void dump(StoreEntry *, const char *, AuthConfig *); - virtual void fixHeader(AuthUserRequest *, HttpReply *, http_hdr_type, HttpRequest *); + virtual void fixHeader(AuthUserRequest::Pointer, HttpReply *, http_hdr_type, HttpRequest *); virtual void init(AuthConfig *); virtual void parse(AuthConfig *, int, char *); virtual void registerWithCacheManager(void); diff --git a/src/auth/negotiate/auth_negotiate.cc b/src/auth/negotiate/auth_negotiate.cc index 948422083c..14594acae6 100644 --- a/src/auth/negotiate/auth_negotiate.cc +++ b/src/auth/negotiate/auth_negotiate.cc @@ -66,7 +66,7 @@ static void authenticateStateFree(authenticateStateData * r) { - AUTHUSERREQUESTUNLOCK(r->auth_user_request, "r"); + r->auth_user_request = NULL; cbdataFree(r); } @@ -301,7 +301,7 @@ AuthNegotiateUserRequest::addHeader(HttpReply * rep, int accel) } void -AuthNegotiateConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, http_hdr_type reqType, HttpRequest * request) +AuthNegotiateConfig::fixHeader(AuthUserRequest::Pointer auth_user_request, HttpReply *rep, http_hdr_type reqType, HttpRequest * request) { AuthNegotiateUserRequest *negotiate_request; @@ -323,8 +323,7 @@ AuthNegotiateConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *re request->flags.proxy_keepalive = 0; } } else { - negotiate_request = dynamic_cast(auth_user_request); - + negotiate_request = dynamic_cast(auth_user_request.getRaw()); assert(negotiate_request != NULL); switch (negotiate_request->auth_state) { @@ -387,7 +386,6 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) int valid; char *blob, *arg = NULL; - AuthUserRequest *auth_user_request; AuthUser *auth_user; NegotiateUser *negotiate_user; AuthNegotiateUserRequest *negotiate_request; @@ -407,20 +405,21 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) reply = (char *)"BH Internal error"; } - auth_user_request = r->auth_user_request; + AuthUserRequest::Pointer auth_user_request = r->auth_user_request; assert(auth_user_request != NULL); - negotiate_request = dynamic_cast(auth_user_request); + negotiate_request = dynamic_cast(auth_user_request.getRaw()); assert(negotiate_request != NULL); + assert(negotiate_request->waiting); negotiate_request->waiting = 0; safe_free(negotiate_request->client_blob); - auth_user = negotiate_request->user(); + auth_user = auth_user_request->user(); assert(auth_user != NULL); assert(auth_user->auth_type == AUTH_NEGOTIATE); - negotiate_user = dynamic_cast(auth_user_request->user()); + negotiate_user = dynamic_cast(auth_user_request->user()); assert(negotiate_user != NULL); if (negotiate_request->authserver == NULL) @@ -533,10 +532,7 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) fatalf("authenticateNegotiateHandleReply: *** Unsupported helper response ***, '%s'\n", reply); } - if (negotiate_request->request) { - HTTPMSGUNLOCK(negotiate_request->request); - negotiate_request->request = NULL; - } + negotiate_request->request = NULL; r->handler(r->data, NULL); cbdataReferenceDone(r->data); authenticateStateFree(r); @@ -577,7 +573,6 @@ AuthNegotiateUserRequest::module_start(RH * handler, void *data) r->handler = handler; r->data = cbdataReference(data); r->auth_user_request = this; - AUTHUSERREQUESTLOCK(r->auth_user_request, "r"); if (auth_state == AUTHENTICATE_STATE_INITIAL) { snprintf(buf, MAX_AUTHTOKEN_LEN, "YR %s\n", client_blob); //CHECKME: can ever client_blob be 0 here? @@ -624,19 +619,20 @@ AuthNegotiateUserRequest::onConnectionClose(ConnStateData *conn) /* unlock the connection based lock */ debugs(29, 9, "AuthNegotiateUserRequest::onConnectionClose: Unlocking auth_user from the connection '" << conn << "'."); - AUTHUSERREQUESTUNLOCK(conn->auth_user_request, "conn"); + conn->auth_user_request = NULL; } /* * Decode a Negotiate [Proxy-]Auth string, placing the results in the passed * Auth_user structure. */ -AuthUserRequest * +AuthUserRequest::Pointer AuthNegotiateConfig::decode(char const *proxy_auth) { NegotiateUser *newUser = new NegotiateUser(&negotiateConfig); - AuthNegotiateUserRequest *auth_user_request = new AuthNegotiateUserRequest (); + AuthUserRequest *auth_user_request = new AuthNegotiateUserRequest(); assert(auth_user_request->user() == NULL); + auth_user_request->user(newUser); auth_user_request->user()->auth_type = AUTH_NEGOTIATE; auth_user_request->user()->addRequest(auth_user_request); @@ -721,7 +717,6 @@ AuthNegotiateUserRequest::authenticate(HttpRequest * aRequest, ConnStateData * c conn->auth_type = AUTH_NEGOTIATE; assert(conn->auth_user_request == NULL); conn->auth_user_request = this; - AUTHUSERREQUESTLOCK(conn->auth_user_request, "conn"); request = aRequest; HTTPMSGLOCK(request); return; diff --git a/src/auth/negotiate/auth_negotiate.h b/src/auth/negotiate/auth_negotiate.h index e28f518e52..7b3707c3b1 100644 --- a/src/auth/negotiate/auth_negotiate.h +++ b/src/auth/negotiate/auth_negotiate.h @@ -36,7 +36,7 @@ typedef enum { /// \ingroup AuthNegotiateAPI typedef struct { void *data; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; RH *handler; } authenticateStateData; #endif @@ -120,10 +120,10 @@ public: AuthNegotiateConfig(); virtual bool active() const; virtual bool configured() const; - virtual AuthUserRequest *decode(char const *proxy_auth); + virtual AuthUserRequest::Pointer decode(char const *proxy_auth); virtual void done(); virtual void dump(StoreEntry *, const char *, AuthConfig *); - virtual void fixHeader(AuthUserRequest *, HttpReply *, http_hdr_type, HttpRequest *); + virtual void fixHeader(AuthUserRequest::Pointer, HttpReply *, http_hdr_type, HttpRequest *); virtual void init(AuthConfig *); virtual void parse(AuthConfig *, int, char *); virtual void registerWithCacheManager(void); diff --git a/src/auth/ntlm/auth_ntlm.cc b/src/auth/ntlm/auth_ntlm.cc index a2b119bf3e..3eaade7d6a 100644 --- a/src/auth/ntlm/auth_ntlm.cc +++ b/src/auth/ntlm/auth_ntlm.cc @@ -54,7 +54,7 @@ static void authenticateStateFree(authenticateStateData * r) { - AUTHUSERREQUESTUNLOCK(r->auth_user_request, "r"); + r->auth_user_request = NULL; cbdataFree(r); } @@ -259,10 +259,8 @@ AuthNTLMUserRequest::module_direction() } void -AuthNTLMConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, http_hdr_type hdrType, HttpRequest * request) +AuthNTLMConfig::fixHeader(AuthUserRequest::Pointer auth_user_request, HttpReply *rep, http_hdr_type hdrType, HttpRequest * request) { - AuthNTLMUserRequest *ntlm_request; - if (!authenticate) return; @@ -280,8 +278,7 @@ AuthNTLMConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, ht request->flags.proxy_keepalive = 0; } } else { - ntlm_request = dynamic_cast(auth_user_request); - + AuthNTLMUserRequest *ntlm_request = dynamic_cast(auth_user_request.getRaw()); assert(ntlm_request != NULL); switch (ntlm_request->auth_state) { @@ -312,7 +309,6 @@ AuthNTLMConfig::fixHeader(AuthUserRequest *auth_user_request, HttpReply *rep, ht safe_free(ntlm_request->server_blob); break; - default: debugs(29, 0, "AuthNTLMConfig::fixHeader: state " << ntlm_request->auth_state << "."); fatal("unexpected state in AuthenticateNTLMFixErrorHeader.\n"); @@ -333,9 +329,9 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) int valid; char *blob; - AuthUserRequest *auth_user_request; AuthUser *auth_user; NTLMUser *ntlm_user; + AuthUserRequest::Pointer auth_user_request; AuthNTLMUserRequest *ntlm_request; debugs(29, 8, "authenticateNTLMHandleReply: helper: '" << lastserver << "' sent us '" << (reply ? reply : "") << "'"); @@ -355,8 +351,8 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) auth_user_request = r->auth_user_request; assert(auth_user_request != NULL); - ntlm_request = dynamic_cast(auth_user_request); + ntlm_request = dynamic_cast(auth_user_request.getRaw()); assert(ntlm_request != NULL); assert(ntlm_request->waiting); ntlm_request->waiting = 0; @@ -493,7 +489,6 @@ AuthNTLMUserRequest::module_start(RH * handler, void *data) r->handler = handler; r->data = cbdataReference(data); r->auth_user_request = this; - AUTHUSERREQUESTLOCK(r->auth_user_request, "r"); if (auth_state == AUTHENTICATE_STATE_INITIAL) { snprintf(buf, 8192, "YR %s\n", client_blob); //CHECKME: can ever client_blob be 0 here? @@ -541,19 +536,20 @@ AuthNTLMUserRequest::onConnectionClose(ConnStateData *conn) /* unlock the connection based lock */ debugs(29, 9, "AuthNTLMUserRequest::onConnectionClose: Unlocking auth_user from the connection '" << conn << "'."); - AUTHUSERREQUESTUNLOCK(conn->auth_user_request, "conn"); + conn->auth_user_request = NULL; } /* * Decode a NTLM [Proxy-]Auth string, placing the results in the passed * Auth_user structure. */ -AuthUserRequest * +AuthUserRequest::Pointer AuthNTLMConfig::decode(char const *proxy_auth) { NTLMUser *newUser = new NTLMUser(&ntlmConfig); - AuthNTLMUserRequest *auth_user_request = new AuthNTLMUserRequest (); + AuthUserRequest::Pointer auth_user_request = new AuthNTLMUserRequest(); assert(auth_user_request->user() == NULL); + auth_user_request->user(newUser); auth_user_request->user()->auth_type = AUTH_NTLM; auth_user_request->user()->addRequest(auth_user_request); @@ -639,7 +635,6 @@ AuthNTLMUserRequest::authenticate(HttpRequest * aRequest, ConnStateData * conn, conn->auth_type = AUTH_NTLM; assert(conn->auth_user_request == NULL); conn->auth_user_request = this; - AUTHUSERREQUESTLOCK(conn->auth_user_request, "conn"); request = aRequest; HTTPMSGLOCK(request); return; diff --git a/src/auth/ntlm/auth_ntlm.h b/src/auth/ntlm/auth_ntlm.h index e26cba6018..949c882a26 100644 --- a/src/auth/ntlm/auth_ntlm.h +++ b/src/auth/ntlm/auth_ntlm.h @@ -27,7 +27,7 @@ typedef enum { typedef struct { void *data; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; RH *handler; } authenticateStateData; #endif @@ -105,10 +105,10 @@ public: AuthNTLMConfig(); virtual bool active() const; virtual bool configured() const; - virtual AuthUserRequest *decode(char const *proxy_auth); + virtual AuthUserRequest::Pointer decode(char const *proxy_auth); virtual void done(); virtual void dump(StoreEntry *, const char *, AuthConfig *); - virtual void fixHeader(AuthUserRequest *, HttpReply *, http_hdr_type, HttpRequest *); + virtual void fixHeader(AuthUserRequest::Pointer, HttpReply *, http_hdr_type, HttpRequest *); virtual void init(AuthConfig *); virtual void parse(AuthConfig *, int, char *); virtual void registerWithCacheManager(void); diff --git a/src/client_side.cc b/src/client_side.cc index 45a13d09c9..1b72d6f9ea 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -489,13 +489,12 @@ prepareLogWithRequestDetails(HttpRequest * request, AccessLogEntry * aLogEntry) aLogEntry->cache.requestSize += request->content_length; aLogEntry->cache.extuser = request->extacl_user.termedBuf(); - if (request->auth_user_request) { + if (request->auth_user_request != NULL) { if (request->auth_user_request->username()) - aLogEntry->cache.authuser = - xstrdup(request->auth_user_request->username()); + aLogEntry->cache.authuser = xstrdup(request->auth_user_request->username()); - AUTHUSERREQUESTUNLOCK(request->auth_user_request, "request via clientPrepareLogWithRequestDetails"); + request->auth_user_request = NULL; } } @@ -672,8 +671,6 @@ ConnStateData::~ConnStateData() if (!flags.swanSang) debugs(33, 1, "BUG: ConnStateData was not destroyed properly; FD " << fd); - AUTHUSERREQUESTUNLOCK(auth_user_request, "~conn"); - cbdataReferenceDone(port); if (bodyPipe != NULL) diff --git a/src/client_side.h b/src/client_side.h index 72fe1195af..d812e22851 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -33,6 +33,7 @@ #ifndef SQUID_CLIENTSIDE_H #define SQUID_CLIENTSIDE_H +#include "auth/UserRequest.h" #include "base/AsyncJob.h" #include "BodyPipe.h" #include "comm.h" @@ -43,15 +44,11 @@ #include "StoreIOBuffer.h" class ConnStateData; - class ClientHttpRequest; - class clientStreamNode; - -class AuthUserRequest; - class ChunkedCodingParser; class HttpParser; +// class AuthUserRequest::Pointer; template class Range; @@ -181,7 +178,7 @@ public: * note this is ONLY connection based because NTLM is against HTTP spec. * the user details for connection based authentication */ - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; /** * used by the owner of the connection, opaque otherwise diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index 8d37cb327d..6f685d8201 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -95,10 +95,9 @@ void clientReplyContext::setReplyToError( err_type err, http_status status, const HttpRequestMethod& method, char const *uri, IpAddress &addr, HttpRequest * failedrequest, const char *unparsedrequest, - AuthUserRequest * auth_user_request) + AuthUserRequest::Pointer auth_user_request) { - ErrorState *errstate = - clientBuildError(err, status, uri, addr, failedrequest); + ErrorState *errstate = clientBuildError(err, status, uri, addr, failedrequest); if (unparsedrequest) errstate->request_hdrs = xstrdup(unparsedrequest); @@ -111,10 +110,7 @@ clientReplyContext::setReplyToError( createStoreEntry(method, request_flags()); - if (auth_user_request) { - errstate->auth_user_request = auth_user_request; - AUTHUSERREQUESTLOCK(errstate->auth_user_request, "errstate"); - } + errstate->auth_user_request = auth_user_request; assert(errstate->callback_data == NULL); errorAppendEntry(http->storeEntry(), errstate); @@ -1361,9 +1357,8 @@ clientReplyContext::buildReplyHeader() * responses */ authenticateFixHeader(reply, request->auth_user_request, request, 0, 1); - } else if (request->auth_user_request) - authenticateFixHeader(reply, request->auth_user_request, request, - http->flags.accel, 0); + } else if (request->auth_user_request != NULL) + authenticateFixHeader(reply, request->auth_user_request, request, http->flags.accel, 0); /* Append X-Cache */ httpHeaderPutStrf(hdr, HDR_X_CACHE, "%s from %s", diff --git a/src/client_side_reply.h b/src/client_side_reply.h index 1d0e827f59..1bee5ced9a 100644 --- a/src/client_side_reply.h +++ b/src/client_side_reply.h @@ -72,7 +72,7 @@ public: int storeOKTransferDone() const; int storeNotOKTransferDone() const; - void setReplyToError(err_type, http_status, const HttpRequestMethod&, char const *, IpAddress &, HttpRequest *, const char *, AuthUserRequest *); + void setReplyToError(err_type, http_status, const HttpRequestMethod&, char const *, IpAddress &, HttpRequest *, const char *, AuthUserRequest::Pointer); void createStoreEntry(const HttpRequestMethod& m, request_flags flags); void removeStoreReference(store_client ** scp, StoreEntry ** ep); void removeClientStoreReference(store_client **scp, ClientHttpRequest *http); diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 434c1ce202..a020a75b39 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -602,7 +602,7 @@ ClientRequestContext::clientAccessCheckDone(int answer) http->getConn() != NULL ? http->getConn()->peer : tmpnoaddr, http->request, NULL, - http->getConn() != NULL && http->getConn()->auth_user_request ? + http->getConn() != NULL && http->getConn()->auth_user_request != NULL ? http->getConn()->auth_user_request : http->request->auth_user_request); node = (clientStreamNode *)http->client_stream.tail->data; @@ -1017,11 +1017,7 @@ ClientRequestContext::clientRedirectDone(char *result) new_request->my_addr = old_request->my_addr; new_request->flags = old_request->flags; new_request->flags.redirected = 1; - - if (old_request->auth_user_request) { - new_request->auth_user_request = old_request->auth_user_request; - AUTHUSERREQUESTLOCK(new_request->auth_user_request, "new request"); - } + new_request->auth_user_request = old_request->auth_user_request; if (old_request->body_pipe != NULL) { new_request->body_pipe = old_request->body_pipe; @@ -1492,7 +1488,7 @@ ClientHttpRequest::handleAdaptationFailure(bool bypassable) repContext->setReplyToError(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request->method, NULL, (c != NULL ? c->peer : noAddr), request, NULL, - (c != NULL && c->auth_user_request ? + (c != NULL && c->auth_user_request != NULL ? c->auth_user_request : request->auth_user_request)); node = (clientStreamNode *)client_stream.tail->data; diff --git a/src/enums.h b/src/enums.h index e06a933f55..47385dc2bc 100644 --- a/src/enums.h +++ b/src/enums.h @@ -249,22 +249,6 @@ typedef enum { STREAM_FAILED } clientStream_status_t; -typedef enum { - AUTH_ACL_CHALLENGE = -2, - AUTH_ACL_HELPER = -1, - AUTH_ACL_CANNOT_AUTHENTICATE = 0, - AUTH_AUTHENTICATED = 1 -} auth_acl_t; - -typedef enum { - AUTH_UNKNOWN, /* default */ - AUTH_BASIC, - AUTH_NTLM, - AUTH_DIGEST, - AUTH_NEGOTIATE, - AUTH_BROKEN /* known type, but broken data */ -} auth_type_t; - /* stateful helper callback response codes */ typedef enum { S_HELPER_UNKNOWN, diff --git a/src/errorpage.cc b/src/errorpage.cc index b0b773ebfd..a8259b8585 100644 --- a/src/errorpage.cc +++ b/src/errorpage.cc @@ -501,7 +501,7 @@ errorStateFree(ErrorState * err) wordlistDestroy(&err->ftp.server_msg); safe_free(err->ftp.request); safe_free(err->ftp.reply); - AUTHUSERREQUESTUNLOCK(err->auth_user_request, "errstate"); + err->auth_user_request = NULL; safe_free(err->err_msg); #if USE_ERR_LOCALES if (err->err_language != Config.errorDefaultLanguage) @@ -609,7 +609,7 @@ ErrorState::Convert(char token, bool url_presentable) switch (token) { case 'a': - if (request && request->auth_user_request) + if (request && request->auth_user_request != NULL) p = request->auth_user_request->username(); if (!p) p = "-"; diff --git a/src/errorpage.h b/src/errorpage.h index 8a22d85fe7..21255fc719 100644 --- a/src/errorpage.h +++ b/src/errorpage.h @@ -35,6 +35,7 @@ #define SQUID_ERRORPAGE_H #include "squid.h" +#include "auth/UserRequest.h" #include "cbdata.h" #include "ip/IpAddress.h" @@ -78,7 +79,6 @@ \endverbatim */ -class AuthUserRequest; class HttpReply; class MemBuf; @@ -127,7 +127,7 @@ public: int page_id; char *err_language; http_status httpStatus; - AuthUserRequest *auth_user_request; + AuthUserRequest::Pointer auth_user_request; HttpRequest *request; char *url; int xerrno; diff --git a/src/external_acl.cc b/src/external_acl.cc index 90ff036841..1b182a19c6 100644 --- a/src/external_acl.cc +++ b/src/external_acl.cc @@ -729,8 +729,7 @@ aclMatchExternal(external_acl_data *acl, ACLFilledChecklist *ch) key = makeExternalAclKey(ch, acl); - if (acl->def->require_auth) - AUTHUSERREQUESTUNLOCK(ch->auth_user_request, "ACLChecklist via aclMatchExternal"); + acl->def->require_auth = NULL; if (!key) { /* Not sufficient data to process */ @@ -845,7 +844,7 @@ makeExternalAclKey(ACLFilledChecklist * ch, external_acl_data * acl_data) switch (format->type) { case _external_acl_format::EXT_ACL_LOGIN: - assert (ch->auth_user_request); + assert (ch->auth_user_request != NULL); str = ch->auth_user_request->username(); break; #if USE_IDENT diff --git a/src/http.cc b/src/http.cc index e8ddbe92c1..206b2b56be 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1490,7 +1490,7 @@ httpFixupAuthentication(HttpRequest * request, HttpRequest * orig_request, const if (orig_request->extacl_user.size()) username = orig_request->extacl_user.termedBuf(); - else if (orig_request->auth_user_request) + else if (orig_request->auth_user_request != NULL) username = orig_request->auth_user_request->username(); snprintf(loginbuf, sizeof(loginbuf), "%s%s", username, orig_request->peer_login + 1); diff --git a/src/peer_userhash.cc b/src/peer_userhash.cc index 5d4255ff0c..b966336c4a 100644 --- a/src/peer_userhash.cc +++ b/src/peer_userhash.cc @@ -180,7 +180,7 @@ peerUserHashSelectParent(HttpRequest * request) if (n_userhash_peers == 0) return NULL; - if (request->auth_user_request) + if (request->auth_user_request != NULL) key = request->auth_user_request->username(); if (!key) diff --git a/src/redirect.cc b/src/redirect.cc index 04bbe8a08a..e6f00fc856 100644 --- a/src/redirect.cc +++ b/src/redirect.cc @@ -136,7 +136,7 @@ redirectStart(ClientHttpRequest * http, RH * handler, void *data) r->client_addr.SetNoAddr(); r->client_ident = NULL; - if (http->request->auth_user_request) + if (http->request->auth_user_request != NULL) r->client_ident = http->request->auth_user_request->username(); else if (http->request->extacl_user.defined()) { r->client_ident = http->request->extacl_user.termedBuf(); diff --git a/src/stat.cc b/src/stat.cc index b90b2d8eec..dd909d9caf 100644 --- a/src/stat.cc +++ b/src/stat.cc @@ -1653,7 +1653,7 @@ statClientRequests(StoreEntry * s) (int) http->start_time.tv_usec, tvSubDsec(http->start_time, current_time)); - if (http->request->auth_user_request) + if (http->request->auth_user_request != NULL) p = http->request->auth_user_request->username(); else if (http->request->extacl_user.defined()) { p = http->request->extacl_user.termedBuf(); diff --git a/src/tests/testAuth.cc b/src/tests/testAuth.cc index 161bd0aa9d..5eb4ce90f4 100644 --- a/src/tests/testAuth.cc +++ b/src/tests/testAuth.cc @@ -11,16 +11,16 @@ CPPUNIT_TEST_SUITE_REGISTRATION( testAuth ); CPPUNIT_TEST_SUITE_REGISTRATION( testAuthConfig ); CPPUNIT_TEST_SUITE_REGISTRATION( testAuthUserRequest ); -#ifdef HAVE_AUTH_MODULE_BASIC +#if HAVE_AUTH_MODULE_BASIC CPPUNIT_TEST_SUITE_REGISTRATION( testAuthBasicUserRequest ); #endif -#ifdef HAVE_AUTH_MODULE_DIGEST +#if HAVE_AUTH_MODULE_DIGEST CPPUNIT_TEST_SUITE_REGISTRATION( testAuthDigestUserRequest ); #endif -#ifdef HAVE_AUTH_MODULE_NTLM +#if HAVE_AUTH_MODULE_NTLM CPPUNIT_TEST_SUITE_REGISTRATION( testAuthNTLMUserRequest ); #endif -#ifdef HAVE_AUTH_MODULE_NEGOTIATE +#if HAVE_AUTH_MODULE_NEGOTIATE CPPUNIT_TEST_SUITE_REGISTRATION( testAuthNegotiateUserRequest ); #endif @@ -74,7 +74,7 @@ getConfig(char const *type_str) config.push_back(theScheme->createConfig()); scheme = config.back(); - assert (scheme); + assert(scheme); } return scheme; @@ -156,7 +156,7 @@ testAuthConfig::create() fake_auth_setup(); for (AuthScheme::const_iterator i = AuthScheme::Schemes().begin(); i != AuthScheme::Schemes().end(); ++i) { - AuthUserRequest *authRequest = AuthConfig::CreateAuthUser(find_proxy_auth((*i)->type())); + AuthUserRequest::Pointer authRequest = AuthConfig::CreateAuthUser(find_proxy_auth((*i)->type())); CPPUNIT_ASSERT(authRequest != NULL); } } @@ -177,12 +177,12 @@ testAuthUserRequest::scheme() for (AuthScheme::const_iterator i = AuthScheme::Schemes().begin(); i != AuthScheme::Schemes().end(); ++i) { // create a user request // check its scheme matches *i - AuthUserRequest *authRequest = AuthConfig::CreateAuthUser(find_proxy_auth((*i)->type())); + AuthUserRequest::Pointer authRequest = AuthConfig::CreateAuthUser(find_proxy_auth((*i)->type())); CPPUNIT_ASSERT_EQUAL(authRequest->scheme(), *i); } } -#ifdef HAVE_AUTH_MODULE_BASIC +#if HAVE_AUTH_MODULE_BASIC #include "auth/basic/auth_basic.h" /* AuthBasicUserRequest::AuthBasicUserRequest works */ @@ -197,18 +197,16 @@ testAuthBasicUserRequest::construction() void testAuthBasicUserRequest::username() { - AuthBasicUserRequest(); - AuthBasicUserRequest *temp=new AuthBasicUserRequest(); + AuthUserRequest::Pointer temp = new AuthBasicUserRequest(); BasicUser *basic_auth=new BasicUser(AuthConfig::Find("basic")); basic_auth->username("John"); temp->user(basic_auth); basic_auth->addRequest(temp); CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username())); - delete temp; } #endif /* HAVE_AUTH_MODULE_BASIC */ -#ifdef HAVE_AUTH_MODULE_DIGEST +#if HAVE_AUTH_MODULE_DIGEST #include "auth/digest/auth_digest.h" /* AuthDigestUserRequest::AuthDigestUserRequest works */ @@ -223,18 +221,16 @@ testAuthDigestUserRequest::construction() void testAuthDigestUserRequest::username() { - AuthDigestUserRequest(); - AuthDigestUserRequest *temp=new AuthDigestUserRequest(); - DigestUser *user=new DigestUser(AuthConfig::Find("digest")); - user->username("John"); - temp->user(user); - user->addRequest(temp); + AuthUserRequest::Pointer temp = new AuthDigestUserRequest(); + DigestUser *duser=new DigestUser(AuthConfig::Find("digest")); + duser->username("John"); + temp->user(duser); + duser->addRequest(temp); CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username())); - delete temp; } #endif /* HAVE_AUTH_MODULE_DIGEST */ -#ifdef HAVE_AUTH_MODULE_NTLM +#if HAVE_AUTH_MODULE_NTLM #include "auth/ntlm/auth_ntlm.h" /* AuthNTLMUserRequest::AuthNTLMUserRequest works */ @@ -249,18 +245,16 @@ testAuthNTLMUserRequest::construction() void testAuthNTLMUserRequest::username() { - AuthNTLMUserRequest(); - AuthNTLMUserRequest *temp=new AuthNTLMUserRequest(); - NTLMUser *user=new NTLMUser(AuthConfig::Find("ntlm")); - user->username("John"); - temp->user(user); - user->addRequest(temp); + AuthUserRequest::Pointer temp = new AuthNTLMUserRequest(); + NTLMUser *nuser=new NTLMUser(AuthConfig::Find("ntlm")); + nuser->username("John"); + temp->user(nuser); + nuser->addRequest(temp); CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username())); - delete temp; } #endif /* HAVE_AUTH_MODULE_NTLM */ -#ifdef HAVE_AUTH_MODULE_NEGOTIATE +#if HAVE_AUTH_MODULE_NEGOTIATE #include "auth/negotiate/auth_negotiate.h" /* AuthNegotiateUserRequest::AuthNegotiateUserRequest works */ @@ -275,14 +269,12 @@ testAuthNegotiateUserRequest::construction() void testAuthNegotiateUserRequest::username() { - AuthNegotiateUserRequest(); - AuthNegotiateUserRequest *temp=new AuthNegotiateUserRequest(); - NegotiateUser *user=new NegotiateUser(AuthConfig::Find("negotiate")); - user->username("John"); - temp->user(user); - user->addRequest(temp); + AuthUserRequest::Pointer temp = new AuthNegotiateUserRequest(); + NegotiateUser *nuser=new NegotiateUser(AuthConfig::Find("negotiate")); + nuser->username("John"); + temp->user(nuser); + nuser->addRequest(temp); CPPUNIT_ASSERT_EQUAL(0, strcmp("John", temp->username())); - delete temp; } #endif /* HAVE_AUTH_MODULE_NEGOTIATE */ -- 2.47.3