Skip to content

Commit dbf145f

Browse files
committed
Refactor authentication and improve maintainability
- Refactored `ChangePassword` logic to handle edge cases, added `CanSetPasswordIfNeverHadAsync`, and improved error handling. - Enhanced `SaveAndRevokeRefreshTokenAsync` with `AuthType` parameter and updated related calls. - Renamed Google login method to `LoginWithGoogle` and consolidated user creation/update logic. - Introduced `AuthType` enum to represent authentication types. - Updated JWT key validation to use UTF-8 byte count and clarified error messages. - Modularized `SimpleConsoleFormatter` for better readability and structured logging. - Improved password hashing with `ParsedPhc` struct, added `ShouldRehash`, and enhanced memory safety. - Added `GetRemoteIPAddress` to `HttpRequestExtensions` for client IP retrieval. - Removed unused code, cleaned imports, and simplified logic. - Enhanced documentation, comments, and error messages for clarity.
2 parents 59e2cd7 + fed6ff3 commit dbf145f

7 files changed

Lines changed: 207 additions & 230 deletions

File tree

‎Sources/EasyExtensions.AspNetCore.Authorization/Controllers/BaseAuthController.cs‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,18 @@ public async Task<IActionResult> ChangePassword([FromBody] ChangePasswordRequest
4444
{
4545
Guid userId = User.GetUserId();
4646
string? phc = await FindUserPhcAsync(userId);
47-
if (string.IsNullOrWhiteSpace(phc))
47+
bool canSetIfNeverHad = await CanSetPasswordIfNeverHadAsync(userId);
48+
if (string.IsNullOrWhiteSpace(phc) && !canSetIfNeverHad)
4849
{
49-
return this.ApiNotFound("User not found");
50+
return this.ApiNotFound("User or password does not exist");
5051
}
51-
bool isValidPassword = _passwordHasher.Verify(request.CurrentPassword, phc);
52-
if (!isValidPassword)
52+
if (!string.IsNullOrWhiteSpace(phc))
5353
{
54-
return this.ApiBadRequest("Invalid current password");
54+
bool isValidPassword = _passwordHasher.Verify(request.CurrentPassword, phc);
55+
if (!isValidPassword)
56+
{
57+
return this.ApiUnauthorized("Current password is incorrect");
58+
}
5559
}
5660
string newPhc = _passwordHasher.Hash(request.NewPassword);
5761
await SetUserPasswordPhcAsync(userId, newPhc);
@@ -79,7 +83,7 @@ public async Task<IActionResult> RefreshToken([FromBody] RefreshTokenRequestDto
7983
string newRefreshToken = StringHelpers.CreateRandomString(64);
8084
var roles = await GetUserRolesAsync(userId.Value);
8185
string accessToken = CreateAccessToken(userId.Value, roles);
82-
await SaveAndRevokeRefreshTokenAsync(userId.Value, request.RefreshToken, newRefreshToken);
86+
await SaveAndRevokeRefreshTokenAsync(userId.Value, request.RefreshToken, newRefreshToken, AuthType.Unknown);
8387
await OnTokenRefreshedAsync(userId.Value, newRefreshToken);
8488
return Ok(new TokenPairDto
8589
{
@@ -119,7 +123,7 @@ public async Task<IActionResult> Login([FromBody] LoginRequestDto request)
119123
var roles = await GetUserRolesAsync(userId.Value);
120124
string accessToken = CreateAccessToken(userId.Value, roles);
121125
string refreshToken = StringHelpers.CreateRandomString(64);
122-
await SaveAndRevokeRefreshTokenAsync(userId.Value, string.Empty, refreshToken);
126+
await SaveAndRevokeRefreshTokenAsync(userId.Value, string.Empty, refreshToken, AuthType.Credentials);
123127
await OnUserLoggedInAsync(userId.Value, AuthType.Credentials);
124128
return Ok(new TokenPairDto
125129
{
@@ -142,7 +146,7 @@ public async Task<IActionResult> Login([FromBody] LoginRequestDto request)
142146
/// email is not verified.</returns>
143147
/// <exception cref="InvalidOperationException">Thrown if user information cannot be retrieved from Google using the provided token.</exception>
144148
[HttpPost("login/google")]
145-
public async Task<IActionResult> Login([FromQuery] string token)
149+
public async Task<IActionResult> LoginWithGoogle([FromQuery] string token)
146150
{
147151
const string url = "https://openidconnect.googleapis.com/v1/userinfo";
148152
using HttpClient http = new();
@@ -154,19 +158,15 @@ public async Task<IActionResult> Login([FromQuery] string token)
154158
{
155159
return this.ApiUnauthorized("Email is not verified");
156160
}
157-
Guid? userId = await FindUserByUsernameAsync(response.Email);
161+
Guid? userId = await CreateOrUpdateUserFromGoogleAsync(response);
158162
if (!userId.HasValue || userId == Guid.Empty)
159163
{
160-
userId = await CreateUserFromGoogleAsync(response);
161-
if (!userId.HasValue || userId == Guid.Empty)
162-
{
163-
return this.ApiUnauthorized("User not found");
164-
}
164+
return this.ApiUnauthorized("User not found");
165165
}
166166
var roles = await GetUserRolesAsync(userId.Value);
167167
string accessToken = CreateAccessToken(userId.Value, roles);
168168
string refreshToken = StringHelpers.CreateRandomString(64);
169-
await SaveAndRevokeRefreshTokenAsync(userId.Value, string.Empty, refreshToken);
169+
await SaveAndRevokeRefreshTokenAsync(userId.Value, string.Empty, refreshToken, AuthType.Google);
170170
await OnUserLoggedInAsync(userId.Value, AuthType.Google);
171171
return Ok(new TokenPairDto
172172
{
@@ -192,8 +192,9 @@ public async Task<IActionResult> Login([FromQuery] string token)
192192
/// <param name="userId">The unique identifier of the user for whom the refresh token is being updated.</param>
193193
/// <param name="oldRefreshToken">The refresh token to be revoked. Cannot be null or empty.</param>
194194
/// <param name="newRefreshToken">The new refresh token to be saved for the user. Cannot be null or empty.</param>
195+
/// <param name="authType">The type of authentication used for the user when logged in for the first time.</param>
195196
/// <returns>A task that represents the asynchronous operation.</returns>
196-
public abstract Task SaveAndRevokeRefreshTokenAsync(Guid userId, string oldRefreshToken, string newRefreshToken);
197+
public abstract Task SaveAndRevokeRefreshTokenAsync(Guid userId, string oldRefreshToken, string newRefreshToken, AuthType authType);
197198

198199
/// <summary>
199200
/// Asynchronously retrieves the unique identifier of a user associated with the specified refresh token.
@@ -227,6 +228,15 @@ public async Task<IActionResult> Login([FromQuery] string token)
227228
/// roles.</returns>
228229
public abstract Task<IEnumerable<string>> GetUserRolesAsync(Guid userId);
229230

231+
/// <summary>
232+
/// Determines whether a password can be set for the specified user if the user has never previously set a
233+
/// password.
234+
/// </summary>
235+
/// <param name="userId">The unique identifier of the user to evaluate. This value must correspond to an existing user.</param>
236+
/// <returns>A task that represents the asynchronous operation. The task result is <see langword="true"/> if the user is
237+
/// eligible to set a password for the first time; otherwise, <see langword="false"/>.</returns>
238+
public abstract Task<bool> CanSetPasswordIfNeverHadAsync(Guid userId);
239+
230240
/// <summary>
231241
/// Handles logic to be executed after a user has changed their password.
232242
/// </summary>
@@ -272,11 +282,12 @@ public virtual bool IsEmailVerificationRequired()
272282

273283
/// <summary>
274284
/// Creates a new user account based on the information provided by a Google OpenID response.
285+
/// If a user with the same email already exists, their information may be updated instead.
275286
/// </summary>
276287
/// <param name="dto">The Google OpenID response data containing user information to create the account. Cannot be null.</param>
277288
/// <returns>A task that represents the asynchronous operation. The task result contains the unique identifier of the
278289
/// newly created user if successful; otherwise, null.</returns>
279-
public virtual Task<Guid?> CreateUserFromGoogleAsync(GoogleOpenIdResponseDto dto)
290+
public virtual Task<Guid?> CreateOrUpdateUserFromGoogleAsync(GoogleOpenIdResponseDto dto)
280291
{
281292
return Task.FromResult<Guid?>(null);
282293
}

‎Sources/EasyExtensions.AspNetCore.Authorization/Extensions/ConfigurationExtensions.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,13 @@ internal static JwtSettings GetJwtSettings(this IConfiguration configuration)
2323
string key = (!jwtSettings.Exists() ? configuration["JwtKey"] : jwtSettings["Key"])
2424
?? throw new KeyNotFoundException("JwtSettings.Key or JwtKey is not set");
2525

26-
string algorithm = key.Length switch
26+
int utf8KeyLength = System.Text.Encoding.UTF8.GetByteCount(key);
27+
string algorithm = utf8KeyLength switch
2728
{
2829
256 / 8 => SecurityAlgorithms.HmacSha256,
2930
384 / 8 => SecurityAlgorithms.HmacSha384,
3031
512 / 8 => SecurityAlgorithms.HmacSha512,
31-
_ => throw new ArgumentOutOfRangeException(nameof(configuration), "Key length must be 128, 192 or 256 bits (16, 24 or 32 symbols), but was " + key.Length + " symbols.")
32+
_ => throw new ArgumentOutOfRangeException(nameof(configuration), "Key length must be 256, 384 or 512 bits (32, 48 or 64 bytes), but was " + utf8KeyLength + " bytes.")
3233
};
3334

3435
return new JwtSettings

‎Sources/EasyExtensions.AspNetCore.Authorization/Models/Dto/Enums/AuthType.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
/// </summary>
66
public enum AuthType
77
{
8+
/// <summary>
9+
/// Indicates that the value is unknown or has not been specified.
10+
/// </summary>
11+
/// <remarks>Use this value when the actual value is not available or cannot be determined. This
12+
/// is typically the default value for the enumeration.</remarks>
13+
Unknown = 0,
14+
815
/// <summary>
916
/// Indicates that the content type is credentials, such as a username and password or authentication token.
1017
/// </summary>

‎Sources/EasyExtensions.AspNetCore.Stack/Extensions/HostApplicationBuilderExtensions.cs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,6 @@ public static IHostApplicationBuilder AddEasyStack(
153153

154154
if (options.AuthorizationEnabled)
155155
{
156-
if (options.ConfigureDatabase == null)
157-
{
158-
throw new Exception("Authorization requires Postgres. Set AddPostgres to true in EasyStackOptions.");
159-
}
160156
if (!isPepperPresented)
161157
{
162158
throw new Exception("Authorization requires Pbkdf2PasswordHashService. Set configuration key 'EasyExtensions:Services:Pbkdf2:Pepper'.");

‎Sources/EasyExtensions.AspNetCore/Extensions/HttpRequestExtensions.cs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Net;
23
using System.Linq;
34
using Microsoft.AspNetCore.Http;
45
using System.Collections.Generic;
@@ -13,6 +14,24 @@ public static class HttpRequestExtensions
1314
{
1415
private static readonly ConcurrentDictionary<string, List<DateTime>> _accesses = new();
1516

17+
/// <summary>
18+
/// Retrieves the remote IP address of the client that initiated the HTTP request.
19+
/// </summary>
20+
/// <remarks>If the remote address cannot be parsed as a valid IP address, the method returns
21+
/// IPAddress.None. This method relies on the value returned by GetRemoteAddress, which may be influenced by
22+
/// proxy headers or server configuration.</remarks>
23+
/// <param name="request">The HTTP request from which to extract the remote IP address. Cannot be null.</param>
24+
/// <returns>An IPAddress representing the remote client's IP address if it can be determined; otherwise, IPAddress.None.</returns>
25+
public static IPAddress GetRemoteIPAddress(this HttpRequest request)
26+
{
27+
string ipString = request.GetRemoteAddress();
28+
if (IPAddress.TryParse(ipString, out var ipAddress))
29+
{
30+
return ipAddress;
31+
}
32+
return IPAddress.None;
33+
}
34+
1635
/// <summary>
1736
/// Get remote host IP address using proxy "X-Real-IP", "CF-Connecting-IP", "X-Forwarded-For" headers, or connection remote IP address.
1837
/// </summary>

0 commit comments

Comments
 (0)