Security: Fix authorization bypass and data exposure in UserProfile/Eventing APIs

[Copilot]

Description

Multiple API endpoints allowed authenticated users to enumerate all users and access other users' data. The /register endpoint used email-based lookup creating Auth0Subject ownership inconsistencies.

EventDB Configuration

services/Visage.Services.Eventing/EventDB.cs

• Added Ignore() directives for EventRegistration.User and EventRegistration.UserId to prevent cross-DbContext shadow properties

entity.Ignore(e => e.User);
entity.Ignore(e => e.UserId);

Eventing API

services/Visage.Services.Eventing/Program.cs

• LookupByPin: Returns 401 Unauthorized instead of 400 BadRequest for missing auth
• Created RegistrationDto to limit response fields (excludes RejectionReason, AdditionalDetails, ApprovedBy)

UserProfile API

Visage.Services.UserProfile/Program.cs

GET /api/users

• Returns only caller's profile instead of all users

GET /api/users/{userId}/registrations

• Added ownership check: caller must be the owner OR have Admin role
• Returns 403 Forbidden for unauthorized access attempts

POST /register

• Changed lookup from u.Email == inputUser.Email to u.Auth0Subject == auth0Subject
• Eliminates email-based identity collision scenarios
• Retains OrderByDescending(u => u.ProfileCompletedAt) for migration data handling

GET /register

• Returns only caller's profile instead of all users

Build Configuration

.gitignore

• Added packages.lock.json to prevent unintended dependency lock file commits

Security Impact

• Enumeration: Prevented authenticated users from listing all system users
• Authorization: Enforced ownership checks on registration access
• Identity: Auth0Subject now primary key for user operations
• Data Exposure: Limited sensitive fields in registration responses

Original prompt

In @services/Visage.Services.Eventing/EventDB.cs:

• Around line 59-95: EventDB's EF configuration for EventRegistration must
  explicitly ignore cross-DbContext navigation/value properties to avoid shadow
  properties or migration failures; in the
  modelBuilder.Entity(...) block add ignore directives for the
  User navigation and UserId scalar (i.e., call Ignore for the User property and
  Ignore for the UserId property on the EventRegistration entity) so EventDB does
  not try to map those UserProfile-specific members.

In @services/Visage.Services.Eventing/Program.cs:

• Around line 478-501: In LookupByPin, replace the BadRequest return for missing
  authentication with a 401 Unauthorized response by returning
  TypedResults.Unauthorized() (or the equivalent Unauthorized result your
  framework uses) when auth0Sub is null/empty; additionally consider returning a
  limited DTO instead of the full EventRegistration object from the final
  TypedResults.Ok(registration) to avoid exposing sensitive fields (create a small
  RegistrationDto and map the allowed properties before returning).

In @Visage.Services.UserProfile/Program.cs:

