[Bug]: Authentication problem with 6.6.2

[kenwonders]

Describe the bug

Upgrading from 6.50 to 6.6.2 caused our authentication system to stop working. We have a separate identity server using client credentials and with 6.6.2 we are always getting the error:

Reverting just Swashbuckle.AspNetCore back to 6.5.0 resolves the issue.

Our .AddSwaggerGen code:

                c.AddSecurityDefinition("Bearer", new OpenApiSecurityScheme
                {
                    Type = SecuritySchemeType.OAuth2,
                    Flows = new OpenApiOAuthFlows
                    {
                        ClientCredentials = new OpenApiOAuthFlow
                        {
                            TokenUrl = new Uri(Configuration["Identity:Authority"] + "/connect/token"),
                            Scopes = new Dictionary<string, string>
                            {
                                { "***.User", "" },
                                { "***.Admin", "" }
                            },
                            AuthorizationUrl = new Uri(Configuration["Identity:Authority"] + "/oauth2/authorize"),
                        }
                    }
                });
                c.AddSecurityRequirement(new OpenApiSecurityRequirement
                {
                    {
                        new OpenApiSecurityScheme
                        {
                            Reference = new OpenApiReference
                            {
                                Id = "Bearer",
                                Type = ReferenceType.SecurityScheme
                            }
                        },
                        new List<string>()
                    }
                });

            services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
                .AddJwtBearer(o =>
                {                    
                    o.Authority = Configuration["Identity:Authority"];
                    o.RequireHttpsMetadata = bool.Parse(Configuration["Identity:RequireHttpsMetadata"]);
                    o.Audience = Configuration["Identity:Audience"];
                    o.TokenValidationParameters = new TokenValidationParameters { ValidateAudience = bool.Parse(Configuration["Identity:ValidateAudience"]) };                                        
                });

            services.AddAuthorization(options =>
            {
                options.AddPolicy("***", policy => policy.RequireClaim("client_Role", "***", "***"));
                options.AddPolicy("***", policy => policy.RequireClaim("client_Role", "***"));
            });

In our UseSwaggerUI:

                C.UseRequestInterceptor("" +
                    "(req) => { if (req.url.endsWith('connect/token') && req.body)" +
                    " req.body += '&client_id=' + client_id.value + '&client_secret=' + client_secret.value; return req; }");

Expected behavior

No response

Actual behavior

No response

Steps to reproduce

No response

Exception(s) (if any)

No response

Swashbuckle.AspNetCore version

6.6.2

.NET Version

NET8

Anything else?

If there's any other information you need please let me know.

[martincostello]

Is it possible for you to share what the rendered JavaScript in the HTML page looks like please?

[kenwonders]

Thank you hugely for helping. Here it is:

<script>
        /* Source: https://gist.github.com/lamberta/3768814
         * Parse a string function definition and return a function object. Does not use eval.
         * @param {string} str
         * @return {function}
         *
         * Example:
         *  var f = function (x, y) { return x * y; };
         *  var g = parseFunction(f.toString());
         *  g(33, 3); //=> 99
         */
        function parseFunction(str) {
            if (!str) return void (0);

            var fn_body_idx = str.indexOf('{'),
                fn_body = str.substring(fn_body_idx + 1, str.lastIndexOf('}')),
                fn_declare = str.substring(0, fn_body_idx),
                fn_params = fn_declare.substring(fn_declare.indexOf('(') + 1, fn_declare.lastIndexOf(')')),
                args = fn_params.split(',');

            args.push(fn_body);

            function Fn() {
                return Function.apply(this, args);
            }
            Fn.prototype = Function.prototype;

            return new Fn();
        }

        window.onload = function () {
            var configObject = JSON.parse('{"urls":[{"url":"/swagger/v1/swagger.json","name":"* v1"}],"deepLinking":false,"persistAuthorization":false,"displayOperationId":false,"defaultModelsExpandDepth":1,"defaultModelExpandDepth":1,"defaultModelRendering":"example","displayRequestDuration":false,"docExpansion":"list","showExtensions":false,"showCommonExtensions":false,"supportedSubmitMethods":["get","put","post","delete","options","head","patch","trace"],"tryItOutEnabled":false}');
            var oauthConfigObject = JSON.parse('{"scopeSeparator":" ","scopes":[],"useBasicAuthenticationWithAccessCodeGrant":false,"usePkceWithAuthorizationCodeGrant":false}');

            // Workaround for https://github.com/swagger-api/swagger-ui/issues/5945
            configObject.urls.forEach(function (item) {
                if (item.url.startsWith("http") || item.url.startsWith("/")) return;
                item.url = window.location.href.replace("index.html", item.url).split('#')[0];
            });

            // If validatorUrl is not explicitly provided, disable the feature by setting to null
            if (!configObject.hasOwnProperty("validatorUrl"))
                configObject.validatorUrl = null

            // If oauth2RedirectUrl isn't specified, use the built-in default
            if (!configObject.hasOwnProperty("oauth2RedirectUrl"))
                configObject.oauth2RedirectUrl = (new URL("oauth2-redirect.html", window.location.href)).href;

            // Apply mandatory parameters
            configObject.dom_id = "#swagger-ui";
            configObject.presets = [SwaggerUIBundle.presets.apis, SwaggerUIStandalonePreset];
            configObject.layout = "StandaloneLayout";

            // Parse and add interceptor functions
            var interceptors = JSON.parse('{"RequestInterceptorFunction":"(req) =\u003E { if (req.url.endsWith(\u0027connect/token\u0027) \u0026\u0026 req.body) req.body \u002B= \u0027\u0026client_id=\u0027 \u002B client_id.value \u002B \u0027\u0026client_secret=\u0027 \u002B client_secret.value; return req; }"}');
            if (interceptors.RequestInterceptorFunction)
                configObject.requestInterceptor = parseFunction(interceptors.RequestInterceptorFunction);
            if (interceptors.ResponseInterceptorFunction)
                configObject.responseInterceptor = parseFunction(interceptors.ResponseInterceptorFunction);

            // Begin Swagger UI call region

            const ui = SwaggerUIBundle(configObject);

            ui.initOAuth(oauthConfigObject);

            // End Swagger UI call region

            window.ui = ui
        }
    </script>

