diff --git a/desktop/src/generated/synkronus-client/docs/DefaultApi.md b/desktop/src/generated/synkronus-client/docs/DefaultApi.md index d3e9ac638..a1a668dee 100644 --- a/desktop/src/generated/synkronus-client/docs/DefaultApi.md +++ b/desktop/src/generated/synkronus-client/docs/DefaultApi.md @@ -1242,6 +1242,8 @@ No authorization required | **200** | Authentication successful | - | | **400** | Bad request | - | | **401** | Authentication failed | - | +| **413** | Authentication request exceeds the configured size limit | - | +| **429** | Too many authentication attempts | * Retry-After - Seconds until the client should retry
| [[Back to top]](#) [[Back to API list]](../README.md#api-endpoints) [[Back to Model list]](../README.md#models) [[Back to README]](../README.md) @@ -1390,6 +1392,8 @@ No authorization required | **200** | Token refresh successful | - | | **400** | Bad request | - | | **401** | Invalid or expired refresh token | - | +| **413** | Authentication request exceeds the configured size limit | - | +| **429** | Too many authentication attempts | * Retry-After - Seconds until the client should retry
| [[Back to top]](#) [[Back to API list]](../README.md#api-endpoints) [[Back to Model list]](../README.md#models) [[Back to README]](../README.md) @@ -1790,6 +1794,8 @@ example().catch(console.error); | **200** | Successful upload | - | | **400** | Bad request (missing or invalid file) | - | | **401** | Unauthorized | - | +| **403** | Authenticated account does not have write access | - | +| **413** | Attachment or multipart request exceeds the configured upload limit | - | | **409** | Conflict — attachment already exists, or repository_generation mismatch (epoch; align before upload) | - | [[Back to top]](#) [[Back to API list]](../README.md#api-endpoints) [[Back to Model list]](../README.md#models) [[Back to README]](../README.md) diff --git a/desktop/src/services/synk/GeneratedSyncGateway.ts b/desktop/src/services/synk/GeneratedSyncGateway.ts index e3a1bf2e5..3b86ff103 100644 --- a/desktop/src/services/synk/GeneratedSyncGateway.ts +++ b/desktop/src/services/synk/GeneratedSyncGateway.ts @@ -8,7 +8,7 @@ import { type SyncPullRequest, type SyncPushRequest, } from '../../generated/synkronus-client'; -import { SyncHttpError } from './syncErrors'; +import { parseRetryAfter, SyncHttpError } from './syncErrors'; import { DEFAULT_OBSERVATION_FORM_TYPE, DEFAULT_OBSERVATION_FORM_VERSION, @@ -176,11 +176,19 @@ async function toSyncGatewayError( const statusLine = `${error.response.status} ${error.response.statusText}`.trim(); const endpoint = error.response.url || baseUrl; + const retryAfterSeconds = parseRetryAfter( + error.response.headers.get('retry-after'), + ); const detailSuffix = responseDetails ? ` | ${responseDetails}` : ''; + const retrySuffix = + error.response.status === 429 && retryAfterSeconds !== undefined + ? ` | retry after ${retryAfterSeconds}s` + : ''; return new SyncHttpError( - `Synk ${operation} failed (HTTP ${statusLine}) at ${endpoint}${detailSuffix}`, + `Synk ${operation} failed (HTTP ${statusLine}) at ${endpoint}${detailSuffix}${retrySuffix}`, error.response.status, operation, + retryAfterSeconds, ); } diff --git a/desktop/src/services/synk/syncErrors.test.ts b/desktop/src/services/synk/syncErrors.test.ts new file mode 100644 index 000000000..680607952 --- /dev/null +++ b/desktop/src/services/synk/syncErrors.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest'; +import { parseRetryAfter, SyncHttpError } from './syncErrors'; + +describe('sync auth errors', () => { + it('parses Retry-After seconds and dates', () => { + expect(parseRetryAfter('12')).toBe(12); + expect( + parseRetryAfter( + 'Wed, 21 Oct 2015 07:28:10 GMT', + Date.parse('Wed, 21 Oct 2015 07:28:00 GMT'), + ), + ).toBe(10); + }); + + it('retains retry metadata without treating 429 as unauthorized', () => { + const error = new SyncHttpError('rate limited', 429, 'refresh', 10); + expect(error.retryAfterSeconds).toBe(10); + expect(error.status).toBe(429); + }); +}); diff --git a/desktop/src/services/synk/syncErrors.ts b/desktop/src/services/synk/syncErrors.ts index b3eefc3f4..c799e50b4 100644 --- a/desktop/src/services/synk/syncErrors.ts +++ b/desktop/src/services/synk/syncErrors.ts @@ -3,19 +3,34 @@ export type SyncGatewayOperation = 'login' | 'refresh' | 'pull' | 'push'; export class SyncHttpError extends Error { readonly status: number; readonly operation: SyncGatewayOperation; + readonly retryAfterSeconds?: number; constructor( message: string, status: number, operation: SyncGatewayOperation, + retryAfterSeconds?: number, ) { super(message); this.name = 'SyncHttpError'; this.status = status; this.operation = operation; + this.retryAfterSeconds = retryAfterSeconds; } } +export function parseRetryAfter( + value: string | null | undefined, + now = Date.now(), +): number | undefined { + if (!value) return undefined; + const seconds = Number(value); + if (Number.isFinite(seconds) && seconds >= 0) return Math.ceil(seconds); + const date = Date.parse(value); + if (Number.isNaN(date)) return undefined; + return Math.max(0, Math.ceil((date - now) / 1000)); +} + export function isSyncHttpUnauthorized(error: unknown): boolean { return ( error instanceof SyncHttpError && diff --git a/desktop/src/store/useCustodianStore.ts b/desktop/src/store/useCustodianStore.ts index a013f7bb4..e6121d91e 100644 --- a/desktop/src/store/useCustodianStore.ts +++ b/desktop/src/store/useCustodianStore.ts @@ -123,6 +123,11 @@ async function reauthenticateActiveProfile( set({ authSessionsByProfileId: merged }); return; } catch (refreshError) { + // Only an invalid refresh credential should fall back to password login. + // Retrying immediately after 429 or a transient failure would amplify load. + if (!isSyncHttpUnauthorized(refreshError)) { + throw refreshError; + } const cred = await tauriClient.credentialGet(id); const password = cred.password ?? ''; if (!password.trim()) { diff --git a/formulus/src/api/synkronus/Auth.ts b/formulus/src/api/synkronus/Auth.ts index 9b23a7157..3ea8e4bbe 100644 --- a/formulus/src/api/synkronus/Auth.ts +++ b/formulus/src/api/synkronus/Auth.ts @@ -3,6 +3,7 @@ import AsyncStorage from '@react-native-async-storage/async-storage'; import * as Keychain from 'react-native-keychain'; import { ODE_VERSION } from '../../version'; import { logger } from '../../diagnostics/logger'; +import { invalidateSettingsHydrationCache } from '../../services/SettingsHydrationCache'; export type UserRole = 'read-only' | 'read-write' | 'admin'; @@ -97,35 +98,87 @@ function decodeJwtPayload(token: string) { } } +const AUTH_STORAGE_KEYS = [ + '@token', + '@refreshToken', + '@tokenExpiresAt', + '@user', +]; + +const getHttpStatus = (error: unknown): number | undefined => { + const httpError = error as HttpError | undefined; + return ( + httpError?.response?.status ?? + httpError?.status ?? + httpError?.statusCode ?? + httpError?.body?.status ?? + httpError?.data?.status + ); +}; + +const clearSession = async (): Promise => { + synkronusApi.clearTokenCache(); + invalidateSettingsHydrationCache(); + await AsyncStorage.multiRemove(AUTH_STORAGE_KEYS); +}; + +const removeCredentialsIfMatching = async ( + username: string, + password: string, +): Promise => { + try { + const saved = await Keychain.getGenericPassword(); + if (saved && saved.username === username && saved.password === password) { + await Keychain.resetGenericPassword(); + } + } catch (error) { + console.warn('Failed to remove rejected saved credentials:', error); + } finally { + invalidateSettingsHydrationCache(); + } +}; + export const login = async ( username: string, password: string, ): Promise => { - logger.info('auth', 'login ok'); const api = await synkronusApi.getApi(); - synkronusApi.clearTokenCache(); - - const res = await api.login({ - xOdeVersion: ODE_VERSION, - loginRequest: { username, password }, - }); + let res; + try { + res = await api.login({ + xOdeVersion: ODE_VERSION, + loginRequest: { username, password }, + }); + } catch (error) { + // A concrete login HTTP 401 confirms that these credentials are invalid. + // Transient failures and compatibility errors must leave the prior session intact. + if (getHttpStatus(error) === 401) { + await clearSession(); + await removeCredentialsIfMatching(username, password); + } + throw error; + } const { token, refreshToken: refreshTokenValue, expiresAt } = res.data; + // Authentication has succeeded, so it is now safe to replace saved credentials. + await Keychain.setGenericPassword(username, password); + invalidateSettingsHydrationCache(); + await AsyncStorage.setItem('@token', token); await AsyncStorage.setItem('@refreshToken', refreshTokenValue); await AsyncStorage.setItem('@tokenExpiresAt', expiresAt.toString()); - // Decode JWT to get user info const claims = decodeJwtPayload(token); const userInfo: UserInfo = { username: claims?.username || username, role: claims?.role || 'read-only', }; - // Store user info await AsyncStorage.setItem('@user', JSON.stringify(userInfo)); + synkronusApi.clearTokenCache(); + logger.info('auth', 'login ok'); return userInfo; }; @@ -143,12 +196,13 @@ export const getUserInfo = async (): Promise => { }; export const logout = async (): Promise => { - await AsyncStorage.multiRemove([ - '@token', - '@refreshToken', - '@tokenExpiresAt', - '@user', + synkronusApi.clearTokenCache(); + invalidateSettingsHydrationCache(); + await Promise.all([ + AsyncStorage.multiRemove(AUTH_STORAGE_KEYS), + Keychain.resetGenericPassword(), ]); + invalidateSettingsHydrationCache(); }; // Function to retrieve the auth token from AsyncStorage @@ -222,15 +276,7 @@ export const isUnauthorizedError = (error: unknown): boolean => { const httpError = error as HttpError; - // Axios errors: error.response.status - if (httpError.response?.status === 401) return true; - - // Direct status properties - if (httpError.status === 401 || httpError.statusCode === 401) return true; - - // ProblemDetail format (from OpenAPI spec) - if (httpError.body?.status === 401 || httpError.data?.status === 401) - return true; + if (getHttpStatus(error) === 401) return true; // Check error message for 401 or unauthorized if (typeof httpError.message === 'string') { @@ -246,6 +292,9 @@ export const isUnauthorizedError = (error: unknown): boolean => { return false; }; +export const isRateLimitedError = (error: unknown): boolean => + getHttpStatus(error) === 429; + /** User-visible explanation when the server rejects observation upload (e.g. read-only role). */ export const SYNC_WRITE_FORBIDDEN_MESSAGE = 'You do not have permission to upload observations. Confirm with your administrator that your account has write access, or sign in with an account that can submit data.'; diff --git a/formulus/src/api/synkronus/__tests__/Auth.test.ts b/formulus/src/api/synkronus/__tests__/Auth.test.ts index d7c4f95ae..6776baa3f 100644 --- a/formulus/src/api/synkronus/__tests__/Auth.test.ts +++ b/formulus/src/api/synkronus/__tests__/Auth.test.ts @@ -59,8 +59,11 @@ import * as Keychain from 'react-native-keychain'; import AsyncStorage from '@react-native-async-storage/async-storage'; import { autoLogin, + isRateLimitedError, isUnauthorizedError, isVersionMismatchError, + login, + logout, } from '../Auth'; import { VersionMismatchError } from '../../../errors/VersionMismatchError'; import { ODE_VERSION } from '../../../version'; @@ -130,6 +133,83 @@ describe('Auth - Auto-Login', () => { }); }); + describe('isRateLimitedError', () => { + test('detects 429 without treating it as unauthorized', () => { + const error = { response: { status: 429 } }; + expect(isRateLimitedError(error)).toBe(true); + expect(isUnauthorizedError(error)).toBe(false); + }); + }); + + describe('session lifecycle', () => { + test('confirmed invalid credentials clear the prior session and rejected saved credentials', async () => { + const credentials = { username: 'testuser', password: 'wrong' }; + (Keychain.getGenericPassword as jest.Mock).mockResolvedValue(credentials); + const mockApi = { + login: jest.fn().mockRejectedValue({ response: { status: 401 } }), + }; + (synkronusApi.getApi as jest.Mock).mockResolvedValue(mockApi); + + await expect( + login(credentials.username, credentials.password), + ).rejects.toEqual({ + response: { status: 401 }, + }); + + expect(AsyncStorage.multiRemove).toHaveBeenCalledWith([ + '@token', + '@refreshToken', + '@tokenExpiresAt', + '@user', + ]); + expect(Keychain.resetGenericPassword).toHaveBeenCalled(); + expect(synkronusApi.clearTokenCache).toHaveBeenCalled(); + }); + + test('rate limiting preserves the prior session and credentials', async () => { + const mockApi = { + login: jest.fn().mockRejectedValue({ response: { status: 429 } }), + }; + (synkronusApi.getApi as jest.Mock).mockResolvedValue(mockApi); + + await expect(login('testuser', 'password')).rejects.toEqual({ + response: { status: 429 }, + }); + + expect(AsyncStorage.multiRemove).not.toHaveBeenCalled(); + expect(Keychain.resetGenericPassword).not.toHaveBeenCalled(); + expect(Keychain.setGenericPassword).not.toHaveBeenCalled(); + }); + + test('successful login persists credentials only after authentication succeeds', async () => { + const mockApi = { + login: jest.fn().mockResolvedValue({ + data: { + token: 'token', + refreshToken: 'refresh', + expiresAt: 123, + }, + }), + }; + (synkronusApi.getApi as jest.Mock).mockResolvedValue(mockApi); + + await login('testuser', 'password'); + + expect(mockApi.login).toHaveBeenCalled(); + expect(Keychain.setGenericPassword).toHaveBeenCalledWith( + 'testuser', + 'password', + ); + }); + + test('logout clears session and saved credentials', async () => { + await logout(); + expect(AsyncStorage.multiRemove).toHaveBeenCalled(); + expect(Keychain.resetGenericPassword).toHaveBeenCalled(); + expect(synkronusApi.clearTokenCache).toHaveBeenCalled(); + }); + }); + describe('isVersionMismatchError', () => { test('should detect VersionMismatchError instance', () => { const error = new VersionMismatchError( diff --git a/formulus/src/api/synkronus/generated/docs/DefaultApi.md b/formulus/src/api/synkronus/generated/docs/DefaultApi.md index 20147efaa..a1ccd98d5 100644 --- a/formulus/src/api/synkronus/generated/docs/DefaultApi.md +++ b/formulus/src/api/synkronus/generated/docs/DefaultApi.md @@ -855,11 +855,13 @@ No authorization required ### HTTP response details -| Status code | Description | Response headers | -| ----------- | ------------------------- | ---------------- | -| **200** | Authentication successful | - | -| **400** | Bad request | - | -| **401** | Authentication failed | - | +| Status code | Description | Response headers | +| ----------- | -------------------------------------------------------- | ----------------------------------------------------------- | +| **200** | Authentication successful | - | +| **400** | Bad request | - | +| **401** | Authentication failed | - | +| **413** | Authentication request exceeds the configured size limit | - | +| **429** | Too many authentication attempts | \* Retry-After - Seconds until the client should retry
| [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) @@ -958,11 +960,13 @@ No authorization required ### HTTP response details -| Status code | Description | Response headers | -| ----------- | -------------------------------- | ---------------- | -| **200** | Token refresh successful | - | -| **400** | Bad request | - | -| **401** | Invalid or expired refresh token | - | +| Status code | Description | Response headers | +| ----------- | -------------------------------------------------------- | ----------------------------------------------------------- | +| **200** | Token refresh successful | - | +| **400** | Bad request | - | +| **401** | Invalid or expired refresh token | - | +| **413** | Authentication request exceeds the configured size limit | - | +| **429** | Too many authentication attempts | \* Retry-After - Seconds until the client should retry
| [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) @@ -1248,6 +1252,8 @@ const { status, data } = await apiInstance.uploadAttachment( | **200** | Successful upload | - | | **400** | Bad request (missing or invalid file) | - | | **401** | Unauthorized | - | +| **403** | Authenticated account does not have write access | - | +| **413** | Attachment or multipart request exceeds the configured upload limit | - | | **409** | Conflict — attachment already exists, or repository_generation mismatch (epoch; align before upload) | - | [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) diff --git a/formulus/src/locales/en.json b/formulus/src/locales/en.json index 0027c3ddf..697b96abb 100644 --- a/formulus/src/locales/en.json +++ b/formulus/src/locales/en.json @@ -205,6 +205,7 @@ "settings.loginSuccess": "Successfully logged in!", "settings.loginFailedCredentials": "Please check your credentials.", "settings.loginFailed": "Login failed: {{message}}", + "settings.loginThrottled": "Too many login attempts. Please wait before trying again.", "settings.qrInvalid": "Invalid QR code format. Please try again.", "settings.qrError": "QR code error: {{message}}", "settings.qrScanFailed": "Failed to scan QR code. Please try again.", diff --git a/formulus/src/locales/fr.json b/formulus/src/locales/fr.json index e71fb2c55..7d09ddc69 100644 --- a/formulus/src/locales/fr.json +++ b/formulus/src/locales/fr.json @@ -205,6 +205,7 @@ "settings.loginSuccess": "Connexion réussie !", "settings.loginFailedCredentials": "Vérifiez vos identifiants.", "settings.loginFailed": "Échec de la connexion : {{message}}", + "settings.loginThrottled": "Trop de tentatives de connexion. Veuillez patienter avant de réessayer.", "settings.qrInvalid": "Format de code QR invalide. Veuillez réessayer.", "settings.qrError": "Erreur de code QR : {{message}}", "settings.qrScanFailed": "Impossible de scanner le code QR. Veuillez réessayer.", diff --git a/formulus/src/locales/pt.json b/formulus/src/locales/pt.json index 8e7e6cc9a..5bde52cdd 100644 --- a/formulus/src/locales/pt.json +++ b/formulus/src/locales/pt.json @@ -205,6 +205,7 @@ "settings.loginSuccess": "Sessão iniciada com sucesso!", "settings.loginFailedCredentials": "Verifique as suas credenciais.", "settings.loginFailed": "Falha no login: {{message}}", + "settings.loginThrottled": "Demasiadas tentativas de início de sessão. Aguarde antes de tentar novamente.", "settings.qrInvalid": "Formato de código QR inválido. Tente novamente.", "settings.qrError": "Erro no código QR: {{message}}", "settings.qrScanFailed": "Não foi possível ler o código QR. Tente novamente.", diff --git a/formulus/src/screens/SettingsScreen.tsx b/formulus/src/screens/SettingsScreen.tsx index fa18ad98e..927f755fb 100644 --- a/formulus/src/screens/SettingsScreen.tsx +++ b/formulus/src/screens/SettingsScreen.tsx @@ -17,9 +17,10 @@ import { Input as ODEInput, PasswordInput } from '../components/common'; import { SafeAreaView } from 'react-native-safe-area-context'; import { useNavigation } from '@react-navigation/native'; import { BottomTabNavigationProp } from '@react-navigation/bottom-tabs'; -import * as Keychain from 'react-native-keychain'; + import { login, + isRateLimitedError, isVersionMismatchError, getUserFacingSyncErrorMessage, } from '../api/synkronus/Auth'; @@ -387,7 +388,6 @@ const SettingsScreen = () => { try { await serverConfigService.saveServerUrl(norm.href); - await Keychain.setGenericPassword(trimmedUsername, trimmedPassword); await login(trimmedUsername, trimmedPassword); void loadSettingsHydrationFromStorage(); ToastService.showShort(t('settings.loginSuccess')); @@ -396,9 +396,11 @@ const SettingsScreen = () => { console.error('Login failed:', error); const message = isVersionMismatchError(error) ? error.message - : t('settings.loginFailed', { - message: t('settings.loginFailedCredentials'), - }); + : isRateLimitedError(error) + ? t('settings.loginThrottled') + : t('settings.loginFailed', { + message: t('settings.loginFailedCredentials'), + }); ToastService.showLong(message); } finally { setIsLoggingIn(false); @@ -441,13 +443,9 @@ const SettingsScreen = () => { setUsername(settings.username); setPassword(settings.password); - if (settings.username && settings.password) { - await serverConfigService.saveServerUrl(settings.serverUrl); + await serverConfigService.saveServerUrl(settings.serverUrl); - await Keychain.setGenericPassword( - settings.username, - settings.password, - ); + if (settings.username && settings.password) { try { await login(settings.username, settings.password); void loadSettingsHydrationFromStorage(); @@ -455,11 +453,14 @@ const SettingsScreen = () => { navigation.navigate('Sync'); } catch (error) { console.error('Auto-login failed:', error); - ToastService.showLong( - t('settings.loginFailed', { - message: t('settings.loginFailedCredentials'), - }), - ); + const message = isVersionMismatchError(error) + ? error.message + : isRateLimitedError(error) + ? t('settings.loginThrottled') + : t('settings.loginFailed', { + message: t('settings.loginFailedCredentials'), + }); + ToastService.showLong(message); } } else { void loadSettingsHydrationFromStorage(); diff --git a/formulus/src/services/QRSettingsService.ts b/formulus/src/services/QRSettingsService.ts index b450ab55b..f9b585610 100644 --- a/formulus/src/services/QRSettingsService.ts +++ b/formulus/src/services/QRSettingsService.ts @@ -1,5 +1,3 @@ -import AsyncStorage from '@react-native-async-storage/async-storage'; -import * as Keychain from 'react-native-keychain'; import { decodeFRMLS } from '../utils/FRMLSHelpers'; import { normalizeServerUrl } from './ServerConfigService'; @@ -32,28 +30,8 @@ export class QRSettingsService { } /** - * Updates app settings with parsed QR data - */ - static async updateSettings(settings: SettingsUpdate): Promise { - try { - // Save server URL to AsyncStorage - await AsyncStorage.setItem( - '@settings', - JSON.stringify({ - serverUrl: settings.serverUrl, - }), - ); - - // Save credentials to Keychain - await Keychain.setGenericPassword(settings.username, settings.password); - } catch (error) { - console.error('Failed to update settings:', error); - throw new Error('Failed to save settings'); - } - } - - /** - * Complete QR code processing: parse and update settings + * Parse and normalize QR settings without persisting credentials. The caller + * saves the server choice and login() stores credentials only after auth succeeds. */ static async processQRCode(qrString: string): Promise { const settings = this.parseQRCode(qrString); @@ -61,8 +39,6 @@ export class QRSettingsService { if (!normalized.ok) { throw new Error(normalized.message); } - const next = { ...settings, serverUrl: normalized.href }; - await this.updateSettings(next); - return next; + return { ...settings, serverUrl: normalized.href }; } } diff --git a/formulus/src/services/SettingsHydrationCache.ts b/formulus/src/services/SettingsHydrationCache.ts index 76baa21a6..8bbc83a71 100644 --- a/formulus/src/services/SettingsHydrationCache.ts +++ b/formulus/src/services/SettingsHydrationCache.ts @@ -12,6 +12,7 @@ export type SettingsHydrationSnapshot = let snapshot: SettingsHydrationSnapshot = { ready: false }; let inflight: Promise | null = null; +let generation = 0; function normalizeCredentials( raw: Awaited>, @@ -22,7 +23,9 @@ function normalizeCredentials( return { username: raw.username, password: raw.password }; } -async function fetchSnapshot(): Promise { +async function fetchSnapshot( + requestedGeneration: number, +): Promise { const [serverUrl, credentials] = await Promise.all([ serverConfigService.getServerUrl(), Keychain.getGenericPassword(), @@ -32,7 +35,9 @@ async function fetchSnapshot(): Promise { serverUrl, credentials: normalizeCredentials(credentials), }; - snapshot = next; + if (requestedGeneration === generation) { + snapshot = next; + } return next; } @@ -45,9 +50,13 @@ export function loadSettingsHydrationFromStorage(): Promise { - inflight = null; + const requestedGeneration = generation; + const request = fetchSnapshot(requestedGeneration).finally(() => { + if (inflight === request) { + inflight = null; + } }); + inflight = request; return inflight; } @@ -67,5 +76,7 @@ export function getSettingsHydrationCredentialPair( /** When storage may no longer match the cache (e.g. after server switch). */ export function invalidateSettingsHydrationCache(): void { + generation += 1; snapshot = { ready: false }; + inflight = null; } diff --git a/formulus/src/webview/FormulusMessageHandlers.ts b/formulus/src/webview/FormulusMessageHandlers.ts index 900142373..cc67da5ee 100644 --- a/formulus/src/webview/FormulusMessageHandlers.ts +++ b/formulus/src/webview/FormulusMessageHandlers.ts @@ -7,7 +7,7 @@ import { sequenceCounterService } from '../services/SequenceCounterService'; import { qrcodeRequestCoordinator } from '../services/QrcodeRequestCoordinator'; import { WebViewMessageEvent, WebView } from 'react-native-webview'; import RNFS from 'react-native-fs'; -import * as Keychain from 'react-native-keychain'; + import AsyncStorage from '@react-native-async-storage/async-storage'; import { Alert, Platform } from 'react-native'; import { i18n } from '../i18n/instance'; @@ -42,6 +42,7 @@ import { persistObservationWithAttachments } from '../services/attachmentStorage import { databaseService } from '../database/DatabaseService'; import { SyncService } from '../services/SyncService'; import { ServerConfigService } from '../services/ServerConfigService'; +import { getUserInfo } from '../api/synkronus/Auth'; // NitroSound is disabled for emulator in react-native.config.js - do not load the module // to avoid "Sound HybridObject not registered" console errors. Load lazily only when @@ -1211,38 +1212,17 @@ export function createFormulusMessageHandlers(): FormulusMessageHandlers { role?: 'read-only' | 'read-write' | 'admin'; }> => { try { - const credentials = await Keychain.getGenericPassword(); - if (!credentials) { + const user = await getUserInfo(); + if (!user) { // Logged out — same shape as authenticated user; empty username is // the contract for callers (e.g. placeholder) and must not throw. return { username: '' }; } - // Retrieve role from stored user info (set during login) - let role: 'read-only' | 'read-write' | 'admin' | undefined; - try { - const userJson = await AsyncStorage.getItem('@user'); - if (userJson) { - const userInfo = JSON.parse(userJson); - if ( - userInfo.role === 'admin' || - userInfo.role === 'read-write' || - userInfo.role === 'read-only' - ) { - role = userInfo.role; - } - } - } catch (roleError) { - console.warn( - 'FormulusMessageHandlers: Failed to retrieve user role:', - roleError, - ); - } - return { - username: credentials.username, - displayName: credentials.username, - role, + username: user.username, + displayName: user.username, + role: user.role, }; } catch (error) { console.error( diff --git a/synkronus-cli/internal/auth/auth.go b/synkronus-cli/internal/auth/auth.go index b79751889..be6927ac4 100644 --- a/synkronus-cli/internal/auth/auth.go +++ b/synkronus-cli/internal/auth/auth.go @@ -3,6 +3,9 @@ package auth import ( "context" "fmt" + "net/http" + "os" + "strconv" "time" "github.com/OpenDataEnsemble/ode/synkronus-cli/internal/utils" @@ -19,6 +22,44 @@ func apiVersion() (string, error) { return version, nil } +// RateLimitError indicates that Synkronus asked the client to back off. +type RateLimitError struct { + RetryAfter time.Duration +} + +func (e *RateLimitError) Error() string { + if e.RetryAfter > 0 { + return fmt.Sprintf("authentication rate limited; retry after %s", e.RetryAfter) + } + return "authentication rate limited; retry later" +} + +func rateLimitError(response *http.Response) error { + if response == nil || response.StatusCode != http.StatusTooManyRequests { + return nil + } + value := response.Header.Get("Retry-After") + if seconds, err := strconv.Atoi(value); err == nil && seconds >= 0 { + return &RateLimitError{RetryAfter: time.Duration(seconds) * time.Second} + } + if retryAt, err := http.ParseTime(value); err == nil { + return &RateLimitError{RetryAfter: time.Until(retryAt).Round(time.Second)} + } + return &RateLimitError{} +} + +func persistAuthConfig() error { + if err := viper.WriteConfig(); err != nil { + return err + } + if path := viper.ConfigFileUsed(); path != "" { + if err := os.Chmod(path, 0600); err != nil { + return fmt.Errorf("secure config permissions: %w", err) + } + } + return nil +} + // TokenResponse represents the response from the authentication endpoint type TokenResponse struct { Token string `json:"token"` @@ -57,8 +98,11 @@ func Login(username, password string) (*TokenResponse, error) { if err != nil { return nil, fmt.Errorf("login request failed: %w", err) } + if err := rateLimitError(resp.HTTPResponse); err != nil { + return nil, err + } if resp.JSON200 == nil { - return nil, fmt.Errorf("login failed with status %d: %s", resp.StatusCode(), string(resp.Body)) + return nil, fmt.Errorf("login failed with status %d", resp.StatusCode()) } tokenResp := &TokenResponse{ Token: resp.JSON200.Token, @@ -66,11 +110,12 @@ func Login(username, password string) (*TokenResponse, error) { ExpiresAt: resp.JSON200.ExpiresAt, } - // Save token to viper config viper.Set("auth.token", tokenResp.Token) viper.Set("auth.refresh_token", tokenResp.RefreshToken) viper.Set("auth.expires_at", tokenResp.ExpiresAt) - viper.WriteConfig() + if err := persistAuthConfig(); err != nil { + return nil, fmt.Errorf("persist authentication: %w", err) + } return tokenResp, nil } @@ -97,8 +142,11 @@ func RefreshToken() (*TokenResponse, error) { if err != nil { return nil, fmt.Errorf("refresh request failed: %w", err) } + if err := rateLimitError(resp.HTTPResponse); err != nil { + return nil, err + } if resp.JSON200 == nil { - return nil, fmt.Errorf("token refresh failed with status %d: %s", resp.StatusCode(), string(resp.Body)) + return nil, fmt.Errorf("token refresh failed with status %d", resp.StatusCode()) } tokenResp := &TokenResponse{ Token: resp.JSON200.Token, @@ -106,11 +154,12 @@ func RefreshToken() (*TokenResponse, error) { ExpiresAt: resp.JSON200.ExpiresAt, } - // Save token to viper config viper.Set("auth.token", tokenResp.Token) viper.Set("auth.refresh_token", tokenResp.RefreshToken) viper.Set("auth.expires_at", tokenResp.ExpiresAt) - viper.WriteConfig() + if err := persistAuthConfig(); err != nil { + return nil, fmt.Errorf("persist authentication: %w", err) + } return tokenResp, nil } @@ -162,5 +211,5 @@ func Logout() error { viper.Set("auth.token", "") viper.Set("auth.refresh_token", "") viper.Set("auth.expires_at", 0) - return viper.WriteConfig() + return persistAuthConfig() } diff --git a/synkronus-cli/internal/auth/auth_test.go b/synkronus-cli/internal/auth/auth_test.go new file mode 100644 index 000000000..2c0ccc495 --- /dev/null +++ b/synkronus-cli/internal/auth/auth_test.go @@ -0,0 +1,27 @@ +package auth + +import ( + "errors" + "net/http" + "testing" + "time" +) + +func TestRateLimitError(t *testing.T) { + response := &http.Response{StatusCode: http.StatusTooManyRequests, Header: make(http.Header)} + response.Header.Set("Retry-After", "12") + err := rateLimitError(response) + var limited *RateLimitError + if !errors.As(err, &limited) { + t.Fatalf("expected RateLimitError, got %v", err) + } + if limited.RetryAfter != 12*time.Second { + t.Fatalf("retry after = %s", limited.RetryAfter) + } +} + +func TestRateLimitErrorIgnoresOtherStatuses(t *testing.T) { + if err := rateLimitError(&http.Response{StatusCode: http.StatusUnauthorized}); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/synkronus-cli/internal/cmd/config.go b/synkronus-cli/internal/cmd/config.go index 195ded4fd..70ed2295f 100644 --- a/synkronus-cli/internal/cmd/config.go +++ b/synkronus-cli/internal/cmd/config.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/OpenDataEnsemble/ode/synkronus-cli/internal/config" "github.com/spf13/cobra" @@ -11,6 +12,34 @@ import ( "gopkg.in/yaml.v3" ) +func isSensitiveConfigKey(key string) bool { + key = strings.ToLower(key) + return strings.Contains(key, "password") || strings.Contains(key, "token") || strings.Contains(key, "secret") +} + +func redactConfigMap(settings map[string]any) map[string]any { + redacted := make(map[string]any, len(settings)) + for key, value := range settings { + if isSensitiveConfigKey(key) { + redacted[key] = "***REDACTED***" + continue + } + if nested, ok := value.(map[string]any); ok { + redacted[key] = redactConfigMap(nested) + } else { + redacted[key] = value + } + } + return redacted +} + +func secureConfigFilePermissions() error { + if path := viper.ConfigFileUsed(); path != "" { + return os.Chmod(path, 0600) + } + return nil +} + func init() { // Config command group configCmd := &cobra.Command{ @@ -53,7 +82,7 @@ func init() { // Create config directory if it doesn't exist configDir := filepath.Dir(configPath) if _, err := os.Stat(configDir); os.IsNotExist(err) { - if err := os.MkdirAll(configDir, 0755); err != nil { + if err := os.MkdirAll(configDir, 0700); err != nil { return fmt.Errorf("error creating config directory: %w", err) } } @@ -68,9 +97,12 @@ func init() { } // Write to file - if err := os.WriteFile(configPath, yamlData, 0644); err != nil { + if err := os.WriteFile(configPath, yamlData, 0600); err != nil { return fmt.Errorf("error writing config file: %w", err) } + if err := os.Chmod(configPath, 0600); err != nil { + return fmt.Errorf("error securing config file: %w", err) + } fmt.Printf("Configuration file created at %s\n", configPath) return nil @@ -87,7 +119,7 @@ func init() { Long: `Display the current configuration settings.`, RunE: func(cmd *cobra.Command, args []string) error { // Get all settings - allSettings := viper.AllSettings() + allSettings := redactConfigMap(viper.AllSettings()) // Convert to YAML yamlData, err := yaml.Marshal(allSettings) @@ -116,8 +148,15 @@ func init() { if err := viper.WriteConfig(); err != nil { return fmt.Errorf("error writing config: %w", err) } + if err := secureConfigFilePermissions(); err != nil { + return fmt.Errorf("error securing config: %w", err) + } - fmt.Printf("Set %s = %s\n", key, value) + displayedValue := value + if isSensitiveConfigKey(key) { + displayedValue = "***REDACTED***" + } + fmt.Printf("Set %s = %s\n", key, displayedValue) return nil }, } @@ -152,9 +191,12 @@ func init() { } pointerPath := filepath.Join(home, ".synkronus_current") - if err := os.WriteFile(pointerPath, []byte(absPath+"\n"), 0644); err != nil { + if err := os.WriteFile(pointerPath, []byte(absPath+"\n"), 0600); err != nil { return fmt.Errorf("error writing current config pointer: %w", err) } + if err := os.Chmod(pointerPath, 0600); err != nil { + return fmt.Errorf("error securing current config pointer: %w", err) + } fmt.Printf("Current config set to %s\n", absPath) return nil diff --git a/synkronus-cli/internal/cmd/config_test.go b/synkronus-cli/internal/cmd/config_test.go new file mode 100644 index 000000000..4c362ea03 --- /dev/null +++ b/synkronus-cli/internal/cmd/config_test.go @@ -0,0 +1,26 @@ +package cmd + +import "testing" + +func TestRedactConfigMap(t *testing.T) { + settings := map[string]any{ + "api": map[string]any{"url": "https://example.test"}, + "auth": map[string]any{ + "token": "access", + "refresh_token": "refresh", + "expires_at": 123, + }, + "database_password": "password", + } + redacted := redactConfigMap(settings) + auth := redacted["auth"].(map[string]any) + if auth["token"] != "***REDACTED***" || auth["refresh_token"] != "***REDACTED***" { + t.Fatalf("tokens were not redacted: %#v", auth) + } + if auth["expires_at"] != 123 { + t.Fatalf("non-secret value changed: %#v", auth) + } + if redacted["database_password"] != "***REDACTED***" { + t.Fatal("password was not redacted") + } +} diff --git a/synkronus-portal/src/api/synkronus/generated/docs/DefaultApi.md b/synkronus-portal/src/api/synkronus/generated/docs/DefaultApi.md index c79d73223..3069f2a70 100644 --- a/synkronus-portal/src/api/synkronus/generated/docs/DefaultApi.md +++ b/synkronus-portal/src/api/synkronus/generated/docs/DefaultApi.md @@ -933,6 +933,8 @@ No authorization required |**200** | Authentication successful | - | |**400** | Bad request | - | |**401** | Authentication failed | - | +|**413** | Authentication request exceeds the configured size limit | - | +|**429** | Too many authentication attempts | * Retry-After - Seconds until the client should retry
| [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) @@ -1047,6 +1049,8 @@ No authorization required |**200** | Token refresh successful | - | |**400** | Bad request | - | |**401** | Invalid or expired refresh token | - | +|**413** | Authentication request exceeds the configured size limit | - | +|**429** | Too many authentication attempts | * Retry-After - Seconds until the client should retry
| [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) @@ -1353,6 +1357,8 @@ const { status, data } = await apiInstance.uploadAttachment( |**200** | Successful upload | - | |**400** | Bad request (missing or invalid file) | - | |**401** | Unauthorized | - | +|**403** | Authenticated account does not have write access | - | +|**413** | Attachment or multipart request exceeds the configured upload limit | - | |**409** | Conflict — attachment already exists, or repository_generation mismatch (epoch; align before upload) | - | [[Back to top]](#) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to Model list]](../README.md#documentation-for-models) [[Back to README]](../README.md) diff --git a/synkronus-portal/src/contexts/AuthContext.tsx b/synkronus-portal/src/contexts/AuthContext.tsx index f86e81a7a..ebf900aa6 100644 --- a/synkronus-portal/src/contexts/AuthContext.tsx +++ b/synkronus-portal/src/contexts/AuthContext.tsx @@ -1,6 +1,6 @@ import { createContext, useContext, useState } from 'react'; import type { ReactNode } from 'react'; -import { api } from '../services/api'; +import { ApiError, api } from '../services/api'; import type { LoginRequest, User, AuthState } from '../types/auth'; interface AuthContextType extends AuthState { @@ -118,8 +118,14 @@ export function AuthProvider({ children }: { children: ReactNode }) { isAuthenticated: true, }); } catch (error) { - // Refresh failed, logout - logout(); + // Preserve the current session during throttling and transient failures. + // Only a rejected refresh credential or malformed successful response is definitive. + if ( + (error instanceof ApiError && error.status === 401) || + (error instanceof Error && error.message === 'Invalid token format') + ) { + logout(); + } throw error; } }; diff --git a/synkronus-portal/src/services/api.ts b/synkronus-portal/src/services/api.ts index 2b31da373..30d4ba53e 100644 --- a/synkronus-portal/src/services/api.ts +++ b/synkronus-portal/src/services/api.ts @@ -40,6 +40,29 @@ const getApiOriginForGeneratedClient = (): string => { const API_BASE_URL = getApiBaseUrl(); const GENERATED_CLIENT_BASE_PATH = getApiOriginForGeneratedClient(); +export class ApiError extends Error { + constructor( + message: string, + readonly status?: number, + readonly retryAfterSeconds?: number, + ) { + super(message); + this.name = 'ApiError'; + } +} + +export function parseRetryAfter( + value: string | null | undefined, + now = Date.now(), +): number | undefined { + if (!value) return undefined; + const seconds = Number(value); + if (Number.isFinite(seconds) && seconds >= 0) return Math.ceil(seconds); + const date = Date.parse(value); + if (Number.isNaN(date)) return undefined; + return Math.max(0, Math.ceil((date - now) / 1000)); +} + const clearStoredAuth = () => { localStorage.removeItem('token'); localStorage.removeItem('refreshToken'); @@ -73,9 +96,30 @@ const toApiError = ( if (redirectOnUnauthorized) { clearStoredAuth(); window.location.href = '/'; - return new Error('Your session has expired. Please log in again.'); + return new ApiError( + 'Your session has expired. Please log in again.', + status, + ); } - return new Error(errorMessage || 'Invalid username or password'); + return new ApiError( + errorMessage || 'Invalid username or password', + status, + ); + } + + if (status === 429) { + const retryAfterSeconds = parseRetryAfter( + axiosError.response?.headers['retry-after'] as string | undefined, + ); + const retryMessage = + retryAfterSeconds !== undefined + ? ` Try again in ${retryAfterSeconds} seconds.` + : ' Try again later.'; + return new ApiError( + `${errorMessage || 'Too many requests.'}${retryMessage}`, + status, + retryAfterSeconds, + ); } if (status === 0 || (status !== undefined && status >= 500)) { @@ -84,7 +128,10 @@ const toApiError = ( errorMessage = 'No internet connection'; } - return new Error(errorMessage || 'Network error: Unable to reach API'); + return new ApiError( + errorMessage || 'Network error: Unable to reach API', + status, + ); } if (error instanceof Error) return error; @@ -172,7 +219,7 @@ export const api = { }); return res.data as LoginResponse; } catch (error) { - throw toApiError(error); + throw toApiError(error, { redirectOnUnauthorized: false }); } }, diff --git a/synkronus/DEPLOYMENT.md b/synkronus/DEPLOYMENT.md index b918be64f..662e18dd0 100644 --- a/synkronus/DEPLOYMENT.md +++ b/synkronus/DEPLOYMENT.md @@ -178,6 +178,21 @@ Your Synkronus instance is now accessible at `https://synkronus.your-domain.com` | `PORT` | `8080` | HTTP server port | | `LOG_LEVEL` | `info` | Logging level (`debug`, `info`, `warn`, `error`) | | `MAX_VERSIONS_KEPT` | `5` | Number of app bundle versions to retain | +| `SYNKRONUS_ACCEPT_LEGACY_UNTYPED_TOKENS` | `true` | Temporarily accept JWTs issued before token-purpose claims were added | +| `SYNKRONUS_AUTH_MAX_BODY_BYTES` | `16384` | Maximum login/refresh request body bytes | +| `SYNKRONUS_AUTH_IP_ATTEMPTS` | `60` | Login/refresh attempts allowed per source in the IP window | +| `SYNKRONUS_AUTH_IP_WINDOW_SECONDS` | `60` | Per-source limiter window in seconds | +| `SYNKRONUS_AUTH_LOGIN_ATTEMPTS` | `10` | Failed logins allowed per source and username | +| `SYNKRONUS_AUTH_LOGIN_WINDOW_SECONDS` | `300` | Source-and-username failure window in seconds | +| `SYNKRONUS_AUTH_ACCOUNT_ATTEMPTS` | `100` | Failed logins allowed per username across sources | +| `SYNKRONUS_AUTH_ACCOUNT_WINDOW_SECONDS` | `900` | Account-wide failure window in seconds | +| `SYNKRONUS_AUTH_LIMITER_MAX_KEYS` | `10000` | Maximum tracked keys in each in-memory limiter | +| `SYNKRONUS_AUTH_TRUSTED_PROXY_CIDRS` | *(empty)* | Comma-separated direct proxy CIDRs allowed to supply `X-Real-IP` | +| `SYNKRONUS_MAX_ATTACHMENT_UPLOAD_BYTES` | `134217728` | Maximum attachment content bytes (128 MiB); proxy request limits need multipart overhead | +| `SYNKRONUS_MAX_CONCURRENT_ATTACHMENT_UPLOADS` | `4` | Maximum concurrent attachment request processing | +| `SYNKRONUS_MAX_CONCURRENT_IMAGE_PROCESSING` | `2` | Maximum concurrent decoded image operations | +| `SYNKRONUS_MAX_DECODED_IMAGE_DIMENSION_PX` | `16384` | Maximum decoded image width or height | +| `SYNKRONUS_MAX_DECODED_IMAGE_PIXELS` | `40000000` | Maximum decoded image pixel count | | `ADMIN_USERNAME` | `admin` | Initial admin username | | `ADMIN_PASSWORD` | `admin` | Initial admin password (CHANGE THIS!) | | `SYNKRONUS_RECOVERY_CREATE_USER` | *(empty)* | Recovery admin username (must be set with pass) | @@ -187,6 +202,18 @@ In the official container image, the process runs from **`/app`** (binary at **` `ADMIN_USERNAME`/`ADMIN_PASSWORD` are bootstrap credentials and only apply when there are no users yet. For emergency recovery, set `SYNKRONUS_RECOVERY_CREATE_USER` and `SYNKRONUS_RECOVERY_CREATE_PASS` together; on startup, Synkronus creates or overwrites that user as admin. Remove these recovery variables after use so credentials are not reset on every restart. +### Authentication security controls + +The login and refresh limits are bounded, in-memory controls. They apply independently to each Synkronus process and reset on restart; multiple replicas multiply the effective allowance. Horizontally scaled installations should add a shared limiter at the load balancer, API gateway, or another shared edge. + +By default, Synkronus ignores client-IP headers and keys source limits by the direct socket peer. When a reverse proxy is used, set `SYNKRONUS_AUTH_TRUSTED_PROXY_CIDRS` to the narrowest CIDR containing the proxy's Synkronus-facing address. Only a direct peer in one of those CIDRs may supply `X-Real-IP`. Never configure unrestricted ranges such as `0.0.0.0/0`, and do not trust a public ingress range unless every address in it is controlled and overwrites inbound `X-Real-IP`. The provided Nginx configuration overwrites `X-Real-IP` with its observed client address. If the setting remains empty behind a proxy, all proxied requests safely share the proxy's source budget rather than trusting spoofable headers. + +Excessive authentication requests receive `429 Too Many Requests` with a `Retry-After` header. Avoid permanent account lockouts: they let an attacker deny service to a known user. These controls reduce online guessing and bcrypt resource exhaustion but do not replace edge denial-of-service protection. + +Newly issued JWTs carry a signed access/refresh purpose. `SYNKRONUS_ACCEPT_LEGACY_UNTYPED_TOKENS` preserves compatibility with tokens from older servers. Disable it only after **all** issuing instances are upgraded and at least the old seven-day refresh-token lifetime, plus clock-skew and operational margin, has elapsed since the final legacy issuer was removed. Purpose-bound tokens used in the wrong context are rejected even while compatibility mode is enabled. + +JWT validation remains stateless. Password changes, role reductions, account deletion, or local sign-out cannot immediately revoke already-issued tokens; they remain valid until expiry. Rotating `JWT_SECRET` provides emergency global invalidation but signs out every user. + ## Volume Management ### Migrating from older layouts diff --git a/synkronus/README.md b/synkronus/README.md index 309ac93f1..05cb9249f 100644 --- a/synkronus/README.md +++ b/synkronus/README.md @@ -77,6 +77,21 @@ Synkronus uses a flexible configuration system that supports both environment va | `JWT_SECRET` | Secret key for JWT token signing | (required, no default) | | `LOG_LEVEL` | Logging level (debug, info, warn, error) | `info` | | `MAX_VERSIONS_KEPT` | Maximum number of app bundle versions to keep | `5` | +| `SYNKRONUS_ACCEPT_LEGACY_UNTYPED_TOKENS` | Temporarily accept JWTs issued before token-purpose claims were added | `true` | +| `SYNKRONUS_AUTH_MAX_BODY_BYTES` | Maximum login/refresh request body bytes | `16384` | +| `SYNKRONUS_AUTH_IP_ATTEMPTS` | Login/refresh attempts allowed per source in the IP window | `60` | +| `SYNKRONUS_AUTH_IP_WINDOW_SECONDS` | Per-source limiter window | `60` | +| `SYNKRONUS_AUTH_LOGIN_ATTEMPTS` | Failed logins allowed per source and username | `10` | +| `SYNKRONUS_AUTH_LOGIN_WINDOW_SECONDS` | Source-and-username failure window | `300` | +| `SYNKRONUS_AUTH_ACCOUNT_ATTEMPTS` | Failed logins allowed per username across sources | `100` | +| `SYNKRONUS_AUTH_ACCOUNT_WINDOW_SECONDS` | Account-wide failure window | `900` | +| `SYNKRONUS_AUTH_LIMITER_MAX_KEYS` | Maximum tracked keys in each in-memory limiter | `10000` | +| `SYNKRONUS_AUTH_TRUSTED_PROXY_CIDRS` | Comma-separated direct proxy CIDRs allowed to supply `X-Real-IP` | (empty) | +| `SYNKRONUS_MAX_ATTACHMENT_UPLOAD_BYTES` | Maximum attachment content bytes | `134217728` (128 MiB) | +| `SYNKRONUS_MAX_CONCURRENT_ATTACHMENT_UPLOADS` | Maximum concurrent attachment uploads | `4` | +| `SYNKRONUS_MAX_CONCURRENT_IMAGE_PROCESSING` | Maximum concurrent image decode/transform jobs | `2` | +| `SYNKRONUS_MAX_DECODED_IMAGE_DIMENSION_PX` | Maximum decoded image width or height | `16384` | +| `SYNKRONUS_MAX_DECODED_IMAGE_PIXELS` | Maximum decoded image pixel count | `40000000` | | `ADMIN_USERNAME` | Initial admin username (bootstrap only) | `admin` | | `ADMIN_PASSWORD` | Initial admin password (bootstrap only) | `admin` | | `SYNKRONUS_RECOVERY_CREATE_USER` | Recovery admin username (must be paired with pass) | (empty) | @@ -89,6 +104,10 @@ Synkronus uses a flexible configuration system that supports both environment va `ADMIN_USERNAME`/`ADMIN_PASSWORD` are only used when no users exist in the database. `SYNKRONUS_RECOVERY_CREATE_USER` + `SYNKRONUS_RECOVERY_CREATE_PASS` provide an emergency recovery flow: on startup, Synkronus creates or overwrites that user as an admin. Remove those recovery variables after regaining access to avoid resetting credentials on each restart. +Authentication limits are in memory, apply independently to each Synkronus process, and reset when the process restarts. Multi-replica deployments should also enforce shared limits at the edge or use a shared limiter. Synkronus trusts `X-Real-IP` only when the direct socket peer is within `SYNKRONUS_AUTH_TRUSTED_PROXY_CIDRS`; configure only the exact CIDRs used by proxies you control. If it is empty, forwarded addresses are ignored. Excessive requests receive `429 Too Many Requests` with `Retry-After`. + +New JWTs are purpose-bound as access or refresh tokens. `SYNKRONUS_ACCEPT_LEGACY_UNTYPED_TOKENS` exists only for a backwards-compatible rollout. Set it to `false` only after all token-issuing Synkronus instances are upgraded and at least the previous seven-day refresh-token lifetime, plus operational and clock-skew margin, has elapsed since the last legacy issuer was removed. + Attachment image processing is optional and only applies to supported image formats. If processing creates a smaller client-facing file, Synkronus stores it in `data/attachments/` and preserves the uploaded original in `data/attachments_uncompressed/` for export and explicit retrieval. ### Running the API diff --git a/synkronus/cmd/synkronus/main.go b/synkronus/cmd/synkronus/main.go index 9d6d8d633..8c04be706 100644 --- a/synkronus/cmd/synkronus/main.go +++ b/synkronus/cmd/synkronus/main.go @@ -144,6 +144,14 @@ func main() { authConfig := auth.DefaultConfig() // Override auth config from configuration authConfig.JWTSecret = cfg.JWTSecret + if raw := os.Getenv("SYNKRONUS_ACCEPT_LEGACY_UNTYPED_TOKENS"); raw != "" { + value, parseErr := strconv.ParseBool(raw) + if parseErr != nil { + log.Error("Invalid SYNKRONUS_ACCEPT_LEGACY_UNTYPED_TOKENS value", "error", parseErr) + return + } + authConfig.AcceptLegacyUntypedTokens = value + } // These can still be overridden by environment variables for security if adminUsername := os.Getenv("ADMIN_USERNAME"); adminUsername != "" { diff --git a/synkronus/docker-compose.example.yml b/synkronus/docker-compose.example.yml index 892882b3b..7f42f25a6 100644 --- a/synkronus/docker-compose.example.yml +++ b/synkronus/docker-compose.example.yml @@ -60,6 +60,26 @@ services: # SYNKRONUS_RECOVERY_CREATE_USER: "admin" # SYNKRONUS_RECOVERY_CREATE_PASS: "TEMPORARY_RECOVERY_PASSWORD" MAX_VERSIONS_KEPT: "5" + # Authentication abuse controls. Limits are per Synkronus process and reset on restart. + SYNKRONUS_AUTH_MAX_BODY_BYTES: "16384" + SYNKRONUS_AUTH_IP_ATTEMPTS: "60" + SYNKRONUS_AUTH_IP_WINDOW_SECONDS: "60" + SYNKRONUS_AUTH_LOGIN_ATTEMPTS: "10" + SYNKRONUS_AUTH_LOGIN_WINDOW_SECONDS: "300" + SYNKRONUS_AUTH_ACCOUNT_ATTEMPTS: "100" + SYNKRONUS_AUTH_ACCOUNT_WINDOW_SECONDS: "900" + SYNKRONUS_AUTH_LIMITER_MAX_KEYS: "10000" + # Set this to the exact Synkronus-facing CIDR of proxies you control. When empty, + # X-Real-IP is ignored and proxied requests safely share the proxy source budget. + # SYNKRONUS_AUTH_TRUSTED_PROXY_CIDRS: "172.20.0.0/24" + # Keep true during the typed-token rollout; see DEPLOYMENT.md before disabling. + SYNKRONUS_ACCEPT_LEGACY_UNTYPED_TOKENS: "true" + # Attachment content limit (128 MiB); keep nginx client_max_body_size above this for multipart overhead. + SYNKRONUS_MAX_ATTACHMENT_UPLOAD_BYTES: "134217728" + SYNKRONUS_MAX_CONCURRENT_ATTACHMENT_UPLOADS: "4" + SYNKRONUS_MAX_CONCURRENT_IMAGE_PROCESSING: "2" + SYNKRONUS_MAX_DECODED_IMAGE_DIMENSION_PX: "16384" + SYNKRONUS_MAX_DECODED_IMAGE_PIXELS: "40000000" volumes: - synkronus_data:/app/data depends_on: diff --git a/synkronus/internal/api/api.go b/synkronus/internal/api/api.go index 89e9e7d31..f1985c796 100644 --- a/synkronus/internal/api/api.go +++ b/synkronus/internal/api/api.go @@ -4,6 +4,7 @@ import ( "net/http" "os" "path/filepath" + "time" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" @@ -12,6 +13,7 @@ import ( "github.com/opendataensemble/synkronus/internal/handlers" "github.com/opendataensemble/synkronus/internal/models" "github.com/opendataensemble/synkronus/pkg/attachment" + "github.com/opendataensemble/synkronus/pkg/authlimit" "github.com/opendataensemble/synkronus/pkg/httptimeout" "github.com/opendataensemble/synkronus/pkg/logger" "github.com/opendataensemble/synkronus/pkg/middleware/auth" @@ -37,6 +39,9 @@ func FileServer(r chi.Router, path string, root http.FileSystem) { func NewRouter(log *logger.Logger, h *handlers.Handler) http.Handler { r := chi.NewRouter() + // Capture the socket peer before RealIP interprets forwarding headers. + r.Use(authlimit.CapturePeer) + // Add middleware r.Use(middleware.RequestID) r.Use(middleware.RealIP) @@ -92,11 +97,29 @@ func NewRouter(log *logger.Logger, h *handlers.Handler) http.Handler { FileServer(r, "/openapi", http.Dir(openapiDir)) } - // All REST API routes (except /health) under /api + // All REST API routes (except /health) under /api. + cfg := h.GetConfig() + authGuard, err := authlimit.New(authlimit.Config{ + MaxBodyBytes: cfg.AuthMaxBodyBytes, + MaxUsernameBytes: 255, + MaxPasswordBytes: 1024, + MaxTokenBytes: 16 << 10, + IPAttempts: cfg.AuthIPAttempts, + IPWindow: time.Duration(cfg.AuthIPWindowSeconds) * time.Second, + LoginAttempts: cfg.AuthLoginAttempts, + LoginWindow: time.Duration(cfg.AuthLoginWindowSeconds) * time.Second, + AccountAttempts: cfg.AuthAccountAttempts, + AccountWindow: time.Duration(cfg.AuthAccountWindowSeconds) * time.Second, + MaxKeys: cfg.AuthLimiterMaxKeys, + TrustedProxyCIDRs: cfg.AuthTrustedProxyCIDRs, + }) + if err != nil { + panic("invalid authentication limiter configuration: " + err.Error()) + } authRoutes := func(r chi.Router) { r.Use(formulusversion.Middleware(log)) - r.Post("/login", h.Login) - r.Post("/refresh", h.RefreshToken) + r.With(authGuard.Middleware(authlimit.EndpointLogin)).Post("/login", h.Login) + r.With(authGuard.Middleware(authlimit.EndpointRefresh)).Post("/refresh", h.RefreshToken) } // Create attachment service @@ -111,6 +134,7 @@ func NewRouter(log *logger.Logger, h *handlers.Handler) http.Handler { attachmentService, h.AttachmentManifestService(), h.SyncService(), + h.GetConfig(), ) r.Route("/api", func(r chi.Router) { diff --git a/synkronus/internal/handlers/attachment.go b/synkronus/internal/handlers/attachment.go index b5e1ed7dd..7a212327e 100644 --- a/synkronus/internal/handlers/attachment.go +++ b/synkronus/internal/handlers/attachment.go @@ -4,39 +4,57 @@ import ( "context" "errors" "io" + "mime" "net/http" "os" + "path/filepath" "strconv" "strings" "github.com/go-chi/chi/v5" "github.com/opendataensemble/synkronus/internal/models" "github.com/opendataensemble/synkronus/pkg/attachment" + "github.com/opendataensemble/synkronus/pkg/config" "github.com/opendataensemble/synkronus/pkg/logger" "github.com/opendataensemble/synkronus/pkg/middleware/auth" "github.com/opendataensemble/synkronus/pkg/sync" ) type AttachmentHandler struct { - service attachment.Service - manifest attachment.ManifestService - syncService sync.ServiceInterface - log *logger.Logger + service attachment.Service + manifest attachment.ManifestService + syncService sync.ServiceInterface + log *logger.Logger + maxUploadBytes int64 + uploadSlots chan struct{} } -const maxAttachmentUploadBytes = 32 << 20 +const multipartMemoryBytes = 1 << 20 func NewAttachmentHandler( log *logger.Logger, service attachment.Service, manifest attachment.ManifestService, syncService sync.ServiceInterface, + configs ...*config.Config, ) *AttachmentHandler { + maxUploadBytes := config.DefaultMaxAttachmentUploadBytes + maxConcurrentUploads := config.DefaultMaxConcurrentAttachmentUploads + if len(configs) > 0 && configs[0] != nil { + if configs[0].MaxAttachmentUploadBytes > 0 { + maxUploadBytes = configs[0].MaxAttachmentUploadBytes + } + if configs[0].MaxConcurrentAttachmentUploads > 0 { + maxConcurrentUploads = configs[0].MaxConcurrentAttachmentUploads + } + } return &AttachmentHandler{ - service: service, - manifest: manifest, - syncService: syncService, - log: log, + service: service, + manifest: manifest, + syncService: syncService, + log: log, + maxUploadBytes: maxUploadBytes, + uploadSlots: make(chan struct{}, maxConcurrentUploads), } } @@ -48,7 +66,7 @@ func (h *AttachmentHandler) RegisterRoutes(r chi.Router, manifestHandler func(ht r.With(auth.RequireRole(models.RoleReadOnly, models.RoleReadWrite, models.RoleAdmin)).Get("/export-zip", h.ExportAllAttachmentsZip) r.Route("/{attachment_id}", func(r chi.Router) { - r.Put("/", h.UploadAttachment) + r.With(auth.RequireRole(models.RoleReadWrite, models.RoleAdmin)).Put("/", h.UploadAttachment) r.Get("/", h.DownloadAttachment) r.Head("/", h.CheckAttachment) }) @@ -104,6 +122,17 @@ func (h *AttachmentHandler) UploadAttachment(w http.ResponseWriter, r *http.Requ SendErrorResponse(w, http.StatusBadRequest, nil, "attachment_id is required") return } + if err := attachment.ValidateAttachmentID(attachmentID); err != nil { + SendErrorResponse(w, http.StatusBadRequest, nil, "attachment_id is invalid") + return + } + select { + case h.uploadSlots <- struct{}{}: + defer func() { <-h.uploadSlots }() + case <-r.Context().Done(): + SendErrorResponse(w, http.StatusRequestTimeout, nil, "Upload cancelled") + return + } clientGen, clientGenSent := sync.ParseClientRepositoryGenerationSent(r, nil) serverGen, err := h.syncService.GetRepositoryGeneration(r.Context()) @@ -121,10 +150,21 @@ func (h *AttachmentHandler) UploadAttachment(w http.ResponseWriter, r *http.Requ return } - // Parse the multipart form - err = r.ParseMultipartForm(maxAttachmentUploadBytes) + // Bound the complete multipart request separately from the attachment content. + r.Body = http.MaxBytesReader(w, r.Body, h.maxUploadBytes+multipartMemoryBytes) + err = r.ParseMultipartForm(multipartMemoryBytes) if err != nil { - SendErrorResponse(w, http.StatusBadRequest, err, "Failed to parse multipart form") + var maxBytesError *http.MaxBytesError + if errors.As(err, &maxBytesError) { + SendErrorResponse(w, http.StatusRequestEntityTooLarge, nil, "Attachment exceeds upload size limit") + return + } + SendErrorResponse(w, http.StatusBadRequest, nil, "Failed to parse multipart form") + return + } + defer r.MultipartForm.RemoveAll() + if len(r.MultipartForm.File) != 1 || len(r.MultipartForm.File["file"]) != 1 || len(r.MultipartForm.Value) != 0 { + SendErrorResponse(w, http.StatusBadRequest, nil, "Exactly one file part is required") return } @@ -140,14 +180,12 @@ func (h *AttachmentHandler) UploadAttachment(w http.ResponseWriter, r *http.Requ } defer file.Close() - data, err := readAllWithLimit(file, maxAttachmentUploadBytes) - if err != nil { - SendErrorResponse(w, http.StatusRequestEntityTooLarge, err, "Attachment exceeds upload size limit") - return - } - - saveResult, err := h.service.SaveUpload(r.Context(), attachmentID, data, header.Header.Get("Content-Type")) + saveResult, err := h.service.SaveUpload(r.Context(), attachmentID, file, header.Header.Get("Content-Type")) if err != nil { + if errors.Is(err, attachment.ErrAttachmentTooLarge) { + SendErrorResponse(w, http.StatusRequestEntityTooLarge, nil, "Attachment exceeds upload size limit") + return + } if os.IsExist(err) { SendErrorResponse(w, http.StatusConflict, err, "Attachment already exists") return @@ -160,7 +198,10 @@ func (h *AttachmentHandler) UploadAttachment(w http.ResponseWriter, r *http.Requ // client_id empty => NULL, meaning all clients (see migration comment on attachment_operations). if err := h.recordAttachmentCreate(r.Context(), attachmentID, saveResult.ServedSize, saveResult.ServedContentType); err != nil { h.log.Error("Failed to record attachment manifest operation", "attachmentId", attachmentID, "error", err) - SendErrorResponse(w, http.StatusInternalServerError, err, "Failed to register attachment for sync") + if rollbackErr := h.service.RemoveUpload(context.WithoutCancel(r.Context()), attachmentID); rollbackErr != nil { + h.log.Error("Failed to roll back attachment after manifest error", "attachmentId", attachmentID, "error", rollbackErr) + } + SendErrorResponse(w, http.StatusInternalServerError, nil, "Failed to register attachment for sync") return } @@ -215,7 +256,8 @@ func (h *AttachmentHandler) DownloadAttachment(w http.ResponseWriter, r *http.Re // Set headers for file download w.Header().Set("Content-Type", "application/octet-stream") - w.Header().Set("Content-Disposition", "attachment; filename="+attachmentID) + disposition := mime.FormatMediaType("attachment", map[string]string{"filename": filepath.Base(attachmentID)}) + w.Header().Set("Content-Disposition", disposition) // Stream the file to the response _, err = io.Copy(w, file) @@ -264,15 +306,3 @@ func preferOriginalAttachment(r *http.Request) bool { return err == nil && parsed } } - -func readAllWithLimit(r io.Reader, maxBytes int64) ([]byte, error) { - limited := io.LimitReader(r, maxBytes+1) - data, err := io.ReadAll(limited) - if err != nil { - return nil, err - } - if int64(len(data)) > maxBytes { - return nil, errors.New("attachment too large") - } - return data, nil -} diff --git a/synkronus/internal/handlers/attachment_test.go b/synkronus/internal/handlers/attachment_test.go index ed77a0c0f..0e1f01692 100644 --- a/synkronus/internal/handlers/attachment_test.go +++ b/synkronus/internal/handlers/attachment_test.go @@ -13,8 +13,10 @@ import ( "github.com/go-chi/chi/v5" "github.com/opendataensemble/synkronus/internal/handlers/mocks" + "github.com/opendataensemble/synkronus/internal/models" "github.com/opendataensemble/synkronus/pkg/attachment" "github.com/opendataensemble/synkronus/pkg/logger" + authmw "github.com/opendataensemble/synkronus/pkg/middleware/auth" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" ) @@ -28,7 +30,11 @@ func (m *mockAttachmentService) Save(ctx context.Context, attachmentID string, f return args.Error(0) } -func (m *mockAttachmentService) SaveUpload(ctx context.Context, attachmentID string, data []byte, contentType string) (attachment.SaveUploadResult, error) { +func (m *mockAttachmentService) SaveUpload(ctx context.Context, attachmentID string, file io.Reader, contentType string) (attachment.SaveUploadResult, error) { + data, err := io.ReadAll(file) + if err != nil { + return attachment.SaveUploadResult{}, err + } args := m.Called(ctx, attachmentID, data, contentType) if args.Get(0) == nil { return attachment.SaveUploadResult{}, args.Error(1) @@ -36,6 +42,11 @@ func (m *mockAttachmentService) SaveUpload(ctx context.Context, attachmentID str return args.Get(0).(attachment.SaveUploadResult), args.Error(1) } +func (m *mockAttachmentService) RemoveUpload(ctx context.Context, attachmentID string) error { + args := m.Called(ctx, attachmentID) + return args.Error(0) +} + func (m *mockAttachmentService) Get(ctx context.Context, attachmentID string) (io.ReadCloser, error) { args := m.Called(ctx, attachmentID) if args.Get(0) == nil { @@ -152,6 +163,24 @@ func TestAttachmentHandler_UploadAttachment(t *testing.T) { } } +func TestAttachmentUploadRequiresWriteRole(t *testing.T) { + mockSvc := &mockAttachmentService{} + handler := NewAttachmentHandler(logger.NewLogger(), mockSvc, &mocks.MockAttachmentManifestService{}, mocks.NewMockSyncService()) + r := chi.NewRouter() + r.Use(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + user := &models.User{Username: "reader", Role: models.RoleReadOnly} + next.ServeHTTP(w, req.WithContext(context.WithValue(req.Context(), authmw.UserKey, user))) + }) + }) + handler.RegisterRoutes(r, func(http.ResponseWriter, *http.Request) {}) + + req := httptest.NewRequest(http.MethodPut, "/attachments/file.txt", nil) + resp := httptest.NewRecorder() + r.ServeHTTP(resp, req) + assert.Equal(t, http.StatusForbidden, resp.Code) +} + func TestAttachmentHandler_DownloadAttachment(t *testing.T) { tests := []struct { name string diff --git a/synkronus/internal/handlers/auth.go b/synkronus/internal/handlers/auth.go index 8057194a4..4d2ac343a 100644 --- a/synkronus/internal/handlers/auth.go +++ b/synkronus/internal/handlers/auth.go @@ -2,6 +2,8 @@ package handlers import ( "encoding/json" + "errors" + "io" "net/http" "time" ) @@ -23,10 +25,10 @@ type LoginResponse struct { func (h *Handler) Login(w http.ResponseWriter, r *http.Request) { var req LoginRequest - // Decode request body - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.log.Error("Failed to decode login request", "error", err) - SendErrorResponse(w, http.StatusBadRequest, err, "Invalid request format") + // Decode exactly one JSON request value. + if err := decodeAuthRequest(r, &req); err != nil { + h.log.Warn("Failed to decode login request", "error", err) + SendErrorResponse(w, http.StatusBadRequest, nil, "Invalid request format") return } @@ -46,8 +48,8 @@ func (h *Handler) Login(w http.ResponseWriter, r *http.Request) { // Authenticate user user, err := h.authService.Authenticate(r.Context(), req.Username, req.Password) if err != nil { - h.log.Error("Authentication failed", "username", req.Username, "error", err) - SendErrorResponse(w, http.StatusUnauthorized, err, "Invalid credentials") + h.log.Warn("Authentication failed", "error", err) + SendErrorResponse(w, http.StatusUnauthorized, nil, "Invalid credentials") return } @@ -55,7 +57,7 @@ func (h *Handler) Login(w http.ResponseWriter, r *http.Request) { token, err := h.authService.GenerateToken(user) if err != nil { h.log.Error("Failed to generate token", "error", err) - SendErrorResponse(w, http.StatusInternalServerError, err, "Failed to generate token") + SendErrorResponse(w, http.StatusInternalServerError, nil, "Failed to generate token") return } @@ -63,14 +65,14 @@ func (h *Handler) Login(w http.ResponseWriter, r *http.Request) { refreshToken, err := h.authService.GenerateRefreshToken(user) if err != nil { h.log.Error("Failed to generate refresh token", "error", err) - SendErrorResponse(w, http.StatusInternalServerError, err, "Failed to generate refresh token") + SendErrorResponse(w, http.StatusInternalServerError, nil, "Failed to generate refresh token") return } // Calculate token expiration expiresAt := time.Now().Add(h.authService.Config().TokenExpiration).Unix() - h.log.Info("User logged in successfully", "username", req.Username) + h.log.Info("User logged in successfully") // Send response SendJSONResponse(w, http.StatusOK, LoginResponse{ @@ -89,10 +91,10 @@ type RefreshRequest struct { func (h *Handler) RefreshToken(w http.ResponseWriter, r *http.Request) { var req RefreshRequest - // Decode request body - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - h.log.Error("Failed to decode refresh token request", "error", err) - SendErrorResponse(w, http.StatusBadRequest, err, "Invalid request format") + // Decode exactly one JSON request value. + if err := decodeAuthRequest(r, &req); err != nil { + h.log.Warn("Failed to decode refresh token request", "error", err) + SendErrorResponse(w, http.StatusBadRequest, nil, "Invalid request format") return } @@ -106,8 +108,8 @@ func (h *Handler) RefreshToken(w http.ResponseWriter, r *http.Request) { // Refresh token token, refreshToken, err := h.authService.RefreshToken(r.Context(), req.RefreshToken) if err != nil { - h.log.Error("Failed to refresh token", "error", err) - SendErrorResponse(w, http.StatusUnauthorized, err, "Invalid refresh token") + h.log.Warn("Failed to refresh token", "error", err) + SendErrorResponse(w, http.StatusUnauthorized, nil, "Invalid refresh token") return } @@ -123,3 +125,18 @@ func (h *Handler) RefreshToken(w http.ResponseWriter, r *http.Request) { ExpiresAt: expiresAt, }) } + +func decodeAuthRequest(r *http.Request, destination any) error { + decoder := json.NewDecoder(r.Body) + if err := decoder.Decode(destination); err != nil { + return err + } + var trailing any + if err := decoder.Decode(&trailing); !errors.Is(err, io.EOF) { + if err == nil { + return errors.New("multiple JSON values are not allowed") + } + return err + } + return nil +} diff --git a/synkronus/nginx.conf b/synkronus/nginx.conf index 681fa9aed..7e7778e3c 100644 --- a/synkronus/nginx.conf +++ b/synkronus/nginx.conf @@ -9,7 +9,8 @@ http { tcp_nodelay on; keepalive_timeout 65; types_hash_max_size 2048; - client_max_body_size 100M; # Allow large file uploads + # 128 MiB attachment content plus bounded multipart overhead. + client_max_body_size 130M; # Logging access_log /var/log/nginx/access.log; diff --git a/synkronus/openapi/synkronus.yaml b/synkronus/openapi/synkronus.yaml index cf6fca05a..73bd857d5 100644 --- a/synkronus/openapi/synkronus.yaml +++ b/synkronus/openapi/synkronus.yaml @@ -391,6 +391,24 @@ paths: application/problem+json: schema: $ref: '#/components/schemas/ProblemDetail' + '413': + description: Authentication request exceeds the configured size limit + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + '429': + description: Too many authentication attempts + headers: + Retry-After: + description: Seconds until the client should retry + schema: + type: integer + minimum: 1 + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' /api/auth/refresh: post: @@ -429,6 +447,24 @@ paths: application/problem+json: schema: $ref: '#/components/schemas/ProblemDetail' + '413': + description: Authentication request exceeds the configured size limit + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + '429': + description: Too many authentication attempts + headers: + Retry-After: + description: Seconds until the client should retry + schema: + type: integer + minimum: 1 + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' /api/users/create: post: @@ -1021,6 +1057,14 @@ paths: description: Bad request (missing or invalid file) '401': description: Unauthorized + '403': + description: Authenticated account does not have write access + '413': + description: Attachment or multipart request exceeds the configured upload limit + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' '409': description: Conflict — attachment already exists, or repository_generation mismatch (epoch; align before upload) content: diff --git a/synkronus/pkg/attachment/image_process.go b/synkronus/pkg/attachment/image_process.go index e6bf39a82..f0d902d25 100644 --- a/synkronus/pkg/attachment/image_process.go +++ b/synkronus/pkg/attachment/image_process.go @@ -17,6 +17,8 @@ type imageProcessOptions struct { MaxWidthPx int MaxHeightPx int ApplyExifOrientation bool + MaxDimensionPx int + MaxPixels int64 } type imageProcessResult struct { @@ -30,14 +32,23 @@ func processImageForStorage(raw []byte, opts imageProcessOptions) (imageProcessR return imageProcessResult{Processed: false}, nil } - img, format, err := image.Decode(bytes.NewReader(raw)) + imageConfig, format, err := image.DecodeConfig(bytes.NewReader(raw)) if err != nil { return imageProcessResult{Processed: false}, nil } if format != "jpeg" && format != "png" { return imageProcessResult{Processed: false}, nil } + if imageConfig.Width <= 0 || imageConfig.Height <= 0 || + (opts.MaxDimensionPx > 0 && (imageConfig.Width > opts.MaxDimensionPx || imageConfig.Height > opts.MaxDimensionPx)) || + (opts.MaxPixels > 0 && int64(imageConfig.Width)*int64(imageConfig.Height) > opts.MaxPixels) { + return imageProcessResult{}, ErrAttachmentTooLarge + } + img, format, err := image.Decode(bytes.NewReader(raw)) + if err != nil { + return imageProcessResult{Processed: false}, nil + } processed := img didTransform := false diff --git a/synkronus/pkg/attachment/image_process_test.go b/synkronus/pkg/attachment/image_process_test.go index e8fd4db22..21a4695a7 100644 --- a/synkronus/pkg/attachment/image_process_test.go +++ b/synkronus/pkg/attachment/image_process_test.go @@ -7,6 +7,8 @@ import ( "image/jpeg" "image/png" "testing" + + "errors" ) func TestJpegQualityForLevel(t *testing.T) { @@ -70,6 +72,18 @@ func TestProcessImageForStorage_MaxBoxDownscale(t *testing.T) { } } +func TestProcessImageForStorageRejectsExcessiveDecodedDimensions(t *testing.T) { + raw := mustEncodePNG(t, makeNoisyImage(10, 10)) + _, err := processImageForStorage(raw, imageProcessOptions{ + CompressionLevel: 1, + MaxDimensionPx: 5, + MaxPixels: 25, + }) + if !errors.Is(err, ErrAttachmentTooLarge) { + t.Fatalf("expected decoded image limit error, got %v", err) + } +} + func TestApplyExifOrientation_Rotate90CW(t *testing.T) { src := image.NewRGBA(image.Rect(0, 0, 2, 3)) out, changed := applyExifOrientation(src, 6) diff --git a/synkronus/pkg/attachment/service.go b/synkronus/pkg/attachment/service.go index 96028a10d..d0f9e17a7 100644 --- a/synkronus/pkg/attachment/service.go +++ b/synkronus/pkg/attachment/service.go @@ -2,7 +2,10 @@ package attachment import ( "archive/zip" + "bufio" + "bytes" "context" + "errors" "fmt" "io" "net/http" @@ -17,8 +20,11 @@ type Service interface { // Save stores the attachment with the given ID Save(ctx context.Context, attachmentID string, file io.Reader) error - // SaveUpload stores attachment bytes and optionally performs image processing. - SaveUpload(ctx context.Context, attachmentID string, data []byte, contentType string) (SaveUploadResult, error) + // SaveUpload streams an attachment and optionally performs bounded image processing. + SaveUpload(ctx context.Context, attachmentID string, file io.Reader, contentType string) (SaveUploadResult, error) + + // RemoveUpload rolls back a newly stored attachment and any retained original. + RemoveUpload(ctx context.Context, attachmentID string) error // Get retrieves the attachment with the given ID Get(ctx context.Context, attachmentID string) (io.ReadCloser, error) @@ -41,9 +47,17 @@ type SaveUploadResult struct { ServedContentType string } +var ErrAttachmentTooLarge = errors.New("attachment too large") + +const maxAttachmentIDBytes = 512 + type service struct { storagePath string originalsPath string + maxUploadBytes int64 + imageSemaphore chan struct{} + maxDecodedImageDimension int + maxDecodedImagePixels int64 imageCompressionLevel int imageMaxWidthPx int imageMaxHeightPx int @@ -61,9 +75,30 @@ func NewService(cfg *config.Config) (Service, error) { return nil, err } + maxUploadBytes := cfg.MaxAttachmentUploadBytes + if maxUploadBytes <= 0 { + maxUploadBytes = config.DefaultMaxAttachmentUploadBytes + } + maxImageWorkers := cfg.MaxConcurrentImageProcessing + if maxImageWorkers <= 0 { + maxImageWorkers = config.DefaultMaxConcurrentImageProcessing + } + maxDimension := cfg.MaxDecodedImageDimensionPx + if maxDimension <= 0 { + maxDimension = config.DefaultMaxDecodedImageDimensionPx + } + maxPixels := cfg.MaxDecodedImagePixels + if maxPixels <= 0 { + maxPixels = config.DefaultMaxDecodedImagePixels + } + return &service{ storagePath: storagePath, originalsPath: originalsPath, + maxUploadBytes: maxUploadBytes, + imageSemaphore: make(chan struct{}, maxImageWorkers), + maxDecodedImageDimension: maxDimension, + maxDecodedImagePixels: maxPixels, imageCompressionLevel: cfg.ImageCompressionLevel, imageMaxWidthPx: cfg.ImageMaxWidthPx, imageMaxHeightPx: cfg.ImageMaxHeightPx, @@ -71,30 +106,41 @@ func NewService(cfg *config.Config) (Service, error) { }, nil } -func (s *service) getAttachmentPath(attachmentID string) (string, error) { - // Basic path traversal protection - if filepath.IsAbs(attachmentID) || filepath.VolumeName(attachmentID) != "" { - return "", os.ErrInvalid +func ValidateAttachmentID(attachmentID string) error { + if attachmentID == "" || len(attachmentID) > maxAttachmentIDBytes || filepath.IsAbs(attachmentID) || filepath.VolumeName(attachmentID) != "" || strings.Contains(attachmentID, `\\`) { + return os.ErrInvalid + } + for _, segment := range strings.Split(attachmentID, "/") { + if segment == "" || segment == "." || segment == ".." { + return os.ErrInvalid + } + for _, r := range segment { + if !(r >= 'a' && r <= 'z') && !(r >= 'A' && r <= 'Z') && !(r >= '0' && r <= '9') && r != '.' && r != '_' && r != '-' { + return os.ErrInvalid + } + } } + return nil +} - // Clean the path to prevent directory traversal - cleanPath := filepath.Clean(attachmentID) - if cleanPath == "." || cleanPath == ".." { +func containedPath(root, attachmentID string) (string, error) { + if err := ValidateAttachmentID(attachmentID); err != nil { + return "", err + } + path := filepath.Join(root, filepath.FromSlash(attachmentID)) + rel, err := filepath.Rel(root, path) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { return "", os.ErrInvalid } + return path, nil +} - return filepath.Join(s.storagePath, cleanPath), nil +func (s *service) getAttachmentPath(attachmentID string) (string, error) { + return containedPath(s.storagePath, attachmentID) } func (s *service) getOriginalAttachmentPath(attachmentID string) (string, error) { - if filepath.IsAbs(attachmentID) || filepath.VolumeName(attachmentID) != "" { - return "", os.ErrInvalid - } - cleanPath := filepath.Clean(attachmentID) - if cleanPath == "." || cleanPath == ".." { - return "", os.ErrInvalid - } - return filepath.Join(s.originalsPath, cleanPath), nil + return containedPath(s.originalsPath, attachmentID) } func (s *service) Save(ctx context.Context, attachmentID string, file io.Reader) error { @@ -121,7 +167,7 @@ func (s *service) Save(ctx context.Context, attachmentID string, file io.Reader) return writeReaderAtomic(path, file) } -func (s *service) SaveUpload(ctx context.Context, attachmentID string, data []byte, contentType string) (SaveUploadResult, error) { +func (s *service) SaveUpload(ctx context.Context, attachmentID string, file io.Reader, contentType string) (SaveUploadResult, error) { if err := ctx.Err(); err != nil { return SaveUploadResult{}, err } @@ -140,15 +186,37 @@ func (s *service) SaveUpload(ctx context.Context, attachmentID string, data []by return SaveUploadResult{}, err } + buffered := bufio.NewReader(file) + sniff, _ := buffered.Peek(512) + processedType := normalizeContentType(contentType, sniff) + if !strings.HasPrefix(processedType, "image/jpeg") && !strings.HasPrefix(processedType, "image/png") { + size, err := writeReaderAtomicNoReplace(path, io.LimitReader(buffered, s.maxUploadBytes+1), s.maxUploadBytes) + if err != nil { + return SaveUploadResult{}, err + } + return SaveUploadResult{ServedSize: int(size), ServedContentType: processedType}, nil + } + + select { + case s.imageSemaphore <- struct{}{}: + defer func() { <-s.imageSemaphore }() + case <-ctx.Done(): + return SaveUploadResult{}, ctx.Err() + } + + data, err := readAllLimited(buffered, s.maxUploadBytes) + if err != nil { + return SaveUploadResult{}, err + } processed := data - processedType := normalizeContentType(contentType, data) shouldPersistOriginal := false - result, err := processImageForStorage(data, imageProcessOptions{ CompressionLevel: s.imageCompressionLevel, MaxWidthPx: s.imageMaxWidthPx, MaxHeightPx: s.imageMaxHeightPx, ApplyExifOrientation: s.imageApplyExifOrientation, + MaxDimensionPx: s.maxDecodedImageDimension, + MaxPixels: s.maxDecodedImagePixels, }) if err != nil { return SaveUploadResult{}, err @@ -167,15 +235,15 @@ func (s *service) SaveUpload(ctx context.Context, attachmentID string, data []by } else if !os.IsNotExist(err) { return SaveUploadResult{}, err } - if err := writeFileAtomic(originalPath, data); err != nil { + if err := writeFileAtomicNoReplace(originalPath, data); err != nil { return SaveUploadResult{}, err } - if err := writeFileAtomic(path, processed); err != nil { + if err := writeFileAtomicNoReplace(path, processed); err != nil { _ = os.Remove(originalPath) return SaveUploadResult{}, err } } else { - if err := writeFileAtomic(path, processed); err != nil { + if err := writeFileAtomicNoReplace(path, processed); err != nil { return SaveUploadResult{}, err } } @@ -186,6 +254,27 @@ func (s *service) SaveUpload(ctx context.Context, attachmentID string, data []by }, nil } +func (s *service) RemoveUpload(ctx context.Context, attachmentID string) error { + if err := ctx.Err(); err != nil { + return err + } + path, err := s.getAttachmentPath(attachmentID) + if err != nil { + return err + } + originalPath, err := s.getOriginalAttachmentPath(attachmentID) + if err != nil { + return err + } + var removalError error + for _, candidate := range []string{path, originalPath} { + if err := os.Remove(candidate); err != nil && !os.IsNotExist(err) { + removalError = errors.Join(removalError, err) + } + } + return removalError +} + func (s *service) Get(ctx context.Context, attachmentID string) (io.ReadCloser, error) { path, err := s.getAttachmentPath(attachmentID) if err != nil { @@ -299,72 +388,70 @@ func normalizeContentType(declared string, data []byte) string { return http.DetectContentType(data) } -func writeFileAtomic(path string, data []byte) error { - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { - return err - } - - tmpFile, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".*.tmp") +func readAllLimited(r io.Reader, maxBytes int64) ([]byte, error) { + data, err := io.ReadAll(io.LimitReader(r, maxBytes+1)) if err != nil { - return err + return nil, err } - tmpPath := tmpFile.Name() - - cleanup := func() { - _ = tmpFile.Close() - _ = os.Remove(tmpPath) + if int64(len(data)) > maxBytes { + return nil, ErrAttachmentTooLarge } + return data, nil +} - if _, err := tmpFile.Write(data); err != nil { - cleanup() - return err - } - if err := tmpFile.Sync(); err != nil { - cleanup() - return err - } - if err := tmpFile.Close(); err != nil { - _ = os.Remove(tmpPath) - return err - } - if err := os.Rename(tmpPath, path); err != nil { - _ = os.Remove(tmpPath) - return err - } - return nil +func writeFileAtomicNoReplace(path string, data []byte) error { + _, err := writeReaderAtomicNoReplace(path, bytes.NewReader(data), int64(len(data))) + return err } func writeReaderAtomic(path string, r io.Reader) error { + _, err := writeReaderAtomicNoReplace(path, r, -1) + return err +} + +func writeReaderAtomicNoReplace(path string, r io.Reader, maxBytes int64) (int64, error) { if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { - return err + return 0, err } - tmpFile, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+".*.tmp") if err != nil { - return err + return 0, err } tmpPath := tmpFile.Name() - cleanup := func() { _ = tmpFile.Close() _ = os.Remove(tmpPath) } - if _, err := io.Copy(tmpFile, r); err != nil { + reader := r + if maxBytes >= 0 { + reader = io.LimitReader(r, maxBytes+1) + } + written, err := io.Copy(tmpFile, reader) + if err != nil { cleanup() - return err + return 0, err + } + if maxBytes >= 0 && written > maxBytes { + cleanup() + return 0, ErrAttachmentTooLarge } if err := tmpFile.Sync(); err != nil { cleanup() - return err + return 0, err } if err := tmpFile.Close(); err != nil { _ = os.Remove(tmpPath) - return err + return 0, err } - if err := os.Rename(tmpPath, path); err != nil { + // Linking within the destination directory atomically fails if path exists; + // unlike Rename, it cannot replace a concurrent upload. + if err := os.Link(tmpPath, path); err != nil { _ = os.Remove(tmpPath) - return err + return 0, err } - return nil + if err := os.Remove(tmpPath); err != nil { + return 0, err + } + return written, nil } diff --git a/synkronus/pkg/attachment/service_test.go b/synkronus/pkg/attachment/service_test.go index b2d749c58..421443c09 100644 --- a/synkronus/pkg/attachment/service_test.go +++ b/synkronus/pkg/attachment/service_test.go @@ -4,6 +4,7 @@ import ( "archive/zip" "bytes" "context" + "errors" "image" "image/color" "image/jpeg" @@ -15,6 +16,54 @@ import ( "github.com/opendataensemble/synkronus/pkg/config" ) +func TestValidateAttachmentID(t *testing.T) { + valid := []string{"file.txt", "photos/cam-01_2.jpg"} + for _, id := range valid { + if err := ValidateAttachmentID(id); err != nil { + t.Fatalf("valid ID %q rejected: %v", id, err) + } + } + invalid := []string{"", "../x", "a/../x", "/absolute", `a\\b`, "a b", "a//b", "a\nb"} + for _, id := range invalid { + if err := ValidateAttachmentID(id); err == nil { + t.Fatalf("invalid ID %q accepted", id) + } + } +} + +func TestService_SaveDoesNotReplaceConcurrentDestination(t *testing.T) { + dir := t.TempDir() + svc, err := NewService(&config.Config{DataDir: dir}) + if err != nil { + t.Fatal(err) + } + if err := svc.Save(context.Background(), "same.txt", bytes.NewReader([]byte("first"))); err != nil { + t.Fatal(err) + } + if err := svc.Save(context.Background(), "same.txt", bytes.NewReader([]byte("second"))); !os.IsExist(err) { + t.Fatalf("expected existence error, got %v", err) + } + body, err := os.ReadFile(filepath.Join(dir, "attachments", "same.txt")) + if err != nil { + t.Fatal(err) + } + if string(body) != "first" { + t.Fatalf("destination replaced: %q", body) + } +} + +func TestService_SaveUploadEnforcesConfiguredSize(t *testing.T) { + dir := t.TempDir() + svc, err := NewService(&config.Config{DataDir: dir, MaxAttachmentUploadBytes: 8}) + if err != nil { + t.Fatal(err) + } + _, err = svc.SaveUpload(context.Background(), "video.bin", bytes.NewReader([]byte("123456789")), "application/octet-stream") + if !errors.Is(err, ErrAttachmentTooLarge) { + t.Fatalf("expected size error, got %v", err) + } +} + func TestService_WriteZip(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir} @@ -89,7 +138,7 @@ func TestService_SaveUpload_CompressedStoresOriginalAndExportUsesOriginal(t *tes } raw := mustEncodeTestJPEG(t, makeTestImage(320, 240), 95) - result, err := svc.SaveUpload(context.Background(), "photos/cam.jpg", raw, "image/jpeg") + result, err := svc.SaveUpload(context.Background(), "photos/cam.jpg", bytes.NewReader(raw), "image/jpeg") if err != nil { t.Fatal(err) } @@ -143,7 +192,7 @@ func TestService_SaveUpload_FallbackWithoutOriginal(t *testing.T) { } data := []byte("plain data") - result, err := svc.SaveUpload(context.Background(), "notes/a.txt", data, "text/plain") + result, err := svc.SaveUpload(context.Background(), "notes/a.txt", bytes.NewReader(data), "text/plain") if err != nil { t.Fatal(err) } diff --git a/synkronus/pkg/auth/auth.go b/synkronus/pkg/auth/auth.go index 83786b6af..3979040ae 100644 --- a/synkronus/pkg/auth/auth.go +++ b/synkronus/pkg/auth/auth.go @@ -18,6 +18,8 @@ import ( type Config struct { // JWTSecret is the secret key used to sign JWT tokens JWTSecret string + // AcceptLegacyUntypedTokens keeps tokens issued before token_use was added valid during migration. + AcceptLegacyUntypedTokens bool // TokenExpiration is the duration for which a token is valid TokenExpiration time.Duration // RefreshTokenExpiration is the duration for which a refresh token is valid @@ -35,20 +37,31 @@ type Config struct { // DefaultConfig returns a default configuration func DefaultConfig() Config { return Config{ - JWTSecret: "change-me-in-production", - TokenExpiration: time.Hour * 24, - RefreshTokenExpiration: time.Hour * 24 * 7, - AdminUsername: "admin", - AdminPassword: "admin", - ForceCreateAdminUser: "", - ForceCreateAdminPassword: "", + JWTSecret: "change-me-in-production", + AcceptLegacyUntypedTokens: true, + TokenExpiration: time.Hour * 24, + RefreshTokenExpiration: time.Hour * 24 * 7, + AdminUsername: "admin", + AdminPassword: "admin", + ForceCreateAdminUser: "", + ForceCreateAdminPassword: "", } } // AuthClaims represents the JWT claims +type TokenUse string + +const ( + TokenUseAccess TokenUse = "access" + TokenUseRefresh TokenUse = "refresh" + tokenIssuer = "synkronus" +) + +// AuthClaims represents the JWT claims. type AuthClaims struct { Username string `json:"username"` Role models.Role `json:"role"` + TokenUse TokenUse `json:"token_use,omitempty"` jwt.RegisteredClaims } @@ -146,7 +159,9 @@ func (s *Service) CheckPasswordHash(password, hash string) bool { return s.VerifyPassword(password, hash) } -// Authenticate verifies user credentials and returns a user if valid +const dummyPasswordHash = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy" + +// Authenticate verifies user credentials and returns a user if valid. func (s *Service) Authenticate(ctx context.Context, username, password string) (*models.User, error) { user, err := s.userRepository.GetByUsername(ctx, username) if err != nil { @@ -154,6 +169,8 @@ func (s *Service) Authenticate(ctx context.Context, username, password string) ( } if user == nil { + // Keep nonexistent-user and wrong-password paths comparable to reduce timing enumeration. + _ = bcrypt.CompareHashAndPassword([]byte(dummyPasswordHash), []byte(password)) return nil, errors.New("invalid credentials") } @@ -171,9 +188,11 @@ func (s *Service) GenerateToken(user *models.User) (string, error) { claims := &AuthClaims{ Username: user.Username, Role: user.Role, + TokenUse: TokenUseAccess, RegisteredClaims: jwt.RegisteredClaims{ ExpiresAt: jwt.NewNumericDate(expirationTime), IssuedAt: jwt.NewNumericDate(time.Now()), + Issuer: tokenIssuer, Subject: user.ID.String(), }, } @@ -194,10 +213,12 @@ func (s *Service) GenerateRefreshToken(user *models.User) (string, error) { claims := &AuthClaims{ Username: user.Username, - Role: user.Role, // Include role in refresh token as well + Role: user.Role, + TokenUse: TokenUseRefresh, RegisteredClaims: jwt.RegisteredClaims{ ExpiresAt: jwt.NewNumericDate(expirationTime), IssuedAt: jwt.NewNumericDate(time.Now()), + Issuer: tokenIssuer, Subject: user.ID.String(), }, } @@ -212,32 +233,46 @@ func (s *Service) GenerateRefreshToken(user *models.User) (string, error) { return tokenString, nil } -// ValidateToken validates a JWT token and returns the claims +// ValidateToken validates an access token and returns the claims. func (s *Service) ValidateToken(tokenString string) (*AuthClaims, error) { - claims := &AuthClaims{} - - token, err := jwt.ParseWithClaims(tokenString, claims, func(token *jwt.Token) (any, error) { - if _, ok := token.Method.(*jwt.SigningMethodHMAC); !ok { - return nil, fmt.Errorf("unexpected signing method: %v", token.Header["alg"]) - } - return []byte(s.config.JWTSecret), nil - }) + return s.validateTokenForUse(tokenString, TokenUseAccess) +} +func (s *Service) validateTokenForUse(tokenString string, expectedUse TokenUse) (*AuthClaims, error) { + claims := &AuthClaims{} + token, err := jwt.ParseWithClaims( + tokenString, + claims, + func(token *jwt.Token) (any, error) { return []byte(s.config.JWTSecret), nil }, + jwt.WithValidMethods([]string{jwt.SigningMethodHS256.Alg()}), + jwt.WithExpirationRequired(), + ) if err != nil { return nil, fmt.Errorf("failed to parse token: %w", err) } - if !token.Valid { return nil, errors.New("invalid token") } + if claims.TokenUse == "" { + if !s.config.AcceptLegacyUntypedTokens { + return nil, errors.New("legacy untyped token is no longer accepted") + } + return claims, nil + } + if claims.Issuer != tokenIssuer { + return nil, errors.New("invalid token issuer") + } + if claims.TokenUse != expectedUse { + return nil, fmt.Errorf("invalid token use: expected %s", expectedUse) + } return claims, nil } // RefreshToken validates a refresh token and generates a new access token func (s *Service) RefreshToken(ctx context.Context, refreshToken string) (string, string, error) { - // Validate the refresh token - claims, err := s.ValidateToken(refreshToken) + // Validate the refresh token in the refresh-token context. + claims, err := s.validateTokenForUse(refreshToken, TokenUseRefresh) if err != nil { return "", "", fmt.Errorf("invalid refresh token: %w", err) } diff --git a/synkronus/pkg/auth/auth_test.go b/synkronus/pkg/auth/auth_test.go index b717731c4..a4d4ee73e 100644 --- a/synkronus/pkg/auth/auth_test.go +++ b/synkronus/pkg/auth/auth_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" "github.com/opendataensemble/synkronus/internal/models" "github.com/opendataensemble/synkronus/internal/repository/mocks" @@ -36,6 +37,11 @@ func setupTestService() (*Service, *mocks.MockUserRepository) { return service, mockRepo } +func TestDummyPasswordHashIsValid(t *testing.T) { + _, err := bcrypt.Cost([]byte(dummyPasswordHash)) + require.NoError(t, err) +} + func TestAuthenticate(t *testing.T) { // Setup service, _ := setupTestService() @@ -127,6 +133,11 @@ func TestRefreshToken(t *testing.T) { err := mockRepo.Create(ctx, user) require.NoError(t, err) + accessToken, err := service.GenerateToken(user) + require.NoError(t, err) + _, _, err = service.RefreshToken(ctx, accessToken) + assert.Error(t, err, "an access token must never be accepted by the refresh endpoint") + // Generate a valid refresh token refreshToken, err := service.GenerateRefreshToken(user) require.NoError(t, err) @@ -185,12 +196,60 @@ func TestValidateToken(t *testing.T) { assert.Equal(t, user.Username, claims.Username) assert.Equal(t, user.Role, claims.Role) assert.Equal(t, user.ID.String(), claims.Subject) + assert.Equal(t, TokenUseAccess, claims.TokenUse) + assert.Equal(t, tokenIssuer, claims.Issuer) + + refreshToken, err := service.GenerateRefreshToken(user) + require.NoError(t, err) + _, err = service.ValidateToken(refreshToken) + assert.Error(t, err, "a refresh token must never authorize a protected endpoint") // Test invalid token _, err = service.ValidateToken("invalid-token") assert.Error(t, err) } +func TestLegacyUntypedTokenCompatibility(t *testing.T) { + service, _ := setupTestService() + now := time.Now() + claims := &AuthClaims{ + Username: "legacy-user", + Role: models.RoleReadOnly, + RegisteredClaims: jwt.RegisteredClaims{ + Subject: uuid.NewString(), + IssuedAt: jwt.NewNumericDate(now), + ExpiresAt: jwt.NewNumericDate(now.Add(time.Hour)), + }, + } + legacy, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString([]byte(service.config.JWTSecret)) + require.NoError(t, err) + + service.config.AcceptLegacyUntypedTokens = true + _, err = service.ValidateToken(legacy) + assert.NoError(t, err) + + service.config.AcceptLegacyUntypedTokens = false + _, err = service.ValidateToken(legacy) + assert.Error(t, err) +} + +func TestRejectsUnexpectedHMACAlgorithm(t *testing.T) { + service, _ := setupTestService() + claims := &AuthClaims{ + Username: "user", + Role: models.RoleReadOnly, + TokenUse: TokenUseAccess, + RegisteredClaims: jwt.RegisteredClaims{ + Issuer: tokenIssuer, + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }, + } + token, err := jwt.NewWithClaims(jwt.SigningMethodHS512, claims).SignedString([]byte(service.config.JWTSecret)) + require.NoError(t, err) + _, err = service.ValidateToken(token) + assert.Error(t, err) +} + func TestInitialize(t *testing.T) { // Setup - use a fresh repository with no users mockRepo := mocks.NewMockUserRepository() diff --git a/synkronus/pkg/authlimit/authlimit.go b/synkronus/pkg/authlimit/authlimit.go new file mode 100644 index 000000000..22053e875 --- /dev/null +++ b/synkronus/pkg/authlimit/authlimit.go @@ -0,0 +1,288 @@ +package authlimit + +import ( + "bytes" + "context" + "crypto/hmac" + "crypto/rand" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "io" + "net" + "net/http" + "strconv" + "strings" + "sync" + "time" +) + +const ( + EndpointLogin = "login" + EndpointRefresh = "refresh" +) + +type Config struct { + MaxBodyBytes int64 + MaxUsernameBytes int + MaxPasswordBytes int + MaxTokenBytes int + IPAttempts int + IPWindow time.Duration + LoginAttempts int + LoginWindow time.Duration + AccountAttempts int + AccountWindow time.Duration + MaxKeys int + TrustedProxyCIDRs []string +} + +type bucket struct { + count int + reset time.Time +} + +type windowLimiter struct { + mu sync.Mutex + buckets map[string]bucket + limit int + window time.Duration + maxKeys int + now func() time.Time +} + +func newWindowLimiter(limit int, window time.Duration, maxKeys int) *windowLimiter { + return &windowLimiter{buckets: make(map[string]bucket), limit: limit, window: window, maxKeys: maxKeys, now: time.Now} +} + +func (l *windowLimiter) available(key string) (bool, time.Duration) { + if l.limit <= 0 { + return true, 0 + } + now := l.now() + l.mu.Lock() + defer l.mu.Unlock() + l.cleanupExpired(now) + b, ok := l.buckets[key] + if !ok { + return true, 0 + } + if b.count >= l.limit { + return false, time.Until(b.reset) + } + return true, 0 +} + +func (l *windowLimiter) take(key string) (bool, time.Duration) { + if l.limit <= 0 { + return true, 0 + } + now := l.now() + l.mu.Lock() + defer l.mu.Unlock() + l.cleanupExpired(now) + b, ok := l.buckets[key] + if !ok { + if l.maxKeys > 0 && len(l.buckets) >= l.maxKeys { + return false, l.window + } + l.buckets[key] = bucket{count: 1, reset: now.Add(l.window)} + return true, 0 + } + if b.count >= l.limit { + return false, b.reset.Sub(now) + } + b.count++ + l.buckets[key] = b + return true, 0 +} + +func (l *windowLimiter) record(key string) { + _, _ = l.take(key) +} + +func (l *windowLimiter) cleanupExpired(now time.Time) { + for key, b := range l.buckets { + if !now.Before(b.reset) { + delete(l.buckets, key) + } + } +} + +type peerContextKey struct{} + +// CapturePeer stores the socket peer before any middleware interprets forwarding headers. +func CapturePeer(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), peerContextKey{}, parseRemoteIP(r.RemoteAddr)) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + +type Guard struct { + config Config + ipLimiter *windowLimiter + loginLimiter *windowLimiter + acctLimiter *windowLimiter + trusted []*net.IPNet + hmacKey []byte +} + +func New(config Config) (*Guard, error) { + trusted := make([]*net.IPNet, 0, len(config.TrustedProxyCIDRs)) + for _, raw := range config.TrustedProxyCIDRs { + raw = strings.TrimSpace(raw) + if raw == "" { + continue + } + _, network, err := net.ParseCIDR(raw) + if err != nil { + return nil, err + } + trusted = append(trusted, network) + } + key := make([]byte, 32) + if _, err := rand.Read(key); err != nil { + return nil, err + } + return &Guard{ + config: config, + ipLimiter: newWindowLimiter(config.IPAttempts, config.IPWindow, config.MaxKeys), + loginLimiter: newWindowLimiter(config.LoginAttempts, config.LoginWindow, config.MaxKeys), + acctLimiter: newWindowLimiter(config.AccountAttempts, config.AccountWindow, config.MaxKeys), + trusted: trusted, + hmacKey: key, + }, nil +} + +type authInput struct { + Username string `json:"username"` + Password string `json:"password"` + RefreshToken string `json:"refreshToken"` +} + +func (g *Guard) Middleware(endpoint string) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ip := g.clientIP(r) + if ok, retry := g.ipLimiter.take("ip:" + ip); !ok { + writeRateLimited(w, retry) + return + } + + body, err := readBoundedBody(w, r, g.config.MaxBodyBytes) + if err != nil { + var maxBytesError *http.MaxBytesError + if errors.As(err, &maxBytesError) { + writeJSONError(w, http.StatusRequestEntityTooLarge, "request_too_large", "Authentication request is too large") + return + } + writeJSONError(w, http.StatusBadRequest, "invalid_request", "Invalid request format") + return + } + r.Body = io.NopCloser(bytes.NewReader(body)) + + var input authInput + _ = json.Unmarshal(body, &input) // The handler remains authoritative for JSON validation. + if len(input.Username) > g.config.MaxUsernameBytes || len(input.Password) > g.config.MaxPasswordBytes || len(input.RefreshToken) > g.config.MaxTokenBytes { + writeJSONError(w, http.StatusBadRequest, "invalid_request", "Authentication field is too long") + return + } + + var sourceIdentityKey, accountKey string + if endpoint == EndpointLogin && input.Username != "" { + identity := g.digest("username:" + input.Username) + sourceIdentityKey = "login-source:" + ip + ":" + identity + accountKey = "login-account:" + identity + if ok, retry := g.loginLimiter.available(sourceIdentityKey); !ok { + writeRateLimited(w, retry) + return + } + if ok, retry := g.acctLimiter.available(accountKey); !ok { + writeRateLimited(w, retry) + return + } + } + + recorder := &statusRecorder{ResponseWriter: w, status: http.StatusOK} + next.ServeHTTP(recorder, r) + if recorder.status == http.StatusUnauthorized && sourceIdentityKey != "" { + g.loginLimiter.record(sourceIdentityKey) + g.acctLimiter.record(accountKey) + } + }) + } +} + +func (g *Guard) digest(value string) string { + mac := hmac.New(sha256.New, g.hmacKey) + _, _ = mac.Write([]byte(value)) + return hex.EncodeToString(mac.Sum(nil)) +} + +func (g *Guard) clientIP(r *http.Request) string { + peer, _ := r.Context().Value(peerContextKey{}).(net.IP) + if peer == nil { + peer = parseRemoteIP(r.RemoteAddr) + } + if g.isTrusted(peer) { + if forwarded := net.ParseIP(strings.TrimSpace(r.Header.Get("X-Real-IP"))); forwarded != nil { + return forwarded.String() + } + } + if peer == nil { + return "unknown" + } + return peer.String() +} + +func (g *Guard) isTrusted(ip net.IP) bool { + for _, network := range g.trusted { + if network.Contains(ip) { + return true + } + } + return false +} + +func parseRemoteIP(remote string) net.IP { + host, _, err := net.SplitHostPort(remote) + if err == nil { + return net.ParseIP(strings.Trim(host, "[]")) + } + return net.ParseIP(strings.Trim(remote, "[]")) +} + +func readBoundedBody(w http.ResponseWriter, r *http.Request, max int64) ([]byte, error) { + if max <= 0 { + max = 16 << 10 + } + r.Body = http.MaxBytesReader(w, r.Body, max) + return io.ReadAll(r.Body) +} + +type statusRecorder struct { + http.ResponseWriter + status int +} + +func (r *statusRecorder) WriteHeader(status int) { + r.status = status + r.ResponseWriter.WriteHeader(status) +} + +func writeRateLimited(w http.ResponseWriter, retry time.Duration) { + seconds := int64((retry + time.Second - 1) / time.Second) + if seconds < 1 { + seconds = 1 + } + w.Header().Set("Retry-After", strconv.FormatInt(seconds, 10)) + writeJSONError(w, http.StatusTooManyRequests, "rate_limited", "Too many authentication attempts; try again later") +} + +func writeJSONError(w http.ResponseWriter, status int, code, message string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(map[string]string{"error": code, "message": message}) +} diff --git a/synkronus/pkg/authlimit/authlimit_test.go b/synkronus/pkg/authlimit/authlimit_test.go new file mode 100644 index 000000000..842c37521 --- /dev/null +++ b/synkronus/pkg/authlimit/authlimit_test.go @@ -0,0 +1,112 @@ +package authlimit + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +func testGuard(t *testing.T) *Guard { + t.Helper() + guard, err := New(Config{ + MaxBodyBytes: 128, + MaxUsernameBytes: 32, + MaxPasswordBytes: 64, + MaxTokenBytes: 64, + IPAttempts: 10, + IPWindow: time.Minute, + LoginAttempts: 2, + LoginWindow: time.Minute, + AccountAttempts: 5, + AccountWindow: time.Minute, + MaxKeys: 100, + TrustedProxyCIDRs: []string{"10.0.0.0/8"}, + }) + if err != nil { + t.Fatal(err) + } + return guard +} + +func TestLoginFailuresAreRateLimited(t *testing.T) { + guard := testGuard(t) + handler := CapturePeer(guard.Middleware(EndpointLogin)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }))) + + for i := 0; i < 2; i++ { + req := httptest.NewRequest(http.MethodPost, "/api/auth/login", strings.NewReader(`{"username":"user","password":"bad"}`)) + req.RemoteAddr = "192.0.2.1:1234" + resp := httptest.NewRecorder() + handler.ServeHTTP(resp, req) + if resp.Code != http.StatusUnauthorized { + t.Fatalf("attempt %d: got %d", i, resp.Code) + } + } + + req := httptest.NewRequest(http.MethodPost, "/api/auth/login", strings.NewReader(`{"username":"user","password":"bad"}`)) + req.RemoteAddr = "192.0.2.1:1234" + resp := httptest.NewRecorder() + handler.ServeHTTP(resp, req) + if resp.Code != http.StatusTooManyRequests { + t.Fatalf("got %d, want 429", resp.Code) + } + if resp.Header().Get("Retry-After") == "" { + t.Fatal("missing Retry-After") + } +} + +func TestUntrustedForwardedIPIsIgnored(t *testing.T) { + guard := testGuard(t) + seen := "" + handler := CapturePeer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + seen = guard.clientIP(r) + })) + req := httptest.NewRequest(http.MethodPost, "/", nil) + req.RemoteAddr = "192.0.2.10:1234" + req.Header.Set("X-Real-IP", "203.0.113.4") + handler.ServeHTTP(httptest.NewRecorder(), req) + if seen != "192.0.2.10" { + t.Fatalf("got %q", seen) + } +} + +func TestTrustedProxyUsesValidatedRealIP(t *testing.T) { + guard := testGuard(t) + req := httptest.NewRequest(http.MethodPost, "/", nil) + req.RemoteAddr = "10.0.0.2:1234" + req.Header.Set("X-Real-IP", "203.0.113.4") + CapturePeer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + if got := guard.clientIP(r); got != "203.0.113.4" { + t.Fatalf("got %q", got) + } + })).ServeHTTP(httptest.NewRecorder(), req) +} + +func TestOversizedAuthBodyIsRejected(t *testing.T) { + guard := testGuard(t) + handler := CapturePeer(guard.Middleware(EndpointLogin)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + }))) + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(strings.Repeat("x", 129))) + resp := httptest.NewRecorder() + handler.ServeHTTP(resp, req) + if resp.Code != http.StatusRequestEntityTooLarge { + t.Fatalf("got %d", resp.Code) + } +} + +func TestLimiterBoundsKeyCount(t *testing.T) { + limiter := newWindowLimiter(1, time.Minute, 2) + if ok, _ := limiter.take("one"); !ok { + t.Fatal("first key rejected") + } + if ok, _ := limiter.take("two"); !ok { + t.Fatal("second key rejected") + } + if ok, _ := limiter.take("three"); ok { + t.Fatal("expected new key to be rejected at capacity") + } +} diff --git a/synkronus/pkg/config/config.go b/synkronus/pkg/config/config.go index 4ccd284d0..73e83cbfe 100644 --- a/synkronus/pkg/config/config.go +++ b/synkronus/pkg/config/config.go @@ -5,12 +5,29 @@ import ( "os" "path/filepath" "strconv" + "strings" "github.com/joho/godotenv" "github.com/opendataensemble/synkronus/pkg/logger" ) // Config holds all configuration for the application +const ( + DefaultMaxAttachmentUploadBytes int64 = 128 << 20 + DefaultMaxConcurrentAttachmentUploads = 4 + DefaultMaxConcurrentImageProcessing = 2 + DefaultMaxDecodedImageDimensionPx = 16384 + DefaultMaxDecodedImagePixels int64 = 40_000_000 + DefaultAuthMaxBodyBytes int64 = 16 << 10 + DefaultAuthIPAttempts = 60 + DefaultAuthIPWindowSeconds = 60 + DefaultAuthLoginAttempts = 10 + DefaultAuthLoginWindowSeconds = 300 + DefaultAuthAccountAttempts = 100 + DefaultAuthAccountWindowSeconds = 900 + DefaultAuthLimiterMaxKeys = 10_000 +) + type Config struct { // Server settings Port string @@ -19,7 +36,16 @@ type Config struct { DatabaseURL string // Authentication - JWTSecret string + JWTSecret string + AuthMaxBodyBytes int64 + AuthIPAttempts int + AuthIPWindowSeconds int + AuthLoginAttempts int + AuthLoginWindowSeconds int + AuthAccountAttempts int + AuthAccountWindowSeconds int + AuthLimiterMaxKeys int + AuthTrustedProxyCIDRs []string // Logging LogLevel string @@ -32,11 +58,16 @@ type Config struct { AppBundleVersionsPath string MaxVersionsKept int - // Attachment image processing (all optional). - ImageCompressionLevel int // 0..10; 0 disables compression - ImageMaxWidthPx int // 0 disables width limit - ImageMaxHeightPx int // 0 disables height limit - ImageApplyExifOrientation bool // true enables EXIF orientation normalization + // Attachment upload and image-processing limits. + MaxAttachmentUploadBytes int64 + MaxConcurrentAttachmentUploads int + MaxConcurrentImageProcessing int + MaxDecodedImageDimensionPx int + MaxDecodedImagePixels int64 + ImageCompressionLevel int // 0..10; 0 disables compression + ImageMaxWidthPx int // 0 disables width limit + ImageMaxHeightPx int // 0 disables height limit + ImageApplyExifOrientation bool // true enables EXIF orientation normalization // Internal tracking Source string // Source of the configuration (env, .env file path, etc.) @@ -148,25 +179,54 @@ func Load(log *logger.Logger) (*Config, error) { appBundlePath := filepath.Join(dataDir, "app-bundle", "active") appBundleVersionsPath := filepath.Join(dataDir, "app-bundle", "versions") + authMaxBodyBytes := getEnvPositiveInt64WithWarnings(log, "SYNKRONUS_AUTH_MAX_BODY_BYTES", DefaultAuthMaxBodyBytes) + authIPAttempts := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_AUTH_IP_ATTEMPTS", DefaultAuthIPAttempts) + authIPWindowSeconds := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_AUTH_IP_WINDOW_SECONDS", DefaultAuthIPWindowSeconds) + authLoginAttempts := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_AUTH_LOGIN_ATTEMPTS", DefaultAuthLoginAttempts) + authLoginWindowSeconds := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_AUTH_LOGIN_WINDOW_SECONDS", DefaultAuthLoginWindowSeconds) + authAccountAttempts := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_AUTH_ACCOUNT_ATTEMPTS", DefaultAuthAccountAttempts) + authAccountWindowSeconds := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_AUTH_ACCOUNT_WINDOW_SECONDS", DefaultAuthAccountWindowSeconds) + authLimiterMaxKeys := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_AUTH_LIMITER_MAX_KEYS", DefaultAuthLimiterMaxKeys) + authTrustedProxyCIDRs := splitCommaSeparated(getEnvOrDefault("SYNKRONUS_AUTH_TRUSTED_PROXY_CIDRS", "")) + + maxAttachmentUploadBytes := getEnvPositiveInt64WithWarnings(log, "SYNKRONUS_MAX_ATTACHMENT_UPLOAD_BYTES", DefaultMaxAttachmentUploadBytes) + maxConcurrentAttachmentUploads := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_MAX_CONCURRENT_ATTACHMENT_UPLOADS", DefaultMaxConcurrentAttachmentUploads) + maxConcurrentImageProcessing := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_MAX_CONCURRENT_IMAGE_PROCESSING", DefaultMaxConcurrentImageProcessing) + maxDecodedImageDimensionPx := getEnvPositiveIntWithWarnings(log, "SYNKRONUS_MAX_DECODED_IMAGE_DIMENSION_PX", DefaultMaxDecodedImageDimensionPx) + maxDecodedImagePixels := getEnvPositiveInt64WithWarnings(log, "SYNKRONUS_MAX_DECODED_IMAGE_PIXELS", DefaultMaxDecodedImagePixels) imageCompressionLevel := getEnvClampedIntWithWarnings(log, "SYNKRONUS_IMAGE_COMPRESSION_LEVEL", 0, 0, 10) imageMaxWidthPx := getEnvNonNegativeIntWithWarnings(log, "SYNKRONUS_IMAGE_MAX_WIDTH_PX", 0) imageMaxHeightPx := getEnvNonNegativeIntWithWarnings(log, "SYNKRONUS_IMAGE_MAX_HEIGHT_PX", 0) imageApplyExifOrientation := getEnvBoolWithWarnings(log, "SYNKRONUS_IMAGE_APPLY_EXIF_ORIENTATION", true) return &Config{ - Port: getEnvOrDefault("PORT", "8080"), - DatabaseURL: getEnvOrDefault("DB_CONNECTION", "postgres://user:password@localhost:5432/synkronus"), - JWTSecret: getEnvOrDefault("JWT_SECRET", ""), - LogLevel: getEnvOrDefault("LOG_LEVEL", "info"), - DataDir: dataDir, - AppBundlePath: appBundlePath, - AppBundleVersionsPath: appBundleVersionsPath, - MaxVersionsKept: getEnvIntOrDefault("MAX_VERSIONS_KEPT", 5), - ImageCompressionLevel: imageCompressionLevel, - ImageMaxWidthPx: imageMaxWidthPx, - ImageMaxHeightPx: imageMaxHeightPx, - ImageApplyExifOrientation: imageApplyExifOrientation, - Source: configSource, + Port: getEnvOrDefault("PORT", "8080"), + DatabaseURL: getEnvOrDefault("DB_CONNECTION", "postgres://user:password@localhost:5432/synkronus"), + JWTSecret: getEnvOrDefault("JWT_SECRET", ""), + AuthMaxBodyBytes: authMaxBodyBytes, + AuthIPAttempts: authIPAttempts, + AuthIPWindowSeconds: authIPWindowSeconds, + AuthLoginAttempts: authLoginAttempts, + AuthLoginWindowSeconds: authLoginWindowSeconds, + AuthAccountAttempts: authAccountAttempts, + AuthAccountWindowSeconds: authAccountWindowSeconds, + AuthLimiterMaxKeys: authLimiterMaxKeys, + AuthTrustedProxyCIDRs: authTrustedProxyCIDRs, + LogLevel: getEnvOrDefault("LOG_LEVEL", "info"), + DataDir: dataDir, + AppBundlePath: appBundlePath, + AppBundleVersionsPath: appBundleVersionsPath, + MaxVersionsKept: getEnvIntOrDefault("MAX_VERSIONS_KEPT", 5), + MaxAttachmentUploadBytes: maxAttachmentUploadBytes, + MaxConcurrentAttachmentUploads: maxConcurrentAttachmentUploads, + MaxConcurrentImageProcessing: maxConcurrentImageProcessing, + MaxDecodedImageDimensionPx: maxDecodedImageDimensionPx, + MaxDecodedImagePixels: maxDecodedImagePixels, + ImageCompressionLevel: imageCompressionLevel, + ImageMaxWidthPx: imageMaxWidthPx, + ImageMaxHeightPx: imageMaxHeightPx, + ImageApplyExifOrientation: imageApplyExifOrientation, + Source: configSource, }, nil } @@ -215,6 +275,46 @@ func getEnvClampedIntWithWarnings(log *logger.Logger, key string, defaultValue, return intValue } +func splitCommaSeparated(value string) []string { + if strings.TrimSpace(value) == "" { + return nil + } + parts := strings.Split(value, ",") + out := make([]string, 0, len(parts)) + for _, part := range parts { + if trimmed := strings.TrimSpace(part); trimmed != "" { + out = append(out, trimmed) + } + } + return out +} + +func getEnvPositiveIntWithWarnings(log *logger.Logger, key string, defaultValue int) int { + value := getEnvPositiveInt64WithWarnings(log, key, int64(defaultValue)) + if value > int64(^uint(0)>>1) { + if log != nil { + log.Warn("Integer environment variable is too large, using default", "key", key, "value", value, "default", defaultValue) + } + return defaultValue + } + return int(value) +} + +func getEnvPositiveInt64WithWarnings(log *logger.Logger, key string, defaultValue int64) int64 { + value, exists := os.LookupEnv(key) + if !exists || value == "" { + return defaultValue + } + intValue, err := strconv.ParseInt(value, 10, 64) + if err != nil || intValue <= 0 { + if log != nil { + log.Warn("Environment variable must be a positive integer, using default", "key", key, "value", value, "default", defaultValue) + } + return defaultValue + } + return intValue +} + func getEnvNonNegativeIntWithWarnings(log *logger.Logger, key string, defaultValue int) int { value, exists := os.LookupEnv(key) if !exists || value == "" {