Skip to content

fix: resolve hardcoded Admin role + Users logging Constitution violations#144

Merged
antosubash merged 3 commits intomainfrom
claude/find-constitution-violations-Dan27
May 3, 2026
Merged

fix: resolve hardcoded Admin role + Users logging Constitution violations#144
antosubash merged 3 commits intomainfrom
claude/find-constitution-violations-Dan27

Conversation

@antosubash
Copy link
Copy Markdown
Owner

Summary

Fixes two categories of Constitution violations surfaced by the audit run on this branch:

  1. Hardcoded "Admin" role string (Constitution §8) — 22 occurrences of policy.RequireRole("Admin") across Admin, PageBuilder, and OpenIddict modules replaced with policy.RequireRole(WellKnownRoles.Admin). The constant already existed in framework/SimpleModule.Core/Authorization/WellKnownRoles.cs (used by ClaimsPrincipalExtensions), so the fix is mechanical and preserves identical semantics.
  2. Direct logger.Log* calls in Users module (Constitution §12) — 19 calls across 11 endpoint files in modules/Users/src/SimpleModule.Users/{Pages,Endpoints}/Account/ converted to source-generated [LoggerMessage] static partial methods, matching the pattern already used in UserService and PageBuilderService. Endpoint classes were promoted to partial.

Files changed

  • Auth fix (16 files): Admin module endpoints (Pages/Admin/*, Endpoints/Admin/*), PageBuilder Pages (Editor, Manage, ViewerDraft), OpenIddict Pages (Clients, ClientsCreate, ClientsEdit).
  • Logging refactor (10 files): Users module RegisterEndpoint, LoginEndpoint, LoginWith2faEndpoint, LoginWithRecoveryCodeEndpoint, LogoutEndpoint, ExternalLoginEndpoint, Manage/ChangePasswordEndpoint, Manage/DeletePersonalDataEndpoint, Endpoints/Account/AccountSecurityEndpoint, Endpoints/Users/DownloadPersonalDataEndpoint.

Verification

  • dotnet build SimpleModule.slnx — succeeds with 0 warnings, 0 errors (TreatWarningsAsErrors is on globally).
  • Tests passing for all four affected modules (run with --no-build):
    • Admin: 28 passed, 1 skipped
    • PageBuilder: 43 passed
    • OpenIddict: 20 passed
    • Users: 40 passed

Out of scope (still pending Constitution violations)

  • 13 entities in Contracts missing [NoDtoGeneration] (Agents, AuditLogs, BackgroundJobs, Chat, Orders, PageBuilder, Products, RateLimiting)
  • SM0052 grandfathered Agents.Module / Rag.Module assembly suffixes (tracked in validate-framework-scope.mjs for Phase 3/4 absorption)
  • Framework scope drift in framework/.allowed-projects (in-flight per Constitution §13)
  • Admin module package.json missing @inertiajs/react peer dep

Test plan

  • CI green on PR
  • Admin/PageBuilder/OpenIddict admin pages still gate on Admin role end-to-end
  • Users login/register/2fa flows still emit log entries (now structured)

Generated by Claude Code

claude added 2 commits May 2, 2026 05:03
….Admin

Replace 22 RequireRole("Admin") literal strings across Admin, PageBuilder,
and OpenIddict modules with the WellKnownRoles.Admin constant from
SimpleModule.Core.Authorization. Eliminates the magic-string violation of
Constitution section 8 ("Never hardcode permission strings — always use the
permission constants") while preserving exact runtime semantics.
…gerMessage]

Replace 19 direct logger.LogInformation/LogWarning calls across the Users
module's account endpoints with [LoggerMessage]-decorated static partial
methods. Aligns with Constitution section 12 ("Use source-generated logging
via [LoggerMessage] attribute for all log messages") and matches the
existing pattern used in UserService and PageBuilderService.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 2, 2026

Deploying simplemodule-website with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9310f41
Status: ✅  Deploy successful!
Preview URL: https://e3d51dfc.simplemodule-website.pages.dev
Branch Preview URL: https://claude-find-constitution-vio.simplemodule-website.pages.dev

View logs

@antosubash antosubash marked this pull request as ready for review May 3, 2026 15:58
@antosubash antosubash merged commit 7c833d1 into main May 3, 2026
6 checks passed
@antosubash antosubash deleted the claude/find-constitution-violations-Dan27 branch May 3, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants