From aa0d62f063b79d63b618a09a893eb682b39316a4 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 12 Sep 2026 17:25:16 +0200 Subject: [PATCH] fix(admin): read key presence from the app config, not the form The place search provider block now renders on a managed install too, and there the key fields are empty by design: the operator owns the credentials and user-profile never sends them. An operator key set through the environment is invisible in the fields either way. The warning behind the provider select read those fields, so it told every managed admin that search "falls back to OpenStreetMap" while their key was answering. app-config already carries the answer as has_maps_key, and it resolves the same chain the search itself uses, environment included. has_amap_key sits beside it, so both come into the store at boot and both follow a save. --- client/src/App.tsx | 5 ++-- .../screens/admin/MAdminSettingsSection.tsx | 6 ++-- .../src/pages/admin/AdminSettingsTab.test.tsx | 12 +++++++- client/src/pages/admin/AdminSettingsTab.tsx | 29 ++++++++++--------- client/src/pages/admin/useAdmin.ts | 7 ++++- client/src/store/authStore.ts | 10 +++++++ client/tests/helpers/mobileAdmin.ts | 2 ++ .../admin/MAdminSettingsSection.test.tsx | 12 ++++++++ 8 files changed, 64 insertions(+), 19 deletions(-) diff --git a/client/src/App.tsx b/client/src/App.tsx index ceadffc3d..9d27ac6d1 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -284,7 +284,7 @@ function RouteFallback() { } export default function App() { - const { loadUser, isAuthenticated, demoMode, setManaged, setDemoMode, setDevMode, setIsPrerelease, setAppVersion, setHasMapsKey, setServerTimezone, setAppRequireMfa, setTripRemindersEnabled, setPlacesPhotosEnabled, setPlacesAutocompleteEnabled, setPlacesDetailsEnabled, setPlacesEnrichEnabled, setPlaceShadowEnabled } = useAuthStore() + const { loadUser, isAuthenticated, demoMode, setManaged, setDemoMode, setDevMode, setIsPrerelease, setAppVersion, setHasMapsKey, setHasAmapKey, setServerTimezone, setAppRequireMfa, setTripRemindersEnabled, setPlacesPhotosEnabled, setPlacesAutocompleteEnabled, setPlacesDetailsEnabled, setPlacesEnrichEnabled, setPlaceShadowEnabled } = useAuthStore() const { loadSettings } = useSettingsStore() const { loadAddons } = useAddonStore() const { loadPlugins } = usePluginStore() @@ -301,13 +301,14 @@ export default function App() { loadUser() } } - authApi.getAppConfig().then(async (config: { managed?: boolean; demo_mode?: boolean; dev_mode?: boolean; is_prerelease?: boolean; has_maps_key?: boolean; version?: string; timezone?: string; require_mfa?: boolean; trip_reminders_enabled?: boolean; places_photos_enabled?: boolean; places_autocomplete_enabled?: boolean; places_details_enabled?: boolean; places_enrich_enabled?: boolean; place_shadow_enabled?: boolean; permissions?: Record }) => { + authApi.getAppConfig().then(async (config: { managed?: boolean; demo_mode?: boolean; dev_mode?: boolean; is_prerelease?: boolean; has_maps_key?: boolean; has_amap_key?: boolean; version?: string; timezone?: string; require_mfa?: boolean; trip_reminders_enabled?: boolean; places_photos_enabled?: boolean; places_autocomplete_enabled?: boolean; places_details_enabled?: boolean; places_enrich_enabled?: boolean; place_shadow_enabled?: boolean; permissions?: Record }) => { setManaged(!!config?.managed) setDemoMode(!!config?.demo_mode) if (config?.dev_mode) setDevMode(true) if (config?.is_prerelease !== undefined) setIsPrerelease(config.is_prerelease) if (config?.version) setAppVersion(config.version) if (config?.has_maps_key !== undefined) setHasMapsKey(config.has_maps_key) + if (config?.has_amap_key !== undefined) setHasAmapKey(config.has_amap_key) if (config?.timezone) setServerTimezone(config.timezone) if (config?.require_mfa !== undefined) setAppRequireMfa(!!config.require_mfa) if (config?.trip_reminders_enabled !== undefined) setTripRemindersEnabled(config.trip_reminders_enabled) diff --git a/client/src/mobile/screens/admin/MAdminSettingsSection.tsx b/client/src/mobile/screens/admin/MAdminSettingsSection.tsx index 64592f54d..0c561a90d 100644 --- a/client/src/mobile/screens/admin/MAdminSettingsSection.tsx +++ b/client/src/mobile/screens/admin/MAdminSettingsSection.tsx @@ -38,7 +38,7 @@ export default function MAdminSettingsSection({ admin, t }: MAdminSettingsSectio passkeyLogin, setPasskeyLogin, passkeyConfigured, webauthnRpId, setWebauthnRpId, webauthnOrigins, setWebauthnOrigins, savingWebauthn, handleSaveWebauthn, allowedFileTypes, setAllowedFileTypes, savingFileTypes, setSavingFileTypes, - mapsKey, setMapsKey, unsplashKey, setUnsplashKey, amapKey, setAmapKey, savingKeys, validating, validation, + mapsKey, setMapsKey, unsplashKey, setUnsplashKey, amapKey, setAmapKey, hasMapsKey, hasAmapKey, savingKeys, validating, validation, placesProvider, savingPlacesProvider, handleSavePlacesProvider, managed, setShowRotateJwtModal, @@ -295,7 +295,9 @@ export default function MAdminSettingsSection({ admin, t }: MAdminSettingsSectio - {((placesProvider === 'google' && !mapsKey) || (placesProvider === 'amap' && !amapKey)) && ( + {/* From app-config, like the desktop card: the key fields are empty + on a managed install and on an operator key set by environment. */} + {((placesProvider === 'google' && !hasMapsKey) || (placesProvider === 'amap' && !hasAmapKey)) && (

{t('admin.placesProvider.missingKey')}

diff --git a/client/src/pages/admin/AdminSettingsTab.test.tsx b/client/src/pages/admin/AdminSettingsTab.test.tsx index 7195f731b..003ed271a 100644 --- a/client/src/pages/admin/AdminSettingsTab.test.tsx +++ b/client/src/pages/admin/AdminSettingsTab.test.tsx @@ -298,7 +298,7 @@ describe('AdminSettingsTab', () => { it('FE-ADMSET-022c: warns when the selected provider has no key behind it', () => { // A provider chosen without a key answers with OpenStreetMap rather than // failing, which is quiet enough to be mistaken for the provider working. - renderTab({ placesProvider: 'amap', amapKey: '' }); + renderTab({ placesProvider: 'amap', hasAmapKey: false }); expect( within(card('API Keys')).getByText(/falls back to OpenStreetMap/i), ).toBeInTheDocument(); @@ -323,6 +323,16 @@ describe('AdminSettingsTab', () => { expect(keys.getByRole('button', { name: 'Automatic' })).toBeInTheDocument(); }); + it('FE-ADMSET-022f: an operator key keeps the managed install quiet', () => { + // The key fields are not on the page here, and an operator key set through + // the environment would not show up in them anyway. app-config knows. + renderTab({ managed: true, placesProvider: 'google', mapsKey: '', hasMapsKey: true }); + + expect( + within(card('API Keys')).queryByText(/falls back to OpenStreetMap/i), + ).not.toBeInTheDocument(); + }); + it('FE-ADMSET-023: the maps Test button is disabled without a key', () => { renderTab({ mapsKey: '' }); diff --git a/client/src/pages/admin/AdminSettingsTab.tsx b/client/src/pages/admin/AdminSettingsTab.tsx index c93cf69a0..13dc91cfb 100644 --- a/client/src/pages/admin/AdminSettingsTab.tsx +++ b/client/src/pages/admin/AdminSettingsTab.tsx @@ -35,7 +35,7 @@ export default function AdminSettingsTab({ admin, t }: AdminSettingsTabProps): R passkeyLogin, setPasskeyLogin, passkeyConfigured, webauthnRpId, setWebauthnRpId, webauthnOrigins, setWebauthnOrigins, savingWebauthn, handleSaveWebauthn, allowedFileTypes, setAllowedFileTypes, savingFileTypes, setSavingFileTypes, - mapsKey, setMapsKey, unsplashKey, setUnsplashKey, amapKey, setAmapKey, showKeys, savingKeys, validating, validation, + mapsKey, setMapsKey, unsplashKey, setUnsplashKey, amapKey, setAmapKey, hasMapsKey, hasAmapKey, showKeys, savingKeys, validating, validation, placesProvider, savingPlacesProvider, handleSavePlacesProvider, managed, setShowRotateJwtModal, @@ -363,17 +363,16 @@ export default function AdminSettingsTab({ admin, t }: AdminSettingsTabProps): R - {/* The card itself stays on a managed install: the TREK index needs no key, - and which keyed provider answers place search is the admin's call there - too (server managed.ts). What folds away below is the operator's: - the keys, what a lookup costs, and the per-place Google switches. */} {/* API Keys. - The key fields stay visible; only the four Google switches fold away. - They are set once and then never touched, and putting them flat next to - the keys made a rarely-used option look as important as the key it - depends on. Weather is a quiet row for the same reason in reverse: it - had the loudest treatment on the card and is the one thing here with - nothing to configure. */} + The card itself stays on a managed install: the TREK index needs no key, + and which keyed provider answers place search is the admin's call there + too (server managed.ts). What folds away is the operator's: the keys, + what a lookup costs, and the per-place Google switches. Those switches + sit under the keys rather than flat beside them because they are set + once and then never touched, and side by side a rarely-used option + looked as important as the key it depends on. Weather is a quiet row + for the same reason in reverse: it had the loudest treatment on the + card and is the one thing here with nothing to configure. */}

{t('admin.apiKeys')}

@@ -656,8 +655,12 @@ export default function AdminSettingsTab({ admin, t }: AdminSettingsTabProps): R /> {/* A provider chosen without a key behind it answers with OpenStreetMap rather than failing, which is quiet enough to be - mistaken for the provider working. Say so. */} - {((placesProvider === 'google' && !mapsKey) || (placesProvider === 'amap' && !amapKey)) && ( + mistaken for the provider working. Say so. Asked of app-config + and not of the fields above: those are empty on a managed + install and on an operator key that came from the environment, + and saying "falls back to OpenStreetMap" to an admin whose + search works is worse than saying nothing. */} + {((placesProvider === 'google' && !hasMapsKey) || (placesProvider === 'amap' && !hasAmapKey)) && (

{t('admin.placesProvider.missingKey')} diff --git a/client/src/pages/admin/useAdmin.ts b/client/src/pages/admin/useAdmin.ts index 6df218f70..99f250f95 100644 --- a/client/src/pages/admin/useAdmin.ts +++ b/client/src/pages/admin/useAdmin.ts @@ -48,6 +48,11 @@ export function useAdmin() { const mcpEnabled = useAddonStore(s => s.isEnabled('mcp')) const devMode = useAuthStore(s => s.devMode) const managed = useAuthStore(s => s.managed) + // Whether a key is actually behind a provider is app-config's answer, not the + // form's: on a managed install the fields below stay empty by design, and an + // operator key set through the environment never reaches them either. + const hasMapsKey = useAuthStore(s => s.hasMapsKey) + const hasAmapKey = useAuthStore(s => s.hasAmapKey) // ?tab= makes a section linkable: a support reply, an onboarding mail or a // bookmark can point at the one panel it is about instead of at the top of a @@ -491,7 +496,7 @@ export function useAdmin() { allowedFileTypes, setAllowedFileTypes, savingFileTypes, setSavingFileTypes, smtpValues, setSmtpValues, smtpLoaded, mapsKey, setMapsKey, weatherKey, setWeatherKey, unsplashKey, setUnsplashKey, - amapKey, setAmapKey, + amapKey, setAmapKey, hasMapsKey, hasAmapKey, placesProvider, savingPlacesProvider, handleSavePlacesProvider, showKeys, setShowKeys, savingKeys, validating, validation, updateInfo, setUpdateInfo, showUpdateModal, setShowUpdateModal, diff --git a/client/src/store/authStore.ts b/client/src/store/authStore.ts index 987bb5741..4a605ecb4 100644 --- a/client/src/store/authStore.ts +++ b/client/src/store/authStore.ts @@ -47,6 +47,10 @@ interface AuthState { isPrerelease: boolean appVersion: string hasMapsKey: boolean + /** The same question for Amap. Kept apart from hasMapsKey rather than folded + * into one "has a search key": which of the two is missing decides what the + * admin has to go and do. */ + hasAmapKey: boolean serverTimezone: string /** Server policy: all users must enable MFA */ appRequireMfa: boolean @@ -75,6 +79,7 @@ interface AuthState { setIsPrerelease: (val: boolean) => void setAppVersion: (val: string) => void setHasMapsKey: (val: boolean) => void + setHasAmapKey: (val: boolean) => void setServerTimezone: (tz: string) => void setAppRequireMfa: (val: boolean) => void setTripRemindersEnabled: (val: boolean) => void @@ -125,6 +130,7 @@ export const useAuthStore = create()( isPrerelease: false, appVersion: '', hasMapsKey: false, + hasAmapKey: false, serverTimezone: Intl.DateTimeFormat().resolvedOptions().timeZone, appRequireMfa: false, tripRemindersEnabled: false, @@ -343,6 +349,9 @@ export const useAuthStore = create()( if ('maps_api_key' in keys) { set({ hasMapsKey: !!keys.maps_api_key }) } + if ('amap_api_key' in keys) { + set({ hasAmapKey: !!keys.amap_api_key }) + } } catch (err: unknown) { throw new Error(getApiErrorMessage(err, 'Error saving API keys')) } @@ -386,6 +395,7 @@ export const useAuthStore = create()( setIsPrerelease: (val: boolean) => set({ isPrerelease: val }), setAppVersion: (val: string) => set({ appVersion: val }), setHasMapsKey: (val: boolean) => set({ hasMapsKey: val }), + setHasAmapKey: (val: boolean) => set({ hasAmapKey: val }), setServerTimezone: (tz: string) => set({ serverTimezone: tz }), setAppRequireMfa: (val: boolean) => set({ appRequireMfa: val }), setTripRemindersEnabled: (val: boolean) => set({ tripRemindersEnabled: val }), diff --git a/client/tests/helpers/mobileAdmin.ts b/client/tests/helpers/mobileAdmin.ts index 40ab3e4c1..d7c4ecb05 100644 --- a/client/tests/helpers/mobileAdmin.ts +++ b/client/tests/helpers/mobileAdmin.ts @@ -146,6 +146,8 @@ export function buildAdminHook(overrides: Record = {}): AdminHo setUnsplashKey: vi.fn(), amapKey: '', setAmapKey: vi.fn(), + hasMapsKey: false, + hasAmapKey: false, placesProvider: 'auto', savingPlacesProvider: false, showKeys: {}, diff --git a/client/tests/unit/mobile/admin/MAdminSettingsSection.test.tsx b/client/tests/unit/mobile/admin/MAdminSettingsSection.test.tsx index b0813e9b3..b778da46d 100644 --- a/client/tests/unit/mobile/admin/MAdminSettingsSection.test.tsx +++ b/client/tests/unit/mobile/admin/MAdminSettingsSection.test.tsx @@ -277,6 +277,18 @@ describe('MAdminSettingsSection', () => { expect(screen.getByDisplayValue('Automatic')).toBeInTheDocument(); }); + it('FE-MOB-ASET-016c: the missing-key notice follows app-config, not the fields', () => { + // A provider chosen without a key answers with OpenStreetMap rather than + // failing. Which of the two is true is app-config's answer: the key fields + // are empty on a managed install and on a key set by the environment. + const { unmount } = render(); + expect(screen.getByText(/falls back to OpenStreetMap/i)).toBeInTheDocument(); + unmount(); + + renderSettings({ managed: true, placesProvider: 'google', mapsKey: '', hasMapsKey: true }); + expect(screen.queryByText(/falls back to OpenStreetMap/i)).not.toBeInTheDocument(); + }); + it('FE-MOB-ASET-017: the Google Places toggles persist optimistically', async () => { const user = userEvent.setup(); const seen: string[] = [];