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
2 changes: 2 additions & 0 deletions .agents/skills/pr-ready/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ Required content:

Length guidance: as compact as the change allows. A typo fix is one sentence. A multi-part feature is a short summary plus a tight bullet list. **No filler, no test-plan boilerplate, no marketing tone.** If you find yourself padding, stop.

Line breaks: **no superfluous line breaks.** Only insert a blank line where it marks a genuine section delimitation (e.g. between the summary and a `**Why:**` block, or before a bullet list). Do not separate every sentence with a blank line, do not pad between bullets, and do not add leading or trailing blank lines.

Example, for a small feature:

```markdown
Expand Down
81 changes: 64 additions & 17 deletions api/src/processings/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import eventsQueue from '@data-fair/lib-node/events-queue.js'
import { reqOrigin, session } from '@data-fair/lib-express/index.js'
import { ensureArtefact } from '@data-fair/lib-node-registry'
import { httpError } from '@data-fair/lib-utils/http-errors.js'
import { axiosBuilder } from '@data-fair/lib-node/axios.js'
import { createNext } from '@data-fair/processings-shared/runs.ts'
import { importPluginModule } from '@data-fair/processings-shared/plugin-load.ts'
import { applyProcessing, deleteProcessing } from '../runs/service.ts'
Expand Down Expand Up @@ -46,18 +47,36 @@ const ajv = ajvFormats(new Ajv({ strict: false, addUsedSchema: false }))
* the schema extracted into a standalone file. Returns `undefined` when
* neither source carries one.
*/
/**
* The registry evaluates artefact access against an `x-account` header. For a
* processing that is always the OWNER — anyone allowed to see the processing
* inherits the owner's plugin access transitively.
*/
const registryAccount = (owner: Processing['owner']) => ({
type: owner.type,
id: owner.id,
...(owner.department ? { department: owner.department } : {})
})

/**
* `lib-express/error-handler.js` collapses every AxiosRequestError to 500, so
* we translate registry failures into user-facing French messages at the
* boundary. 403/404 keep their status (the UI relies on them to show its
* "plugin broken" banner); anything else is a registry outage → 502.
*/
const translateRegistryError = (err: any, processing: Pick<Processing, 'plugin' | 'owner'>) => {
const status = err?.status ?? err?.statusCode
const ownerLabel = processing.owner.name ?? processing.owner.id
if (status === 403) return httpError(403, `Le compte ${ownerLabel} n'a pas accès au plugin ${processing.plugin}.`)
if (status === 404) return httpError(404, `Le plugin ${processing.plugin} est introuvable dans le registre.`)
return httpError(502, 'Le registre des plugins est temporairement indisponible.')
}

