Skip to content

Commit 6e223d7

Browse files
committed
Refactor erroror (#22)
* feat(validation): add ValidationHelper for FluentValidation integration refactor(validators): update password validation to use IdmtPasswordOptions chore(solution): remove old .sln file and add new .slnx format fix(dependencies): update package versions for Idmt.BasicSample project test(integration): remove obsolete system info tests from AdminIntegrationTests test(integration): update AuthIntegrationTests to reflect new endpoint naming conventions test(integration): modify ManageIntegrationTests to use new reset-password endpoint test(integration): adjust MultiTenancyIntegrationTests for updated endpoint paths chore(tests): update Idmt.UnitTests project dependencies and remove unused tests test(unit): refactor ValidatorsTests to use IdmtPasswordOptions and remove obsolete validations * feat(auth): require authorization for logout endpoint feat(auth): enhance refresh token handling with tenant validation fix(auth): add logging for password reset errors refactor(health): simplify health check by removing tenant user count feat(manage): update GetUserInfo to return detailed errors refactor(manage): streamline user registration process fix(manage): ensure user info updates only when changes occur fix(middleware): improve error handling in ValidateBearerTokenTenantMiddleware fix(persistence): use enum for audit actions in IdmtDbContext chore(tests): add unit tests for tenant operation service chore(tests): implement fluent validation tests for various requests chore(tests): update integration tests for tenant management * fix: address 8 high-priority bugs across auth, admin, and health endpoints Security: - Close refresh token tenant validation bypass by rejecting when tenant claim or context is null instead of silently allowing through - Block inactive users from resetting passwords via ResetPassword endpoint Correctness: - Return Unhealthy when database CanConnectAsync returns false (was always reporting Healthy) and propagate CancellationToken - Convert Logout handler to ErrorOr pattern with proper error handling instead of re-throwing raw exceptions to the client - Map Tenant.NotResolved (Validation type) to 400 in login endpoints instead of falling through to the default 500 arm - Return 409 Conflict when creating a tenant that already exists and is active, instead of silently returning 201 Created API consistency: - Rename RevokeTenantAccess route param from tenantId to tenantIdentifier to match GrantTenantAccess endpoint convention - Remove dead Ok<AccessTokenResponse> from RefreshToken Results<> type union since only SignInHttpResult is returned on success * feat: enhance tenant management with validation and logging improvements * Add unit tests for user info update handler and middleware - Implement `UpdateUserInfoHandlerTests` to validate user info update logic, including scenarios for missing claims, inactive users, and unchanged fields. - Add `CurrentUserMiddlewareTests` to ensure current user is set correctly from authenticated requests. - Enhance `ValidateBearerTokenTenantMiddlewareTests` with additional tests for empty tenant claims and custom claim types. - Create `IdmtTenantInfoTests` to validate tenant info model behavior and constraints. - Introduce `TokenRevocationServiceTests` to verify token revocation logic and cleanup of expired tokens. - Update `FluentValidatorTests` to check password reset request validation for weak passwords. - Modify `IdmtLinkGeneratorTests` to ensure email confirmation links are generated correctly with Base64 URL encoding. - Update project dependencies in `Idmt.UnitTests.csproj` for SQLite and testing utilities. * fix(tests): update RefreshTokenHandlerTests to handle null tenant context correctly * feat: add TokenRevocationCleanupService and enhance authorization for tenant management endpoints * feat(auth): Implement rate limiting for authentication endpoints - Added a rate limiter policy to the authentication endpoints to prevent brute-force attacks and email flooding. - Integrated the rate limiting feature based on configuration options. refactor(manage): Change user role representation to a list - Updated the GetUserInfo response to return a list of roles instead of a single role. - Adjusted related tests to validate the new roles structure. chore(project): Add Microsoft.AspNetCore.App framework reference - Included a framework reference for Microsoft.AspNetCore.App in the project file. fix(token): Handle concurrent token revocation gracefully - Implemented a retry mechanism for concurrent token revocation to ensure correct expiration handling. - Added tests to verify the behavior under race conditions. test(tests): Enhance integration tests for user management and authentication - Updated tests to reflect changes in user roles and authentication flow. - Added new tests for pagination and role retrieval in user management. test(tests): Add unit tests for IdmtOptions and RateLimitingOptions - Created unit tests to validate the configuration options for IdmtOptions and RateLimitingOptions. - Ensured default values and custom configurations are correctly handled. test(tests): Improve LogoutHandler tests for tenant context handling - Enhanced tests for LogoutHandler to verify behavior with tenant context and claims. - Added assertions for logging warnings when tenant information is missing. * feat(auth): Enhance login and refresh token handling with improved error responses and options integration - Added handling for locked-out users during login attempts in Login.cs. - Integrated IdmtOptions for cookie expiration in login authentication properties. - Updated refresh token logic to utilize DateTimeOffset for issued and expiration times in RefreshToken.cs. - Refactored middleware to return ProblemDetails for unauthorized and forbidden responses in ValidateBearerTokenTenantMiddleware.cs. - Changed timestamp properties in IdmtAuditLog, IdmtUser, RevokedToken, and TenantAccess models to DateTimeOffset for better time zone handling. - Updated database context to store DateTimeOffset as UTC ticks for compatibility across providers. - Modified token revocation service to accept DateTimeOffset for issuedAt parameter. - Introduced IdmtEmailSenderStartupCheck to warn about unconfigured email sender at startup. - Enhanced unit tests to cover new locked-out scenarios and updated date handling. * refactor(errors): Clean up error definitions and formatting in IdmtErrors class * refactor(auth): Update authorization policies to require SysUserPolicy for tenant access operations * refactor(ci): Update solution file references from .sln to .slnx in CI workflows ---------
1 parent c27aee4 commit 6e223d7

119 files changed

Lines changed: 8837 additions & 2219 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,19 @@ jobs:
2020
dotnet-version: 9.0.x
2121

2222
- name: Restore dependencies
23-
run: dotnet restore src/Idmt.sln
23+
run: dotnet restore src/Idmt.slnx
2424

2525
- name: Format check
26-
run: dotnet format src/Idmt.sln --verify-no-changes --verbosity diagnostic
26+
run: dotnet format src/Idmt.slnx --verify-no-changes --verbosity diagnostic
2727

2828
- name: Build
29-
run: dotnet build src/Idmt.sln --no-restore --configuration Release /p:TreatWarningsAsErrors=true
29+
run: dotnet build src/Idmt.slnx --no-restore --configuration Release /p:TreatWarningsAsErrors=true
3030

3131
- name: Run analyzers
32-
run: dotnet build src/Idmt.sln --no-restore --configuration Release /p:RunAnalyzers=true /p:EnforceCodeStyleInBuild=true
32+
run: dotnet build src/Idmt.slnx --no-restore --configuration Release /p:RunAnalyzers=true /p:EnforceCodeStyleInBuild=true
3333

3434
- name: Test
35-
run: dotnet test src/Idmt.sln --no-build --configuration Release --verbosity normal
35+
run: dotnet test src/Idmt.slnx --no-build --configuration Release --verbosity normal
3636

3737
- name: Pack
3838
run: dotnet pack src/Idmt.Plugin/Idmt.Plugin.csproj --no-build --configuration Release --output .

.github/workflows/publish.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ jobs:
1616
dotnet-version: 9.0.x
1717

1818
- name: Restore dependencies
19-
run: dotnet restore src/Idmt.sln
19+
run: dotnet restore src/Idmt.slnx
2020

2121
- name: Build
22-
run: dotnet build src/Idmt.sln --no-restore --configuration Release
22+
run: dotnet build src/Idmt.slnx --no-restore --configuration Release
2323

2424
- name: Test
25-
run: dotnet test src/Idmt.sln --no-build --configuration Release
25+
run: dotnet test src/Idmt.slnx --no-build --configuration Release
2626

2727
- name: Set Version
2828
id: get_version

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,5 @@ nunit-*.xml
5858
*.db
5959
*.db-shm
6060
*.db-wal
61+
62+
.claude/settings.local.json
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace Idmt.Plugin.Configuration;
2+
3+
internal static class IdmtEndpointNames
4+
{
5+
public const string ConfirmEmail = "ConfirmEmail";
6+
public const string ConfirmEmailDirect = "ConfirmEmail-direct";
7+
public const string PasswordReset = "ResetPassword";
8+
}

src/Idmt.Plugin/Configuration/IdmtOptions.cs

Lines changed: 116 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class IdmtOptions
2424
/// <summary>
2525
/// Identity configuration options
2626
/// </summary>
27-
public AuthOptions Identity { get; set; } = new();
27+
public IdmtAuthOptions Identity { get; set; } = new();
2828

2929
/// <summary>
3030
/// Multi-tenant configuration options
@@ -35,15 +35,45 @@ public class IdmtOptions
3535
/// Database configuration options
3636
/// </summary>
3737
public DatabaseOptions Database { get; set; } = new();
38+
39+
/// <summary>
40+
/// Rate limiting configuration options for auth endpoints
41+
/// </summary>
42+
public RateLimitingOptions RateLimiting { get; set; } = new();
43+
44+
}
45+
46+
/// <summary>
47+
/// Controls how email confirmation links behave.
48+
/// </summary>
49+
public enum EmailConfirmationMode
50+
{
51+
/// <summary>
52+
/// Email link points to GET /auth/confirm-email on the server, which confirms
53+
/// the email directly (like Microsoft's reference implementation).
54+
/// No client-side form needed for email confirmation.
55+
/// </summary>
56+
ServerConfirm,
57+
58+
/// <summary>
59+
/// Email link points to ClientUrl/ConfirmEmailFormPath on the client app.
60+
/// The client reads the token from the URL and calls POST /auth/confirm-email.
61+
/// Default for SPA/mobile apps.
62+
/// </summary>
63+
ClientForm
3864
}
3965

