Fix/introspection basic auth base64

CHANGELOG.md

@@ -488,6 +488,7 @@ follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- `TMCPIntrospectionAuthorizer` sends the client credentials for the introspection endpoint as a single-line `Authorization: Basic` header. It encoded them with `TNetEncoding.Base64`, which inserts a line break after every 76 characters, so a `client_id:client_secret` of 57 bytes or more broke the request and every token was rejected; it now uses `TNetEncoding.Base64String`.
 - The `resource_metadata` URL in the `WWW-Authenticate` challenge is derived from `[Auth] ResourceUri` when it is set (RFC 9728: origin of the resource, `/.well-known/oauth-protected-resource`, then the path of the resource). It used to be built from `Protocol`, `Host` and the listening port, so a server behind a reverse proxy sent clients to `http://localhost:8080/...` instead of its public address. `TMCPProtectedResourceMetadata.UrlFor` does the derivation and `PathFor` gives the matching path, which the server now also serves without a token when it differs from `/.well-known/oauth-protected-resource<Endpoint>`. `TMCPResourceLocation.TryParse` splits a resource URI into scheme, authority and path.
 - A record that failed to convert halfway leaked the objects it already held:
   `TMCPSerializer.JsonToValue` adds them to the caller's list only once the

src/Core/MCPServer.Authorization.pas

