From 13676aa0caa915161567787f13a88fda641b0558 Mon Sep 17 00:00:00 2001 From: Prajon Date: Wed, 14 Aug 2019 15:12:31 -0400 Subject: [PATCH] Removed Logging Part from the current PR as suggested. Logging will be done in follow up PR. --- .../MAuthAppBuilderExtensions.cs | 7 +----- .../MAuthMiddleware.cs | 5 ++-- src/Medidata.MAuth.Core/MAuthAuthenticator.cs | 15 ++---------- src/Medidata.MAuth.Core/UtilityExtensions.cs | 6 ++--- .../MAuthAppBuilderExtensions.cs | 8 +------ src/Medidata.MAuth.Owin/MAuthMiddleware.cs | 5 ++-- .../MAuthAuthenticatingHandler.cs | 15 ++++-------- .../Infrastructure/MAuthServerHandler.cs | 3 +-- .../MAuthAuthenticatorTests.cs | 23 +++++++++---------- .../UtilityExtensionsTest.cs | 3 +-- 10 files changed, 27 insertions(+), 63 deletions(-) diff --git a/src/Medidata.MAuth.AspNetCore/MAuthAppBuilderExtensions.cs b/src/Medidata.MAuth.AspNetCore/MAuthAppBuilderExtensions.cs index f242c4b..35c90a1 100644 --- a/src/Medidata.MAuth.AspNetCore/MAuthAppBuilderExtensions.cs +++ b/src/Medidata.MAuth.AspNetCore/MAuthAppBuilderExtensions.cs @@ -1,8 +1,5 @@ using System; using Microsoft.AspNetCore.Builder; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; namespace Medidata.MAuth.AspNetCore { @@ -29,10 +26,8 @@ public static IApplicationBuilder UseMAuthAuthentication(this IApplicationBuilde if (options == null) throw new ArgumentNullException(nameof(options)); - var loggerFactory = app.ApplicationServices.GetService() ?? - NullLoggerFactory.Instance; - return app.UseMiddleware(options, loggerFactory); + return app.UseMiddleware(options); } /// diff --git a/src/Medidata.MAuth.AspNetCore/MAuthMiddleware.cs b/src/Medidata.MAuth.AspNetCore/MAuthMiddleware.cs index 69f679d..0efdee7 100644 --- a/src/Medidata.MAuth.AspNetCore/MAuthMiddleware.cs +++ b/src/Medidata.MAuth.AspNetCore/MAuthMiddleware.cs @@ -3,7 +3,6 @@ using Medidata.MAuth.Core; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Internal; -using Microsoft.Extensions.Logging; namespace Medidata.MAuth.AspNetCore { @@ -13,11 +12,11 @@ internal class MAuthMiddleware private readonly MAuthAuthenticator authenticator; private readonly RequestDelegate next; - public MAuthMiddleware(RequestDelegate next, MAuthMiddlewareOptions options, ILoggerFactory loggerFactory) + public MAuthMiddleware(RequestDelegate next, MAuthMiddlewareOptions options) { this.next = next; this.options = options; - this.authenticator = new MAuthAuthenticator(options, loggerFactory); + this.authenticator = new MAuthAuthenticator(options); } public async Task Invoke(HttpContext context) diff --git a/src/Medidata.MAuth.Core/MAuthAuthenticator.cs b/src/Medidata.MAuth.Core/MAuthAuthenticator.cs index 31aa579..df5eb95 100644 --- a/src/Medidata.MAuth.Core/MAuthAuthenticator.cs +++ b/src/Medidata.MAuth.Core/MAuthAuthenticator.cs @@ -5,7 +5,6 @@ using Microsoft.Extensions.Caching.Memory; using Org.BouncyCastle.Crypto; using Medidata.MAuth.Core.Models; -using Microsoft.Extensions.Logging; namespace Medidata.MAuth.Core { @@ -13,11 +12,10 @@ internal class MAuthAuthenticator { private readonly MAuthOptionsBase options; private readonly IMemoryCache cache = new MemoryCache(new MemoryCacheOptions()); - private readonly ILogger _logger; public Guid ApplicationUuid => options.ApplicationUuid; - public MAuthAuthenticator(MAuthOptionsBase options, ILoggerFactory loggerFactory) + public MAuthAuthenticator(MAuthOptionsBase options) { if (options.ApplicationUuid == default(Guid)) throw new ArgumentException(nameof(options.ApplicationUuid)); @@ -29,8 +27,6 @@ public MAuthAuthenticator(MAuthOptionsBase options, ILoggerFactory loggerFactory throw new ArgumentNullException(nameof(options.PrivateKey)); this.options = options; - - this._logger = loggerFactory.CreateLogger(); } /// @@ -42,11 +38,8 @@ public async Task AuthenticateRequest(HttpRequestMessage request) { try { - _logger.LogInformation($"Initiating Authentication of request", request); var version = request.GetAuthHeaderValue().GetVersionFromAuthenticationHeader(); - _logger.LogInformation($"Authentication is for the request with {version} version."); - if (options.DisableV1 && version == MAuthVersion.MWS) throw new InvalidVersionException($"Authentication with {version} version is disabled."); @@ -59,29 +52,25 @@ public async Task AuthenticateRequest(HttpRequestMessage request) } catch (ArgumentException ex) { - _logger.LogError($"Unable to authenticate due to invalid MAuth authentication headers. Exception: {ex.Message}"); throw new AuthenticationException("The request has invalid MAuth authentication headers.", ex); } catch (RetriedRequestException ex) { - _logger.LogError($"Unable to query the application information from MAuth server. Exception:{ex.Message}"); throw new AuthenticationException( "Could not query the application information for the application from the MAuth server.", ex); } catch (InvalidCipherTextException ex) { - _logger.LogError($"Unable to authenticate due to invalid payload information. Exception: {ex.Message}"); + throw new AuthenticationException( "The request verification failed due to an invalid payload information.", ex); } catch (InvalidVersionException ex) { - _logger.LogError(ex, $"Unable to authenticate due to invalid version. Exception: {ex.Message}"); throw new InvalidVersionException(ex.Message, ex); } catch (Exception ex) { - _logger.LogError($"Unable to authenticate due to unexpected error. Exception: {ex.Message}"); throw new AuthenticationException( "An unexpected error occured during authentication. Please see the inner exception for details.", ex diff --git a/src/Medidata.MAuth.Core/UtilityExtensions.cs b/src/Medidata.MAuth.Core/UtilityExtensions.cs index bc7ee05..7fb474b 100644 --- a/src/Medidata.MAuth.Core/UtilityExtensions.cs +++ b/src/Medidata.MAuth.Core/UtilityExtensions.cs @@ -2,7 +2,6 @@ using System.Net.Http; using System.Threading.Tasks; using Medidata.MAuth.Core.Models; -using Microsoft.Extensions.Logging; namespace Medidata.MAuth.Core { @@ -66,11 +65,10 @@ public static bool TryParseAuthenticationHeader(this string headerValue, /// /// The request message to authenticate. /// The MAuth options to use for the authentication. - /// The logger factory used with authentication. /// The task for the operation that is when completes will result in if /// the authentication is successful; otherwise . - public static Task Authenticate(this HttpRequestMessage request, MAuthOptionsBase options, ILoggerFactory loggerFactory) => - new MAuthAuthenticator(options, loggerFactory).AuthenticateRequest(request); + public static Task Authenticate(this HttpRequestMessage request, MAuthOptionsBase options) => + new MAuthAuthenticator(options).AuthenticateRequest(request); /// /// Determines the MAuth version enumerator reading authHeader. diff --git a/src/Medidata.MAuth.Owin/MAuthAppBuilderExtensions.cs b/src/Medidata.MAuth.Owin/MAuthAppBuilderExtensions.cs index 8f4e596..5a9cd17 100644 --- a/src/Medidata.MAuth.Owin/MAuthAppBuilderExtensions.cs +++ b/src/Medidata.MAuth.Owin/MAuthAppBuilderExtensions.cs @@ -1,8 +1,5 @@ using System; -using System.Linq; -using Microsoft.Extensions.Logging.Abstractions; using Owin; -using Microsoft.Extensions.Logging; namespace Medidata.MAuth.Owin { @@ -28,10 +25,7 @@ public static IAppBuilder UseMAuthAuthentication(this IAppBuilder app, MAuthMidd if (options == null) throw new ArgumentNullException(nameof(options)); - var loggerFactory = (ILoggerFactory)app.Properties.Where(x => x.Key == "ILoggerFactory")? - .FirstOrDefault().Value ?? NullLoggerFactory.Instance; - - return app.Use(options, loggerFactory); + return app.Use(options); } /// diff --git a/src/Medidata.MAuth.Owin/MAuthMiddleware.cs b/src/Medidata.MAuth.Owin/MAuthMiddleware.cs index 702d837..3883f9e 100644 --- a/src/Medidata.MAuth.Owin/MAuthMiddleware.cs +++ b/src/Medidata.MAuth.Owin/MAuthMiddleware.cs @@ -1,7 +1,6 @@ using System.Net; using System.Threading.Tasks; using Medidata.MAuth.Core; -using Microsoft.Extensions.Logging; using Microsoft.Owin; namespace Medidata.MAuth.Owin @@ -11,10 +10,10 @@ internal class MAuthMiddleware: OwinMiddleware private readonly MAuthMiddlewareOptions options; private readonly MAuthAuthenticator authenticator; - public MAuthMiddleware(OwinMiddleware next, MAuthMiddlewareOptions options, ILoggerFactory loggerFactory): base(next) + public MAuthMiddleware(OwinMiddleware next, MAuthMiddlewareOptions options): base(next) { this.options = options; - authenticator = new MAuthAuthenticator(options, loggerFactory); + authenticator = new MAuthAuthenticator(options); } public override async Task Invoke(IOwinContext context) diff --git a/src/Medidata.MAuth.WebApi/MAuthAuthenticatingHandler.cs b/src/Medidata.MAuth.WebApi/MAuthAuthenticatingHandler.cs index 26c67c7..c652d69 100644 --- a/src/Medidata.MAuth.WebApi/MAuthAuthenticatingHandler.cs +++ b/src/Medidata.MAuth.WebApi/MAuthAuthenticatingHandler.cs @@ -4,8 +4,6 @@ using System.Threading; using System.Threading.Tasks; using Medidata.MAuth.Core; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; namespace Medidata.MAuth.WebApi { @@ -26,13 +24,11 @@ public class MAuthAuthenticatingHandler : DelegatingHandler /// . /// /// The options for this message handler. - /// The logger factory used by this message handler. - public MAuthAuthenticatingHandler(MAuthWebApiOptions options, ILoggerFactory loggerFactory = null) + public MAuthAuthenticatingHandler(MAuthWebApiOptions options) { this.options = options; - var loggerFact = loggerFactory ?? NullLoggerFactory.Instance; - authenticator = new MAuthAuthenticator(options, loggerFact); + authenticator = new MAuthAuthenticator(options); } /// @@ -43,13 +39,10 @@ public MAuthAuthenticatingHandler(MAuthWebApiOptions options, ILoggerFactory log /// /// The inner handler which is responsible for processing the HTTP response messages. /// - /// The logger factory used by this message handler. - public MAuthAuthenticatingHandler(MAuthWebApiOptions options, HttpMessageHandler innerHandler, - ILoggerFactory loggerFactory = null) : base(innerHandler) + public MAuthAuthenticatingHandler(MAuthWebApiOptions options, HttpMessageHandler innerHandler) : base(innerHandler) { this.options = options; - var loggerFact = loggerFactory ?? NullLoggerFactory.Instance; - authenticator = new MAuthAuthenticator(options, loggerFact); + authenticator = new MAuthAuthenticator(options); } /// diff --git a/tests/Medidata.MAuth.Tests/Infrastructure/MAuthServerHandler.cs b/tests/Medidata.MAuth.Tests/Infrastructure/MAuthServerHandler.cs index 0515bb5..2d497a3 100644 --- a/tests/Medidata.MAuth.Tests/Infrastructure/MAuthServerHandler.cs +++ b/tests/Medidata.MAuth.Tests/Infrastructure/MAuthServerHandler.cs @@ -4,7 +4,6 @@ using System.Threading; using System.Threading.Tasks; using Medidata.MAuth.Core; -using Microsoft.Extensions.Logging.Abstractions; using Newtonsoft.Json; namespace Medidata.MAuth.Tests.Infrastructure @@ -26,7 +25,7 @@ protected override async Task SendAsync( if (currentNumberOfAttempts < SucceedAfterThisManyAttempts) return new HttpResponseMessage(HttpStatusCode.ServiceUnavailable); - var authenticator = new MAuthAuthenticator(TestExtensions.ServerOptions, NullLoggerFactory.Instance); + var authenticator = new MAuthAuthenticator(TestExtensions.ServerOptions); var authInfo = authenticator.GetAuthenticationInfo(request, version); diff --git a/tests/Medidata.MAuth.Tests/MAuthAuthenticatorTests.cs b/tests/Medidata.MAuth.Tests/MAuthAuthenticatorTests.cs index 82d5bde..442924e 100644 --- a/tests/Medidata.MAuth.Tests/MAuthAuthenticatorTests.cs +++ b/tests/Medidata.MAuth.Tests/MAuthAuthenticatorTests.cs @@ -6,7 +6,6 @@ using Medidata.MAuth.Core.Exceptions; using Medidata.MAuth.Core.Models; using Medidata.MAuth.Tests.Infrastructure; -using Microsoft.Extensions.Logging.Abstractions; using Xunit; namespace Medidata.MAuth.Tests @@ -23,14 +22,14 @@ public static void MAuthAuthenticator_WithInvalidOptions_WillThrowException( ApplicationUuid = TestExtensions.ClientUuid, MAuthServiceUrl = mauthServiceUrl != null ? new Uri(mauthServiceUrl) : null, PrivateKey = privateKey - }, NullLoggerFactory.Instance)); + })); [Fact] public static void MAuthAuthenticator_WithDefaultUuid_WillThrowException() => Assert.Throws(() => new MAuthAuthenticator(new MAuthTestOptions() { ApplicationUuid = default(Guid) - }, NullLoggerFactory.Instance)); + })); [Theory] [InlineData("GET")] @@ -42,7 +41,7 @@ public static async Task AuthenticateRequest_WithValidRequest_WillAuthenticate(s // Arrange var testData = await method.FromResource(); - var authenticator = new MAuthAuthenticator(TestExtensions.ServerOptions, NullLoggerFactory.Instance); + var authenticator = new MAuthAuthenticator(TestExtensions.ServerOptions); var mAuthCore = new MAuthCore(); var signedRequest = await mAuthCore @@ -70,7 +69,7 @@ public static async Task AuthenticateRequest_WithValidMWSV2Request_WillAuthentic // Arrange var testData = await method.FromResourceV2(); var version = MAuthVersion.MWSV2; - var authenticator = new MAuthAuthenticator(TestExtensions.ServerOptions, NullLoggerFactory.Instance); + var authenticator = new MAuthAuthenticator(TestExtensions.ServerOptions); var mAuthCore = new MAuthCoreV2(); var signedRequest = await mAuthCore @@ -101,7 +100,7 @@ public static async Task AuthenticateRequest_WithNumberOfAttempts_WillAuthentica var testData = await "GET".FromResource(); var authenticator = new MAuthAuthenticator(TestExtensions.GetServerOptionsWithAttempts( - policy, shouldSucceedWithin: true), NullLoggerFactory.Instance); + policy, shouldSucceedWithin: true)); var mAuthCore = new MAuthCore(); var signedRequest = await mAuthCore @@ -131,7 +130,7 @@ public static async Task AuthenticateRequest_WithMWSV2Request_WithNumberOfAttemp var testData = await "GET".FromResourceV2(); var version = MAuthVersion.MWSV2; var authenticator = new MAuthAuthenticator(TestExtensions.GetServerOptionsWithAttempts( - policy, shouldSucceedWithin: true), NullLoggerFactory.Instance); + policy, shouldSucceedWithin: true)); var mAuthCore = new MAuthCoreV2(); var signedRequest = await mAuthCore @@ -161,7 +160,7 @@ public static async Task AuthenticateRequest_AfterNumberOfAttempts_WillThrowExce var testData = await "GET".FromResource(); var authenticator = new MAuthAuthenticator(TestExtensions.GetServerOptionsWithAttempts( - policy, shouldSucceedWithin: false), NullLoggerFactory.Instance); + policy, shouldSucceedWithin: false)); var mAuthCore = new MAuthCore(); var signedRequest = await mAuthCore @@ -196,7 +195,7 @@ public static async Task AuthenticateRequest_WithMWSV2Request_AfterNumberOfAttem var testData = await "GET".FromResource(); var version = MAuthVersion.MWSV2; var authenticator = new MAuthAuthenticator(TestExtensions.GetServerOptionsWithAttempts( - policy, shouldSucceedWithin: false), NullLoggerFactory.Instance); + policy, shouldSucceedWithin: false)); var mAuthCore = new MAuthCoreV2(); var signedRequest = await mAuthCore @@ -278,7 +277,7 @@ public static async Task AuthenticateRequest_WithMWSVersion_WithDisableV1_WillTh var testData = await method.FromResource(); var testOptions = TestExtensions.ServerOptions; testOptions.DisableV1 = true; - var authenticator = new MAuthAuthenticator(testOptions, NullLoggerFactory.Instance); + var authenticator = new MAuthAuthenticator(testOptions); var mAuthCore = new MAuthCore(); var signedRequest = await mAuthCore @@ -309,7 +308,7 @@ public static async Task GetAuthenticationInfo_WithSignedRequest_ForMWSV2Version var testData = await method.FromResourceV2(); var version = MAuthVersion.MWSV2; var testOptions = TestExtensions.ServerOptions; - var authenticator = new MAuthAuthenticator(testOptions, NullLoggerFactory.Instance); + var authenticator = new MAuthAuthenticator(testOptions); // Act var actual = authenticator.GetAuthenticationInfo(testData.ToHttpRequestMessage(version), version); @@ -331,7 +330,7 @@ public static async Task GetAuthenticationInfo_WithSignedRequest_ForMWSVersion_W var testData = await method.FromResource(); var version = MAuthVersion.MWS; var testOptions = TestExtensions.ServerOptions; - var authenticator = new MAuthAuthenticator(testOptions, NullLoggerFactory.Instance); + var authenticator = new MAuthAuthenticator(testOptions); // Act var actual = authenticator.GetAuthenticationInfo(testData.ToHttpRequestMessage(version), version); diff --git a/tests/Medidata.MAuth.Tests/UtilityExtensionsTest.cs b/tests/Medidata.MAuth.Tests/UtilityExtensionsTest.cs index d07c67b..432f3dc 100644 --- a/tests/Medidata.MAuth.Tests/UtilityExtensionsTest.cs +++ b/tests/Medidata.MAuth.Tests/UtilityExtensionsTest.cs @@ -2,7 +2,6 @@ using System.Threading.Tasks; using Medidata.MAuth.Core; using Medidata.MAuth.Tests.Infrastructure; -using Microsoft.Extensions.Logging.Abstractions; using Xunit; namespace Medidata.MAuth.Tests @@ -68,7 +67,7 @@ public static async Task Authenticate_WithValidRequest_WillAuthenticate(string m }); // Act - var isAuthenticated = await signedRequest.Authenticate(TestExtensions.ServerOptions, NullLoggerFactory.Instance); + var isAuthenticated = await signedRequest.Authenticate(TestExtensions.ServerOptions); // Assert Assert.True(isAuthenticated);