[martincostello]

Thanks - it looks like the changes I made to use System.Text.Json to support native AoT have broken the rendering here:

var interceptors = JSON.parse('{"RequestInterceptorFunction":"(req) =\u003E { if (req.url.endsWith(\u0027connect/token\u0027) \u0026\u0026 req.body) req.body \u002B= \u0027\u0026client_id=\u0027 \u002B client_id.value \u002B \u0027\u0026client_secret=\u0027 \u002B client_secret.value; return req; }"}');

[kenwonders]

Really appreciate you for looking into it, than you. We'll stay on 6.5.0 as it's not problem to do so, I was just keeping our versions up to date.

[martincostello]

I notice we don't seem to have any test coverage for this area, so that's something we'll also need to sort out when this is fixed.

To help validate my hunch on what the issue is, if you explicitly set the following as the value of SwaggerUIOptions.JsonSerializerOptions does your code work as expected again?

Ignore, doesn't work.

app.UseSwaggerUI(c =>
{
    // Your configuration, then:
    c.JsonSerializerOptions = new()
    {
        Converters = { new JsonStringEnumConverter(JsonNamingPolicy.CamelCase, false) },
        Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
        PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
    };
});

Edit: nevermind, that isn't the fix or reason.

[martincostello]

I've found the reason for this issue and it both is and isn't our fault 😄

In swagger-ui-dist@v4.15.5, which is included with v6.5.0, the name of the HTML element highlighted in red below has an client_id, which is in the scope of the closure were the interceptor function runs:

<input id="client_id" type="text" data-name="clientId">

In swagger-ui-dist@5.17.10, which is included with v6.6.2, the element has a different ID of client_id_authorizationCode:

<input id="client_id_authorizationCode" type="text" data-name="clientId">

client_secret was similarly renamed to client_secret_authorizationCode.

If I change your interceptor function like below to use client_id_authorizationCode, then things work as expected:

c.UseRequestInterceptor(
    "" +
    "(req) => {" +
    "  if (req.url.endsWith('connect/token') && req.body) {" +
    "    req.body += '&client_id=' + client_id_authorizationCode.value + '&client_secret=' + client_secret_authorizationCode.value;" +
    "    return req;" +
    "  }" +
    "}");

So this is our fault in one way because we updated swagger-ui-dist to v5, but it also has two other contributing factors.

The first is that swagger-ui did some UI refactoring and renamed the page element. I don't know if this is considered part of their public API surface area or not, but at face value it looks like an implementation detail to me that isn't intended to be something that's relied on. If it is part of their public surface area and covered by the v5 breaking changes, this feels quite out of scope of the sort of things we should be expected to find/detect/react-to within the library.

We did test that there was nothing obviously breaking with the update from v4 to v5 and didn't find anything, which is why it seemed safe to include in v6.6 (rather than needing it be part of a v7 as it was a major update).

If it isn't part of their public API surface area, then in that case the second issue is that you depended on an implementation detail of Swagger UI to get the client ID in your interceptor function.

From looking at req in the Chrome Developer Tools, the JSON equivalent of the value appears to be something like this:

{
  "url":"/auth-server/connect/token",
  "method":"post",
  "headers":{"Accept":"application/json, text/plain, */*","Content-Type":"application/x-www-form-urlencoded","X-Requested-With":"XMLHttpRequest"},
"body":"grant_type=authorization_code&code=Skg5ZRCelpGhVkzSao1QGsdZno78PONgZikGhSt0KbA&client_id=test-id&client_secret=test-secret&redirect_uri=https%3A%2F%2Flocalhost%3A5001%2Fresource-server%2Fswagger%2Foauth2-redirect.html&code_verifier=xPn7nrFvaxiTu2Z2K1JcMtHNkWY3JW5KrJtaO3Toz3c"
}

The client ID and client secret appear to be already set, so I'm not sure why you need to set it.

I'm going to leave this issue open for now as I found a few things to tweak while investigating this, but for the original issue you need to either remove your interceptor (because the client ID and secret appear to be set anyway), or to use the new input values to access their values.

[martincostello]

See #2942 for the changes I mentioned, otherwise I think this is now resolved from our perspective.