mirror of
https://github.com/kennethnym/aris.git
synced 2026-04-16 14:51:17 +01:00
fix: unified source config + credentials
Accept optional credentials in PUT /api/sources/:sourceId so the dashboard can send config and credentials in a single request, eliminating the race condition between parallel config/credential updates that left sources uninitialized until server restart. The existing /credentials endpoint is preserved for independent credential updates. Co-authored-by: Ona <no-reply@ona.com>
This commit is contained in:
@@ -81,6 +81,9 @@ mock.module("../sources/user-sources.ts", () => ({
|
||||
updatedAt: now,
|
||||
}
|
||||
},
|
||||
async upsertConfig(_sourceId: string, _data: { enabled: boolean; config: unknown }) {
|
||||
// no-op for tests
|
||||
},
|
||||
async updateCredentials(sourceId: string, credentials: Buffer) {
|
||||
if (mockUpdateCredentialsError) {
|
||||
throw mockUpdateCredentialsError
|
||||
@@ -824,3 +827,121 @@ describe("UserSessionManager.updateSourceCredentials", () => {
|
||||
expect(factory).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe("UserSessionManager.upsertSourceConfig", () => {
|
||||
test("upserts config without credentials (existing behavior)", async () => {
|
||||
setEnabledSources(["test"])
|
||||
const factory = mock(async () => createStubSource("test"))
|
||||
const provider: FeedSourceProvider = { sourceId: "test", feedSourceForUser: factory }
|
||||
const manager = new UserSessionManager({
|
||||
db: fakeDb,
|
||||
providers: [provider],
|
||||
credentialEncryptor: testEncryptor,
|
||||
})
|
||||
|
||||
// Create a session first so we can verify the source is refreshed
|
||||
await manager.getOrCreate("user-1")
|
||||
|
||||
await manager.upsertSourceConfig("user-1", "test", {
|
||||
enabled: true,
|
||||
config: { key: "value" },
|
||||
})
|
||||
|
||||
// feedSourceForUser called once for session creation, once for upsert refresh
|
||||
expect(factory).toHaveBeenCalledTimes(2)
|
||||
// No credentials should have been persisted
|
||||
expect(mockUpdateCredentialsCalls).toHaveLength(0)
|
||||
})
|
||||
|
||||
test("upserts config with credentials — persists both and passes credentials to source", async () => {
|
||||
setEnabledSources(["test"])
|
||||
let receivedCredentials: unknown = null
|
||||
const factory = mock(async (_userId: string, _config: unknown, creds: unknown) => {
|
||||
receivedCredentials = creds
|
||||
return createStubSource("test")
|
||||
})
|
||||
const provider: FeedSourceProvider = { sourceId: "test", feedSourceForUser: factory }
|
||||
const manager = new UserSessionManager({
|
||||
db: fakeDb,
|
||||
providers: [provider],
|
||||
credentialEncryptor: testEncryptor,
|
||||
})
|
||||
|
||||
// Create a session so the source refresh path runs
|
||||
await manager.getOrCreate("user-1")
|
||||
|
||||
const creds = { username: "alice", password: "s3cret" }
|
||||
await manager.upsertSourceConfig("user-1", "test", {
|
||||
enabled: true,
|
||||
config: { serverUrl: "https://example.com" },
|
||||
credentials: creds,
|
||||
})
|
||||
|
||||
// Credentials were encrypted and persisted
|
||||
expect(mockUpdateCredentialsCalls).toHaveLength(1)
|
||||
const decrypted = JSON.parse(testEncryptor.decrypt(mockUpdateCredentialsCalls[0]!.credentials))
|
||||
expect(decrypted).toEqual(creds)
|
||||
|
||||
// feedSourceForUser received the provided credentials (not null)
|
||||
expect(receivedCredentials).toEqual(creds)
|
||||
})
|
||||
|
||||
test("upserts config with credentials adds source to session when not already present", async () => {
|
||||
// Start with no enabled sources so the session is empty
|
||||
setEnabledSources([])
|
||||
const factory = mock(async () => createStubSource("test"))
|
||||
const provider: FeedSourceProvider = { sourceId: "test", feedSourceForUser: factory }
|
||||
const manager = new UserSessionManager({
|
||||
db: fakeDb,
|
||||
providers: [provider],
|
||||
credentialEncryptor: testEncryptor,
|
||||
})
|
||||
|
||||
const session = await manager.getOrCreate("user-1")
|
||||
expect(session.hasSource("test")).toBe(false)
|
||||
|
||||
// Set mockFindResult to undefined so find() returns a row (simulating the row was just created by upsertConfig)
|
||||
await manager.upsertSourceConfig("user-1", "test", {
|
||||
enabled: true,
|
||||
config: {},
|
||||
credentials: { token: "abc" },
|
||||
})
|
||||
|
||||
// Source should now be in the session
|
||||
expect(session.hasSource("test")).toBe(true)
|
||||
expect(mockUpdateCredentialsCalls).toHaveLength(1)
|
||||
})
|
||||
|
||||
test("throws CredentialStorageUnavailableError when credentials provided without encryptor", async () => {
|
||||
setEnabledSources(["test"])
|
||||
const provider = createStubProvider("test")
|
||||
const manager = new UserSessionManager({
|
||||
db: fakeDb,
|
||||
providers: [provider],
|
||||
// No credentialEncryptor
|
||||
})
|
||||
|
||||
await expect(
|
||||
manager.upsertSourceConfig("user-1", "test", {
|
||||
enabled: true,
|
||||
config: {},
|
||||
credentials: { token: "abc" },
|
||||
}),
|
||||
).rejects.toBeInstanceOf(CredentialStorageUnavailableError)
|
||||
})
|
||||
|
||||
test("throws SourceNotFoundError for unknown provider", async () => {
|
||||
const manager = new UserSessionManager({
|
||||
db: fakeDb,
|
||||
providers: [],
|
||||
credentialEncryptor: testEncryptor,
|
||||
})
|
||||
|
||||
await expect(
|
||||
manager.upsertSourceConfig("user-1", "unknown", {
|
||||
enabled: true,
|
||||
config: {},
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SourceNotFoundError)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -11,6 +11,7 @@ import type { FeedSourceProvider } from "./feed-source-provider.ts"
|
||||
import {
|
||||
CredentialStorageUnavailableError,
|
||||
InvalidSourceConfigError,
|
||||
InvalidSourceCredentialsError,
|
||||
SourceNotFoundError,
|
||||
} from "../sources/errors.ts"
|
||||
import { sources } from "../sources/user-sources.ts"
|
||||
@@ -171,13 +172,18 @@ export class UserSessionManager {
|
||||
* inserts a new row if one doesn't exist and fully replaces config
|
||||
* (no merge).
|
||||
*
|
||||
* When `credentials` is provided, they are encrypted and persisted
|
||||
* alongside the config in the same flow, avoiding the race condition
|
||||
* of separate config + credential requests.
|
||||
*
|
||||
* @throws {SourceNotFoundError} if the sourceId has no registered provider
|
||||
* @throws {InvalidSourceConfigError} if config fails schema validation
|
||||
* @throws {CredentialStorageUnavailableError} if credentials are provided but no encryptor is configured
|
||||
*/
|
||||
async upsertSourceConfig(
|
||||
userId: string,
|
||||
sourceId: string,
|
||||
data: { enabled: boolean; config?: unknown },
|
||||
data: { enabled: boolean; config?: unknown; credentials?: unknown },
|
||||
): Promise<void> {
|
||||
const provider = this.providers.get(sourceId)
|
||||
if (!provider) {
|
||||
@@ -191,6 +197,10 @@ export class UserSessionManager {
|
||||
}
|
||||
}
|
||||
|
||||
if (data.credentials !== undefined && !this.encryptor) {
|
||||
throw new CredentialStorageUnavailableError()
|
||||
}
|
||||
|
||||
const config = data.config ?? {}
|
||||
|
||||
// Fetch existing row before upsert to capture credentials for session refresh.
|
||||
@@ -202,14 +212,24 @@ export class UserSessionManager {
|
||||
config,
|
||||
})
|
||||
|
||||
// Persist credentials after the upsert so the row exists.
|
||||
if (data.credentials !== undefined && this.encryptor) {
|
||||
const encrypted = this.encryptor.encrypt(JSON.stringify(data.credentials))
|
||||
await sources(this.db, userId).updateCredentials(sourceId, encrypted)
|
||||
}
|
||||
|
||||
const session = this.sessions.get(userId)
|
||||
if (session) {
|
||||
if (!data.enabled) {
|
||||
session.removeSource(sourceId)
|
||||
} else {
|
||||
const credentials = existingRow?.credentials
|
||||
? this.decryptCredentials(existingRow.credentials)
|
||||
: null
|
||||
// Prefer the just-provided credentials over what was in the DB.
|
||||
const credentials =
|
||||
data.credentials !== undefined
|
||||
? data.credentials
|
||||
: existingRow?.credentials
|
||||
? this.decryptCredentials(existingRow.credentials)
|
||||
: null
|
||||
const source = await provider.feedSourceForUser(userId, config, credentials)
|
||||
if (session.hasSource(sourceId)) {
|
||||
session.replaceSource(sourceId, source)
|
||||
|
||||
@@ -738,6 +738,42 @@ describe("PUT /api/sources/:sourceId", () => {
|
||||
|
||||
expect(res.status).toBe(204)
|
||||
})
|
||||
|
||||
test("returns 204 when credentials are included alongside config", async () => {
|
||||
activeStore = createInMemoryStore()
|
||||
const { app } = createAppWithEncryptor(
|
||||
[createStubProvider("aelis.weather", weatherConfig)],
|
||||
MOCK_USER_ID,
|
||||
)
|
||||
|
||||
const res = await put(app, "aelis.weather", {
|
||||
enabled: true,
|
||||
config: { units: "metric" },
|
||||
credentials: { apiKey: "secret123" },
|
||||
})
|
||||
|
||||
expect(res.status).toBe(204)
|
||||
const row = activeStore.rows.get(`${MOCK_USER_ID}:aelis.weather`)
|
||||
expect(row).toBeDefined()
|
||||
expect(row!.enabled).toBe(true)
|
||||
expect(row!.config).toEqual({ units: "metric" })
|
||||
})
|
||||
|
||||
test("returns 503 when credentials are provided but no encryptor is configured", async () => {
|
||||
activeStore = createInMemoryStore()
|
||||
// createApp does NOT configure an encryptor
|
||||
const { app } = createApp([createStubProvider("aelis.weather", weatherConfig)], MOCK_USER_ID)
|
||||
|
||||
const res = await put(app, "aelis.weather", {
|
||||
enabled: true,
|
||||
config: { units: "metric" },
|
||||
credentials: { apiKey: "secret123" },
|
||||
})
|
||||
|
||||
expect(res.status).toBe(503)
|
||||
const body = (await res.json()) as { error: string }
|
||||
expect(body.error).toContain("not configured")
|
||||
})
|
||||
})
|
||||
|
||||
describe("PUT /api/sources/:sourceId/credentials", () => {
|
||||
|
||||
@@ -34,11 +34,13 @@ const ReplaceSourceConfigRequestBody = type({
|
||||
"+": "reject",
|
||||
enabled: "boolean",
|
||||
config: "unknown",
|
||||
"credentials?": "unknown",
|
||||
})
|
||||
|
||||
const ReplaceSourceConfigNoConfigRequestBody = type({
|
||||
"+": "reject",
|
||||
enabled: "boolean",
|
||||
"credentials?": "unknown",
|
||||
})
|
||||
|
||||
export function registerSourcesHttpHandlers(
|
||||
@@ -161,7 +163,7 @@ async function handleReplaceSource(c: Context<Env>) {
|
||||
return c.json({ error: parsed.summary }, 400)
|
||||
}
|
||||
|
||||
const { enabled } = parsed
|
||||
const { enabled, credentials } = parsed
|
||||
const config = "config" in parsed ? parsed.config : undefined
|
||||
const user = c.get("user")!
|
||||
|
||||
@@ -169,6 +171,7 @@ async function handleReplaceSource(c: Context<Env>) {
|
||||
await sessionManager.upsertSourceConfig(user.id, sourceId, {
|
||||
enabled,
|
||||
config,
|
||||
credentials,
|
||||
})
|
||||
} catch (err) {
|
||||
if (err instanceof SourceNotFoundError) {
|
||||
@@ -177,6 +180,9 @@ async function handleReplaceSource(c: Context<Env>) {
|
||||
if (err instanceof InvalidSourceConfigError) {
|
||||
return c.json({ error: err.message }, 400)
|
||||
}
|
||||
if (err instanceof CredentialStorageUnavailableError) {
|
||||
return c.json({ error: err.message }, 503)
|
||||
}
|
||||
throw err
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user