Implement internal pages - #714
Conversation
|
Great work on this PR — the internal pages feature is exactly the foundation we need. @rajnandan1 — Is this PR on track to be merged? We'd like to base our work on it. Thanks to both of you! |
|
Thanks again for your work on this, @Pefington! |
|
Ah, shame, I initially implemented a role-based visibility toggle, but simplified it as we do not need it. |
|
@Pefington I’ve implemented a role-based solution and uploaded it as PR #727—feel free to take a look. I’d appreciate any feedback on it, especially since you’ve likely implemented something similar yourselves. |
|
checking |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a ChangesInternal Pages Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant dashboardController
participant signInPageServer
participant signInPageSvelte
Browser->>dashboardController: request dashboard data for internal page
dashboardController->>Browser: redirect to /account/signin?next=...
Browser->>signInPageServer: load sign-in page with next
signInPageServer->>signInPageSvelte: return next in page data
Browser->>signInPageSvelte: submit login form with next
signInPageSvelte->>signInPageServer: POST login + next form data
signInPageServer->>Browser: redirect to next path or default route
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| migrations/20260417120000_add_page_is_internal.ts | Adds page_is_internal column with NOT NULL DEFAULT 0; idempotent hasColumn guards on both up and down paths. |
| src/lib/server/controllers/dashboardController.ts | Adds internal-page access check with redirect to sign-in and properly encoded ?next= return URL; correctly uses layoutData.loggedInUser. |
| src/routes/(account)/account/signin/+page.server.ts | Reads ?next= from URL on load and passes it as a hidden form field; post-login redirect uses isSafePath guard (startsWith('/') + no double-slash) preventing open-redirect. |
| src/lib/server/api-server/pages/get.ts | Filters internal pages server-side based on GetLoggedInSession result before mapping to PageNavItem; internal flag is not exposed in the response. |
| src/lib/server/db/repositories/pages.ts | Adds page_is_internal ?? 0 to createPage insert; updatePage uses Partial spread so the field propagates correctly on edit. |
| src/lib/server/types/db.ts | Adds page_is_internal: number to PageRecord and page_is_internal?: number to PageRecordInsert; types are consistent with migration and repository usage. |
| src/lib/components/PageSelector.svelte | Changes threshold from > 0 to > 1 so the dropdown is hidden when only one page is available to the current user. |
| src/routes/(manage)/manage/app/pages/[page_id]/+page.svelte | Adds Internal Page toggle; correctly converts DB number to boolean for the Switch component and back to 0/1 on save. |
| src/routes/(manage)/manage/app/pages/+page.svelte | Adds Access column showing Internal/Public badge based on page_is_internal value. |
| src/routes/(account)/account/signin/+page.svelte | Renders a hidden next input inside the form only when a next value is present, correctly threading the return URL through the login flow. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant U as Unauthenticated User
participant SC as SvelteKit Server
participant DC as dashboardController
participant DB as Database
U->>SC: GET /internal-page
SC->>DC: GetPageDashboardData(pagePath, layoutData)
DC->>DB: GetPageByPathWithMonitors(pagePath)
DB-->>DC: "{ page: { page_is_internal: 1 }, monitors }"
DC->>DC: Check layoutData.loggedInUser → null
DC-->>SC: "throw redirect(302, /account/signin?next=%2Finternal-page)"
SC-->>U: "302 → /account/signin?next=%2Finternal-page"
U->>SC: "POST /account/signin (email, password, next=/internal-page)"
SC->>SC: Validate credentials
SC->>SC: isSafePath check on next
SC-->>U: 302 → /internal-page (authenticated)
U->>SC: GET /dashboard-apis/pages
SC->>SC: GetLoggedInSession(cookies) → null
SC->>DB: GetAllPages()
DB-->>SC: [public_page, internal_page]
SC->>SC: "filter(p => !p.page_is_internal || !!loggedInUser)"
SC-->>U: [public_page only]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant U as Unauthenticated User
participant SC as SvelteKit Server
participant DC as dashboardController
participant DB as Database
U->>SC: GET /internal-page
SC->>DC: GetPageDashboardData(pagePath, layoutData)
DC->>DB: GetPageByPathWithMonitors(pagePath)
DB-->>DC: "{ page: { page_is_internal: 1 }, monitors }"
DC->>DC: Check layoutData.loggedInUser → null
DC-->>SC: "throw redirect(302, /account/signin?next=%2Finternal-page)"
SC-->>U: "302 → /account/signin?next=%2Finternal-page"
U->>SC: "POST /account/signin (email, password, next=/internal-page)"
SC->>SC: Validate credentials
SC->>SC: isSafePath check on next
SC-->>U: 302 → /internal-page (authenticated)
U->>SC: GET /dashboard-apis/pages
SC->>SC: GetLoggedInSession(cookies) → null
SC->>DB: GetAllPages()
DB-->>SC: [public_page, internal_page]
SC->>SC: "filter(p => !p.page_is_internal || !!loggedInUser)"
SC-->>U: [public_page only]
Reviews (3): Last reviewed commit: "fix: reject backslash-prefixed paths in ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/server/controllers/dashboardController.ts`:
- Around line 314-321: The internal-page redirect in dashboardController’s
access check currently sends anonymous users to /account/signin without
preserving the requested page. Update the redirect logic around the
pageDetails.page_is_internal / layoutData.loggedInUser check to include a return
URL for the original page, and make the sign-in flow consume that parameter
after successful login so users are sent back to the page they requested instead
of the default dashboard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bf4755a7-3975-4eeb-8328-53081d8c391c
📒 Files selected for processing (8)
migrations/20260417120000_add_page_is_internal.tssrc/lib/components/PageSelector.sveltesrc/lib/server/api-server/pages/get.tssrc/lib/server/controllers/dashboardController.tssrc/lib/server/db/repositories/pages.tssrc/lib/server/types/db.tssrc/routes/(manage)/manage/app/pages/+page.sveltesrc/routes/(manage)/manage/app/pages/[page_id]/+page.svelte
…gn-in Internal page redirects now append ?next=<encoded path> to the sign-in URL. The sign-in page threads next through load → hidden form field → login action, and redirects there after successful authentication. Open-redirect guard: only relative paths starting with / are honoured.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/routes/`(account)/account/signin/+page.server.ts:
- Around line 83-86: The redirect handling in signin/+page.server.ts still
allows backslash-prefixed relative paths like "/\\evil.com" through the
next/redirectTo check. Update the validation around redirectTo so the signin
flow only accepts safe in-app paths by rejecting any value whose first two
characters could form a protocol-relative or backslash-based escape, or by
parsing and allowlisting the pathname before calling redirect() in the signin
page server handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5c045602-3ccb-453f-b068-4ecf6d11e696
📒 Files selected for processing (3)
src/lib/server/controllers/dashboardController.tssrc/routes/(account)/account/signin/+page.server.tssrc/routes/(account)/account/signin/+page.svelte
3 participants
Summary
Adds an "Internal" toggle to the page creation/edit UI. When enabled, unauthenticated users are redirected to sign in when they try to access the page. Internal pages are also hidden from the page switcher for signed out users.
Changes
8e288327feat: implement internal pagesAdds an "Internal" toggle to pages. When enabled, unauthenticated users are redirected to sign-in when visiting the page, and the page is hidden from the page switcher/nav. Changes include:
page_is_internal INTEGER NOT NULL DEFAULT 0to thepagestable2077bcdfux: hide page switcher if single page availableThe page switcher component was always rendered even when only one page was visible to the current user (e.g., all other pages are internal). It now hides itself when there's only one page to switch between.
How to test
Summary by CodeRabbit