4066
/// <summary>
4167
/// Application configuration options
4268
/// </summary>
4369
public class ApplicationOptions
4470
{
45-
public const string PasswordResetEndpointName = "ResetPassword";
46-
public const string ConfirmEmailEndpointName = "ConfirmEmail";
71+
/// <summary>
72+
/// URI prefix applied to all IDMT endpoint groups (/auth, /manage, /admin, /health).
73+
/// Defaults to "/api/v1". Set to "" to restore the legacy unprefixed behavior.
74+
/// Examples: "/api/v1", "/v2", "/api/v2", ""
75+
/// </summary>
76+
public string ApiPrefix { get; set; } = "/api/v1";
4777

4878
/// <summary>
4979
/// Base URL of the client application, if any (e.g. "https://myapp.com")
@@ -57,12 +87,19 @@ public class ApplicationOptions
5787

5888
public string ResetPasswordFormPath { get; set; } = "/reset-password";
5989
public string ConfirmEmailFormPath { get; set; } = "/confirm-email";
90+
91+
/// <summary>
92+
/// Controls how email confirmation links are generated.
93+
/// ServerConfirm: link hits GET /auth/confirm-email which confirms directly.
94+
/// ClientForm: link points to ClientUrl/ConfirmEmailFormPath for SPA handling.
95+
/// </summary>
96+
public EmailConfirmationMode EmailConfirmationMode { get; set; } = EmailConfirmationMode.ClientForm;
6097
}
6198

