GFlisch / Arc4u

Apache License 2.0
23 stars 18 forks source link

Arc4u 8+: incorrect assumption in AddContextToPrincipalMiddleware causes crash in integration tests #124

Open vvdb-architecture opened 3 months ago

vvdb-architecture commented 3 months ago

The code in AddContextToPrincipalMiddleware contains the following:

     // Ensure we have a traceparent.
            if (!context.Request.Headers.ContainsKey("traceparent"))
            {
                // Activity.Current is not null just because we are in the context of an activity.
                context.Request.Headers.Add("traceparent", Activity.Current!.Id);
            }

            activity?.SetTag(LoggingConstants.ActivityId, Activity.Current!.Id);

Note the comment: "Activity.Current is not null just because we are in the context of an activity.".

This is not correct. Activity.Current can be null even if you are within an activity. This can be the case because at least one of the two following conditions are satisfied:

This can be the case during integration testing, when you are building your own WebApplicationFactory. Such tests will fail like this:

Message: 
  ....RestException : The HTTP status code of the response was not expected (500).

  Status: 500
  Response: 
  System.NullReferenceException: Object reference not set to an instance of an object.
     at Arc4u.OAuth2.Middleware.AddContextToPrincipalMiddleware.Invoke(HttpContext context) in /_/src/Arc4u.Standard.OAuth2.AspNetCore/Middleware/AddContextToPrincipalMiddleware.cs:line 40
     at Arc4u.OAuth2.Middleware.ValidateResourcesRightMiddleware.Invoke(HttpContext context) in /_/src/Arc4u.Standard.OAuth2.AspNetCore/Middleware/ValidateResourcesRightMiddleware.cs:line 40
     at Microsoft.AspNetCore.Authorization.Authori

Stack Trace: 
    ... (stack trace of actual test)
  --- End of stack trace from previous location ---

The System.NullReferenceException: Object reference not set to an instance of an object. corresponds to the line where Activity.Current!.Id is accessed but fails because Activity.Current is null.

The above code should be guarded as follows:

if(Activity.Current is not null)
{
     // Ensure we have a traceparent.
     if (!context.Request.Headers.ContainsKey("traceparent"))
     {
                context.Request.Headers.Add("traceparent", Activity.Current!.Id);
     }

     activity?.SetTag(LoggingConstants.ActivityId, Activity.Current!.Id);
}