From 6e7de29f5bd39a0276af608482c806f0da5b640d Mon Sep 17 00:00:00 2001 From: Matthieu Sieben Date: Fri, 22 Nov 2024 17:56:06 +0100 Subject: [PATCH] Prevent use of another user's server-config (#241) * Prevent use of another user's server-config * do not read from localstorage on server --- components/shell/ConfigContext.tsx | 48 ++++--- components/shell/ConfigurationContext.tsx | 17 +-- .../useServerConfigQuery.ts | 120 +++++++++--------- lib/useStoredQuery.ts | 34 +++++ 4 files changed, 132 insertions(+), 87 deletions(-) create mode 100644 lib/useStoredQuery.ts diff --git a/components/shell/ConfigContext.tsx b/components/shell/ConfigContext.tsx index ac97beee..288f4cd0 100644 --- a/components/shell/ConfigContext.tsx +++ b/components/shell/ConfigContext.tsx @@ -1,12 +1,11 @@ 'use client' -import { useQuery } from '@tanstack/react-query' -import { createContext, ReactNode, useContext, useEffect, useMemo } from 'react' -import { useLocalStorage } from 'react-use' +import { createContext, ReactNode, useContext, useMemo } from 'react' import { Loading } from '@/common/Loader' import { SetupModal } from '@/common/SetupModal' import { getConfig, OzoneConfig } from '@/lib/client-config' +import { useStoredQuery } from '@/lib/useStoredQuery' import { GLOBAL_QUERY_CONTEXT } from './QueryClient' export type ConfigContextData = { @@ -18,36 +17,45 @@ export type ConfigContextData = { const ConfigContext = createContext(null) export const ConfigProvider = ({ children }: { children: ReactNode }) => { - const [cachedConfig, setCachedConfig] = - useLocalStorage('labeler-config') - - const { data, error, refetch } = useQuery({ + const { data, error, refetch } = useStoredQuery({ // Use the global query client to avoid clearing the cache when the user // changes. context: GLOBAL_QUERY_CONTEXT, - retry: (failureCount: number, error: Error): boolean => { - // TODO: change getConfig() to throw a specific error when a network - // error occurs, so we can distinguish between network errors and - // configuration errors. - return false - }, + // TODO: change getConfig() to throw a specific error when a network + // error occurs, so we can distinguish between network errors and + // configuration errors. + retry: false, queryKey: ['labeler-config'], queryFn: getConfig, - initialData: cachedConfig, // Refetching will be handled manually refetchOnWindowFocus: false, + // Initialize with data from the legacy key (can be removed in the future) + initialData: + typeof window === 'undefined' + ? undefined + : ((legacyKey: string) => { + try { + const data = localStorage.getItem(legacyKey) + if (data) return JSON.parse(data) + } catch { + // Ignore + } finally { + localStorage.removeItem(legacyKey) + } + })('labeler-config'), }) - useEffect(() => { - if (data) setCachedConfig(data) - }, [data, setCachedConfig]) - - const value = useMemo( + const value = useMemo( () => data ? { config: data, - configError: error, + configError: + error == null + ? null + : error instanceof Error + ? error + : new Error('Unknown error', { cause: error }), refetchConfig: refetch, } : null, diff --git a/components/shell/ConfigurationContext.tsx b/components/shell/ConfigurationContext.tsx index 949fd070..0dfcae26 100644 --- a/components/shell/ConfigurationContext.tsx +++ b/components/shell/ConfigurationContext.tsx @@ -69,14 +69,14 @@ export const ConfigurationProvider = ({ // Reset "skipRecord" on credential change useEffect(() => setSkipRecord(false), [labelerAgent]) - const accountDid = labelerAgent?.did + const isServiceAccount = labelerAgent.did === config.did const state = serverConfigError?.['status'] === 401 ? ConfigurationState.Unauthorized : config.needs.key || config.needs.service || - (config.needs.record && config.did === accountDid && !skipRecord) + (config.needs.record && isServiceAccount && !skipRecord) ? ConfigurationState.Unconfigured : !serverConfig ? isServerConfigLoading @@ -100,13 +100,13 @@ export const ConfigurationProvider = ({ labelerAgent ? { config, - isServiceAccount: accountDid === config.did, + isServiceAccount, serverConfig, labelerAgent, reconfigure, } : null, - [state, accountDid, config, serverConfig, labelerAgent, reconfigure], + [state, config, isServiceAccount, serverConfig, labelerAgent, reconfigure], ) if (!configurationContextData) { @@ -144,7 +144,7 @@ export const ConfigurationProvider = ({ ) } -export const useConfigurationContext = () => { +export function useConfigurationContext() { const value = useContext(ConfigurationContext) if (value) return value @@ -166,12 +166,9 @@ export function usePermission(name: PermissionName) { } export function useAppviewAgent() { - const { appview } = useConfigurationContext().serverConfig - + const { appview } = useServerConfig() return useMemo(() => { - if (appview) { - return new Agent(new CredentialSession(new URL(appview))) - } + if (appview) return new Agent(appview) return null }, [appview]) } diff --git a/components/shell/ConfigurationContext/useServerConfigQuery.ts b/components/shell/ConfigurationContext/useServerConfigQuery.ts index ee634d83..ca0b0057 100644 --- a/components/shell/ConfigurationContext/useServerConfigQuery.ts +++ b/components/shell/ConfigurationContext/useServerConfigQuery.ts @@ -1,71 +1,77 @@ import { Agent } from '@atproto/api' import { ResponseType, XRPCError } from '@atproto/xrpc' -import { useQuery } from '@tanstack/react-query' -import { useEffect } from 'react' -import { useLocalStorage } from 'react-use' -import { parseServerConfig, ServerConfig } from '@/lib/server-config' +import { parseServerConfig } from '@/lib/server-config' +import { useStoredQuery } from '@/lib/useStoredQuery' export function useServerConfigQuery(agent: Agent) { - const [cachedServerConfig, setCachedServerConfig] = - useLocalStorage('labeler-server-config') - - const response = useQuery({ - retry: (failureCount, error): boolean => { - if (error instanceof XRPCError) { - if (error.status === ResponseType.InternalServerError) { - // The server is misconfigured - return false - } - - if ( - error.status === ResponseType.InvalidRequest && - error.message === 'could not resolve proxy did service url' - ) { - // Labeler service not configured in the user's DID document (yet) - return false - } - - if (error.status === ResponseType.AuthRequired) { - // User is logged in with a user that is not member of the labeler's - // group. - return false - } - } - - return failureCount < 3 - }, - retryDelay: (attempt, error) => { - if ( - error instanceof XRPCError && - error.status === ResponseType.RateLimitExceeded && - error.headers?.['ratelimit-remaining'] === '0' && - error.headers?.['ratelimit-reset'] - ) { - // ratelimit-limit: 3000 - // ratelimit-policy: 3000;w=300 - // ratelimit-remaining: 2977 - // ratelimit-reset: 1724927309 - - const reset = Number(error.headers['ratelimit-reset']) * 1e3 - return reset - Date.now() - } - - // Exponential backoff with a maximum of 30 seconds - return Math.min(1000 * 2 ** attempt, 30000) - }, - queryKey: ['server-config', agent.assertDid, agent.proxy], + return useStoredQuery({ + queryKey: ['server-config', agent.assertDid, agent.proxy ?? null], queryFn: async ({ signal }) => { const { data } = await agent.tools.ozone.server.getConfig({}, { signal }) return parseServerConfig(data) }, - initialData: cachedServerConfig, + retry, + retryDelay, refetchOnWindowFocus: false, + // Initialize with data from the legacy key (can be removed in the future) + initialData: + typeof window === 'undefined' + ? undefined + : ((legacyKey: string) => { + try { + const data = localStorage.getItem(legacyKey) + if (data) return JSON.parse(data) + } catch { + // Ignore + } finally { + localStorage.removeItem(legacyKey) + } + })('labeler-server-config'), }) +} + +const retry = (failureCount: number, error: unknown): boolean => { + if (error instanceof XRPCError) { + if (error.status === ResponseType.InternalServerError) { + // The server is misconfigured + return false + } + + if ( + error.status === ResponseType.InvalidRequest && + error.message === 'could not resolve proxy did service url' + ) { + // Labeler service not configured in the user's DID document (yet) + return false + } + + if (error.status === ResponseType.AuthRequired) { + // User is logged in with a user that is not member of the labeler's + // group. + return false + } + } + + return failureCount < 3 +} + +const retryDelay = (attempt: number, error: unknown): number => { + if ( + error instanceof XRPCError && + error.status === ResponseType.RateLimitExceeded && + error.headers?.['ratelimit-remaining'] === '0' && + error.headers?.['ratelimit-reset'] + ) { + // ratelimit-limit: 3000 + // ratelimit-policy: 3000;w=300 + // ratelimit-remaining: 2977 + // ratelimit-reset: 1724927309 - useEffect(() => { - if (response.data) setCachedServerConfig(response.data) - }, [response.data, setCachedServerConfig]) + const reset = Number(error.headers['ratelimit-reset']) * 1e3 + return reset - Date.now() + } - return response + // Exponential backoff with a maximum of 30 seconds + return Math.min(1000 * 2 ** attempt, 30000) } diff --git a/lib/useStoredQuery.ts b/lib/useStoredQuery.ts new file mode 100644 index 00000000..83a0c2de --- /dev/null +++ b/lib/useStoredQuery.ts @@ -0,0 +1,34 @@ +import { + useQuery, + UseQueryOptions, + UseQueryResult, +} from '@tanstack/react-query' +import { useEffect } from 'react' +import { useLocalStorage } from 'react-use' + +export function useStoredQuery< + TData extends NonNullable | null, + TError, + TQueryKey extends (string | number | boolean | null)[], +>({ + initialData, + ...options +}: Omit< + UseQueryOptions, + 'queryKey' | 'initialData' +> & { + queryKey: TQueryKey + initialData?: TData +}): UseQueryResult { + const key = `storedQuery:${JSON.stringify(options.queryKey).slice(1, -1)}` + + const [storedData, setStoredData] = useLocalStorage(key, initialData) + + const response = useQuery({ ...options, initialData: storedData }) + + useEffect(() => { + setStoredData(response.data) + }, [response.data, setStoredData]) + + return response +}