6299
/// <summary>
63100
/// ASP.NET Core Identity configuration
64101
/// </summary>
65-
public class AuthOptions
102+
public class IdmtAuthOptions
66103
{
67104
public const string CookieOrBearerScheme = "CookieOrBearer";
68105

@@ -75,7 +112,7 @@ public class AuthOptions
75112
/// <summary>
76113
/// Password requirements
77114
/// </summary>
78-
public PasswordOptions Password { get; set; } = new();
115+
public IdmtPasswordOptions Password { get; set; } = new();
79116

80117
/// <summary>
81118
/// User requirements
@@ -90,7 +127,7 @@ public class AuthOptions
90127
/// <summary>
91128
/// Cookie configuration options
92129
/// </summary>
93-
public CookieOptions Cookie { get; set; } = new();
130+
public IdmtCookieOptions Cookie { get; set; } = new();
94131

95132
/// <summary>
96133
/// Bearer token configuration options
@@ -106,13 +143,13 @@ public class AuthOptions
106143
/// <summary>
107144
/// Password configuration options
108145
/// </summary>
109-
public class PasswordOptions
146+
public class IdmtPasswordOptions
110147
{
111148
public bool RequireDigit { get; set; } = true;
112149
public bool RequireLowercase { get; set; } = true;
113150
public bool RequireUppercase { get; set; } = true;
114151
public bool RequireNonAlphanumeric { get; set; } = false;
115-
public int RequiredLength { get; set; } = 6;
152+
public int RequiredLength { get; set; } = 8;
116153
public int RequiredUniqueChars { get; set; } = 1;
117154
}
118155

@@ -130,19 +167,34 @@ public class UserOptions
130167
/// </summary>
131168
public class SignInOptions
132169
{
133-
public bool RequireConfirmedEmail { get; set; } = false;
170+
public bool RequireConfirmedEmail { get; set; } = true;
134171
public bool RequireConfirmedPhoneNumber { get; set; } = false;
135172
}
136173

137174
/// <summary>
138175
/// Cookie configuration options
139176
/// </summary>
140-
public class CookieOptions
177+
public class IdmtCookieOptions
141178
{
142179
public string Name { get; set; } = ".Idmt.Application";
143180
public bool HttpOnly { get; set; } = true;
144-
public Microsoft.AspNetCore.Http.CookieSecurePolicy SecurePolicy { get; set; } = Microsoft.AspNetCore.Http.CookieSecurePolicy.SameAsRequest;
145-
public Microsoft.AspNetCore.Http.SameSiteMode SameSite { get; set; } = Microsoft.AspNetCore.Http.SameSiteMode.Lax;
181+
public Microsoft.AspNetCore.Http.CookieSecurePolicy SecurePolicy { get; set; } = Microsoft.AspNetCore.Http.CookieSecurePolicy.Always;
182+
183+
/// <summary>
184+
/// Controls the SameSite attribute of the authentication cookie.
185+
/// Defaults to <see cref="Microsoft.AspNetCore.Http.SameSiteMode.Strict"/>, which means the
186+
/// browser will never send the cookie on cross-site requests (neither top-level navigations
187+
/// nor sub-resource loads). This is the strongest available CSRF protection at the cookie
188+
/// layer and removes the need for anti-forgery tokens on state-mutating endpoints that rely
189+
/// solely on cookie authentication.
190+
///
191+
/// Change to <see cref="Microsoft.AspNetCore.Http.SameSiteMode.Lax"/> only if your
192+
/// application requires cookie preservation on top-level cross-site GET navigations (e.g.
193+
/// OAuth / OIDC redirect flows), and compensate with explicit anti-forgery validation on
194+
/// every state-mutating endpoint.
195+
/// </summary>
196+
public Microsoft.AspNetCore.Http.SameSiteMode SameSite { get; set; } = Microsoft.AspNetCore.Http.SameSiteMode.Strict;
197+
146198
public TimeSpan ExpireTimeSpan { get; set; } = TimeSpan.FromDays(14);
147199
public bool SlidingExpiration { get; set; } = true;
148200
public bool IsRedirectEnabled { get; set; } = false;
@@ -191,7 +243,7 @@ public class MultiTenantOptions
191243
/// </summary>
192244
public const string DefaultTenantIdentifier = "system-tenant";
193245

194-
public string DefaultTenantDisplayName { get; set; } = "System Tenant";
246+
public string DefaultTenantName { get; set; } = "System Tenant";
195247

196248
/// <summary>
197249
/// Tenant resolution strategy (header, subdomain, etc.)
@@ -204,18 +256,65 @@ public class MultiTenantOptions
204256
public Dictionary<string, string> StrategyOptions { get; set; } = [];
205257
}
206258

259+
/// <summary>
260+
/// Controls how the IDMT database schema is initialized on startup.
261+
/// </summary>
262+
public enum DatabaseInitializationMode
263+
{
264+
/// <summary>
265+
/// Use EF Core Migrations. Consumers must create and apply migrations themselves.
266+
/// Recommended for production. Default.
267+
/// </summary>
268+
Migrate,
269+
270+
/// <summary>
271+
/// Use EnsureCreated for quick setup. Not compatible with migrations.
272+
/// Suitable for development, testing, and prototyping only.
273+
/// </summary>
274+
EnsureCreated,
275+
276+
/// <summary>
277+
/// Skip automatic database initialization. Consumer manages the database schema externally.
278+
/// </summary>
279+
None
280+
}
281+
207282
/// <summary>
208283
/// Database configuration options
209284
/// </summary>
210285
public class DatabaseOptions
211286
{
212287
/// <summary>
213-
/// Connection string template with placeholder for tenant's properties
288+
/// Controls how the database schema is initialized on startup.
289+
/// <see cref="DatabaseInitializationMode.Migrate"/> runs EF Core migrations and is the
290+
/// default for production use. <see cref="DatabaseInitializationMode.EnsureCreated"/> is
291+
/// suitable for development, testing, and prototyping where migrations are not used.
292+
/// <see cref="DatabaseInitializationMode.None"/> skips initialization entirely and leaves
293+
/// schema management to the consumer.
294+
/// </summary>
295+
public DatabaseInitializationMode DatabaseInitialization { get; set; } = DatabaseInitializationMode.Migrate;
296+
}
297+
298+
/// <summary>
299+
/// Rate limiting configuration for IDMT auth endpoints.
300+
/// When enabled, a fixed-window limiter named "idmt-auth" is registered and applied
301+
/// to all authentication endpoints (login, token, forgot-password, etc.) to protect
302+
/// against brute-force and email-flooding attacks.
303+
/// </summary>
304+
public class RateLimitingOptions
305+
{
306+
/// <summary>
307+
/// Enable built-in rate limiting for auth endpoints. Default: true.
308+
/// </summary>
309+
public bool Enabled { get; set; } = true;
310+
311+
/// <summary>
312+
/// Maximum number of requests allowed per window for auth endpoints. Default: 10.
214313
/// </summary>
215-
public string ConnectionStringTemplate { get; set; } = string.Empty;
314+
public int PermitLimit { get; set; } = 10;
216315

217316
/// <summary>
218-
/// Auto-migrate database on startup
317+
/// Duration of the sliding window in seconds. Default: 60.
219318
/// </summary>
220-
public bool AutoMigrate { get; set; } = false;
319+
public int WindowInSeconds { get; set; } = 60;
221320
}
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
using Microsoft.Extensions.Options;
2+
3+
namespace Idmt.Plugin.Configuration;
4+
5+
/// <summary>
6+
/// Validates <see cref="IdmtOptions"/> at application startup so that
7+
/// misconfigured options are reported immediately rather than causing
8+
/// unexpected failures at runtime.
9+
/// </summary>
10+
public sealed class IdmtOptionsValidator : IValidateOptions<IdmtOptions>
11+
{
12+
/// <inheritdoc />
13+
public ValidateOptionsResult Validate(string? name, IdmtOptions options)
14+
{
15+
var failures = new List<string>();
16+
17+
ValidateApplicationOptions(options.Application, failures);
18+
ValidateMultiTenantOptions(options.MultiTenant, failures);
19+
20+
return failures.Count > 0
21+
? ValidateOptionsResult.Fail(failures)
22+
: ValidateOptionsResult.Success;
23+
}
24+
25+
private static void ValidateApplicationOptions(ApplicationOptions application, List<string> failures)
26+
{
27+
// Rule 1: ApiPrefix must not be null or empty.
28+
// An empty string is the documented opt-out for legacy unprefixed behavior,
29+
// but null indicates the property was explicitly cleared and is misconfigured.
30+
if (application.ApiPrefix is null)
31+
{
32+
failures.Add(
33+
$"{nameof(IdmtOptions.Application)}.{nameof(ApplicationOptions.ApiPrefix)} must not be null. " +
34+
"Use an empty string \"\" to disable the prefix or provide a value such as \"/api/v1\".");
35+
}
36+
37+
// Rule 2: When EmailConfirmationMode is ClientForm, ClientUrl is required.
38+
if (application.EmailConfirmationMode == EmailConfirmationMode.ClientForm &&
39+
string.IsNullOrWhiteSpace(application.ClientUrl))
40+
{
41+
failures.Add(
42+
$"{nameof(IdmtOptions.Application)}.{nameof(ApplicationOptions.ClientUrl)} must not be null or empty " +
43+
$"when {nameof(ApplicationOptions.EmailConfirmationMode)} is {nameof(EmailConfirmationMode.ClientForm)}.");
44+
}
45+
46+
// Rule 3: When ClientUrl is set, the client-side form paths must also be configured.
47+
if (!string.IsNullOrWhiteSpace(application.ClientUrl))
48+
{
49+
if (string.IsNullOrWhiteSpace(application.ConfirmEmailFormPath))
50+
{
51+
failures.Add(
52+
$"{nameof(IdmtOptions.Application)}.{nameof(ApplicationOptions.ConfirmEmailFormPath)} must not be null or empty " +
53+
$"when {nameof(ApplicationOptions.ClientUrl)} is set.");
54+
}
55+
56+
if (string.IsNullOrWhiteSpace(application.ResetPasswordFormPath))
57+
{
58+
failures.Add(
59+
$"{nameof(IdmtOptions.Application)}.{nameof(ApplicationOptions.ResetPasswordFormPath)} must not be null or empty " +
60+
$"when {nameof(ApplicationOptions.ClientUrl)} is set.");
61+
}
62+
}
63+
}
64+
65+
private static void ValidateMultiTenantOptions(MultiTenantOptions multiTenant, List<string> failures)
66+
{
67+
// Rule 4: The constant DefaultTenantIdentifier is a compile-time value and cannot be
68+
// null or empty. Validate it defensively so that any future refactor to an instance
69+
// property is caught immediately.
70+
if (string.IsNullOrWhiteSpace(MultiTenantOptions.DefaultTenantIdentifier))
71+
{
72+
failures.Add(
73+
$"{nameof(IdmtOptions.MultiTenant)}.{nameof(MultiTenantOptions.DefaultTenantIdentifier)} must not be null or empty.");
74+
}
75+
76+
// Rule 5: DefaultTenantName is configurable and must not be null or empty.
77+
if (string.IsNullOrWhiteSpace(multiTenant.DefaultTenantName))
78+
{
79+
failures.Add(
80+
$"{nameof(IdmtOptions.MultiTenant)}.{nameof(MultiTenantOptions.DefaultTenantName)} must not be null or empty.");
81+
}
82+
83+
// Rule 6: At least one tenant resolution strategy must be configured.
84+
// The Strategies array controls which strategies (header, claim, route, basepath) are
85+
// active. An empty array means no tenant can ever be resolved, which is always a
86+
// misconfiguration. StrategyOptions holds per-strategy overrides and may be empty.
87+
if (multiTenant.Strategies.Length == 0)
88+
{
89+
failures.Add(
90+
$"{nameof(IdmtOptions.MultiTenant)}.{nameof(MultiTenantOptions.Strategies)} must contain at least one entry. " +
91+
$"Supported values: {IdmtMultiTenantStrategy.Header}, {IdmtMultiTenantStrategy.Claim}, " +
92+
$"{IdmtMultiTenantStrategy.Route}, {IdmtMultiTenantStrategy.BasePath}.");
93+
}
94+
}
95+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
namespace Idmt.Plugin.Constants;
2+
3+
public enum AuditAction
4+
{
5+
Created,
6+
Modified,
7+
Deleted
8+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
namespace Idmt.Plugin.Constants;
2+
3+
public static class IdmtClaimTypes
4+
{
5+
public const string IsActive = "is_active";
6+
}

0 commit comments

Comments
 (0)