Added refresh endpoint for access token #120
Conversation
| @@ -14,5 +14,6 @@ public static void MapEndpoints(WebApplication app) | |||
| app.MapPost("/authentication/login", Login.PostLogin).WithTags("Authentication").WithOpenApi(); | |||
| app.MapPut("/authentication/changePassword", ChangePassword.PutChangePassword).WithTags("Authentication").WithOpenApi().RequireAuthorization("user"); | |||
| app.MapPut("/authentication/logout", Logout.PutLogout).WithTags("Authentication").WithOpenApi().RequireAuthorization("user"); | |||
| app.MapPut("/authentication/refreshAccess", RefreshEndpointAccessToken.PostRefreshAccessToken).WithTags("Authentication").WithOpenApi().RequireAuthorization("user"); | |||
There was a problem hiding this comment.
change the endpoint path to "authentication/refreshAccessToken" please
| { | ||
|
|
||
| /// <summary> | ||
| /// Creates a new access token. |
There was a problem hiding this comment.
... based on a refresh token
| return TypedResults.Unauthorized(); | ||
| } | ||
|
|
||
| User? user = databaseHandle.Users.Where(u => u.Username == requestBody.UserName).FirstOrDefault(); |
There was a problem hiding this comment.
You dont need to use a body here, just use the claims from the token to get the username
|
|
||
| User? user = databaseHandle.Users.Where(u => u.Username == requestBody.UserName).FirstOrDefault(); | ||
|
|
||
| // If no user is found matching the redentials, return 404. |
There was a problem hiding this comment.
There should be no possibility for that, since the user is authenticated using a refresh token. Still, should the user be null, just return unauthorized. This is not a login request.
| { | ||
| databaseHandle.Database.EnsureCreated(); | ||
|
|
||
| if (!AnonKeyBackend.Authentication.TokenActions.ValidateClaimsOnRequest(userClaimsPrincipal, databaseHandle)) |
There was a problem hiding this comment.
set isRefreshRequest = true here
| } | ||
|
|
||
| // Generate a new token and return it to the user. | ||
| Token refreshToken = tokenService.GenerateNewToken(user, "RefreshToken"); |
|
|
||
| // Generate a new token and return it to the user. | ||
| Token refreshToken = tokenService.GenerateNewToken(user, "RefreshToken"); | ||
| Token accessToken = tokenService.GenerateNewToken(user, "AccessToken"); |
There was a problem hiding this comment.
Pass parent token uuid to this call
| Ok<ApiDatastructures.Authentication.Login.AuthenticationLoginResponseBody>, | ||
| NotFound<ApiDatastructures.RequestError.ErrorResponseBody>, | ||
| UnauthorizedHttpResult> | ||
| PostRefreshAccessToken(ClaimsPrincipal userClaimsPrincipal, ApiDatastructures.Authentication.Login.AuthenticationLoginRequestBody requestBody, AnonKeyBackend.Authentication.TokenService tokenService, Data.DatabaseHandle databaseHandle) |
There was a problem hiding this comment.
Please define your own return schemas in ApiDatastructures.
| @@ -0,0 +1,20 @@ | |||
| namespace AnonKeyBackend.ApiDatastructures.Authentication.RefreshTokens; | |||
There was a problem hiding this comment.
Ideally name the Namespace more something like "RefreshAccessToken" (make sure to rename the folder appropriately)
| /// <summary> | ||
| /// Represents a token object in the response to a Refresh request. | ||
| /// </summary> | ||
| public class AuthenticationRefreshTokens |
There was a problem hiding this comment.
Rename the class to something like "AuthenticationRefreshAccessToken" to avoid name conflicts
| /// <summary> | ||
| /// The body of the response to a Refresh request. | ||
| /// </summary> | ||
| public class AuthenticationRefreshTokensResponseBody |
There was a problem hiding this comment.
Rename the class to something like "AuthenticationRefreshAccessTokenResponseBody" to avoid name conflicts
| /// <summary> | ||
| /// The token for refreshing the access token or the refresh token itself | ||
| /// </summary> | ||
| public AuthenticationRefreshTokens? RefreshToken { get; set; } |
There was a problem hiding this comment.
There wont be a RefreshToken returned by this request, so this property can be removed.
| return TypedResults.Unauthorized(); | ||
| } | ||
|
|
||
| User? userName = databaseHandle.Users.Where(u => u.Username == user.Identity.Name).FirstOrDefault(); |
There was a problem hiding this comment.
- This variable name may be misleading, as it implies type "string", while being "User?".
- You are missing a null check on user.Identity (formal requirement mostly...)
| return TypedResults.Unauthorized(); | ||
| } | ||
|
|
||
| string tokenUuid = user.Claims.First(c => c.Type == "TokenParent").Value; |
There was a problem hiding this comment.
The token used for authentication against this endpoint is a RefreshToken, use the TokenUuid claim instead of TokenParent. (TokenParent is not populated for refresh tokens)
No description provided.