Skip to content

Fix read permissions for stock routes. Change preview script from pm2 to - #1229

Open
stagrim wants to merge 2 commits into
daisyfrom
daisy-fix-admin-read-permissions
Open

Fix read permissions for stock routes. Change preview script from pm2 to#1229
stagrim wants to merge 2 commits into
daisyfrom
daisy-fix-admin-read-permissions

Conversation

@stagrim

@stagrim stagrim commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

should work

pls look and merge 🥺️

@Isak-Kallini

Copy link
Copy Markdown
Member

Add the check to +layout.server.ts instead so it runs for all /admin/stocklist routes without having to add it in many places?

@danieladugyan danieladugyan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the check to +layout.server.ts instead so it runs for all /admin/stocklist routes without having to add it in many places?

That's a common footgun in SvelteKit that should be avoided1 (see https://svelte.dev/docs/kit/load#Implications-for-authentication).

There are a few alternatives. You could revert the latest commit, but the proper solution would be to change the read permission in schema.zmodel for DrinkItem from @@allow("read", true) to @@allow("read", has(auth().policies, "drinkitem:read")).

Long-term if we'd like to move away from ZenStack in favour of route level access, the recommended approach seems to be having a Map<Route, RequiredPermission> that's checked in hooks.server.ts.

Footnotes

  1. Why it should be avoided is another question. FWIW I think we could just about get away with it here, but it will encourage people to use the same approach in places where it actually ends up causing an issue.

@danieladugyan danieladugyan moved this from 🆕 New to 📝In review in CPU Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 📝In review

Development

Successfully merging this pull request may close these issues.

3 participants