async function ensurePluginAndReadSchema (processing: Pick<Processing, 'plugin' | 'owner'>) {
const account = {
type: processing.owner.type,
id: processing.owner.id,
...(processing.owner.department ? { department: processing.owner.department } : {})
}
const account = registryAccount(processing.owner)
// `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.
//
// 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({
Expand All @@ -68,15 +87,7 @@ async function ensurePluginAndReadSchema (processing: Pick<Processing, 'plugin'
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.')
throw translateRegistryError(err, processing)
}
const schemaPath = path.join(ensured.path, 'processing-config-schema.json')
let processingConfigSchema: Record<string, unknown> | undefined
Expand All @@ -92,6 +103,28 @@ async function ensurePluginAndReadSchema (processing: Pick<Processing, 'plugin'
return { ensured, processingConfigSchema }
}

/**
* Fetch plugin artefact METADATA (title, description, thumbnail ref, ...) from
* the registry on behalf of the processing's owner. Lightweight — unlike
* `ensureArtefact` it does not download the tarball. Used by the `/:id/plugin`
* endpoint so individually-permitted viewers don't need their own plugin grant.
*/
async function fetchPluginArtefact (processing: Pick<Processing, 'plugin' | 'owner'>) {
const ax = axiosBuilder({
baseURL: config.privateRegistryUrl,
headers: {
'x-secret-key': config.secretKeys.registry,
'x-account': JSON.stringify(registryAccount(processing.owner))
}
})
try {
const res = await ax.get(`/api/v1/artefacts/${encodeURIComponent(processing.plugin)}`)
return res.data
} catch (err) {
throw translateRegistryError(err, processing)
}
}

const sensitiveParts = ['permissions', 'webhookKey', 'config']

/**
Expand Down Expand Up @@ -466,6 +499,20 @@ router.get('/:id/plugin-config-schema', async (req, res, next) => {
} catch (err) { next(err) }
})

// Get the plugin's registry metadata for this processing. Permission is checked
// against the requesting user; the registry fetch runs as the owner so a viewer
// with only an individual permission inherits the owner's plugin access.
router.get('/:id/plugin', async (req, res, next) => {
try {
const sessionState = await session.reqAuthenticated(req)
const processing = await mongo.processings.findOne({ _id: req.params.id })
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 artefact = await fetchPluginArtefact(processing)
res.status(200).json(artefact)
} catch (err) { next(err) }
})

// Delete a processing
router.delete('/:id', async (req, res) => {
const sessionState = await session.reqAuthenticated(req)
Expand Down
23 changes: 23 additions & 0 deletions docs/architecture/v6-registry-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,29 @@ For v6.0, list plugins flat with a search box. No sub-categories.

Same-domain deployment: registry sits on the same host at `/registry`, so the browser call to `/registry/api/v1/artefacts?...` is same-origin. No CORS configuration is required and the existing SimpleDirectory session cookie is naturally sent. The dev environment must mirror this — see E.1.

#### C.x Plugin metadata under processing permission

The picker above (a creation flow for owners/admins) keeps its direct
same-origin `/registry/api/v1/artefacts` list call. But **viewing an existing
processing** must not require the viewer to have their own registry grant: a
user can hold an individual `read`/`exec` permission on a processing they don't
own. So plugin **metadata** for an existing processing is fetched through the
processings API, not directly from `/registry`:

- `GET /api/v1/processings/:id/plugin` — gates on the caller's processing
permission (`getUserResourceProfile` ∈ `{admin, exec, read}`), then fetches
`GET /api/v1/artefacts/:pluginId` from the private registry with
`x-account: <processing.owner>`. Registry access is therefore evaluated
against the **owner**, mirroring `/:id/plugin-config-schema`. Registry
403/404 are translated and passed through (the UI shows its "plugin broken"
banner on those); other failures become 502.
- UI: `pages/processings/[id]/index.vue` and `usePluginFetch`
(`processing-card.vue`) call this endpoint instead of `/registry/...`.

Thumbnails are still only rendered in the picker (`new.vue`), so no
processing-scoped thumbnail proxy exists yet; if a thumbnail is ever shown on a
permitted-but-ungranted viewer's card/detail it would need the same treatment.

#### C.2 Drop admin pages and nav
- Delete `ui/src/pages/admin/plugins.vue` and matching route in `ui/src/router/`.
- Drop the `/admin/plugins` nav entry.
Expand Down
2 changes: 1 addition & 1 deletion skills-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"source": "data-fair/lib",
"sourceType": "github",
"skillPath": "skills/pr-ready/SKILL.md",
"computedHash": "dd62e51657c77f3f44972b9ee62d3082a0503b3deb10f40d9fc64b45cd778451"
"computedHash": "88ee2f1b52748972924a4c8755be1be39916005a08cf97e6aa3b3076d38e27b3"
},
"upgrade-scripts": {
"source": "data-fair/lib",
Expand Down
81 changes: 81 additions & 0 deletions tests/features/processings/plugin-info-transitivity.api.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { test, expect } from '@playwright/test'
import { axiosAuth, clean } from '../../support/axios.ts'
import { publishFixturePlugin } from '../../support/registry.ts'

/**
* A user with an *individual* read permission on a processing (not via
* ownership) must be able to read the plugin metadata even when they have no
* personal grant on the (private) plugin. The endpoint checks the processing
* permission against the user, then fetches the artefact from the registry on
* behalf of the OWNER (who does have the grant).
*/
test.describe('plugin info permission transitivity', () => {
test.beforeEach(clean)
test.afterAll(clean)

test('individually-permitted user reads plugin metadata fetched as the owner', async () => {
// Private plugin: only test_org1 (the owner) is on privateAccess. The
// fixture auto-grants privateAccess accounts + superadmin; test_alone has
// NO grant of its own — that's the whole point.
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 adminTestOrg1 = await axiosAuth({ email: 'test_admin1@test.com', org: 'test_org1' })
const aloneOutsider = await axiosAuth('test_alone@test.com')

const processing = (await adminTestOrg1.post('/api/v1/processings', {
title: 'Transitive plugin read',
plugin: fixture.pluginId,
owner: { type: 'organization', id: 'test_org1', name: 'Test Org 1' }
})).data

// Before the grant, the outsider cannot even see the processing.
await expect(aloneOutsider.get(`/api/v1/processings/${processing._id}/plugin`))
.rejects.toMatchObject({ status: 403 })

// Grant test_alone an individual read permission on the processing.
await adminTestOrg1.patch(`/api/v1/processings/${processing._id}`, {
permissions: [{ profile: 'read', target: { type: 'userEmail', email: 'test_alone@test.com' } }]
})

// Now the outsider reads the plugin metadata — fetched as the owner.
const res = await aloneOutsider.get(`/api/v1/processings/${processing._id}/plugin`)
expect(res.status).toBe(200)
expect(res.data._id).toBe(fixture.pluginId)
})

test('owner lacking the plugin grant gets a translated 403/404', async () => {
// Owner test_user1 has the global access-grant but is NOT on privateAccess,
// so the owner-scoped registry fetch is denied.
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 superadmin = await axiosAuth('test_superadmin@test.com')

const processing = (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 res = await superadmin.get(
`/api/v1/processings/${processing._id}/plugin`,
{ validateStatus: () => true }
)
expect([403, 404]).toContain(res.status)
const body = typeof res.data === 'string' ? res.data : JSON.stringify(res.data)
expect(body).toContain(fixture.pluginId)
})
})
2 changes: 1 addition & 1 deletion ui/src/components/processing/processing-card.vue
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ const props = defineProps({
showOwner: Boolean
})

const pluginFetch = usePluginFetch(props.processing.plugin)
const pluginFetch = usePluginFetch(props.processing._id)

</script>

Expand Down
28 changes: 15 additions & 13 deletions ui/src/composables/use-plugin-fetch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { $apiPath } from '../context'

// Subset of registry's Artefact shape that the UI uses. The artefact `_id` is
// exactly what we store on `processing.plugin`, so we fetch the artefact by that.
// exactly what we store on `processing.plugin`.
export interface RegistryArtefact {
_id: string
name: string
Expand All @@ -16,24 +18,24 @@ export interface RegistryArtefact {
const fetches: Record<string, ReturnType<typeof useFetch<RegistryArtefact>>> = {}

/**
* Fetch artefact metadata from the registry for a processing's `plugin`
* (the registry artefact id).
* Fetch a processing's plugin metadata through the processings API
* (`GET /processings/:id/plugin`). The API checks the caller's permission on
* the processing, then fetches the artefact from the registry as the owner —
* so a user with only an individual permission still sees the plugin even
* without a personal registry grant.
*
* Errors (404 deleted, 403 no access) are NOT broadcast as a global ui
* notification — callers read `error.value` and render their own inline
* state. See processing-card.vue and pages/processings/[id]/index.vue.
*
* Same-domain assumption: registry is always mounted at `/registry` of the
* current domain. The session cookie is sent automatically.
* notification — callers read `error.value` and render their own inline state.
* See processing-card.vue and pages/processings/[id]/index.vue.
*/
export const usePluginFetch = (pluginId: string) => {
if (!fetches[pluginId]) {
fetches[pluginId] = useFetch<RegistryArtefact>(
`/registry/api/v1/artefacts/${encodeURIComponent(pluginId)}`,
export const usePluginFetch = (processingId: string) => {
if (!fetches[processingId]) {
fetches[processingId] = useFetch<RegistryArtefact>(
`${$apiPath}/processings/${encodeURIComponent(processingId)}/plugin`,
{ notifError: false }
)
}
return fetches[pluginId]
return fetches[processingId]
}

export default usePluginFetch
10 changes: 6 additions & 4 deletions ui/src/pages/processings/[id]/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,15 @@ async function fetchProcessing () {
- configSchema null: legitimate "this plugin ships no schema" — distinct
from a registry error and does NOT trigger the banner.

Same-domain assumption: registry is mounted at `/registry` of the current
domain, so an absolute path bypasses `$fetch`'s `/processings/api/v1`
baseURL (which would rewrite `/registry/...` to a 404).
Plugin metadata now flows through the processings API
(`/processings/:id/plugin`) rather than a direct same-origin `/registry`
call: the API gates on the caller's processing permission, then fetches the
artefact as the owner. This lets individually-permitted viewers see the
plugin without their own registry grant.
*/
const pluginFetch = useFetch<RegistryArtefact>(
computed(() => processing.value?.plugin
? `/registry/api/v1/artefacts/${encodeURIComponent(processing.value.plugin)}`
? `${$apiPath}/processings/${processingId}/plugin`
: null),
{ notifError: false }
)
Expand Down
Loading