[squid-dev] Extremely questionable code in Basic authentication module

[Joshua Rogers]

Hi there,

I was looking at the file src/auth/basic/UserRequest.cc, in
function Auth::Basic::UserRequest::module_direction:


    case Auth::Ok:
        if (user()->expiretime +
static_cast<Auth::Basic::Config*>(Auth::SchemeConfig::Find("basic"))->credentialsTTL
<= squid_curtime)
            return Auth::CRED_LOOKUP;
        return Auth::CRED_VALID;

    case Auth::Failed:
        return Auth::CRED_VALID;


I was a bit alarmed that if an auth fails, it returns Auth::CRED_VALID.
Why is CRED_ERROR or CRED_CHALLENGE not used here?

In negotiate and NTLM code, there is a note:
"XXX: really? not VALID or CHALLENGE?" when CRED_ERROR is returned.

Thankfully Squid doesn't really rely on this return value to determine
whether a login is correct or not as it
calls authenticateUserAuthenticated() which eventually checks credentials()
== Auth::Ok. It all seems like quite a round-about method, however.

According to
http://www.squid-cache.org/Doc/code/namespaceAuth.html?#afd721f7bc874e61ad0111999abf22a19a2d0cf49d6f94b0664c99dffb68cb4d5d
each of these calls should return CRED_CHALLENGE.

What are your thoughts on this? Should it be changed?

Cheers,
Josh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20210324/da27ac7c/attachment.htm>