Updating TIdHTTP DoOnAuthorization() and DoOnProxyAuthorization() met…

…hods to no longer check if the On(Proxy)Authorization events or Password properties are assigned before entering the authentication loops. Individual TIdAuthentication-derived classes should handle the Username and Password as needed (in the case of SSPI, they can both be blank to use the current identity of the calling thread!). The On(Proxy)Authentication events are now checked only when TIdAuthentication.Next returns wnAskTheProgram.

Tweaks to overridden TIdAuthentication.DoNext() implementations.

Lib/Protocols/IdAuthentication.pas

@@ -298,36 +298,93 @@ function TIdBasicAuthentication.Authentication: String;
   end;
 end;
 
+// TODO: move this into the 'interface' section, or maybe the IdGlobalProtocols unit...
+function Unquote(var S: String): String;
+var
+  I, Len: Integer;
+begin
+  Len := Length(S);
+  I := 2; // skip first quote
+  while I <= Len do
+  begin
+    if S[I] = '"' then begin
+      Break;
+    end;
+    if S[I] = '\' then begin
+      Inc(I);
+    end;
+    Inc(I);
+  end;
+  Result := Copy(S, 2, I-2);
+  S := Copy(S, I+1, MaxInt);
+
+  // TODO: use a PosEx() loop instead
+  {
+  I := Pos('\', Result);
+  while I <> 0 do
+  begin
+    IdDelete(Result, I, 1);
+    I := PosEx('\', Result, I+1);
+  end;
+  }
+  Len := Length(Result);
+  I := 1;
+  while I <= Len do
+  begin
+    if Result[I] = '\' then begin
+      IdDelete(Result, I, 1);
+    end;
+    Inc(I);
+  end;
+end;
+
 function TIdBasicAuthentication.DoNext: TIdAuthWhatsNext;
 var
-  S, LSep: String;
+  S, LName, LValue: String;
+  LParams: TStringList;
 begin
   S := ReadAuthInfo('Basic');        {Do not Localize}
   Fetch(S);
 
-  LSep := Params.NameValueSeparator;
-  while Length(S) > 0 do begin
-    // realm have 'realm="SomeRealmValue"' format    {Do not Localize}
-    // FRealm never assigned without StringReplace
-    Params.Add(ReplaceOnlyFirst(Fetch(S, ', '), '=', LSep));  {do not localize}
-  end;
+  LParams := TStringList.Create;
+  try
+    {$IFDEF HAS_TStringList_CaseSensitive}
+    LParams.CaseSensitive := False;
+    {$ENDIF}
 
-  FRealm := UnquotedStr(Params.Values['realm']);   {Do not Localize}
+    while Length(S) > 0 do begin
+      // RLebeau: Apache sends a space after each comma, but IIS does not!
+      LName := Trim(Fetch(S, '=')); {do not localize}
+      S := TrimLeft(S);
+      if TextStartsWith(S, '"') then begin {do not localize}
+        LValue := Unquote(S); {do not localize}
+        Fetch(S, ','); {do not localize}
+      end else begin
+        LValue := Trim(Fetch(S, ','));
+      end;
+      IndyAddPair(LParams, LName, LValue);
+      S := TrimLeft(S);
+    end;
+
+    FRealm := LParams.Values['realm'];   {Do not Localize}
 
-  FCharset := UnquotedStr(Params.Values['charset']); // RFC 7617
-  if FCharset = '' then begin
-    FCharset := UnquotedStr(Params.Values['accept-charset']); // draft-reschke-basicauth-enc-05 onwards
+    FCharset := LParams.Values['charset']; // RFC 7617
     if FCharset = '' then begin
-      FCharset := UnquotedStr(Params.Values['encoding']); // draft-reschke-basicauth-enc-04
+      FCharset := LParams.Values['accept-charset']; // draft-reschke-basicauth-enc-05 onwards
       if FCharset = '' then begin
-        FCharset := UnquotedStr(Params.Values['enc']); // I saw this mentioned in a Mozilla bug report, and apparently Opera supports it
-      end;
-      if FCharset = '' then begin
-        // TODO: check the user's input and encode using ISO-8859-1 only if
-        // the characters will actually fit, otherwise use UTF-8 instead?
-        FCharset := 'ISO-8859-1';
+        FCharset := LParams.Values['encoding']; // draft-reschke-basicauth-enc-04
+        if FCharset = '' then begin
+          FCharset := LParams.Values['enc']; // I saw this mentioned in a Mozilla bug report, and apparently Opera supports it
+        end;
+        if FCharset = '' then begin
+          // TODO: check the user's input and encode using ISO-8859-1 only if
+          // the characters will actually fit, otherwise use UTF-8 instead?
+          FCharset := 'ISO-8859-1';
+        end;
       end;
     end;
+  finally
+    FreeAndNil(LParams);
   end;
 
   if FCurrentStep = 0 then

Lib/Protocols/IdAuthenticationDigest.pas

@@ -199,6 +199,7 @@ function TIdDigestAuthentication.Authentication: String;
   end;
 end;
 
+// TODO: move this to the IdAuthentication unit, or maybe the IdGlobalProtocols unit...
 function Unquote(var S: String): String;
 var
   I, Len: Integer;
@@ -217,6 +218,25 @@ function Unquote(var S: String): String;
   end;
   Result := Copy(S, 2, I-2);
   S := Copy(S, I+1, MaxInt);
+
+  // TODO: use a PosEx() loop instead
+  {
+  I := Pos('\', Result);
+  while I <> 0 do
+  begin
+    IdDelete(Result, I, 1);
+    I := PosEx('\', Result, I+1);
+  end;
+  }
+  Len := Length(Result);
+  I := 1;
+  while I <= Len do
+  begin
+    if Result[I] = '\' then begin
+      IdDelete(Result, I, 1);
+    end;
+    Inc(I);
+  end;
 end;
 
 function TIdDigestAuthentication.DoNext: TIdAuthWhatsNext;
@@ -247,6 +267,10 @@ function TIdDigestAuthentication.DoNext: TIdAuthWhatsNext;
 
         LParams := TStringList.Create;
         try
+          {$IFDEF HAS_TStringList_CaseSensitive}
+          LParams.CaseSensitive := False;
+          {$ENDIF}
+
           while Length(S) > 0 do begin
             // RLebeau: Apache sends a space after each comma, but IIS does not!
             LName := Trim(Fetch(S, '=')); {do not localize}
@@ -262,6 +286,7 @@ function TIdDigestAuthentication.DoNext: TIdAuthWhatsNext;
           end;
 
           FRealm := LParams.Values['realm']; {do not localize}
+
           LTempNonce := LParams.Values['nonce']; {do not localize}
           if FNonce <> LTempNonce then
           begin

Lib/Protocols/IdAuthenticationNTLM.pas

@@ -118,14 +118,9 @@ function TIdNTLMAuthentication.DoNext: TIdAuthWhatsNext;
           Result := wnAskTheProgram;
         end;
       end;
-    1:
-      begin
-        FCurrentStep := 2;
-        Result := wnDoRequest;
-      end;
-    2:
+    1, 2:
       begin
-        FCurrentStep := 3;
+        Inc(FCurrentStep);
         Result := wnDoRequest;
       end;
     3:

Lib/Protocols/IdAuthenticationSSPI.pas

@@ -1199,26 +1199,10 @@ function TIdSSPINTLMAuthentication.DoNext: TIdAuthWhatsNext;
 begin
   Result := wnDoRequest;
   case FCurrentStep of
-    0:
+    //Authentication() does the 2>3 progression
+    0, 1, 3:
       begin
-        {if (Length(Username) > 0) and (Length(Password) > 0) then
-        begin}
-        Result := wnDoRequest;
-        FCurrentStep := 1;
-        {end
-        else begin
-          result := wnAskTheProgram;
-        end;}
-      end;
-    1:
-      begin
-        FCurrentStep := 2;
-        Result := wnDoRequest;
-      end;
-    //Authentication does the 2>3 progression
-    3:
-      begin
-        FCurrentStep := 4;
+        Inc(FCurrentStep);
         Result := wnDoRequest;
       end;
     4:
@@ -1297,13 +1281,15 @@ function TIdSSPINTLMAuthentication.GetDomain: String;
 end;
 
 procedure TIdSSPINTLMAuthentication.SetUserName(const Value: String);
-Var
+var
   S: String;
+  Idx: Integer;
 begin
   S := Value;
-  if IndyPos('\', S) > 0 then begin
-    Domain := Copy(S, 1, IndyPos('\', S) - 1);
-    Delete(S, 1, Length(Domain) + 1);
+  Idx := IndyPos('\', S);
+  if Idx > 0 then begin
+    Domain := Copy(S, 1, Idx - 1);
+    Delete(S, 1, Idx);
   end;
   inherited SetUserName(S);
 end;

Lib/Protocols/IdHTTP.pas

@@ -2269,15 +2269,6 @@ function TIdCustomHTTP.DoOnAuthorization(ARequest: TIdHTTPRequest; AResponse: TI
   // username/password as it can use the identity of the user token associated
   // with the calling thread!
 
-  // TODO: get rid of this check here.  Let Request.Authentication validate the
-  // username/password as needed.  Don't validate OnAuthorization unless
-  // wnAskTheProgram is requested...  
-  Result := Assigned(FOnAuthorization) or (Trim(ARequest.Password) <> '');
-
-  if not Result then begin
-    Exit;
-  end;
-
   LAuth := ARequest.Authentication;
   LAuth.Username := ARequest.Username;
   LAuth.Password := ARequest.Password;
@@ -2292,21 +2283,23 @@ function TIdCustomHTTP.DoOnAuthorization(ARequest: TIdHTTPRequest; AResponse: TI
     case LAuth.Next of
       wnAskTheProgram:
         begin // Ask the user porgram to supply us with authorization information
-          if Assigned(FOnAuthorization) then
+          if not Assigned(FOnAuthorization) then
           begin
-            LAuth.UserName := ARequest.Username;
-            LAuth.Password := ARequest.Password;
+            Result := False;
+            Break;
+          end;
 
-            OnAuthorization(Self, LAuth, Result);
+          LAuth.UserName := ARequest.Username;
+          LAuth.Password := ARequest.Password;
 
-            if Result then begin
-              ARequest.BasicAuthentication := True;
-              ARequest.Username := LAuth.UserName;
-              ARequest.Password := LAuth.Password;
-            end else begin
-              Break;
-            end;
+          OnAuthorization(Self, LAuth, Result);
+          if not Result then begin
+            Break;
           end;
+
+          ARequest.BasicAuthentication := True;
+          ARequest.Username := LAuth.UserName;
+          ARequest.Password := LAuth.Password;
         end;
       wnDoRequest:
         begin
@@ -2377,15 +2370,6 @@ function TIdCustomHTTP.DoOnProxyAuthorization(ARequest: TIdHTTPRequest; ARespons
   // username/password as it can use the identity of the user token associated
   // with the calling thread!
 
-  // TODO: get rid of this check here.  Let ProxyParams.Authentication validate
-  // the username/password as needed.  Don't validate OnProxyAuthorization unless
-  // wnAskTheProgram is requested...  
-  Result := Assigned(OnProxyAuthorization) or (Trim(ProxyParams.ProxyPassword) <> '');
-
-  if not Result then begin
-    Exit;
-  end;
-
   LAuth := ProxyParams.Authentication;
   LAuth.Username := ProxyParams.ProxyUsername;
   LAuth.Password := ProxyParams.ProxyPassword;
@@ -2399,22 +2383,23 @@ function TIdCustomHTTP.DoOnProxyAuthorization(ARequest: TIdHTTPRequest; ARespons
     case LAuth.Next of
       wnAskTheProgram: // Ask the user porgram to supply us with authorization information
         begin
-          if Assigned(OnProxyAuthorization) then
-          begin
-            LAuth.Username := ProxyParams.ProxyUsername;
-            LAuth.Password := ProxyParams.ProxyPassword;
+          if not Assigned(OnProxyAuthorization) then begin
+            Result := False;
+            Break;
+          end;
 
-            OnProxyAuthorization(Self, LAuth, Result);
+          LAuth.Username := ProxyParams.ProxyUsername;
+          LAuth.Password := ProxyParams.ProxyPassword;
 
-            if Result then begin
-              // TODO: do we need to set this, like DoOnAuthorization does?
-              //ProxyParams.BasicAuthentication := True;
-              ProxyParams.ProxyUsername := LAuth.Username;
-              ProxyParams.ProxyPassword := LAuth.Password;
-            end else begin
-              Break;
-            end;
+          OnProxyAuthorization(Self, LAuth, Result);
+          if not Result then begin
+            Break;
           end;
+
+          // TODO: do we need to set this, like DoOnAuthorization does?
+          //ProxyParams.BasicAuthentication := True;
+          ProxyParams.ProxyUsername := LAuth.Username;
+          ProxyParams.ProxyPassword := LAuth.Password;
         end;
       wnDoRequest:
         begin