From 94ddad7138d12e3c7eb31ec683efc1fe9502c804 Mon Sep 17 00:00:00 2001 From: antoineatstariongroup Date: Tue, 21 Jan 2025 14:24:48 +0100 Subject: [PATCH] SonarQube fixes, Sam's comment. Missing username and logout implement --- .../AuthenticationPersonAuthenticator.cs | 4 - CometServer/Authentication/JwtTokenService.cs | 7 +- CometServer/CometServer.csproj.orig | 113 ------------------ .../LocalJwtAuthenticationConfig.cs | 14 ++- .../Extensions/HttpRequestExtensions.cs | 3 + CometServer/Modules/10-25/ApiBase.cs | 36 ++++-- .../Modules/10-25/EngineeringModelApi.cs | 11 +- CometServer/Modules/10-25/SiteDirectoryApi.cs | 5 +- .../Authentication/AuthenticationModule.cs | 6 +- CometServer/Startup.cs | 15 +++ CometServer/appsettings.Development.json | 3 +- 11 files changed, 69 insertions(+), 148 deletions(-) delete mode 100644 CometServer/CometServer.csproj.orig diff --git a/CometServer/Authentication/AuthenticationPersonAuthenticator.cs b/CometServer/Authentication/AuthenticationPersonAuthenticator.cs index 32284e1c..8c55f05e 100644 --- a/CometServer/Authentication/AuthenticationPersonAuthenticator.cs +++ b/CometServer/Authentication/AuthenticationPersonAuthenticator.cs @@ -111,16 +111,12 @@ public async Task Authenticate(string username, string pas } catch (NpgsqlException ex) { - transaction?.RollbackAsync(); - this.Logger.LogCritical( "The AuthenticationPersonAuthenticator could not interact with the CDP4-COMET database"); throw new AuthenticatorException("The authenticator could not connect to the CDP4-COMET database", innerException: ex); } catch (Exception ex) { - transaction?.RollbackAsync(); - this.Logger.LogCritical(ex, "There was an error while authenticating the user credentials"); throw new AuthenticatorException("There was an error while authenticating the user credentials", innerException: ex); diff --git a/CometServer/Authentication/JwtTokenService.cs b/CometServer/Authentication/JwtTokenService.cs index 7f90d6b3..3ac66b4c 100644 --- a/CometServer/Authentication/JwtTokenService.cs +++ b/CometServer/Authentication/JwtTokenService.cs @@ -43,11 +43,6 @@ namespace CometServer.Authentication /// public class JwtTokenService : IJwtTokenService { - /// - /// Provides the expiration times of a generated JWT token, in minutes - /// - private const int ExpirationMinutes = 30; - /// /// The (injected) logger /// @@ -83,7 +78,7 @@ public JwtTokenService(ILogger logger, IAppConfigService appCon /// The created JWT token public string CreateToken(AuthenticationPerson authenticationPerson) { - var expiration = DateTime.UtcNow.AddMinutes(ExpirationMinutes); + var expiration = DateTime.UtcNow.AddMinutes(this.appConfigService.AppConfig.AuthenticationConfig.LocalJwtAuthenticationConfig.TokenExpirationMinutes); var jwtSecurityToken = this.CreateJwtSecurityToken( CreateClaims(authenticationPerson), diff --git a/CometServer/CometServer.csproj.orig b/CometServer/CometServer.csproj.orig deleted file mode 100644 index 2068e6b0..00000000 --- a/CometServer/CometServer.csproj.orig +++ /dev/null @@ -1,113 +0,0 @@ - - - - net8.0 - RHEA System S.A. - CometServer-CE - 8.0.0-RC21 - CDP4-COMET Services API - Copyright © RHEA System S.A. - AGPL-3.0-only - Sam Gerené, Alex Vorobiev, Alexander van Delft - Debug;Release - Git - https://github.com/RHEAGROUP/COMET-WebServices-Community-Edition.git - false - latest - en - OnOutputUpdated - - - - - - - - - - - -<<<<<<< HEAD - - -======= - - - ->>>>>>> e1f2727 (WIP - JWT) - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - <_StaticWebAssetEmbeddedProjectConfigurationCanonicalMetadata Remove="VERSION" /> - - - - <_StaticWebAssetProjectConfigurationCanonicalMetadata Remove="VERSION" /> - - - - - Always - - - - - ../VersionFileCreator/bin/$(Configuration)/$(TargetFramework)/VersionFileCreator.dll - False - True - - - - - - - - - - - - - - - - - - - - diff --git a/CometServer/Configuration/LocalJwtAuthenticationConfig.cs b/CometServer/Configuration/LocalJwtAuthenticationConfig.cs index ea54173f..c68827c2 100644 --- a/CometServer/Configuration/LocalJwtAuthenticationConfig.cs +++ b/CometServer/Configuration/LocalJwtAuthenticationConfig.cs @@ -41,7 +41,7 @@ public LocalJwtAuthenticationConfig() this.ValidIssuer = "CDP4-COMET"; this.ValidAudience = "localhost:5000"; this.SymmetricSecurityKey = "needs-to-be-updated-with-a-secret"; - } + } /// /// Initializes a new instance of the class @@ -55,8 +55,13 @@ public LocalJwtAuthenticationConfig(IConfiguration configuration) this.ValidIssuer = configuration["Authentication:LocalJwtBearer:ValidIssuer"]; this.ValidAudience = configuration["Authentication:LocalJwtBearer:ValidAudience"]; this.SymmetricSecurityKey = configuration["Authentication:LocalJwtBearer:SymmetricSecurityKey"]; + + if (int.TryParse(configuration["Authentication:LocalJwtBearer:TokenExpirationMinutes"], out var tokenExpirationMinutes)) + { + this.TokenExpirationMinutes = tokenExpirationMinutes; + } } - + /// /// Gets or sets a value indicating whether Local JWT Authentication is enabled or not /// @@ -82,5 +87,10 @@ public LocalJwtAuthenticationConfig(IConfiguration configuration) /// Gets or sets the symmetric security key with which the bearer tokens are generated and also validated /// public string SymmetricSecurityKey { get; set; } + + /// + /// Gets or sets the expiration time of a generated JWT Token, in minutes (defaults: 30) + /// + public int TokenExpirationMinutes { get; set; } = 30; } } diff --git a/CometServer/Extensions/HttpRequestExtensions.cs b/CometServer/Extensions/HttpRequestExtensions.cs index 407cc174..46b387ad 100644 --- a/CometServer/Extensions/HttpRequestExtensions.cs +++ b/CometServer/Extensions/HttpRequestExtensions.cs @@ -107,6 +107,9 @@ public static string QueryNameMethodPath(this HttpRequest httpRequest) /// Value of the assert public static bool DoesAuthorizationHeaderMatches(this HttpRequest request, string expectedAuthorizationScheme) { + ArgumentNullException.ThrowIfNull(request); + ArgumentNullException.ThrowIfNull(expectedAuthorizationScheme); + var authorizationHeader = request.Headers.Authorization; if (string.IsNullOrEmpty(authorizationHeader)) diff --git a/CometServer/Modules/10-25/ApiBase.cs b/CometServer/Modules/10-25/ApiBase.cs index b0162688..114749cb 100644 --- a/CometServer/Modules/10-25/ApiBase.cs +++ b/CometServer/Modules/10-25/ApiBase.cs @@ -224,7 +224,7 @@ protected async Task Authorize(IAppConfigService appConfigService, ICred case PersonIdentifierPropertyKind.Iid: if (!Guid.TryParse(claim.Value, out var userId)) { - throw new AuthenticationException(); + throw new AuthenticationException("Provided claim value is not a valid GUID."); } await this.Authorize(appConfigService, credentialsService, userId); @@ -236,10 +236,10 @@ protected async Task Authorize(IAppConfigService appConfigService, ICred return claim.Value; } - this.logger.LogWarning("Identifier claim {ClaimName} missing User Claims", + this.logger.LogWarning("Identifier claim {ClaimName} missing from User Claims", appConfigService.AppConfig.AuthenticationConfig.ExternalJwtAuthenticationConfig.IdentifierClaimName); - throw new AuthorizationException(); + throw new AuthorizationException($"Identifer claim {appConfigService.AppConfig.AuthenticationConfig.ExternalJwtAuthenticationConfig.IdentifierClaimName} missing from User Claims"); } /// @@ -642,13 +642,13 @@ protected async Task WriteMessagePackResponse(IHeaderInfoProvider headerInfoProv /// /// The . /// - protected void WriteMultipartResponse(IHeaderInfoProvider headerInfoProvider, IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileBinaryService fileBinaryService, IPermissionInstanceFilterService permissionInstanceFilterService, List fileRevisions, List resourceResponse, Version version, HttpResponse httpResponse, HttpStatusCode statusCode = HttpStatusCode.OK) + protected Task WriteMultipartResponse(IHeaderInfoProvider headerInfoProvider, IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileBinaryService fileBinaryService, IPermissionInstanceFilterService permissionInstanceFilterService, List fileRevisions, List resourceResponse, Version version, HttpResponse httpResponse, HttpStatusCode statusCode = HttpStatusCode.OK) { headerInfoProvider.RegisterResponseHeaders(httpResponse, ContentTypeKind.MULTIPARTMIXED, HttpConstants.BoundaryString); httpResponse.StatusCode = (int)statusCode; - this.PrepareMultiPartResponse(metaInfoProvider,jsonSerializer, fileBinaryService, permissionInstanceFilterService, httpResponse.Body, fileRevisions, resourceResponse, version); + return this.PrepareMultiPartResponse(metaInfoProvider,jsonSerializer, fileBinaryService, permissionInstanceFilterService, httpResponse.Body, fileRevisions, resourceResponse, version); } /// @@ -690,12 +690,12 @@ protected void WriteMultipartResponse(IHeaderInfoProvider headerInfoProvider, IM /// /// The . /// - protected void WriteArchivedResponse(IHeaderInfoProvider headerInfoProvider, IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileArchiveService fileArchiveService, IPermissionInstanceFilterService permissionInstanceFilterService, List resourceResponse, string partition, string[] routeSegments, Version version, HttpResponse httpResponse, HttpStatusCode statusCode = HttpStatusCode.OK) + protected Task WriteArchivedResponse(IHeaderInfoProvider headerInfoProvider, IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileArchiveService fileArchiveService, IPermissionInstanceFilterService permissionInstanceFilterService, List resourceResponse, string partition, string[] routeSegments, Version version, HttpResponse httpResponse, HttpStatusCode statusCode = HttpStatusCode.OK) { headerInfoProvider.RegisterResponseHeaders(httpResponse, ContentTypeKind.MULTIPARTMIXED, HttpConstants.BoundaryString); httpResponse.StatusCode = (int)statusCode; - this.PrepareArchivedResponse(metaInfoProvider,jsonSerializer, fileArchiveService, permissionInstanceFilterService, httpResponse.Body, resourceResponse, version, partition, routeSegments); + return this.PrepareArchivedResponse(metaInfoProvider,jsonSerializer, fileArchiveService, permissionInstanceFilterService, httpResponse.Body, resourceResponse, version, partition, routeSegments); } /// @@ -858,7 +858,7 @@ private void CreateFilteredMessagePackResponseStream(IMessagePackSerializer mess /// /// The used to serialize data to JSOIN /// - private void PrepareMultiPartResponse(IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileBinaryService fileBinaryService, IPermissionInstanceFilterService permissionInstanceFilterService, Stream targetStream, List fileRevisions, List resourceResponse, Version requestDataModelVersion) + private async Task PrepareMultiPartResponse(IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileBinaryService fileBinaryService, IPermissionInstanceFilterService permissionInstanceFilterService, Stream targetStream, List fileRevisions, List resourceResponse, Version requestDataModelVersion) { if (fileRevisions.Count == 0) { @@ -892,7 +892,12 @@ private void PrepareMultiPartResponse(IMetaInfoProvider metaInfoProvider, ICdp4J { fileSize = fileStream.Length; buffer = new byte[(int)fileSize]; - fileStream.Read(buffer, 0, (int)fileSize); + var readBytes = fileStream.Read(buffer, 0, (int)fileSize); + + if (readBytes != fileSize) + { + this.logger.LogWarning("Failed to read {FileSize} bytes, only read {ReadBytes}", fileSize, readBytes); + } } // write out the binary content to the first multipart content entry @@ -906,7 +911,7 @@ private void PrepareMultiPartResponse(IMetaInfoProvider metaInfoProvider, ICdp4J } // stream the multipart content to the request contents target stream - content.CopyToAsync(targetStream).Wait(); + await content.CopyToAsync(targetStream); AddMultiPartMimeEndpoint(targetStream); } @@ -940,7 +945,7 @@ private void PrepareMultiPartResponse(IMetaInfoProvider metaInfoProvider, ICdp4J /// /// The route segments. /// - private void PrepareArchivedResponse(IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileArchiveService fileArchiveService, IPermissionInstanceFilterService permissionInstanceFilterService, Stream targetStream, List resourceResponse, Version requestDataModelVersion, string partition, string[] routeSegments) + private async Task PrepareArchivedResponse(IMetaInfoProvider metaInfoProvider, ICdp4JsonSerializer jsonSerializer, IFileArchiveService fileArchiveService, IPermissionInstanceFilterService permissionInstanceFilterService, Stream targetStream, List resourceResponse, Version requestDataModelVersion, string partition, string[] routeSegments) { var temporaryTopFolder = fileArchiveService.CreateFolderAndFileStructureOnDisk(resourceResponse, partition, routeSegments); @@ -973,7 +978,12 @@ private void PrepareArchivedResponse(IMetaInfoProvider metaInfoProvider, ICdp4Js { fileSize = fileStream.Length; buffer = new byte[(int)fileSize]; - fileStream.Read(buffer, 0, (int)fileSize); + var readBytes = fileStream.Read(buffer, 0, (int)fileSize); + + if (readBytes != fileSize) + { + this.logger.LogWarning("Failed to read {FileSize} bytes, only read {ReadBytes}", fileSize, readBytes); + } } var binaryContent = new ByteArrayContent(buffer); @@ -989,7 +999,7 @@ private void PrepareArchivedResponse(IMetaInfoProvider metaInfoProvider, ICdp4Js content.Add(binaryContent); // stream the multipart content to the request contents target stream - content.CopyToAsync(targetStream).Wait(); + await content.CopyToAsync(targetStream); AddMultiPartMimeEndpoint(targetStream); } diff --git a/CometServer/Modules/10-25/EngineeringModelApi.cs b/CometServer/Modules/10-25/EngineeringModelApi.cs index 8f6419b2..9e1384c5 100644 --- a/CometServer/Modules/10-25/EngineeringModelApi.cs +++ b/CometServer/Modules/10-25/EngineeringModelApi.cs @@ -649,7 +649,7 @@ protected async Task GetResponseData(HttpRequest httpRequest, HttpResponse httpR if (requestUtils.QueryParameters.IncludeFileData && fileRevisions.Any()) { // return multipart response including file binaries - this.WriteMultipartResponse(headerInfoProvider, metaInfoProvider, jsonSerializer, fileBinaryService, permissionInstanceFilterService, fileRevisions, resourceResponse, version, httpResponse); + await this.WriteMultipartResponse(headerInfoProvider, metaInfoProvider, jsonSerializer, fileBinaryService, permissionInstanceFilterService, fileRevisions, resourceResponse, version, httpResponse); return; } @@ -661,13 +661,13 @@ protected async Task GetResponseData(HttpRequest httpRequest, HttpResponse httpR { var iterationPartition = requestUtils.GetIterationPartitionString(modelSetup.EngineeringModelIid); - this.WriteArchivedResponse(headerInfoProvider, metaInfoProvider, jsonSerializer, fileArchiveService, permissionInstanceFilterService, resourceResponse, iterationPartition, routeSegments, version, httpResponse); + await this.WriteArchivedResponse(headerInfoProvider, metaInfoProvider, jsonSerializer, fileArchiveService, permissionInstanceFilterService, resourceResponse, iterationPartition, routeSegments, version, httpResponse); return; } if (this.IsValidCommonFileStoreArchiveRoute(routeSegmentList)) { - this.WriteArchivedResponse(headerInfoProvider, metaInfoProvider, jsonSerializer, fileArchiveService, permissionInstanceFilterService, resourceResponse, partition, routeSegments, version, httpResponse); + await this.WriteArchivedResponse(headerInfoProvider, metaInfoProvider, jsonSerializer, fileArchiveService, permissionInstanceFilterService, resourceResponse, partition, routeSegments, version, httpResponse); return; } } @@ -905,10 +905,11 @@ protected async Task EnqueCometTaskForPostRequest(PostRequestData postRequestDat this.logger.LogTrace(task.IsCompletedSuccessfully.ToString()); }); - var completed = longRunningCometTask.Wait(TimeSpan.FromSeconds(requestUtils.QueryParameters.WaitTime)); + var completedTask = await Task.WhenAny(longRunningCometTask, Task.Delay(TimeSpan.FromSeconds(requestUtils.QueryParameters.WaitTime))); - if (completed) + if (completedTask == longRunningCometTask) { + await longRunningCometTask; this.logger.LogInformation("The task {id} for actor {actor} completed within the requsted wait time {waittime}", cometTask.Id, cometTask.Actor, requestUtils.QueryParameters.WaitTime); } else diff --git a/CometServer/Modules/10-25/SiteDirectoryApi.cs b/CometServer/Modules/10-25/SiteDirectoryApi.cs index e58c2527..f91fbb4f 100644 --- a/CometServer/Modules/10-25/SiteDirectoryApi.cs +++ b/CometServer/Modules/10-25/SiteDirectoryApi.cs @@ -547,10 +547,11 @@ protected async Task EnqueCometTaskForPostRequest(PostRequestData postRequestDat this.logger.LogTrace(task.IsCompletedSuccessfully.ToString()); }); - var completed = longRunningCometTask.Wait(TimeSpan.FromSeconds(requestUtils.QueryParameters.WaitTime)); + var completedTask = await Task.WhenAny(longRunningCometTask, Task.Delay(TimeSpan.FromSeconds(requestUtils.QueryParameters.WaitTime))); - if (completed) + if (completedTask == longRunningCometTask) { + await longRunningCometTask; this.logger.LogInformation("The task {id} for actor {actor} completed within the requested wait time {waittime}", cometTask.Id, cometTask.Actor, requestUtils.QueryParameters.WaitTime); } else diff --git a/CometServer/Modules/Authentication/AuthenticationModule.cs b/CometServer/Modules/Authentication/AuthenticationModule.cs index 1c09327c..6bd4298f 100644 --- a/CometServer/Modules/Authentication/AuthenticationModule.cs +++ b/CometServer/Modules/Authentication/AuthenticationModule.cs @@ -35,6 +35,7 @@ namespace CometServer.Modules using CometServer.Authentication; using CometServer.Configuration; + using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; @@ -68,11 +69,12 @@ public override void AddRoutes(IEndpointRouteBuilder app) await res.AsJson("not authenticated"); }); - app.MapPost("/logout", async (HttpRequest req, HttpResponse res) => + app.MapPost("/logout", async (HttpRequest req, HttpResponse res) => { + await req.HttpContext.SignInAsync(req.Headers.Authorization.); //return webServiceAuthentication.LogOutResponse(req.HttpContext); throw new NotImplementedException(); - }); + }).RequireAuthorization(ApiBase.AuthenticationSchemes); app.MapGet("/auth/schemes", ProvideEnabledAuthenticationScheme); } diff --git a/CometServer/Startup.cs b/CometServer/Startup.cs index 03d82c47..e1d75085 100644 --- a/CometServer/Startup.cs +++ b/CometServer/Startup.cs @@ -233,6 +233,21 @@ private static void SetUpAuthentication(IServiceCollection services, IConfigurat throw new ConfigurationErrorsException("The Authentication:LocalJwtBearer:SymmetricSecurityKey setting must be available"); } + var expirationTime = configuration["Authentication:LocalJwtBearer:TokenExpirationMinutes"]; + + if (!string.IsNullOrEmpty(expirationTime)) + { + if (!int.TryParse(expirationTime, out var tokenExpirationMinutes)) + { + throw new ConfigurationErrorsException("The Authentication:LocalJwtBearer:TokenExpirationMinutes setting must an integer value"); + } + + if (tokenExpirationMinutes <= 0) + { + throw new ConfigurationErrorsException("The Authentication:LocalJwtBearer:TokenExpirationMinutes setting must be strickly greater than 0"); + } + } + authenticationBuilder.AddLocalJwtBearerAuthentication(configure: options => { options.TokenValidationParameters = new TokenValidationParameters diff --git a/CometServer/appsettings.Development.json b/CometServer/appsettings.Development.json index c90d4ea5..0e661981 100644 --- a/CometServer/appsettings.Development.json +++ b/CometServer/appsettings.Development.json @@ -60,7 +60,8 @@ "IsEnabled": true, "ValidIssuer": "CDP4-COMET", "ValidAudience": "localhost:5000", - "SymmetricSecurityKey": "needs-to-be-updated-with-a-secret" + "SymmetricSecurityKey": "needs-to-be-updated-with-a-secret", + "TokenExpirationMinutes": 150 }, "ExternalJwtBearer": { "IsEnabled": false,