Add caller IP address to audit log. (#606)

Add caller IP address to audit log.
microsoft · Aug 14, 2019 · b8d124c · b8d124c
1 parent fd031c1
commit b8d124c
Show file tree

Hide file tree

Showing 29 changed files with 368 additions and 364 deletions.
diff --git a/src/Microsoft.Health.Fhir.Api/Controllers/AadSmartOnFhirClaimsExtractor.cs b/src/Microsoft.Health.Fhir.Api/Controllers/AadSmartOnFhirClaimsExtractor.cs
@@ -0,0 +1,39 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Linq;
+using EnsureThat;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Primitives;
+using Microsoft.Health.Fhir.Core.Features.Security;
+
+namespace Microsoft.Health.Fhir.Api.Controllers
+{
+    internal class AadSmartOnFhirClaimsExtractor : IClaimsExtractor
+    {
+        private const string ClientId = "client_id";
+
+        private readonly IHttpContextAccessor _httpContextAccessor;
+
+        public AadSmartOnFhirClaimsExtractor(IHttpContextAccessor httpContextAccessor)
+        {
+            EnsureArg.IsNotNull(httpContextAccessor, nameof(httpContextAccessor));
+
+            _httpContextAccessor = httpContextAccessor;
+        }
+
+        public IReadOnlyCollection<KeyValuePair<string, string>> Extract()
+        {
+            HttpContext context = _httpContextAccessor.HttpContext;
+
+            StringValues clientId = context.Request.HasFormContentType ? context.Request.Form[ClientId] : context.Request.Query[ClientId];
+
+            ReadOnlyCollection<KeyValuePair<string, string>> claims = clientId.Select(x => new KeyValuePair<string, string>(ClientId, x)).ToList().AsReadOnly();
+            return claims;
+        }
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Api/Controllers/AadSmartOnFhirProxyAuditLoggingFilterAttribute.cs b/src/Microsoft.Health.Fhir.Api/Controllers/AadSmartOnFhirProxyAuditLoggingFilterAttribute.cs
@@ -0,0 +1,21 @@
+// -------------------------------------------------------------------------------------------------
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
+// -------------------------------------------------------------------------------------------------
+
+using System;
+using Microsoft.Health.Fhir.Api.Features.Audit;
+
+namespace Microsoft.Health.Fhir.Api.Controllers
+{
+    [AttributeUsage(AttributeTargets.Class)]
+    internal class AadSmartOnFhirProxyAuditLoggingFilterAttribute : AuditLoggingFilterAttribute
+    {
+        public AadSmartOnFhirProxyAuditLoggingFilterAttribute(
+           AadSmartOnFhirClaimsExtractor claimsExtractor,
+           IAuditHelper auditHelper)
+            : base(claimsExtractor, auditHelper)
+        {
+        }
+    }
+}
diff --git a/src/Microsoft.Health.Fhir.Api/Controllers/AadSmartOnFhirProxyController.cs b/src/Microsoft.Health.Fhir.Api/Controllers/AadSmartOnFhirProxyController.cs
@@ -15,7 +15,6 @@
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
-using Microsoft.Health.Fhir.Api.Features.ActionResults;
 using Microsoft.Health.Fhir.Api.Features.Audit;
 using Microsoft.Health.Fhir.Api.Features.Exceptions;
 using Microsoft.Health.Fhir.Api.Features.Filters;
@@ -32,7 +31,7 @@ namespace Microsoft.Health.Fhir.Api.Controllers
     /// <summary>
     /// Controller class enabling Azure Active Directory SMART on FHIR Proxy Capability
     /// </summary>
-    [TypeFilter(typeof(AadSmartOnFhirProxyFeatureFilterAttribute))]
+    [ServiceFilter(typeof(AadSmartOnFhirProxyAuditLoggingFilterAttribute))]
     [TypeFilter(typeof(AadSmartOnFhirProxyExceptionFilterAttribute))]
     [Route("AadSmartOnFhirProxy")]
     public class AadSmartOnFhirProxyController : Controller
@@ -112,7 +111,7 @@ public AadSmartOnFhirProxyController(IOptions<SecurityConfiguration> securityCon
         /// <param name="state">state URL parameter.</param>
         /// <param name="aud">aud (audience) URL parameter.</param>
         [HttpGet]
-        [TypeFilter(typeof(SmartOnFhirAuditLoggingFilterAttribute), Arguments = new object[] { AuditEventSubType.SmartOnFhirAuthorize })]
+        [AuditEventType(AuditEventSubType.SmartOnFhirAuthorize)]
         [Route("authorize", Name = RouteNames.AadSmartOnFhirProxyAuthorize)]
         public ActionResult Authorize(
             [FromQuery(Name = "response_type")] string responseType,
@@ -205,7 +204,7 @@ public ActionResult Authorize(
         /// <param name="error">error URL parameter.</param>
         /// <param name="errorDescription">error_description URL parameter.</param>
         [HttpGet]
-        [TypeFilter(typeof(SmartOnFhirAuditLoggingFilterAttribute), Arguments = new object[] { AuditEventSubType.SmartOnFhirCallback })]
+        [AuditEventType(AuditEventSubType.SmartOnFhirCallback)]
         [Route("callback/{encodedRedirect}", Name = RouteNames.AadSmartOnFhirProxyCallback)]
         public ActionResult Callback(
             string encodedRedirect,
@@ -280,7 +279,7 @@ public ActionResult Callback(
         /// <param name="clientId">client_id request parameter.</param>
         /// <param name="clientSecret">client_secret request parameter.</param>
         [HttpPost]
-        [TypeFilter(typeof(SmartOnFhirAuditLoggingFilterAttribute), Arguments = new object[] { AuditEventSubType.SmartOnFhirToken })]
+        [AuditEventType(AuditEventSubType.SmartOnFhirToken)]
         [Route("token", Name = RouteNames.AadSmartOnFhirProxyToken)]
         public async Task<ActionResult> Token(
             [FromForm(Name = "grant_type")] string grantType,

diff --git a/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditHelper.cs b/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditHelper.cs
@@ -9,6 +9,7 @@
 using System.Net;
 using EnsureThat;
 using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc.Controllers;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.Extensions.Logging;
@@ -26,7 +27,6 @@ public class AuditHelper : IAuditHelper, IStartable
     {
         private readonly IActionDescriptorCollectionProvider _actionDescriptorCollectionProvider;
         private readonly IFhirRequestContextAccessor _fhirRequestContextAccessor;
-        private readonly IClaimsExtractor _claimsExtractor;
         private readonly IAuditLogger _auditLogger;
         private readonly ILogger<AuditHelper> _logger;
 
@@ -35,19 +35,16 @@ public class AuditHelper : IAuditHelper, IStartable
         public AuditHelper(
             IActionDescriptorCollectionProvider actionDescriptorCollectionProvider,
             IFhirRequestContextAccessor fhirRequestContextAccessor,
-            IClaimsExtractor claimsExtractor,
             IAuditLogger auditLogger,
             ILogger<AuditHelper> logger)
         {
             EnsureArg.IsNotNull(actionDescriptorCollectionProvider, nameof(actionDescriptorCollectionProvider));
             EnsureArg.IsNotNull(fhirRequestContextAccessor, nameof(fhirRequestContextAccessor));
-            EnsureArg.IsNotNull(claimsExtractor, nameof(claimsExtractor));
             EnsureArg.IsNotNull(auditLogger, nameof(auditLogger));
             EnsureArg.IsNotNull(logger, nameof(logger));
 
             _actionDescriptorCollectionProvider = actionDescriptorCollectionProvider;
             _fhirRequestContextAccessor = fhirRequestContextAccessor;
-            _claimsExtractor = claimsExtractor;
             _auditLogger = auditLogger;
             _logger = logger;
         }
@@ -66,15 +63,21 @@ public string GetAuditEventType(string controllerName, string actionName)
         }
 
         /// <inheritdoc />
-        public void LogExecuting(string controllerName, string actionName)
+        public void LogExecuting(string controllerName, string actionName, HttpContext httpContext, IClaimsExtractor claimsExtractor)
         {
-            Log(AuditAction.Executing, controllerName, actionName, statusCode: null, resourceType: null);
+            EnsureArg.IsNotNull(claimsExtractor, nameof(claimsExtractor));
+            EnsureArg.IsNotNull(httpContext, nameof(httpContext));
+
+            Log(AuditAction.Executing, controllerName, actionName, statusCode: null, resourceType: null, httpContext, claimsExtractor);
         }
 
         /// <inheritdoc />
-        public void LogExecuted(string controllerName, string actionName, HttpStatusCode statusCode, string responseResultType)
+        public void LogExecuted(string controllerName, string actionName, string responseResultType, HttpContext httpContext, IClaimsExtractor claimsExtractor)
         {
-            Log(AuditAction.Executed, controllerName, actionName, statusCode, responseResultType);
+            EnsureArg.IsNotNull(claimsExtractor, nameof(claimsExtractor));
+            EnsureArg.IsNotNull(httpContext, nameof(httpContext));
+
+            Log(AuditAction.Executed, controllerName, actionName, (HttpStatusCode)httpContext.Response.StatusCode, responseResultType, httpContext, claimsExtractor);
         }
 
         void IStartable.Start()
@@ -104,7 +107,7 @@ private Attribute GetAttribute(string controllerName, string actionName)
             throw new AuditException(controllerName, actionName);
         }
 
-        private void Log(AuditAction auditAction, string controllerName, string actionName, HttpStatusCode? statusCode, string resourceType)
+        private void Log(AuditAction auditAction, string controllerName, string actionName, HttpStatusCode? statusCode, string resourceType, HttpContext httpContext, IClaimsExtractor claimsExtractor)
         {
             Attribute attribute = GetAttribute(controllerName, actionName);
 
@@ -119,12 +122,13 @@ private void Log(AuditAction auditAction, string controllerName, string actionNa
 
                 _auditLogger.LogAudit(
                     auditAction,
-                    action: auditEventTypeAttribute.AuditEventType,
+                    operation: auditEventTypeAttribute.AuditEventType,
                     resourceType: resourceType,
                     requestUri: fhirRequestContext.Uri,
                     statusCode: statusCode,
                     correlationId: fhirRequestContext.CorrelationId,
-                    claims: _claimsExtractor.Extract());
+                    callerIpAddress: httpContext.Connection?.RemoteIpAddress?.ToString(),
+                    callerClaims: claimsExtractor.Extract());
             }
         }
     }

diff --git a/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditLogger.cs b/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditLogger.cs
@@ -14,9 +14,12 @@
 
 namespace Microsoft.Health.Fhir.Api.Features.Audit
 {
+    /// <summary>
+    /// Provides mechanism to log the audit event using default logger.
+    /// </summary>
     public class AuditLogger : IAuditLogger
     {
-        internal const string AuditEventType = "AuditEvent";
+        private const string AuditEventType = "AuditEvent";
 
         private static readonly string AuditMessageFormat =
             "ActionType: {ActionType}" + Environment.NewLine +
@@ -28,34 +31,39 @@ public class AuditLogger : IAuditLogger
             "Action: {Action}" + Environment.NewLine +
             "StatusCode: {StatusCode}" + Environment.NewLine +
             "CorrelationId: {CorrelationId}" + Environment.NewLine +
+            "CallerIPAddress: {CallerIPAddress}" + Environment.NewLine +
             "Claims: {Claims}";
 
-        private readonly ILogger<IAuditLogger> _logger;
         private readonly SecurityConfiguration _securityConfiguration;
+        private readonly ILogger<IAuditLogger> _logger;
 
-        public AuditLogger(ILogger<IAuditLogger> logger, IOptions<SecurityConfiguration> securityConfiguration)
+        public AuditLogger(
+            IOptions<SecurityConfiguration> securityConfiguration,
+            ILogger<IAuditLogger> logger)
         {
-            EnsureArg.IsNotNull(logger, nameof(logger));
             EnsureArg.IsNotNull(securityConfiguration?.Value, nameof(securityConfiguration));
+            EnsureArg.IsNotNull(logger, nameof(logger));
 
-            _logger = logger;
             _securityConfiguration = securityConfiguration.Value;
+            _logger = logger;
         }
 
+        /// <inheritdoc />
         public void LogAudit(
             AuditAction auditAction,
             string action,
             string resourceType,
             Uri requestUri,
             HttpStatusCode? statusCode,
             string correlationId,
-            IReadOnlyCollection<KeyValuePair<string, string>> claims)
+            string callerIpAddress,
+            IReadOnlyCollection<KeyValuePair<string, string>> callerClaims)
         {
             string claimsInString = null;
 
-            if (claims != null)
+            if (callerClaims != null)
             {
-                claimsInString = string.Join(";", claims.Select(claim => $"{claim.Key}={claim.Value}"));
+                claimsInString = string.Join(";", callerClaims.Select(claim => $"{claim.Key}={claim.Value}"));
             }
 
             _logger.LogInformation(
@@ -69,6 +77,7 @@ public void LogAudit(
                 action,
                 statusCode,
                 correlationId,
+                callerIpAddress,
                 claimsInString);
         }
     }

diff --git a/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditLoggingFilterAttribute.cs b/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditLoggingFilterAttribute.cs
@@ -5,23 +5,26 @@
 
 using System;
 using System.Diagnostics;
-using System.Net;
 using EnsureThat;
 using Microsoft.AspNetCore.Mvc.Controllers;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.Health.Fhir.Api.Features.ActionResults;
+using Microsoft.Health.Fhir.Core.Features.Security;
 
 namespace Microsoft.Health.Fhir.Api.Features.Audit
 {
     [AttributeUsage(AttributeTargets.Class)]
     internal class AuditLoggingFilterAttribute : ActionFilterAttribute
     {
+        private readonly IClaimsExtractor _claimsExtractor;
         private readonly IAuditHelper _auditHelper;
 
-        public AuditLoggingFilterAttribute(IAuditHelper auditHelper)
+        public AuditLoggingFilterAttribute(IClaimsExtractor claimsExtractor, IAuditHelper auditHelper)
         {
+            EnsureArg.IsNotNull(claimsExtractor, nameof(claimsExtractor));
             EnsureArg.IsNotNull(auditHelper, nameof(auditHelper));
 
+            _claimsExtractor = claimsExtractor;
             _auditHelper = auditHelper;
         }
 
@@ -35,7 +38,7 @@ public override void OnActionExecuting(ActionExecutingContext context)
 
             if (actionDescriptor != null)
             {
-                _auditHelper.LogExecuting(actionDescriptor.ControllerName, actionDescriptor.ActionName);
+                _auditHelper.LogExecuting(actionDescriptor.ControllerName, actionDescriptor.ActionName, context.HttpContext, _claimsExtractor);
             }
 
             base.OnActionExecuting(context);
@@ -55,8 +58,9 @@ public override void OnResultExecuted(ResultExecutedContext context)
             _auditHelper.LogExecuted(
                 actionDescriptor.ControllerName,
                 actionDescriptor.ActionName,
-                (HttpStatusCode)context.HttpContext.Response.StatusCode,
-                result?.GetResultTypeName());
+                result?.GetResultTypeName(),
+                context.HttpContext,
+                _claimsExtractor);
 
             base.OnResultExecuted(context);
         }

diff --git a/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditMiddleware.cs b/src/Microsoft.Health.Fhir.Api/Features/Audit/AuditMiddleware.cs
@@ -10,6 +10,7 @@
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Health.Fhir.Api.Features.Routing;
 using Microsoft.Health.Fhir.Core.Features.Context;
+using Microsoft.Health.Fhir.Core.Features.Security;
 
 namespace Microsoft.Health.Fhir.Api.Features.Audit
 {
@@ -20,19 +21,23 @@ public class AuditMiddleware
     {
         private readonly RequestDelegate _next;
         private readonly IFhirRequestContextAccessor _fhirRequestContextAccessor;
+        private readonly IClaimsExtractor _claimsExtractor;
         private readonly IAuditHelper _auditHelper;
 
         public AuditMiddleware(
             RequestDelegate next,
             IFhirRequestContextAccessor fhirRequestContextAccessor,
+            IClaimsExtractor claimsExtractor,
             IAuditHelper auditHelper)
         {
             EnsureArg.IsNotNull(next, nameof(next));
             EnsureArg.IsNotNull(fhirRequestContextAccessor, nameof(fhirRequestContextAccessor));
+            EnsureArg.IsNotNull(claimsExtractor, nameof(claimsExtractor));
             EnsureArg.IsNotNull(auditHelper, nameof(auditHelper));
 
             _next = next;
             _fhirRequestContextAccessor = fhirRequestContextAccessor;
+            _claimsExtractor = claimsExtractor;
             _auditHelper = auditHelper;
         }
 
@@ -61,8 +66,9 @@ public async Task Invoke(HttpContext context)
                     _auditHelper.LogExecuted(
                         controllerName?.ToString(),
                         actionName?.ToString(),
-                        statusCode,
-                        resourceType?.ToString());
+                        resourceType?.ToString(),
+                        context,
+                        _claimsExtractor);
                 }
             }
         }

diff --git a/src/Microsoft.Health.Fhir.Api/Features/Audit/IAuditHelper.cs b/src/Microsoft.Health.Fhir.Api/Features/Audit/IAuditHelper.cs
@@ -3,7 +3,8 @@
 // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
 // -------------------------------------------------------------------------------------------------
 
-using System.Net;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Health.Fhir.Core.Features.Security;
 
 namespace Microsoft.Health.Fhir.Api.Features.Audit
 {
@@ -26,15 +27,18 @@ public interface IAuditHelper
         /// </summary>
         /// <param name="controllerName">The controller name.</param>
         /// <param name="actionName">The action name.</param>
-        void LogExecuting(string controllerName, string actionName);
+        /// <param name="httpContext">The HTTP context.</param>
+        /// <param name="claimsExtractor">The extractor used to extract claims.</param>
+        void LogExecuting(string controllerName, string actionName, HttpContext httpContext, IClaimsExtractor claimsExtractor);
 
         /// <summary>
         /// Logs an audit entry for executed the <paramref name="controllerName"/> and <paramref name="actionName"/>.
         /// </summary>
         /// <param name="controllerName">The controller name.</param>
         /// <param name="actionName">The action name.</param>
-        /// <param name="statusCode">The HTTP status code.</param>
         /// <param name="responseResultType">The optional response result type.</param>
-        void LogExecuted(string controllerName, string actionName, HttpStatusCode statusCode, string responseResultType);
+        /// <param name="httpContext">The HTTP context.</param>
+        /// <param name="claimsExtractor">The extractor used to extract claims.</param>
+        void LogExecuted(string controllerName, string actionName, string responseResultType, HttpContext httpContext, IClaimsExtractor claimsExtractor);
     }
 }