• Around line 253-257: The /api/users endpoint (app.MapGet("/api/users", ... )
  which currently returns db.Users.ToListAsync() and is only protected by
  RequireAuthorization() allows any authenticated user to enumerate all users;
  either restrict it to admins by applying a role-based policy (e.g., define an
  "AdminOnly" policy and call .RequireAuthorization("AdminOnly") or use the
  appropriate RequireRole/Authorize attribute for the endpoint), or change the
  handler to return only the caller's profile by extracting the user id from the
  HttpContext/ClaimsPrincipal inside the MapGet handler and querying
  db.Users.Where(u => u.Id == userId).FirstOrDefaultAsync() instead of returning
  all users.
• Around line 370-388: The current /register flow looks up users by email
  (existing = db.Users...FirstOrDefaultAsync(u => u.Email == inputUser.Email) )
  then checks Auth0Subject, which creates the inconsistency described; change the
  lookup to find by Auth0Subject (u => u.Auth0Subject == auth0Subject) so
  ownership is the primary key, and when creating a new user ensure
  inputUser.Auth0Subject is set to auth0Subject (and keep setting
  inputUser.CreatedAt and saving as before); retain the OrderByDescending(u =>
  u.ProfileCompletedAt) if you need to prefer the most recent profile but do not
  rely on email for identification.
• Around line 324-339: The endpoint registered with
  app.MapGet("/api/users/{userId}/registrations") currently returns any user's
  registrations; update the route handler to extract the authenticated user id
  from HttpContext.User (e.g., the sub or nameidentifier claim) and compare it to
  parsedUserId, and also allow access if HttpContext.User.IsInRole("Admin") (or
  equivalent admin claim); if the caller is neither the owner nor an admin return
  a 403 (Results.Forbid) instead of the registrations; keep the existing
  StrictId.Id.TryParse validation and the db.EventRegistrations query but
  gate the query behind this ownership/admin check and log an unauthorized access
  attempt with the existing ILogger logger.
• Around line 436-439: The GET handler registered with app.MapGet("/register",
  ...) currently returns db.Users.ToListAsync() and only calls
  RequireAuthorization(), allowing any authenticated user to enumerate all users;
  change the handler to either (a) restrict access to admins by applying a role
  policy (RequireAuthorization with an admin role/Policy) so only admins can call
  this endpoint, or (b) convert it to return only the current user's profile by
  resolving HttpContext.User inside the MapGet handler, extracting the user
  identifier (e.g., sub/NameIdentifier) and querying
  db.Users.SingleOrDefaultAsync(...) for that id instead of
  db.Users.ToListAsync(); update the MapGet registration (and its
  RequireAuthorization call) accordingly

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[indcoder]

We do not need to amend package.lock.json

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

We do not need to amend package.lock.json

Reverted packages.lock.json files and added them to .gitignore in commit b629d3f.

[Copilot]

Pull request overview

This pull request addresses critical security vulnerabilities by fixing authorization bypass and data exposure issues in the UserProfile and Eventing APIs. The changes prevent authenticated users from enumerating all system users and accessing other users' registration data.

Changes:

• Fixed user enumeration vulnerabilities by restricting GET /api/users and GET /register to return only the authenticated caller's profile
• Added ownership and role-based authorization checks to GET /api/users/{userId}/registrations endpoint
• Changed POST /register to use Auth0Subject instead of email for user lookup, eliminating identity collision scenarios
• Modified LookupByPin to return 401 Unauthorized for missing authentication and introduced RegistrationDto to limit exposed sensitive fields
• Added EF Core Ignore() directives for cross-DbContext properties in EventDB configuration

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
services/Visage.Services.Eventing/EventDB.cs	Added Ignore() directives for User navigation and UserId to prevent cross-DbContext EF Core mapping issues
services/Visage.Services.Eventing/Program.cs	Changed LookupByPin to return 401 instead of 400, created RegistrationDto excluding RejectionReason/AdditionalDetails/ApprovedBy fields
Visage.Services.UserProfile/Program.cs	Restricted GET /api/users and GET /register to caller's profile only, added ownership/admin checks to GET /api/users/{userId}/registrations, changed POST /register to lookup by Auth0Subject instead of email
Visage.AppHost/packages.lock.json	Platform switch from win-x64 to linux-x64, added Azure AppContainers support
Visage.FrontEnd/Visage.FrontEnd.Web/packages.lock.json	Updated Microsoft packages from 10.0.1 to 10.0.2
Visage.FrontEnd/Visage.FrontEnd.Web.Client/packages.lock.json	Updated Microsoft packages including HotReload, ILLink.Tasks, and SDK.WebAssembly.Pack
tests/Visage.Test.Aspire/packages.lock.json	Platform switch from win-x64 to linux-x64 for Aspire components, added Azure provisioning packages

[Copilot]

The RegistrationDto exposes the Auth0Subject field, which is a user identifier that could be considered sensitive. While this may be intentional for staff/admin lookup scenarios, consider whether exposing the Auth0Subject to any authenticated caller is appropriate for your security model. If this endpoint is meant for staff-only use, add role-based authorization. Otherwise, consider removing Auth0Subject from the DTO or redacting it for non-admin users.

[Copilot]

The role check uses "Admin" but other parts of the codebase (e.g., CheckOutFromSession in Eventing API) use "VisageAdmin". This inconsistency in role naming could lead to authorization bypass if the actual role name in Auth0 is "VisageAdmin" rather than "Admin". Verify the correct role name and ensure consistency across all authorization checks in the codebase.

Suggested change

	bool isAdmin = httpContext.User.IsInRole("Admin");
	bool isAdmin = httpContext.User.IsInRole("VisageAdmin");

[Copilot]

The removed Auth0Subject mismatch check was necessary to prevent ownership bypass. Since the lookup now uses Auth0Subject directly, an existing user record will always belong to the authenticated user, making the removed check redundant. However, the redundant assignment on line 430 (inputUser.Auth0Subject = auth0Subject) is unnecessary since inputUser.Auth0Subject was already set on line 411 and won't be used when existing is not null. Consider removing line 430 for code clarity.

Suggested change

inputUser.Auth0Subject = auth0Subject;

[Copilot]

The security fixes in this PR (authorization checks on /api/users, /api/users/{userId}/registrations, /register endpoints, and LookupByPin) lack test coverage. Given the critical security nature of these changes, consider adding integration tests to verify: 1) users can only access their own profile data, 2) unauthorized access to other users' registrations returns 403, 3) admin role grants appropriate access, and 4) LookupByPin properly restricts access. Reference existing test patterns in tests/Visage.Test.Aspire/ for examples.