Fix #378: Authentication handlers #379
Conversation
…tern; add AnonymousAuthenticationHandler to support Carter modules that do not require authentication
…nd Serilog.AspNetCore
…openid-authentication
antoineatstariongroup
requested review from
samatstariongroup and
lxatstariongroup
| @@ -2,7 +2,7 @@ | |||
| // <copyright file="IAuthenticationPersonDao.cs" company="Starion Group S.A."> | |||
| // Copyright (c) 2015-2023 Starion Group S.A. | |||
| // | |||
| // Author: Sam Geren�, Alex Vorobiev, Alexander van Delft, Nathanael Smiechowski, Antoine Th�ate | |||
| // Author: Sam Geren�, Alex Vorobiev, Alexander van Delft, Nathanael Smiechowski, Antoine Th�ate | |||
There was a problem hiding this comment.
Something wrong with encoding?
There was a problem hiding this comment.
fixed
| @@ -108,12 +109,21 @@ public async Task<AuthenticationPerson> Authenticate(string username, string pas | |||
|
|
|||
| return null; | |||
| } | |||
| catch (NpgsqlException ex) | |||
| { | |||
| transaction?.RollbackAsync(); | |||
There was a problem hiding this comment.
do we need to rollback? i don;t think we are writing anything, so nothing to rollback to
There was a problem hiding this comment.
Fixed
| /// <summary> | ||
| /// Provides the expiration times of a generated JWT token, in minutes | ||
| /// </summary> | ||
| private const int ExpirationMinutes = 30; |
There was a problem hiding this comment.
i propose we make this configurable
There was a problem hiding this comment.
Fixed
CometServer/CometServer.csproj.orig
Outdated
| <Exec Command="dotnet $(VersionFileCreatorDllPath) path:$(OutDir) " YieldDuringToolExecution="True" ConsoleToMSBuild="True" /> | ||
| <Message Importance="High" Text="------ VersionFileCreator tool Finished ------ " /> | ||
| </Target> | ||
|
|
There was a problem hiding this comment.
this file can be removed
There was a problem hiding this comment.
Fixed
| /// <returns>Value of the assert</returns> | ||
| public static bool DoesAuthorizationHeaderMatches(this HttpRequest request, string expectedAuthorizationScheme) | ||
| { | ||
| var authorizationHeader = request.Headers.Authorization; |
There was a problem hiding this comment.
please add argumentnull check
There was a problem hiding this comment.
Fixed
CometServer/Modules/10-25/ApiBase.cs
Outdated
| this.logger.LogWarning("Identifier claim {ClaimName} missing User Claims", | ||
| appConfigService.AppConfig.AuthenticationConfig.ExternalJwtAuthenticationConfig.IdentifierClaimName); | ||
|
|
||
| throw new AuthorizationException(); |
There was a problem hiding this comment.
should we not add textual description to exceptino, similar to log statemnt?
There was a problem hiding this comment.
Fixed
| }); | ||
|
|
||
| app.MapGet("/logout", async (HttpRequest req, HttpResponse res) => { | ||
| app.MapPost("/logout", async (HttpRequest req, HttpResponse res) => | ||
| { | ||
| //return webServiceAuthentication.LogOutResponse(req.HttpContext); | ||
| throw new NotImplementedException(); |
There was a problem hiding this comment.
can we implement this? not sure what we need to do though
| app.MapGet("/username", async (HttpRequest req, HttpResponse res) => | ||
| { | ||
| await res.WriteAsync(res.HttpContext.User.Identity.Name); | ||
| }).RequireAuthorization(new[] { BasicAuthenticationDefaults.AuthenticationScheme }); |
There was a problem hiding this comment.
does this only work with basic then?
There was a problem hiding this comment.
Indeed, I support everything is necessary?
|
Prerequisites
Description
Fix #378
Allows 3 kind of authentication: