mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 08:21:39 -06:00
fix(metadata): require google books api key
Block Google Books from being enabled without credentials because unauthenticated API calls now fail quota checks. Normalize legacy enabled configs without keys to disabled.
This commit is contained in:
+32
-4
@@ -2,6 +2,7 @@
|
||||
import { onMounted, onUnmounted, ref, watch } from 'vue'
|
||||
import { Loader2, Save } from 'lucide-vue-next'
|
||||
import type { MetadataProviderKey, ProviderConfigurations, ProviderStatus, ProviderThrottleRuntimeState } from '@bookorbit/types'
|
||||
import { toast } from 'vue-sonner'
|
||||
import { Badge } from '@/components/ui/badge'
|
||||
|
||||
const props = defineProps<{
|
||||
@@ -78,7 +79,7 @@ const rows: { key: keyof ProviderConfigurations; label: string; hint?: string; f
|
||||
{
|
||||
key: 'google',
|
||||
label: 'Google Books',
|
||||
hint: "Broad coverage from Google's book index. Add an API key to avoid rate limits.",
|
||||
hint: "Broad coverage from Google's book index. Google Books now requires an API key before this provider can be enabled.",
|
||||
fields: [{ key: 'apiKey', label: 'API Key', type: 'password' }],
|
||||
},
|
||||
{
|
||||
@@ -163,6 +164,33 @@ function save() {
|
||||
emit('save', draft.value)
|
||||
}
|
||||
|
||||
function isGoogleApiKeyField(rowKey: keyof ProviderConfigurations, fieldKey: string): boolean {
|
||||
return rowKey === 'google' && fieldKey === 'apiKey'
|
||||
}
|
||||
|
||||
function hasGoogleApiKey(): boolean {
|
||||
return !!draft.value?.google.apiKey.trim()
|
||||
}
|
||||
|
||||
function canEnableProvider(key: keyof ProviderConfigurations): boolean {
|
||||
return key !== 'google' || hasGoogleApiKey()
|
||||
}
|
||||
|
||||
function canEditField(rowKey: keyof ProviderConfigurations, fieldKey: string): boolean {
|
||||
if (!draft.value) return false
|
||||
return draft.value[rowKey].enabled || isGoogleApiKeyField(rowKey, fieldKey)
|
||||
}
|
||||
|
||||
function toggleProvider(key: keyof ProviderConfigurations) {
|
||||
if (!draft.value) return
|
||||
const provider = draft.value[key]
|
||||
if (!provider.enabled && !canEnableProvider(key)) {
|
||||
toast.error('Google Books requires an API key before it can be enabled')
|
||||
return
|
||||
}
|
||||
provider.enabled = !provider.enabled
|
||||
}
|
||||
|
||||
function onSecretFieldFocus(event: FocusEvent) {
|
||||
const input = event.target as HTMLInputElement | null
|
||||
if (input?.readOnly) input.readOnly = false
|
||||
@@ -229,7 +257,7 @@ function onSecretFieldFocus(event: FocusEvent) {
|
||||
<select
|
||||
v-if="field.type === 'select'"
|
||||
v-model="(draft[row.key] as unknown as Record<string, string>)[field.key]"
|
||||
:disabled="!draft[row.key].enabled"
|
||||
:disabled="!canEditField(row.key, field.key)"
|
||||
class="h-9 w-full sm:w-auto rounded-md border border-input bg-background px-3 text-sm font-medium focus:outline-none focus:ring-1 focus:ring-primary disabled:opacity-40 transition-all"
|
||||
>
|
||||
<option v-for="opt in field.options" :key="opt" :value="opt">{{ opt }}</option>
|
||||
@@ -240,7 +268,7 @@ function onSecretFieldFocus(event: FocusEvent) {
|
||||
:type="field.type === 'password' ? 'text' : field.type"
|
||||
:name="`metadata-${row.key}-${field.key}`"
|
||||
:placeholder="field.label"
|
||||
:disabled="!draft[row.key].enabled"
|
||||
:disabled="!canEditField(row.key, field.key)"
|
||||
:readonly="field.type === 'password'"
|
||||
:autocomplete="field.type === 'password' ? 'off' : 'off'"
|
||||
autocorrect="off"
|
||||
@@ -267,7 +295,7 @@ function onSecretFieldFocus(event: FocusEvent) {
|
||||
:aria-checked="draft[row.key].enabled"
|
||||
class="relative inline-flex h-5 w-9 shrink-0 items-center rounded-full transition-colors focus:outline-none"
|
||||
:class="draft[row.key].enabled ? 'bg-primary' : 'bg-muted border border-border'"
|
||||
@click="draft[row.key].enabled = !draft[row.key].enabled"
|
||||
@click="toggleProvider(row.key)"
|
||||
>
|
||||
<span
|
||||
class="inline-block h-3.5 w-3.5 rounded-full bg-white shadow-xs transition-transform"
|
||||
|
||||
@@ -34,7 +34,9 @@ export function useProviderConfig() {
|
||||
await fetchConfig()
|
||||
toast.success('Provider settings saved')
|
||||
} else {
|
||||
toast.error('Failed to save provider settings')
|
||||
const data = await res.json().catch(() => null)
|
||||
const message = typeof data?.message === 'string' ? data.message : 'Failed to save provider settings'
|
||||
toast.error(message)
|
||||
}
|
||||
} finally {
|
||||
saving.value = false
|
||||
|
||||
@@ -52,6 +52,18 @@ describe('GoogleProvider', () => {
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array if API key is missing', async () => {
|
||||
global.fetch = vi.fn();
|
||||
vi.spyOn(providerConfig, 'getConfig').mockResolvedValue({
|
||||
...mockConfig,
|
||||
google: { enabled: true, apiKey: '' },
|
||||
});
|
||||
|
||||
const result = await provider.search({ title: 'Test' });
|
||||
expect(result).toEqual([]);
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should fetch from Google Books and return mapped volumes', async () => {
|
||||
const mockVolume = {
|
||||
id: 'vol1',
|
||||
@@ -102,6 +114,18 @@ describe('GoogleProvider', () => {
|
||||
});
|
||||
|
||||
describe('lookupById', () => {
|
||||
it('should return null if API key is missing', async () => {
|
||||
global.fetch = vi.fn();
|
||||
vi.spyOn(providerConfig, 'getConfig').mockResolvedValue({
|
||||
...mockConfig,
|
||||
google: { enabled: true, apiKey: '' },
|
||||
});
|
||||
|
||||
const result = await provider.lookupById('vol1');
|
||||
expect(result).toBeNull();
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should fetch volume by id and return mapped volume', async () => {
|
||||
const mockVolume = {
|
||||
id: 'vol1',
|
||||
|
||||
@@ -26,7 +26,7 @@ export class GoogleProvider implements IdentifiableProvider {
|
||||
|
||||
async search(params: MetadataSearchParams): Promise<MetadataCandidate[]> {
|
||||
const { enabled, apiKey } = await this.providerConfig.getConfig().then((c) => c.google);
|
||||
if (!enabled) return [];
|
||||
if (!enabled || !apiKey.trim()) return [];
|
||||
const query = this.buildQuery(params);
|
||||
if (!query) return [];
|
||||
return this.fetchVolumes(query, apiKey, params.signal);
|
||||
@@ -34,7 +34,7 @@ export class GoogleProvider implements IdentifiableProvider {
|
||||
|
||||
async lookupById(providerId: string, signal?: AbortSignal): Promise<MetadataCandidate | null> {
|
||||
const { enabled, apiKey } = await this.providerConfig.getConfig().then((c) => c.google);
|
||||
if (!enabled) return null;
|
||||
if (!enabled || !apiKey.trim()) return null;
|
||||
const url = this.buildUrl(`/volumes/${providerId}`, {}, apiKey);
|
||||
const startedAt = Date.now();
|
||||
this.logger.log(`[google] [start] op=lookup providerId="${providerId}"`);
|
||||
|
||||
@@ -59,7 +59,7 @@ describe('ProviderConfigService', () => {
|
||||
const config = await service.getConfig();
|
||||
|
||||
expect(config).toEqual({
|
||||
google: { enabled: true, apiKey: '' },
|
||||
google: { enabled: false, apiKey: '' },
|
||||
amazon: { enabled: true, domain: 'amazon.com', cookie: '' },
|
||||
goodreads: { enabled: true },
|
||||
hardcover: { enabled: false, apiKey: '' },
|
||||
@@ -80,7 +80,7 @@ describe('ProviderConfigService', () => {
|
||||
|
||||
const second = await service.getConfig();
|
||||
|
||||
expect(second.google.enabled).toBe(true);
|
||||
expect(second.google.enabled).toBe(false);
|
||||
expect(second.amazon.domain).toBe('amazon.com');
|
||||
});
|
||||
|
||||
@@ -95,7 +95,7 @@ describe('ProviderConfigService', () => {
|
||||
|
||||
const config = await service.getConfig();
|
||||
|
||||
expect(config.google).toEqual({ enabled: true, apiKey: 'key-1' });
|
||||
expect(config.google).toEqual({ enabled: false, apiKey: 'key-1' });
|
||||
expect(config.amazon).toEqual({ enabled: true, domain: 'amazon.com', cookie: '' });
|
||||
expect(config.hardcover).toEqual({ enabled: true, apiKey: 'hardcover-key' });
|
||||
expect(config.itunes).toEqual({ enabled: false, coverResolution: 'high' });
|
||||
@@ -117,6 +117,18 @@ describe('ProviderConfigService', () => {
|
||||
expect(config.goodreads).toEqual({ enabled: true });
|
||||
});
|
||||
|
||||
it('normalizes stored Google Books config to disabled when API key is missing', async () => {
|
||||
db.query.appSettings.findFirst.mockResolvedValue({
|
||||
value: JSON.stringify({
|
||||
google: { enabled: true, apiKey: '' },
|
||||
}),
|
||||
});
|
||||
|
||||
const config = await service.getConfig();
|
||||
|
||||
expect(config.google).toEqual({ enabled: false, apiKey: '' });
|
||||
});
|
||||
|
||||
it('drops unsupported properties from stored provider sections', async () => {
|
||||
db.query.appSettings.findFirst.mockResolvedValue({
|
||||
value: JSON.stringify({
|
||||
@@ -186,6 +198,30 @@ describe('ProviderConfigService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('rejects enabling Google Books without an API key', async () => {
|
||||
db.__tx.query.appSettings.findFirst.mockResolvedValue({
|
||||
value: JSON.stringify({
|
||||
google: { enabled: false, apiKey: '' },
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(service.updateConfig({ google: { enabled: true } })).rejects.toThrow('Google Books requires an API key');
|
||||
expect(db.__tx.insert).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('normalizes legacy enabled Google Books config during unrelated updates', async () => {
|
||||
db.__tx.query.appSettings.findFirst.mockResolvedValue({
|
||||
value: JSON.stringify({
|
||||
google: { enabled: true, apiKey: '' },
|
||||
}),
|
||||
});
|
||||
|
||||
const updated = await service.updateConfig({ amazon: { cookie: 'session-cookie' } });
|
||||
|
||||
expect(updated.google).toEqual({ enabled: false, apiKey: '' });
|
||||
expect(updated.amazon.cookie).toBe('session-cookie');
|
||||
});
|
||||
|
||||
it('acquires advisory lock before reading config inside update transaction', async () => {
|
||||
db.__tx.query.appSettings.findFirst.mockResolvedValue(undefined);
|
||||
|
||||
@@ -234,7 +270,8 @@ describe('ProviderConfigService', () => {
|
||||
MetadataProviderKey.AUDNEXUS,
|
||||
MetadataProviderKey.COMICVINE,
|
||||
]);
|
||||
expect(statuses.find((s) => s.key === MetadataProviderKey.GOOGLE)?.hint).toContain('Recommended for higher rate limits');
|
||||
expect(statuses.find((s) => s.key === MetadataProviderKey.GOOGLE)?.configured).toBe(false);
|
||||
expect(statuses.find((s) => s.key === MetadataProviderKey.GOOGLE)?.hint).toContain('API key required');
|
||||
expect(statuses.find((s) => s.key === MetadataProviderKey.AMAZON)?.hint).toContain('Cookie recommended');
|
||||
expect(statuses.find((s) => s.key === MetadataProviderKey.HARDCOVER)?.configured).toBe(false);
|
||||
expect(statuses.find((s) => s.key === MetadataProviderKey.GOODREADS)?.enabled).toBe(false);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { Inject, Injectable, Logger } from '@nestjs/common';
|
||||
import { BadRequestException, Inject, Injectable, Logger } from '@nestjs/common';
|
||||
import { eq, sql } from 'drizzle-orm';
|
||||
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
|
||||
import { MetadataProviderKey, ProviderConfigurations, ProviderStatus } from '@bookorbit/types';
|
||||
@@ -15,7 +15,7 @@ type ProviderConfigPatch = {
|
||||
const PROVIDER_CONFIG_KEY = 'metadata_provider_config';
|
||||
|
||||
const DEFAULT_CONFIG: ProviderConfigurations = {
|
||||
google: { enabled: true, apiKey: '' },
|
||||
google: { enabled: false, apiKey: '' },
|
||||
amazon: { enabled: true, domain: 'amazon.com', cookie: '' },
|
||||
goodreads: { enabled: true },
|
||||
hardcover: { enabled: false, apiKey: '' },
|
||||
@@ -143,6 +143,20 @@ export class ProviderConfigService {
|
||||
};
|
||||
}
|
||||
|
||||
private validateConfig(config: ProviderConfigurations): void {
|
||||
if (config.google.enabled && !config.google.apiKey.trim()) {
|
||||
throw new BadRequestException('Google Books requires an API key before it can be enabled');
|
||||
}
|
||||
}
|
||||
|
||||
private normalizeConfig(config: ProviderConfigurations): ProviderConfigurations {
|
||||
if (!config.google.enabled || config.google.apiKey.trim()) return config;
|
||||
return {
|
||||
...config,
|
||||
google: { ...config.google, enabled: false },
|
||||
};
|
||||
}
|
||||
|
||||
private parsePersistedConfig(
|
||||
rawValue: string,
|
||||
fallback: ProviderConfigurations,
|
||||
@@ -170,7 +184,7 @@ export class ProviderConfigService {
|
||||
where: eq(schema.appSettings.key, PROVIDER_CONFIG_KEY),
|
||||
});
|
||||
if (!row) return defaults;
|
||||
return this.parsePersistedConfig(row.value, defaults, 'get', startedAt);
|
||||
return this.normalizeConfig(this.parsePersistedConfig(row.value, defaults, 'get', startedAt));
|
||||
}
|
||||
|
||||
async updateConfig(patch: ProviderConfigPatch): Promise<ProviderConfigurations> {
|
||||
@@ -182,8 +196,9 @@ export class ProviderConfigService {
|
||||
const row = await tx.query.appSettings.findFirst({
|
||||
where: eq(schema.appSettings.key, PROVIDER_CONFIG_KEY),
|
||||
});
|
||||
const current = row ? this.parsePersistedConfig(row.value, defaults, 'update', startedAt) : defaults;
|
||||
const current = row ? this.normalizeConfig(this.parsePersistedConfig(row.value, defaults, 'update', startedAt)) : defaults;
|
||||
const next = this.mergeConfig(current, patch);
|
||||
this.validateConfig(next);
|
||||
const value = JSON.stringify(next);
|
||||
await tx
|
||||
.insert(schema.appSettings)
|
||||
@@ -200,8 +215,8 @@ export class ProviderConfigService {
|
||||
key: MetadataProviderKey.GOOGLE,
|
||||
label: PROVIDER_LABELS[MetadataProviderKey.GOOGLE],
|
||||
enabled: cfg.google.enabled,
|
||||
configured: true,
|
||||
hint: !cfg.google.apiKey ? 'Recommended for higher rate limits' : undefined,
|
||||
configured: !!cfg.google.apiKey.trim(),
|
||||
hint: !cfg.google.apiKey.trim() ? 'API key required' : undefined,
|
||||
},
|
||||
{
|
||||
key: MetadataProviderKey.AMAZON,
|
||||
|
||||
Reference in New Issue
Block a user