Skip to main content
Version: Latest

15. Security Notes

15.1 Current posture (as of Phase 1.6)

  • Every onboarding endpoint requires a valid JWT (401 otherwise) and the correct permission (403 otherwise) — see Chapter 9.
  • answers/recommendation/apply/skip — the actions that can change tenant configuration — are restricted to the tenant Admin role. view-only actions (status, survey) are open to any authenticated tenant user.
  • The internal apply bridge requires a shared secret key, is hidden from Swagger, and independently verifies the resolved tenant matches the request before touching any settings.
  • All non-onboarding permissions in the MultiTenancy host deny by default — there is no "allow all" fallback anywhere in this feature's authorization code.

15.2 Hardening history (why this matters)

This feature's authorization model was built and hardened incrementally, and two of the issues found along the way were not related to onboarding at all but were serious platform-wide findings surfaced while scanning Shumoul.MultiTenancyApi's controllers during this work:

  • OnboardingController originally had a class-level [AllowAnonymous] that silently bypassed every [MustHavePermission] check on every action, regardless of the permission attribute being present — this is a general ASP.NET Core behavior ([AllowAnonymous] anywhere in an endpoint's metadata skips authorization entirely) worth knowing if you ever add a new controller here.
  • SharedDbConnectionStringsController.GetActiveListAsync() had no permission check of any kind and was fully anonymous — it returned raw ServerName, UserName, Password, and ConnectionString for every shared database connection on the platform to any unauthenticated caller. Found and fixed during this feature's security review, unrelated to onboarding itself. See Chapter 18 — Phase History for the full list of what was found and fixed, and the Operational Notes chapter for the recommended remediation (credential rotation) if this endpoint was ever reachable in a live environment before the fix.
  • TestNotificationsController had no permission check at all and could trigger a real notification send to an arbitrary user/tenant/topic anonymously — gated behind a new, deny-by-default permission.

None of these three were onboarding bugs, but all three were found specifically because this feature's implementation required a systematic authorization review of the host it lives in. They are documented here, not just in a changelog, because anyone extending this host should be aware [AllowAnonymous] stacked with a permission attribute is a known trap in this codebase — always check both.

15.3 What this feature deliberately does not do

  • It does not introduce a new identity or permission storage system — see §9.2 for why a claims-based approach was chosen instead of a database-backed one.
  • It never accepts a tenant identifier from request body/query for authorization decisions — only from the authenticated JWT.
  • The internal bridge never accepts a user JWT — only the shared internal key.

15.4 Reporting a concern

If you find another [AllowAnonymous]-related gap while working in Shumoul.MultiTenancyApi, treat it the same way this feature's review did: confirm whether the action also declares [MustHavePermission] (a conflict — fix it) or genuinely has none at all (assess whether it's really meant to be public before touching it), rather than assuming the class-level attribute reflects current intent.