Allow users to view admin-owned websites#4330
Conversation
|
Someone is attempting to deploy a commit to the Umami Software Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR allows regular users to view websites owned by admin accounts by adding a new
Confidence Score: 2/5Not safe to merge as-is — the authorization model change is broader than the PR title implies and may expose private admin website data to all users. The core change grants every authenticated user unconditional read access to every website owned by any admin account. In a single-admin setup this may be acceptable, but in any multi-admin deployment it silently exposes one admin's private analytics to every other user. The implementation has no opt-in mechanism (no isPublic flag, no explicit share, no per-user grant), so there is no way to create a private admin website after this change lands. src/permissions/website.ts and src/queries/prisma/website.ts contain the core authorization logic that needs the most scrutiny before merging. Important Files Changed
Sequence DiagramsequenceDiagram
participant User as Regular User
participant Route as /api/me/websites (GET)
participant Query as getUserWebsitesIncludingAdminOwned
participant DB as Database
User->>Route: GET /api/me/websites
Route->>Query: getUserWebsitesIncludingAdminOwned(userId)
Query->>DB: "WHERE userId=X OR user.role='admin'"
DB-->>Query: User's own websites + ALL admin-owned websites
Query-->>Route: Combined result set
Route-->>User: Returns admin websites from any/all admins
Note over User,DB: canViewWebsite path (direct access by ID)
User->>Route: GET /api/websites/[adminWebsiteId]
Route->>+Route: canViewWebsite(auth, adminWebsiteId)
Route->>DB: isAdminOwnedWebsite(adminWebsiteId)
DB-->>Route: true (owner has admin role)
Route-->>User: 200 OK — grants access to any admin website
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (entity.userId) { | ||
| return user.id === entity.userId; | ||
| if (user.id === entity.userId) { | ||
| return true; | ||
| } | ||
|
|
||
| return isAdminOwnedWebsite(websiteId); |
There was a problem hiding this comment.
isAdminOwnedWebsite only checks whether the website's owner has the admin role — it does not verify any relationship between the requesting user and that admin. Any authenticated non-admin user can now directly view the data of every website owned by any admin account simply by knowing (or guessing) its ID. There is no per-website opt-in or per-admin sharing intent captured here; if an admin has websites intended to remain private, this check will still return true and grant access to all authenticated users.
| export async function getUserWebsitesIncludingAdminOwned(userId: string, filters?: QueryFilters) { | ||
| return getWebsites( | ||
| { | ||
| where: { | ||
| OR: [ | ||
| { userId }, | ||
| { | ||
| user: { | ||
| role: ROLES.admin, | ||
| deletedAt: null, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| include: { | ||
| user: { | ||
| select: { | ||
| username: true, | ||
| id: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| { | ||
| orderBy: 'name', | ||
| ...filters, | ||
| }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
getUserWebsitesIncludingAdminOwned returns websites owned by userId OR owned by any user with the admin role — without any relationship to the caller. In a multi-admin deployment every non-admin user now receives the full website list of every admin account in the system (not just the "system" admin that may have set up shared websites). Combined with the permission change in canViewWebsite, this means one admin's private analytics data is unconditionally visible to all users. If the intent is to share only certain admin-managed websites, a marker on the website (e.g. isPublic / sharedWithAll) or an explicit team membership would be safer than a blanket role-based filter.
1 participant
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.