From a325d07c6be6950f8aeada3bb71303cd3a8fe0ea Mon Sep 17 00:00:00 2001 From: Developer 02 Date: Tue, 29 Oct 2024 14:24:13 +0100 Subject: [PATCH] refactor: AuthController-Methoden optimieren und Login-Logik verbessern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AuthController aktualisiert, um eine klarere Struktur zu implementieren. - Login-Methode vereinfacht, um die Benutzerauthentifizierung direkt zu behandeln. - Neue LoginById-Methode für den Benutzerlogin über ID eingeführt. - Fehlerprotokollierung und -behandlung in den Login-Methoden verbessert. - Überflüssigen Code entfernt und Lesbarkeit verbessert. - TODO für weitere Integration mit UserManager hinzugefügt. --- .../Attributes/ApiKeyAuthAttribute.cs | 1 + WorkFlow.API/Controllers/AuthController.cs | 134 +++++++++--------- WorkFlow.API/Extensions/DIExtensions.cs | 14 +- .../Filters/APIKeyAuthHeaderOpFilter.cs | 10 +- WorkFlow.API/Models/Login.cs | 2 +- WorkFlow.API/Program.cs | 2 +- WorkFlow.API/WFKey.cs | 8 ++ 7 files changed, 96 insertions(+), 75 deletions(-) create mode 100644 WorkFlow.API/WFKey.cs diff --git a/WorkFlow.API/Attributes/ApiKeyAuthAttribute.cs b/WorkFlow.API/Attributes/ApiKeyAuthAttribute.cs index f4614a2..944590a 100644 --- a/WorkFlow.API/Attributes/ApiKeyAuthAttribute.cs +++ b/WorkFlow.API/Attributes/ApiKeyAuthAttribute.cs @@ -3,6 +3,7 @@ using WorkFlow.API.Filters; namespace WorkFlow.API.Attributes { + //TODO: move APIKeyAuthAttribute to Core.API public class APIKeyAuthAttribute : ServiceFilterAttribute { public APIKeyAuthAttribute() diff --git a/WorkFlow.API/Controllers/AuthController.cs b/WorkFlow.API/Controllers/AuthController.cs index 56911cd..f30bd2f 100644 --- a/WorkFlow.API/Controllers/AuthController.cs +++ b/WorkFlow.API/Controllers/AuthController.cs @@ -14,17 +14,12 @@ using WorkFlow.API.Attributes; namespace WorkFlow.API.Controllers { + //TODO: implement up-to-date AuthController in UserManager [APIKeyAuth] [Route("api/[controller]")] [ApiController] - public class AuthController(IUserService userService, IGroupOfUserService gouService, IDirectorySearchService directorySearchService, IStringLocalizer localizer, ILogger logger) : ControllerBase + public class AuthController(IUserService userService, IDirectorySearchService dirSearchService, IStringLocalizer localizer, ILogger logger) : ControllerBase { - private readonly IUserService _userService = userService; - private readonly IGroupOfUserService _gouService = gouService; - private readonly IDirectorySearchService _dirSearchService = directorySearchService; - private readonly IStringLocalizer _localizer = localizer; - private readonly ILogger _logger = logger; - [AllowAnonymous] [HttpGet("check")] public IActionResult CheckAuthentication() @@ -35,76 +30,81 @@ namespace WorkFlow.API.Controllers } catch(Exception ex) { - _logger.LogError(ex, "{Message}", ex.Message); + logger.LogError(ex, "{Message}", ex.Message); return StatusCode(StatusCodes.Status500InternalServerError); } } + [NonAction] + public async Task Login(UserReadDto user) + { + // Create claimsIdentity + var claimsIdentity = new ClaimsIdentity(user.ToClaimList(), CookieAuthenticationDefaults.AuthenticationScheme); + + // Create authProperties + var authProperties = new AuthenticationProperties + { + IsPersistent = true, + AllowRefresh = true, + ExpiresUtc = DateTime.UtcNow.AddMinutes(60) + }; + + // Sign in + await HttpContext.SignInAsync( + CookieAuthenticationDefaults.AuthenticationScheme, + new ClaimsPrincipal(claimsIdentity), + authProperties); + + return Ok(); + } + [AllowAnonymous] [HttpPost("login")] public async Task Login([FromBody] Login login) { try { - var username = string.Empty; - DataResult? uRes = null; + return dirSearchService.ValidateCredentials(login.Username, login.Password) + ? await userService.ReadByUsernameAsync(login.Username).ThenAsync( + SuccessAsync: Login, + Fail: IActionResult (msg, ntc) => + { + logger.LogNotice(ntc); + logger.LogError("User could not be found, although verified by directory-search-service. It needs to be imported by UserManager. User name is {username}.", login.Username); + return StatusCode(StatusCodes.Status500InternalServerError); + }) + : Unauthorized(localizer[WFKey.UserNotFoundOrWrongPassword].Value); + } + catch(Exception ex) + { + logger.LogError(ex, "{Message}", ex.Message); + return StatusCode(StatusCodes.Status500InternalServerError); + } + } - if(login.Username is not null && login.UserId is not null) - return BadRequest("Invalid request: either 'UserId' or 'Username' must be provided, but not both."); - else if(login.Username is not null) - username = login.Username; - else if(login.UserId is int userId) - { - uRes = await _userService.ReadByIdAsync(userId); - if (!uRes.IsSuccess || uRes.Data is null) + [AllowAnonymous] + [HttpPost("login/{id}")] + public async Task LoginById([FromRoute] int id, [FromQuery] string password) + { + try + { + return await userService.ReadByIdAsync(id).ThenAsync( + SuccessAsync: async user + => dirSearchService.ValidateCredentials(user.Username, password) + ? await Login(user) + : Unauthorized(localizer[WFKey.WrongPassword].Value), + Fail: (msg, ntc) => { - return Unauthorized(uRes); - } - } - else - return BadRequest("Invalid request: either 'UserId' or 'Username' must be provided, but not both."); - - bool isValid = _dirSearchService.ValidateCredentials(username, login.Password); - - if (!isValid) - return Unauthorized(Result.Fail().Message(_localizer[Key.UserNotFound])); - - var gouMsg = await _gouService.HasGroup(username, "PM_USER", caseSensitive: false); - if (!gouMsg.IsSuccess) - return Unauthorized(Result.Fail().Message(_localizer[Key.UnauthorizedUser])); - - //find the user - uRes ??= await _userService.ReadByUsernameAsync(username); - if (!uRes.IsSuccess || uRes.Data is null) - { - _logger.LogNotice(uRes.Notices); - return Unauthorized(); - } + if (ntc.HasFlag(Flag.NotFound)) + return Unauthorized(Key.UserNotFound); - UserReadDto user = uRes.Data; - - // Create claimsIdentity - var claimsIdentity = new ClaimsIdentity(user.ToClaimList(), CookieAuthenticationDefaults.AuthenticationScheme); - - // Create authProperties - var authProperties = new AuthenticationProperties - { - IsPersistent = true, - AllowRefresh = true, - ExpiresUtc = DateTime.UtcNow.AddMinutes(60) - }; - - // Sign in - await HttpContext.SignInAsync( - CookieAuthenticationDefaults.AuthenticationScheme, - new ClaimsPrincipal(claimsIdentity), - authProperties); - - return Ok(); + logger.LogNotice(ntc); + return StatusCode(StatusCodes.Status500InternalServerError); + }); } - catch(Exception ex) + catch (Exception ex) { - _logger.LogError(ex, "{Message}", ex.Message); + logger.LogError(ex, "{Message}", ex.Message); return StatusCode(StatusCodes.Status500InternalServerError); } } @@ -121,16 +121,16 @@ namespace WorkFlow.API.Controllers if (string.IsNullOrEmpty(username)) return Unauthorized(); - return await _userService.ReadByUsernameAsync(username) + return await userService.ReadByUsernameAsync(username) .ThenAsync(Ok, IActionResult (m, n) => { - _logger.LogNotice(n); - return NotFound(Result.Fail().Message(_localizer[Key.UserNotFound])); + logger.LogNotice(n); + return NotFound(Result.Fail().Message(localizer[Key.UserNotFound].Value)); }); } catch (Exception ex) { - _logger.LogError(ex, "{Message}", ex.Message); + logger.LogError(ex, "{Message}", ex.Message); return StatusCode(StatusCodes.Status500InternalServerError); } } @@ -146,7 +146,7 @@ namespace WorkFlow.API.Controllers } catch(Exception ex) { - _logger.LogError(ex, "{Message}", ex.Message); + logger.LogError(ex, "{Message}", ex.Message); return StatusCode(StatusCodes.Status500InternalServerError); } } diff --git a/WorkFlow.API/Extensions/DIExtensions.cs b/WorkFlow.API/Extensions/DIExtensions.cs index 933c9ac..4c9297d 100644 --- a/WorkFlow.API/Extensions/DIExtensions.cs +++ b/WorkFlow.API/Extensions/DIExtensions.cs @@ -1,4 +1,7 @@ -using WorkFlow.API.Filters; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Options; +using WorkFlow.API.Filters; using WorkFlow.API.Models; namespace WorkFlow.API.Extensions @@ -8,7 +11,12 @@ namespace WorkFlow.API.Extensions public static IServiceCollection AddAPIKeyAuth(this IServiceCollection services, Func isValidKey, string headerName = "X-API-Key") => services.AddSingleton(provider => new(isValidKey: isValidKey, headerName: headerName)); - public static IServiceCollection AddAPIKeyAuth(this IServiceCollection services, APIKeyAuthOptions options) - => services.AddAPIKeyAuth(isValidKey: key => options.Key is null || options.Key == key, headerName: options.HeaderName); + public static IServiceCollection AddAPIKeyAuth(this IServiceCollection services, APIKeyAuthOptions options, bool configureOptions = true) + { + if(configureOptions) + services.TryAddSingleton(Options.Create(options)); + + return services.AddAPIKeyAuth(isValidKey: key => options.Key is null || options.Key == key, headerName: options.HeaderName); + } } } \ No newline at end of file diff --git a/WorkFlow.API/Filters/APIKeyAuthHeaderOpFilter.cs b/WorkFlow.API/Filters/APIKeyAuthHeaderOpFilter.cs index ae1e670..bb022a5 100644 --- a/WorkFlow.API/Filters/APIKeyAuthHeaderOpFilter.cs +++ b/WorkFlow.API/Filters/APIKeyAuthHeaderOpFilter.cs @@ -1,11 +1,12 @@ using Microsoft.Extensions.Options; +using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Models; using Swashbuckle.AspNetCore.SwaggerGen; using WorkFlow.API.Models; namespace WorkFlow.API.Filters { - public class APIKeyAuthHeaderOpFilter(IOptions options) : IOperationFilter + public class APIKeyAuthHeaderOpFilter(IOptions options, IWebHostEnvironment environment) : IOperationFilter { private readonly APIKeyAuthOptions apiKeyAuthOptions = options.Value; @@ -23,10 +24,13 @@ namespace WorkFlow.API.Filters } }; - if(apiKeyAuthOptions.SwaggerDescription is not null) + if(environment.IsDevelopment()) + param.Schema.Default = new OpenApiString(apiKeyAuthOptions.Key); + + if (apiKeyAuthOptions.SwaggerDescription is not null) param.Description = apiKeyAuthOptions.SwaggerDescription; - operation.Parameters = [param]; + operation.Parameters.Add(param); } } } \ No newline at end of file diff --git a/WorkFlow.API/Models/Login.cs b/WorkFlow.API/Models/Login.cs index 9c111c9..4959890 100644 --- a/WorkFlow.API/Models/Login.cs +++ b/WorkFlow.API/Models/Login.cs @@ -1,4 +1,4 @@ namespace WorkFlow.API.Models { - public record Login(int? UserId, string? Username, string Password); + public record Login(string Username, string Password); } \ No newline at end of file diff --git a/WorkFlow.API/Program.cs b/WorkFlow.API/Program.cs index 5ea2b3e..8dfa9ad 100644 --- a/WorkFlow.API/Program.cs +++ b/WorkFlow.API/Program.cs @@ -40,7 +40,7 @@ try bool disableAPIKeyAuth = config.GetValue("DisableAPIKeyAuth") && builder.IsDevOrDiP(); if (disableAPIKeyAuth) - builder.Services.AddAPIKeyAuth(new()); + builder.Services.AddAPIKeyAuth(new APIKeyAuthOptions()); else if (config.GetSection("APIKeyAuth").Get() is APIKeyAuthOptions options) builder.Services.AddAPIKeyAuth(options); diff --git a/WorkFlow.API/WFKey.cs b/WorkFlow.API/WFKey.cs new file mode 100644 index 0000000..0345869 --- /dev/null +++ b/WorkFlow.API/WFKey.cs @@ -0,0 +1,8 @@ +namespace WorkFlow.API +{ + public static class WFKey + { + public static readonly string WrongPassword = nameof(WrongPassword); + public static readonly string UserNotFoundOrWrongPassword = nameof(UserNotFoundOrWrongPassword); + } +}