-
Notifications
You must be signed in to change notification settings - Fork 5
feat: qol esigner and file manager improvements #658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
00869bb
8443cb1
ffe2e12
1a4e450
54037cf
591902a
ab6f64b
aae3fe0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export { default as component } from "../../../../src/routes/(protected)/files/+page.svelte"; | ||
| export { default as component } from "../../../../src/routes/(auth)/deeplink-login/+page.svelte"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export { default as component } from "../../../../src/routes/(protected)/files/[id]/+page.svelte"; | ||
| export { default as component } from "../../../../src/routes/(protected)/files/+page.svelte"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export { default as component } from "../../../../src/routes/(protected)/files/new/+page.svelte"; | ||
| export { default as component } from "../../../../src/routes/(protected)/files/[id]/+page.svelte"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export { default as component } from "../../../../src/routes/(protected)/files/new/+page.svelte"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import type * as Kit from '@sveltejs/kit'; | ||
|
|
||
| type Expand<T> = T extends infer O ? { [K in keyof O]: O[K] } : never; | ||
| // @ts-ignore | ||
| type MatcherParam<M> = M extends (param : string) => param is infer U ? U extends string ? U : string : string; | ||
| type RouteParams = { }; | ||
| type RouteId = '/(auth)/deeplink-login'; | ||
| type MaybeWithVoid<T> = {} extends T ? T | void : T; | ||
| export type RequiredKeys<T> = { [K in keyof T]-?: {} extends { [P in K]: T[K] } ? never : K; }[keyof T]; | ||
| type OutputDataShape<T> = MaybeWithVoid<Omit<App.PageData, RequiredKeys<T>> & Partial<Pick<App.PageData, keyof T & keyof App.PageData>> & Record<string, any>> | ||
| type EnsureDefined<T> = T extends null | undefined ? {} : T; | ||
| type OptionalUnion<U extends Record<string, any>, A extends keyof U = U extends U ? keyof U : never> = U extends unknown ? { [P in Exclude<A, keyof U>]?: never } & U : never; | ||
| export type Snapshot<T = any> = Kit.Snapshot<T>; | ||
| type PageParentData = EnsureDefined<import('../../$types.js').LayoutData>; | ||
|
|
||
| export type PageServerData = null; | ||
| export type PageData = Expand<PageParentData>; | ||
| export type PageProps = { params: RouteParams; data: PageData } |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||||
| export function isMobileDevice(): boolean { | ||||||||
| if (typeof window === 'undefined') return false; | ||||||||
|
|
||||||||
| return /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) || | ||||||||
| (window.innerWidth <= 768); | ||||||||
| } | ||||||||
|
Comment on lines
+1
to
+6
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mixed detection methods may cause inconsistent mobile identification. Combining user agent regex with viewport width ( Consider separating these concerns or choosing one primary detection method for more predictable behavior. 🤖 Prompt for AI Agents |
||||||||
|
|
||||||||
| export function getDeepLinkUrl(qrData: string): string { | ||||||||
| return qrData; | ||||||||
| } | ||||||||
|
Comment on lines
+8
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Remove unnecessary passthrough function. The 🔎 Proposed fix to remove the passthrough functionRemove the function from this file: -export function getDeepLinkUrl(qrData: string): string {
- return qrData;
-}Then update the import and usage in -import { isMobileDevice, getDeepLinkUrl } from '$lib/utils/mobile-detection';
+import { isMobileDevice } from '$lib/utils/mobile-detection';And replace the function call with direct usage: <a
- href={getDeepLinkUrl(signingSession.qrData)}
+ href={signingSession.qrData}
class="w-full px-6 py-3 bg-blue-600 text-white rounded-lg hover:bg-blue-700 transition-colors text-center font-medium"
>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| <script lang="ts"> | ||
| import { onMount } from 'svelte'; | ||
| import { goto } from '$app/navigation'; | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| import { apiClient } from '$lib/utils/axios'; | ||
| import { login } from '$lib/stores/auth'; | ||
| let isLoading = $state(true); | ||
| let error = $state<string | null>(null); | ||
| onMount(() => { | ||
| const handleDeeplinkLogin = async () => { | ||
| try { | ||
| // Try parsing from search string first | ||
| let params: URLSearchParams; | ||
| let searchString = window.location.search; | ||
| // If search is empty, try parsing from hash or full URL | ||
| if (!searchString || searchString === '') { | ||
| const hash = window.location.hash; | ||
| if (hash && hash.includes('?')) { | ||
| searchString = hash.substring(hash.indexOf('?')); | ||
| } else { | ||
| try { | ||
| const fullUrl = new URL(window.location.href); | ||
| searchString = fullUrl.search; | ||
| } catch (e) { | ||
| // Ignore parsing errors | ||
| } | ||
| } | ||
| } | ||
| // Remove leading ? if present | ||
| if (searchString.startsWith('?')) { | ||
| searchString = searchString.substring(1); | ||
| } | ||
| // Parse the search string | ||
| params = new URLSearchParams(searchString); | ||
| let ename = params.get('ename'); | ||
| let session = params.get('session'); | ||
| let signature = params.get('signature'); | ||
| const appVersion = params.get('appVersion'); | ||
| if (!ename || !session || !signature) { | ||
| // Add a small delay to allow URL to fully parse before showing error | ||
| await new Promise(resolve => setTimeout(resolve, 500)); | ||
| // Re-check one more time after delay | ||
| const finalParams = new URLSearchParams(window.location.search || (window.location.hash.includes('?') ? window.location.hash.substring(window.location.hash.indexOf('?') + 1) : '') || ''); | ||
| ename = finalParams.get('ename') || ename; | ||
| session = finalParams.get('session') || session; | ||
| signature = finalParams.get('signature') || signature; | ||
| if (!ename || !session || !signature) { | ||
| error = "Missing required authentication parameters"; | ||
| isLoading = false; | ||
| return; | ||
| } | ||
| } | ||
|
Comment on lines
+45
to
+59
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Investigate the root cause instead of using delay-based retry. Same timing/retry issue as in file-manager. The 500ms delay workaround is unreliable and should be replaced with proper URL parameter handling using SvelteKit's page store. See the file-manager review for the detailed alternative approach. 🤖 Prompt for AI Agents |
||
| // Clean up URL | ||
| window.history.replaceState({}, '', window.location.pathname); | ||
| // Make POST request to login endpoint using apiClient | ||
| const requestBody = { ename, session, signature, appVersion: appVersion || '0.4.0' }; | ||
| const response = await apiClient.post('/api/auth', requestBody); | ||
| if (response.data.token && response.data.user) { | ||
| login(response.data.token, response.data.user); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find platforms/esigner/src/lib/stores -name "*.ts" -o -name "*.js" -o -name "*.svelte" | head -20Repository: MetaState-Prototype-Project/prototype Length of output: 300 🏁 Script executed: fd "auth" platforms/esigner/src/lib/stores --type fRepository: MetaState-Prototype-Project/prototype Length of output: 121 🏁 Script executed: cat platforms/esigner/src/lib/stores/auth.ts 2>/dev/null || cat platforms/esigner/src/lib/stores/auth.js 2>/dev/null || echo "auth store not found in expected locations"Repository: MetaState-Prototype-Project/prototype Length of output: 2223 🏁 Script executed: cat platforms/esigner/src/lib/utils/axios.ts 2>/dev/null || cat platforms/esigner/src/lib/utils/axios.js 2>/dev/nullRepository: MetaState-Prototype-Project/prototype Length of output: 781 Authentication tokens are stored in localStorage, creating an XSS vulnerability. The Migrate to httpOnly cookies set by the server instead. This prevents JavaScript from accessing the token, significantly improving security posture. |
||
| goto('/files'); | ||
| } else { | ||
| error = "Invalid response from server"; | ||
| isLoading = false; | ||
| } | ||
| } catch (err: any) { | ||
| console.error('Login request failed:', err); | ||
| if (err.response?.data?.error) { | ||
| error = err.response.data.error; | ||
| } else if (err.response?.status) { | ||
| error = `Server error: ${err.response.status}`; | ||
| } else { | ||
| error = "Failed to connect to server"; | ||
| } | ||
| isLoading = false; | ||
| } | ||
| }; | ||
| handleDeeplinkLogin(); | ||
| }); | ||
| </script> | ||
|
|
||
| {#if isLoading} | ||
| <div class="flex h-screen items-center justify-center"> | ||
| <div class="text-center"> | ||
| <div class="animate-spin rounded-full h-32 w-32 border-b-2 border-blue-600 mx-auto mb-4"></div> | ||
| <p class="text-lg text-gray-600">Authenticating...</p> | ||
| </div> | ||
| </div> | ||
| {:else if error} | ||
| <div class="flex h-screen items-center justify-center"> | ||
| <div class="text-center"> | ||
| <div class="text-red-600 mb-4">{error}</div> | ||
| <button | ||
| onclick={() => window.location.href = "/"} | ||
| class="px-6 py-3 bg-blue-600 text-white rounded-lg hover:bg-blue-700" | ||
| > | ||
| Go to Login | ||
| </button> | ||
| </div> | ||
| </div> | ||
| {/if} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 470
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 1137
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 135
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 1104
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 1889
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 1846
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 251
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 82
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 920
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 205
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 380
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 213
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 270
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 3673
🏁 Script executed:
Repository: MetaState-Prototype-Project/prototype
Length of output: 764
Remove localhost fallback from PUBLIC_ESIGNER_BASE_URL—environment variable must be explicitly configured in production.
The localhost default will not work in production and conflicts with the architectural pattern of removing static fallbacks from PUBLIC_* environment variables. Additionally,
file.idshould be URL-encoded to safely handle special characters.This pattern appears in:
NotificationService.tslines 288, 319, 347SignatureService.tsline 75Recommended fix
📝 Committable suggestion
🤖 Prompt for AI Agents