@@ -546,7 +546,9 @@ function TMCPIntrospectionAuthorizer.TryValidateToken(const Token: string; out C
       const HasClientId = (FClientId <> '');
       if HasClientId then
       begin
-        const Credentials = TNetEncoding.Base64.Encode(Format('%s:%s', [FClientId, FClientSecret]));
+        const ClientCredentials = Format('%s:%s', [FClientId, FClientSecret]);
+        const Encoding = TNetEncoding.Base64String;
+        const Credentials = Encoding.Encode(ClientCredentials);
         Headers := Headers + [TNetHeader.Create('Authorization', Format('Basic %s', [Credentials]))];
       end;
 

tests/MCPServer.Tests.Authorization.pas

@@ -31,6 +31,8 @@   TAuthorizationTests = class
     FSeenBody: string;
     procedure HandleIntrospection(Context: TIdContext; Request: TIdHTTPRequestInfo; Response: TIdHTTPResponseInfo);
     function Decide(const Authorizer: IMCPAuthorizer; const Token: string): TMCPAuthResult;
+    procedure StartIntrospectionServer;
+    function IntrospectionUrl: string;
   public
     [TearDown]
     procedure TearDown;
@@ -80,6 +82,9 @@   TAuthorizationTests = class
 
     [Test]
     procedure Introspection_PostsTokenWithClientCredentials;
+
+    [Test]
+    procedure Introspection_LongClientSecret_SendsBasicHeaderOnOneLine;
   end;
 
 implementation
@@ -88,7 +93,13 @@ implementation
   System.DateUtils;
 
 const
-  AUDIENCE = 'https://mcp.example/mcp';
+  Audience = 'https://mcp.example/mcp';
+  ClaimsWithExpiryFormat = '{"sub":"u","aud":"%s","exp":%d}';
+  IntrospectionClientId = 'mcp';
+  ActiveToken = 'good';
+  LongSecretLength = 64;
+  LongSecretAuthorization =
+    'Basic bWNwOnh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHg=';
 
 { TClaimsAuthorizer }
 
@@ -179,26 +190,31 @@ procedure TAuthorizationTests.OAuth_RejectsWrongAudience_Expiry_AndScope;
   var Future := System.DateUtils.DateTimeToUnix(Now, False) + 600;
   var Past := System.DateUtils.DateTimeToUnix(Now, False) - 600;
 
-  var Invalid: IMCPAuthorizer := TClaimsAuthorizer.Create(AUDIENCE, Format('{"sub":"u","aud":"%s","exp":%d}', [AUDIENCE, Future]));
+  const InvalidClaims = Format(ClaimsWithExpiryFormat, [Audience, Future]);
+  const Invalid: IMCPAuthorizer = TClaimsAuthorizer.Create(Audience, InvalidClaims);
   var Rejected := Decide(Invalid, 'nope');
   Assert.AreEqual(TMCPAuthDecision.Unauthorized, Rejected.Decision);
   Assert.AreEqual('invalid_token', Rejected.Challenge.Error);
 
-  var WrongAudience: IMCPAuthorizer := TClaimsAuthorizer.Create(AUDIENCE, Format('{"sub":"u","aud":"https://other","exp":%d}', [Future]));
+  const OtherAudienceClaims = Format('{"sub":"u","aud":"https://other","exp":%d}', [Future]);
+  const WrongAudience: IMCPAuthorizer = TClaimsAuthorizer.Create(Audience, OtherAudienceClaims);
   var ForAnother := Decide(WrongAudience, 'valid');
   Assert.AreEqual(TMCPAuthDecision.Unauthorized, ForAnother.Decision);
   Assert.IsTrue(ForAnother.Challenge.ErrorDescription.Contains('not issued for this server'),
     ForAnother.Challenge.ErrorDescription);
 
-  var Expired: IMCPAuthorizer := TClaimsAuthorizer.Create(AUDIENCE, Format('{"sub":"u","aud":"%s","exp":%d}', [AUDIENCE, Past]));
+  const ExpiredClaims = Format(ClaimsWithExpiryFormat, [Audience, Past]);
+  const Expired: IMCPAuthorizer = TClaimsAuthorizer.Create(Audience, ExpiredClaims);
   var Stale := Decide(Expired, 'valid');
   Assert.AreEqual(TMCPAuthDecision.Unauthorized, Stale.Decision);
   Assert.IsTrue(Stale.Challenge.ErrorDescription.Contains('expired'), Stale.Challenge.ErrorDescription);
 
-  var NoExpiry: IMCPAuthorizer := TClaimsAuthorizer.Create(AUDIENCE, Format('{"sub":"u","aud":"%s"}', [AUDIENCE]));
+  const NoExpiryClaims = Format('{"sub":"u","aud":"%s"}', [Audience]);
+  const NoExpiry: IMCPAuthorizer = TClaimsAuthorizer.Create(Audience, NoExpiryClaims);
   Assert.AreEqual(TMCPAuthDecision.Unauthorized, Decide(NoExpiry, 'valid').Decision, 'exp is mandatory');
 
-  var Scoped := TClaimsAuthorizer.Create(AUDIENCE, Format('{"sub":"u","aud":"%s","exp":%d,"scope":"read"}', [AUDIENCE, Future]));
+  const ScopedClaims = Format('{"sub":"u","aud":"%s","exp":%d,"scope":"read"}', [Audience, Future]);
+  const Scoped = TClaimsAuthorizer.Create(Audience, ScopedClaims);
   var ScopedRef: IMCPAuthorizer := Scoped;
   Scoped.RequiredScopes := ['read', 'write'];
   var Denied := Decide(ScopedRef, 'valid');
@@ -217,8 +233,9 @@ procedure TAuthorizationTests.OAuth_RejectsWrongAudience_Expiry_AndScope;
 procedure TAuthorizationTests.OAuth_AcceptsAudienceArray_AndScopeArray;
 begin
   var Future := System.DateUtils.DateTimeToUnix(Now, False) + 600;
-  var Authorizer: IMCPAuthorizer := TClaimsAuthorizer.Create(AUDIENCE,
-    Format('{"sub":"u","aud":["https://other","%s"],"exp":%d,"scp":["a","b"]}', [AUDIENCE.ToUpper, Future]));
+  const UpperAudience = Audience.ToUpper;
+  const ArrayClaims = Format('{"sub":"u","aud":["https://other","%s"],"exp":%d,"scp":["a","b"]}', [UpperAudience, Future]);
+  const Authorizer: IMCPAuthorizer = TClaimsAuthorizer.Create(Audience, ArrayClaims);
   var Outcome := Decide(Authorizer, 'valid');
   Assert.AreEqual(TMCPAuthDecision.Allow, Outcome.Decision);
   Assert.IsTrue(Outcome.Principal.HasScope('a'));
@@ -293,24 +310,36 @@ procedure TAuthorizationTests.HandleIntrospection(Context: TIdContext; Request:
   FSeenAuthorization := Request.RawHeaders.Values['Authorization'];
   FSeenBody := Request.FormParams;
   Response.ContentType := 'application/json';
+  const Expiry = System.DateUtils.DateTimeToUnix(Now, False) + 600;
   if FSeenBody.Contains('token=good') then
-    Response.ContentText := Format('{"active":true,"sub":"alice","aud":"%s","exp":%d,"scope":"read"}',
-      [AUDIENCE, System.DateUtils.DateTimeToUnix(Now, False) + 600])
+    Response.ContentText := Format('{"active":true,"sub":"alice","aud":"%s","exp":%d,"scope":"read"}', [Audience, Expiry])
   else
     Response.ContentText := '{"active":false}';
 end;
 
-procedure TAuthorizationTests.Introspection_PostsTokenWithClientCredentials;
+procedure TAuthorizationTests.StartIntrospectionServer;
 begin
   FIntrospection := TIdHTTPServer.Create(nil);
-  FIntrospection.Bindings.Add.IP := '127.0.0.1';
-  FIntrospection.Bindings[0].Port := 0;
+  const Binding = FIntrospection.Bindings.Add;
+  Binding.IP := '127.0.0.1';
+  Binding.Port := 0;
   FIntrospection.OnCommandGet := HandleIntrospection;
   FIntrospection.Active := True;
-  var Url := Format('http://127.0.0.1:%d/introspect', [FIntrospection.Bindings[0].Port]);
+end;
+
+function TAuthorizationTests.IntrospectionUrl: string;
+begin
+  const Binding = FIntrospection.Bindings[0];
+  Result := Format('http://127.0.0.1:%d/introspect', [Binding.Port]);
+end;
+
+procedure TAuthorizationTests.Introspection_PostsTokenWithClientCredentials;
+begin
+  StartIntrospectionServer;
+  const Url = IntrospectionUrl;
 
-  var Authorizer: IMCPAuthorizer := TMCPIntrospectionAuthorizer.Create(AUDIENCE, Url, 'mcp', 's3cret');
-  var Active := Decide(Authorizer, 'good');
+  const Authorizer: IMCPAuthorizer = TMCPIntrospectionAuthorizer.Create(Audience, Url, IntrospectionClientId, 's3cret');
+  const Active = Decide(Authorizer, ActiveToken);
   Assert.AreEqual(TMCPAuthDecision.Allow, Active.Decision);
   Assert.AreEqual('alice', Active.Principal.Subject);
   Assert.IsTrue(Active.Principal.HasScope('read'));
@@ -322,4 +351,18 @@ procedure TAuthorizationTests.Introspection_PostsTokenWithClientCredentials;
   Assert.AreEqual('invalid_token', Stale.Challenge.Error);
 end;
 
+procedure TAuthorizationTests.Introspection_LongClientSecret_SendsBasicHeaderOnOneLine;
+begin
+  FSeenAuthorization := '';
+  StartIntrospectionServer;
+  const Url = IntrospectionUrl;
+  const LongSecret = StringOfChar('x', LongSecretLength);
+  const Authorizer: IMCPAuthorizer = TMCPIntrospectionAuthorizer.Create(Audience, Url, IntrospectionClientId, LongSecret);
+
+  const Active = Decide(Authorizer, ActiveToken);
+
+  Assert.AreEqual(LongSecretAuthorization, FSeenAuthorization, False);
+  Assert.AreEqual(TMCPAuthDecision.Allow, Active.Decision);
+end;
+
 end.