Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 52 additions & 18 deletions api/src/processings/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@ const ajv = ajvFormats(new Ajv({ strict: false }))
/**
* Ensure the plugin tarball is in the API's local cache (downloads on miss).
* Registry enforces that `processing.owner` has access to the artefact; a 403
* here surfaces directly to the caller.
* here surfaces directly to the caller as a user-facing French message.
*
* The plugin's processing config schema is read from the conventional
* `processing-config-schema.json` shipped alongside the plugin (same convention
* as v5). Returns `undefined` when the plugin doesn't ship one.
* `processing-config-schema.json` shipped alongside the plugin (v6 convention),
* falling back to `plugin.json#processingConfigSchema` for legacy v5 plugins
* whose tarballs were packed verbatim by the registry migration and never had
* the schema extracted into a standalone file. Returns `undefined` when
* neither source carries one.
*/
async function ensurePluginAndReadSchema (processing: Pick<Processing, 'plugin' | 'owner'>) {
const account = {
Expand All @@ -50,17 +53,39 @@ async function ensurePluginAndReadSchema (processing: Pick<Processing, 'plugin'
// `processing.plugin` is the registry artefact id, passed through as-is.
// lib-node downloads + extracts the tarball into registryCacheDir on cache
// miss; the cache is invalidated when the artefact's dataUpdatedAt bumps.
const ensured = await ensureArtefact({
registryUrl: config.privateRegistryUrl,
secretKey: config.secretKeys.registry,
artefactId: processing.plugin,
cacheDir: registryCacheDir,
account
})
//
// Axios errors carry `status` but `lib-express/error-handler.js` collapses
// every AxiosRequestError to 500 — so we translate at the boundary here.
let ensured
try {
ensured = await ensureArtefact({
registryUrl: config.privateRegistryUrl,
secretKey: config.secretKeys.registry,
artefactId: processing.plugin,
cacheDir: registryCacheDir,
account
})
} catch (err: any) {
const status = err?.status ?? err?.statusCode
const ownerLabel = processing.owner.name ?? processing.owner.id
if (status === 403) {
throw httpError(403, `Le compte ${ownerLabel} n'a pas accès au plugin ${processing.plugin}.`)
}
if (status === 404) {
throw httpError(404, `Le plugin ${processing.plugin} est introuvable dans le registre.`)
}
throw httpError(502, 'Le registre des plugins est temporairement indisponible.')
}
const schemaPath = path.join(ensured.path, 'processing-config-schema.json')
let processingConfigSchema: Record<string, unknown> | undefined
if (await fs.pathExists(schemaPath)) {
processingConfigSchema = await fs.readJson(schemaPath)
} else {
const pluginJsonPath = path.join(ensured.path, 'plugin.json')
if (await fs.pathExists(pluginJsonPath)) {
const pluginJson = await fs.readJson(pluginJsonPath)
if (pluginJson?.processingConfigSchema) processingConfigSchema = pluginJson.processingConfigSchema
}
}
return { ensured, processingConfigSchema }
}
Expand Down Expand Up @@ -101,9 +126,8 @@ const sendProcessingEvent = (
* - validate the config against the plugin's processingConfigSchema (read
* from the cached package.json#registry block, downloaded on cache miss)
*
* Errors from registry (404 unknown artefact, 403 owner has no access) bubble
* up as the corresponding HTTP errors and replace the explicit access check
* that used to live in the create/update endpoints.
* Skipped entirely when the processing has no config. Registry errors are
* translated to user-facing French messages inside ensurePluginAndReadSchema.
*/
async function validateFullProcessing (processing: any): Promise<Processing> {
(await import('#types/processing/index.ts')).returnValid(processing)
Expand All @@ -118,7 +142,12 @@ async function validateFullProcessing (processing: any): Promise<Processing> {
}

const prepareProcessing = async (processing: Processing) => {
// Get the plugin file and execute the prepare function if it exists.
// `prepare` operates on the user-supplied config (e.g. exchanging OAuth
// refresh tokens for ciphered secrets). With no config there's nothing to
// prepare — and skipping the registry fetch lets a processing be created
// even when the owner has no access to the plugin yet. The same broken
// state is what the UI banner already shows for deleted plugins.
if (!processing.config) return
const { ensured } = await ensurePluginAndReadSchema(processing)
const plugin = await importPluginModule<{ prepare?: PrepareFunction }>(ensured.path, { cacheBust: true })
if (!(plugin.prepare && typeof plugin.prepare === 'function')) return
Expand Down Expand Up @@ -336,9 +365,10 @@ router.post('', async (req, res) => {
date: new Date().toISOString()
}

// Plugin access check is delegated to registry: validateFullProcessing
// calls ensureArtefact with owner=body.owner, and registry returns 403 if
// the artefact is private and the owner has no privateAccess entry.
// Plugin access is only checked when actually needed: validateFullProcessing
// and prepareProcessing both short-circuit when `body.config` is absent. A
// processing created without config lands in the same "plugin not loaded"
// state that existed for deleted/revoked plugins — see ui pluginBroken.
const processing = await validateFullProcessing(body)
Object.assign(processing, await prepareProcessing(processing))
await mongo.processings.insertOne(processing)
Expand Down Expand Up @@ -425,7 +455,11 @@ router.get('/:id/plugin-config-schema', async (req, res, next) => {
if (!processing) return res.status(404).send()
if (!['admin', 'exec', 'read'].includes(permissions.getUserResourceProfile(processing.owner, processing.permissions, sessionState) ?? '')) return res.status(403).send()
const { processingConfigSchema } = await ensurePluginAndReadSchema(processing)
if (!processingConfigSchema) return res.status(404).send()
if (!processingConfigSchema) {
return res.status(404).type('text/plain').send(
`Plugin "${processing.plugin}" ships no processing config schema (no processing-config-schema.json in tarball and no processingConfigSchema field in plugin.json).`
)
}
res.status(200).json(processingConfigSchema)
} catch (err) { next(err) }
})
Expand Down
25 changes: 17 additions & 8 deletions tests/features/processings/broken-plugin.api.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,30 @@ test.describe('processing with unavailable plugin', () => {
expect(res.data.plugin).toBe('@test-never-existed-1')
})

test('PATCH /processings/:id surfaces the registry error', async () => {
test('PATCH /processings/:id with no config field succeeds even when the plugin is gone', async () => {
const superadmin = await axiosAuth('test_superadmin@test.com')
const id = await createBrokenProcessing(superadmin)

// Renaming/changing scheduling on a broken-plugin processing must work —
// ensurePluginAndReadSchema is only invoked when there's a config to
// validate or prepare. Lets users fix metadata, switch owner, or delete.
const res = await superadmin.patch(`/api/v1/processings/${id}`, { title: 'New title' })
expect(res.status).toBe(200)
expect(res.data.title).toBe('New title')
})

test('PATCH /processings/:id with a config field surfaces a 404 with the plugin id', async () => {
const superadmin = await axiosAuth('test_superadmin@test.com')
const id = await createBrokenProcessing(superadmin)

const res = await superadmin.patch(
`/api/v1/processings/${id}`,
{ title: 'New title' },
{ config: { message: 'hello' } },
{ validateStatus: () => true }
)
// Registry returns 404 for an unknown artefact. In the current
// implementation ensurePluginAndReadSchema does NOT forward the registry
// status — the unhandled AxiosRequestError propagates and Express returns
// 500. We include 403/404 in the set so the test also passes if the
// forwarding behaviour is improved in a future refactor.
expect([403, 404, 500]).toContain(res.status)
expect(res.status).toBe(404)
expect(typeof res.data === 'string' ? res.data : JSON.stringify(res.data))
.toContain('@test-never-existed-1')
})

test('DELETE /processings/:id returns 204', async () => {
Expand Down
128 changes: 128 additions & 0 deletions tests/features/processings/plugin-access.api.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
import { test, expect } from '@playwright/test'
import { axiosAuth, clean } from '../../support/axios.ts'
import { publishFixturePlugin } from '../../support/registry.ts'

/**
* A superadmin can see every artefact in the registry, so the create-form
* picker lets them pick a plugin no matter who they set as owner. When the
* picked plugin is private and the owner is not on `privateAccess`, the
* registry's /:id/download route returns 403 (no longer 404, since the
* caller already knows the id). The processings API translates that into
* a French user-facing message and routes the response status correctly.
*
* The owner used here is `test_user1` because the default fixture grants only
* include `test_superadmin` + the two orgs — a plain user account is the
* cleanest "has a grant but no privateAccess for THIS plugin" subject.
*/
test.describe('processing creation with restricted plugin access', () => {
test.beforeEach(clean)
test.afterAll(clean)

test('superadmin can create a processing without config even if the owner has no plugin access (broken state)', async () => {
const superadmin = await axiosAuth('test_superadmin@test.com')
const fixture = await publishFixturePlugin({
name: '@data-fair/processing-hello-world',
version: '1.2.2',
isPublic: false,
privateAccess: [{ type: 'organization', id: 'test_org1', name: 'Test Org 1' }],
// Grant test_user1 the global access-grant so the only thing missing
// is the privateAccess entry — that's the scenario we want to test.
grants: [
{ type: 'user', id: 'test_superadmin' },
{ type: 'user', id: 'test_user1' },
{ type: 'organization', id: 'test_org1' }
]
})

const res = await superadmin.post('/api/v1/processings', {
title: 'Owner has no access',
plugin: fixture.pluginId,
owner: { type: 'user', id: 'test_user1', name: 'Test User 1' }
})
expect(res.status).toBe(200)
expect(res.data._id).toBeTruthy()
expect(res.data.owner.id).toBe('test_user1')

// The schema fetch is the first call that hits the registry on behalf of
// the owner. Pre-rebuild registry image hides the artefact behind 404;
// post-rebuild it returns 403 (since the caller already knows the id and
// we want to distinguish "no access" from "doesn't exist"). Either way
// the processings API translates to a French message containing the
// plugin id, NOT the previous opaque 500.
const schema = await superadmin.get(
`/api/v1/processings/${res.data._id}/plugin-config-schema`,
{ validateStatus: () => true }
)
expect([403, 404]).toContain(schema.status)
const body = typeof schema.data === 'string' ? schema.data : JSON.stringify(schema.data)
expect(body).toContain(fixture.pluginId)
// Post-rebuild only: the message names the owner. Skip if the registry
// still returns 404 — that's just "Le plugin … est introuvable".
if (schema.status === 403) expect(body).toContain('Test User 1')
})

test('PATCH with a config field on a processing whose owner has no access fails with 403 + message', async () => {
const superadmin = await axiosAuth('test_superadmin@test.com')
const fixture = await publishFixturePlugin({
name: '@data-fair/processing-hello-world',
version: '1.2.2',
isPublic: false,
privateAccess: [{ type: 'organization', id: 'test_org1', name: 'Test Org 1' }],
grants: [
{ type: 'user', id: 'test_superadmin' },
{ type: 'user', id: 'test_user1' },
{ type: 'organization', id: 'test_org1' }
]
})

const created = (await superadmin.post('/api/v1/processings', {
title: 'Owner has no access',
plugin: fixture.pluginId,
owner: { type: 'user', id: 'test_user1', name: 'Test User 1' }
})).data

const patch = await superadmin.patch(
`/api/v1/processings/${created._id}`,
{
config: {
datasetMode: 'create',
dataset: { id: 'test_x', title: 'X' },
overwrite: false,
message: 'x'
}
},
{ validateStatus: () => true }
)
// 403 once the registry change deploys; 404 in the meantime (registry
// currently hides the artefact behind 404 from the owner's view).
expect([403, 404]).toContain(patch.status)
const body = typeof patch.data === 'string' ? patch.data : JSON.stringify(patch.data)
expect(body).toContain(fixture.pluginId)
if (patch.status === 403) expect(body).toContain('Test User 1')
})

test('owner with privateAccess can have a processing created with config', async () => {
const superadmin = await axiosAuth('test_superadmin@test.com')
const fixture = await publishFixturePlugin({
name: '@data-fair/processing-hello-world',
version: '1.2.2',
isPublic: false,
privateAccess: [{ type: 'organization', id: 'test_org1', name: 'Test Org 1' }]
})

const res = await superadmin.post('/api/v1/processings', {
title: 'Authorized owner with config',
plugin: fixture.pluginId,
owner: { type: 'organization', id: 'test_org1', name: 'Test Org 1' },
config: {
datasetMode: 'create',
dataset: { id: 'test_y', title: 'Y' },
overwrite: false,
message: 'y'
}
})
expect(res.status).toBe(200)
expect(res.data._id).toBeTruthy()
expect(res.data.config?.message).toBe('y')
})
})
11 changes: 11 additions & 0 deletions ui/src/pages/processings/[id]/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,17 @@ const configSchemaFetch = useFetch<Record<string, unknown>>(
computed(() => processing.value?.plugin
? `${$apiPath}/processings/${processingId}/plugin-config-schema`
: null),
// We handle notifications ourselves below — useFetch's built-in toast
// would also fire on the legitimate "plugin ships no schema" 404.
{ notifError: false }
)
const { sendUiNotif } = useUiNotif()
watch(() => configSchemaFetch.error.value, (err) => {
if (!err) return
const status = err.statusCode ?? err.status
if (status === 404) return // legitimate "plugin ships no schema"
sendUiNotif({ msg: t('configSchemaFetchError'), error: err })
})
const configSchema = computed<Record<string, unknown> | null>(() => {
const err = configSchemaFetch.error.value
// The endpoint returns 404 to mean "this plugin doesn't ship a schema"; we
Expand Down Expand Up @@ -326,6 +335,7 @@ const timezoneLabel = (timeZone: string) => {
updateError: Error while updating the processing
pluginUnavailableTitle: Plugin unavailable
pluginUnavailableBody: This processing's plugin has been removed or its access revoked. You can no longer edit or run this processing, but you can still view its run history and delete it.
configSchemaFetchError: Failed to load the plugin's configuration schema

fr:
frequency:
Expand All @@ -340,6 +350,7 @@ const timezoneLabel = (timeZone: string) => {
updateError: Erreur lors de la mise à jour du traitement
pluginUnavailableTitle: Plugin indisponible
pluginUnavailableBody: Le plugin de ce traitement a été supprimé ou son accès retiré. Vous ne pouvez plus modifier ni exécuter ce traitement, mais vous pouvez consulter son historique et le supprimer.
configSchemaFetchError: Échec du chargement du schéma de configuration du plugin

</i18n>

Expand Down
Loading