From e94f4e769c7c17bd59e712f1c4c2c6558ff1769c Mon Sep 17 00:00:00 2001
From: Joe DeCock <josephdecock@gmail.com>
Date: Wed, 5 Jul 2023 17:22:19 -0500
Subject: [PATCH] Return 500 error from incorrectly configured yarp endpoints

---
 .../AccessTokenTransformProvider.cs           | 20 ++++++++++++++++---
 src/Duende.Bff/General/Log.cs                 | 10 ++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/src/Duende.Bff.Yarp/AccessTokenTransformProvider.cs b/src/Duende.Bff.Yarp/AccessTokenTransformProvider.cs
index 4e000813..30f31089 100644
--- a/src/Duende.Bff.Yarp/AccessTokenTransformProvider.cs
+++ b/src/Duende.Bff.Yarp/AccessTokenTransformProvider.cs
@@ -5,7 +5,9 @@
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
+using System.Threading.Tasks;
 using Duende.AccessTokenManagement;
+using Duende.Bff.Logging;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Yarp.ReverseProxy.Transforms;
@@ -19,6 +21,7 @@ namespace Duende.Bff.Yarp;
 public class AccessTokenTransformProvider : ITransformProvider
 {
     private readonly BffOptions _options;
+    private readonly ILogger<AccessTokenTransformProvider> _logger;
     private readonly ILoggerFactory _loggerFactory;
     private readonly IDPoPProofService _dPoPProofService;
 
@@ -26,11 +29,13 @@ public class AccessTokenTransformProvider : ITransformProvider
     /// ctor
     /// </summary>
     /// <param name="options"></param>
+    /// <param name="logger"></param>
     /// <param name="loggerFactory"></param>
     /// <param name="dPoPProofService"></param>
-    public AccessTokenTransformProvider(IOptions<BffOptions> options, ILoggerFactory loggerFactory, IDPoPProofService dPoPProofService)
+    public AccessTokenTransformProvider(IOptions<BffOptions> options, ILogger<AccessTokenTransformProvider> logger, ILoggerFactory loggerFactory, IDPoPProofService dPoPProofService)
     {
         _options = options.Value;
+        _logger = logger;
         _loggerFactory = loggerFactory;
         _dPoPProofService = dPoPProofService;
     }
@@ -78,10 +83,19 @@ public void Apply(TransformBuilderContext transformBuildContext)
         bool optional;
         if(GetMetadataValue(transformBuildContext, Constants.Yarp.OptionalUserTokenMetadata, out var optionalTokenMetadata))
         {
+            if (GetMetadataValue(transformBuildContext, Constants.Yarp.TokenTypeMetadata, out var tokenTypeMetadata))
+            {
+                transformBuildContext.AddRequestTransform(ctx =>
+                {
+                    ctx.HttpContext.Response.StatusCode = 500;
+                    _logger.InvalidRouteConfiguration(transformBuildContext.Route.ClusterId, transformBuildContext.Route.RouteId);
+
+                    return ValueTask.CompletedTask;
+                });
+                return;
+            }
             optional = true;
             tokenType = TokenType.User;
-            // TODO - is it an error to set both OptionalUserToken and a token type? I think yes, because setting a token type means
-            // setting a *required* token type.
         } 
         else if (GetMetadataValue(transformBuildContext, Constants.Yarp.TokenTypeMetadata, out var tokenTypeMetadata))
         {
diff --git a/src/Duende.Bff/General/Log.cs b/src/Duende.Bff/General/Log.cs
index d526cc47..d8049bb2 100644
--- a/src/Duende.Bff/General/Log.cs
+++ b/src/Duende.Bff/General/Log.cs
@@ -18,6 +18,7 @@ internal static class EventIds
     public static readonly EventId BackChannelLogout = new (2, "BackChannelLogout");
     public static readonly EventId BackChannelLogoutError = new (3, "BackChannelLogoutError");
     public static readonly EventId AccessTokenMissing = new (4, "AccessTokenMissing");
+    public static readonly EventId InvalidRouteConfiguration = new (5, "InvalidRouteConfiguration");
 }
     
 internal static class Log
@@ -42,6 +43,10 @@ internal static class Log
         EventIds.AccessTokenMissing,
         "Access token is missing. token type: '{tokenType}', local path: '{localpath}', detail: '{detail}'");
 
+    private static readonly Action<ILogger, string, string, Exception?> _invalidRouteConfiguration = LoggerMessage.Define<string, string>(
+        LogLevel.Warning,
+        EventIds.InvalidRouteConfiguration,
+        "Invalid route configuration. Cannot combine a required access token (a call to WithAccessToken) and an optional access token (a call to WithOptionalUserAccessToken). clusterId: '{clusterId}', routeId: '{routeId}'");
 
     public static void AntiForgeryValidationFailed(this ILogger logger, string localPath)
     {
@@ -62,4 +67,9 @@ public static void AccessTokenMissing(this ILogger logger, string tokenType, str
     {
         _accessTokenMissing(logger, tokenType, localPath, detail, null);
     }
+
+    public static void InvalidRouteConfiguration(this ILogger logger, string? clusterId, string routeId)
+    {
+        _invalidRouteConfiguration(logger, clusterId ?? "no cluster id", routeId, null);
+    }
 }
\ No newline at end of file