diff --git a/src/lib/media/video/__tests__/upload.shared.test.ts b/src/lib/media/video/__tests__/upload.shared.test.ts new file mode 100644 index 0000000000..7ab8b88ed3 --- /dev/null +++ b/src/lib/media/video/__tests__/upload.shared.test.ts @@ -0,0 +1,70 @@ +import {type Client} from '@atproto/lex' + +import {com} from '#/lexicons' +import { + getServiceAuthToken, + SERVICE_AUTH_TTL_SEC, + serviceAuthExp, +} from '../upload.shared' + +function createClient() { + const call = jest.fn().mockResolvedValue({token: 'token'}) + return {client: {call} as unknown as Client, call} +} + +describe('serviceAuthExp', () => { + it('returns an integer even when the clock has sub-second precision', () => { + jest.spyOn(Date, 'now').mockReturnValue(1_700_000_000_500) + expect(serviceAuthExp()).toBe(1_700_000_000 + SERVICE_AUTH_TTL_SEC) + expect(Number.isInteger(serviceAuthExp())).toBe(true) + }) + + it('accepts a custom ttl and keeps the result integral', () => { + jest.spyOn(Date, 'now').mockReturnValue(1_700_000_000_999) + expect(serviceAuthExp(90.7)).toBe(1_700_000_090) + }) +}) + +describe('getServiceAuthToken', () => { + it('floors a fractional exp before sending it', async () => { + const {client, call} = createClient() + await getServiceAuthToken({ + client, + aud: 'did:web:video.bsky.app', + lxm: 'com.atproto.repo.uploadBlob', + exp: 1_700_001_800.5, + }) + expect(call).toHaveBeenCalledWith(com.atproto.server.getServiceAuth, { + aud: 'did:web:video.bsky.app', + lxm: 'com.atproto.repo.uploadBlob', + exp: 1_700_001_800, + }) + }) + + it('leaves exp undefined when the caller omits it', async () => { + const {client, call} = createClient() + await getServiceAuthToken({ + client, + aud: 'did:web:video.bsky.app', + lxm: 'app.bsky.video.getUploadLimits', + }) + expect(call).toHaveBeenCalledWith(com.atproto.server.getServiceAuth, { + aud: 'did:web:video.bsky.app', + lxm: 'app.bsky.video.getUploadLimits', + exp: undefined, + }) + }) + + it('rejects a non-finite exp rather than sending NaN', async () => { + const {client, call} = createClient() + await expect( + getServiceAuthToken({ + client, + aud: 'did:web:video.bsky.app', + lxm: 'com.atproto.repo.uploadBlob', + exp: NaN, + }), + ).rejects.toThrow('Invalid service auth exp') + expect(call).not.toHaveBeenCalled() + }) +}) diff --git a/src/lib/media/video/multipart/upload.ts b/src/lib/media/video/multipart/upload.ts index d3e5210873..3b0048ecd2 100644 --- a/src/lib/media/video/multipart/upload.ts +++ b/src/lib/media/video/multipart/upload.ts @@ -5,7 +5,7 @@ import {AbortError} from '#/lib/async/cancelable' import {type CompressedVideo} from '#/lib/media/video/types' import {shouldRetryError} from '#/lib/strings/errors' import {type app} from '#/lexicons' -import {getServiceAuthToken} from '../upload.shared' +import {getServiceAuthToken, serviceAuthExp} from '../upload.shared' import {mimeToExt} from '../util' import { abortUpload, @@ -280,7 +280,7 @@ function createTokenProvider( async function get(forceRefresh = false) { if (!forceRefresh && token && Date.now() < expiresAt - 60_000) return token if (!refresh) { - const exp = Math.floor(Date.now() / 1000) + 60 * 30 + const exp = serviceAuthExp() refresh = getServiceAuthTokenWithRetry(client, dispatchUrl, exp, signal) .then(nextToken => { token = nextToken diff --git a/src/lib/media/video/upload.shared.ts b/src/lib/media/video/upload.shared.ts index 9e8d99a185..1ea0c62a9c 100644 --- a/src/lib/media/video/upload.shared.ts +++ b/src/lib/media/video/upload.shared.ts @@ -27,6 +27,11 @@ export async function getServiceAuthToken({ dispatchUrl?: string | URL aud?: string lxm: NsidString + /** + * Unix timestamp in *seconds* at which the token expires. Fractional values + * are floored - see {@link toIntegerExp}. Defaults to the server's own + * short expiry when omitted. + */ exp?: number }) { let resolvedAud = aud @@ -43,11 +48,42 @@ export async function getServiceAuthToken({ const {token} = await client.call(com.atproto.server.getServiceAuth, { aud: resolvedAud as DidString, lxm, - exp, + exp: exp === undefined ? undefined : toIntegerExp(exp), }) return token } +/** + * Default lifetime for the video upload service auth token. Long enough to + * cover a slow upload of a large file, short enough to limit the damage if the + * token leaks. + */ +export const SERVICE_AUTH_TTL_SEC = 60 * 30 + +/** + * Build a service auth `exp` claim `ttlSec` seconds from now. + * + * Always use this instead of hand-rolling the arithmetic: `Date.now()` is in + * milliseconds, and dividing by 1000 without flooring yields a fractional + * timestamp that the endpoint rejects. + */ +export function serviceAuthExp(ttlSec: number = SERVICE_AUTH_TTL_SEC) { + return Math.floor(Date.now() / 1000) + Math.floor(ttlSec) +} + +/** + * The lexicon types `exp` as an integer and it is serialized straight into the + * query string, so a fractional value fails validation and the upload dies + * before it starts. Floor here, at the single chokepoint every caller goes + * through, so a call site that forgets to cannot reintroduce the bug. + */ +function toIntegerExp(exp: number) { + if (!Number.isFinite(exp)) { + throw new Error(`Invalid service auth exp: ${exp}`) + } + return Math.floor(exp) +} + export async function getVideoUploadLimits(client: Client, i18n: I18n) { const token = await getServiceAuthToken({ client, diff --git a/src/lib/media/video/upload.ts b/src/lib/media/video/upload.ts index 287c8f09c6..d5a2af5146 100644 --- a/src/lib/media/video/upload.ts +++ b/src/lib/media/video/upload.ts @@ -13,7 +13,11 @@ import { import {Features, features} from '#/analytics/features' import {type app} from '#/lexicons' import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload' -import {getServiceAuthToken, getVideoUploadLimits} from './upload.shared' +import { + getServiceAuthToken, + getVideoUploadLimits, + serviceAuthExp, +} from './upload.shared' import {createVideoEndpointUrl, mimeToExt} from './util' export async function uploadVideo({ @@ -72,7 +76,7 @@ export async function uploadVideo({ client, dispatchUrl, lxm: 'com.atproto.repo.uploadBlob', - exp: Date.now() / 1000 + 60 * 30, // 30 minutes + exp: serviceAuthExp(), }) const uploadTask = createUploadTask( uri, diff --git a/src/lib/media/video/upload.web.ts b/src/lib/media/video/upload.web.ts index 90c10a0684..b0fcbaaf09 100644 --- a/src/lib/media/video/upload.web.ts +++ b/src/lib/media/video/upload.web.ts @@ -12,7 +12,11 @@ import { import {Features, features} from '#/analytics/features' import {type app} from '#/lexicons' import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload' -import {getServiceAuthToken, getVideoUploadLimits} from './upload.shared' +import { + getServiceAuthToken, + getVideoUploadLimits, + serviceAuthExp, +} from './upload.shared' import {createVideoEndpointUrl, mimeToExt} from './util' export async function uploadVideo({ @@ -79,7 +83,7 @@ export async function uploadVideo({ client, dispatchUrl, lxm: 'com.atproto.repo.uploadBlob', - exp: Date.now() / 1000 + 60 * 30, // 30 minutes + exp: serviceAuthExp(), }) if (signal.aborted) {