Skip to content

Commit

Permalink
fix: PSA compliance fixes (#2091)
Browse files Browse the repository at this point in the history 
resolves #1579
  • Loading branch information
@flea89
flea89 authored Nov 30, 2022
1 parent 0013efd commit 8b1c1d3
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 38 deletions.
7 changes: 4 additions & 3 deletions packages/api/src/pins.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export async function pinsGet (request, env, ctx) {
/**
* Transform a PinRequest into a PinStatus
*
* @param { Object } pinRequest
* @param { import('../../db/db-client-types.js').PsaPinRequestUpsertOutput } pinRequest
* @returns { PsaPinStatusResponse }
*/
export function toPinStatusResponse (pinRequest) {
Expand All @@ -197,9 +197,10 @@ export function toPinStatusResponse (pinRequest) {
created: pinRequest.created,
pin: {
cid: pinRequest.sourceCid,
...pinRequest
...pinRequest.name && { name: pinRequest.name },
...pinRequest.origins && { origins: pinRequest.origins },
...pinRequest.meta && { meta: pinRequest.meta }
},
// TODO(https://github.com/web3-storage/web3.storage/issues/792)
delegates: []
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/api/src/utils/psa.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const MAX_PIN_LISTING_LIMIT = 1000
// Validation schemas
const listPinsValidator = new Validator({
type: 'object',
required: ['status'],
required: [],
properties: {
name: { type: 'string', maxLength: 255 },
after: { type: 'string', format: 'date-time' },
Expand Down Expand Up @@ -234,7 +234,7 @@ export function validateSearchParams (queryString) {

if (result.valid) {
// Map statuses for DB compatibility.
opts.statuses = psaStatusesToDBStatuses(opts.status)
opts.statuses = opts.status && psaStatusesToDBStatuses(opts.status)
data = opts
} else {
error = parseValidatorErrors(result.errors)
Expand Down
71 changes: 43 additions & 28 deletions packages/api/test/pin.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import {
} from '../src/utils/psa.js'
import { PSAErrorResourceNotFound, PSAErrorInvalidData, PSAErrorRequiredData } from '../src/errors.js'

const REQUEST_EXPECTED_PROPERTIES = ['requestid', 'status', 'created', 'delegates', 'info', 'pin']
const PIN_EXPECTED_PROPERTIES = ['cid', 'name', 'origins', 'meta']

/**
*
* @param {import('../../db/postgres/pg-rest-api-types').definitions['pin']['status']} status
Expand Down Expand Up @@ -41,24 +44,41 @@ const assertCorrectPinResponse = (data) => {
assert.ok(Date.parse(data.created), 'created should be valid date string')
assert.ok(Array.isArray(data.delegates), 'delegates should be an array')

if (data.info) {
// @ts-ignore https://github.com/microsoft/TypeScript/issues/44253
if (Object.hasOwn(data, 'info')) {
assert.ok(typeof data.info === 'object', 'info should be an object')
}

assert.ok(typeof data.pin === 'object', 'pin should be an object')
assert.ok(typeof data.pin.cid === 'string', 'pin.cid should be an string')

if (data.pin.name) {
// @ts-ignore https://github.com/microsoft/TypeScript/issues/44253
if (Object.hasOwn(data.pin, 'name')) {
assert.ok(typeof data.pin.name === 'string', 'pin.name should be an string')
}

if (data.pin.origins) {
// @ts-ignore https://github.com/microsoft/TypeScript/issues/44253
if (Object.hasOwn(data.pin, 'origins')) {
assert.ok(Array.isArray(data.pin.origins), 'pin.origins should be an array')
}

if (data.pin.meta) {
// @ts-ignore https://github.com/microsoft/TypeScript/issues/44253
if (Object.hasOwn(data.pin, 'meta')) {
assert.ok(typeof data.pin.meta === 'object', 'pin.meta should be an object')
}

assert.ok(Object.keys(data).every(property => REQUEST_EXPECTED_PROPERTIES.includes(property)), 'request contains not valid properties')
assert.ok(Object.keys(data.pin).every(property => PIN_EXPECTED_PROPERTIES.includes(property)), 'request.pin contains not valid properties')
}

/**
*
* @param {object} data
*/
const assertCorretPinResponseList = (data) => {
assert.ok(typeof data.count === 'number', 'count should be a number')
data.results.forEach(r => assertCorrectPinResponse(r))
Object.keys(data).every(property => ['count', 'results'].includes(property))
}

/**
Expand Down Expand Up @@ -121,24 +141,6 @@ describe('Pinning APIs endpoints', () => {
assert.strictEqual(error.details, '#/limit: Instance type "number" is invalid. Expected "integer".')
})

it('requires status', async () => {
const url = new URL(`${baseUrl}`).toString()
const res = await fetch(
url, {
method: 'GET',
headers: {
Authorization: `Bearer ${token}`,
'Content-Type': 'application/json'
}
})

assert(res, 'Server responded')
assert.strictEqual(res.status, ERROR_CODE)
const { error } = await res.json()
assert.strictEqual(error.reason, PSAErrorRequiredData.CODE)
assert.strictEqual(error.details, 'Instance does not have required property "status".')
})

it('validates meta filter is json object', async () => {
const opts = new URLSearchParams({
meta: `[
Expand Down Expand Up @@ -251,9 +253,7 @@ describe('Pinning APIs endpoints', () => {
})

it('returns only successful pins when no filter values are specified', async () => {
const opts = new URLSearchParams({
status: 'pinned'
})
const opts = new URLSearchParams({})
const url = new URL(`${baseUrl}?${opts}`).toString()
const res = await fetch(
url, {
Expand All @@ -268,6 +268,7 @@ describe('Pinning APIs endpoints', () => {
assert(res.ok, 'Server response is ok')
const data = await res.json()
assert.strictEqual(data.count, 1)
assertCorretPinResponseList(data)
})

it('filters pins on CID, for this user', async () => {
Expand All @@ -294,6 +295,7 @@ describe('Pinning APIs endpoints', () => {
assert(res.ok, 'Server response is ok')
const data = await res.json()
assert.strictEqual(data.count, 2)
assertCorretPinResponseList(data)
})

it('filters case sensitive exact match on name', async () => {
Expand All @@ -315,6 +317,7 @@ describe('Pinning APIs endpoints', () => {
assert(res.ok, 'Server response is ok')
const data = await res.json()
assert.strictEqual(data.count, 1)
assertCorretPinResponseList(data)
})

it('filters case insensitive partial match on name', async () => {
Expand All @@ -337,6 +340,7 @@ describe('Pinning APIs endpoints', () => {
assert(res.ok, 'Server response is ok')
const data = await res.json()
assert.strictEqual(data.count, 4)
assertCorretPinResponseList(data)
})

it('filters pins by status', async () => {
Expand All @@ -359,6 +363,7 @@ describe('Pinning APIs endpoints', () => {
assert.strictEqual(data.count, 1)
assert.strictEqual(data.results.length, 1)
assert.strictEqual(data.results[0].pin.name, 'FailedPinning.doc')
assertCorretPinResponseList(data)
})

it('filters pins by multiple statuses', async () => {
Expand All @@ -379,6 +384,7 @@ describe('Pinning APIs endpoints', () => {
assert(res.ok, 'Server response is ok')
const data = await res.json()
assert.strictEqual(data.count, 5)
assertCorretPinResponseList(data)
})

it('filters pins by queued', async () => {
Expand All @@ -399,6 +405,7 @@ describe('Pinning APIs endpoints', () => {
assert(res.ok, 'Server response is ok')
const data = await res.json()
assert.strictEqual(data.count, 1)
assertCorretPinResponseList(data)
})

it('filters pins created before a date', async () => {
Expand All @@ -421,6 +428,7 @@ describe('Pinning APIs endpoints', () => {
const data = await res.json()
assert.strictEqual(data.results.length, 1)
assert.strictEqual(data.count, 1)
assertCorretPinResponseList(data)
})

it('filters pins created after a date', async () => {
Expand All @@ -443,6 +451,7 @@ describe('Pinning APIs endpoints', () => {
const data = await res.json()
assert.strictEqual(data.results.length, 2)
assert.strictEqual(data.count, 2)
assertCorretPinResponseList(data)
})

it('limits the number of pins returned for this user and includes the total', async () => {
Expand All @@ -465,6 +474,7 @@ describe('Pinning APIs endpoints', () => {
const data = await res.json()
assert.strictEqual(data.count, 7)
assert.strictEqual(data.results.length, 3)
assertCorretPinResponseList(data)
})

it('filters pins by meta', async () => {
Expand All @@ -487,6 +497,7 @@ describe('Pinning APIs endpoints', () => {
const data = await res.json()
assert.strictEqual(data.count, 1)
assert.strictEqual(data.results[0].pin.name, 'Image.jpeg')
assertCorretPinResponseList(data)
})
})

Expand All @@ -509,10 +520,12 @@ describe('Pinning APIs endpoints', () => {
assert(res, 'Server responded')
assert(res.ok, 'Server response ok')
const data = await res.json()
assertCorrectPinResponse(data)
assert.strictEqual(data.pin.cid, cid)
assert.strictEqual(data.pin.name, null)
assert.strictEqual(data.pin.origins, null)
assert.strictEqual(data.pin.meta, null)
console.log(data.pin.cid, cid)
assert.strictEqual(data.pin.name, undefined)
assert.strictEqual(data.pin.origins, undefined)
assert.strictEqual(data.pin.meta, undefined)
})

it('requires cid', async () => {
Expand Down Expand Up @@ -574,6 +587,7 @@ describe('Pinning APIs endpoints', () => {
const data = await res.json()
assert(res, 'Server responded')
assert(res.ok, 'Server response ok')
assertCorrectPinResponse(data)
assert.strictEqual(data.pin.cid, cid)
assert.deepStrictEqual(data.pin.name, name)
assert.deepStrictEqual(data.pin.origins, origins)
Expand Down Expand Up @@ -971,6 +985,7 @@ describe('Pinning APIs endpoints', () => {
assert(replaceResponse.ok, 'Replace request was not successful')
assert.strictEqual(replaceResponse.status, 202)
const data = await replaceResponse.json()
assertCorrectPinResponse(data)
assert.strictEqual(data.pin.cid, newCid)
assert.deepStrictEqual(data.pin.name, name)
assert.deepStrictEqual(data.pin.origins, origins)
Expand Down
2 changes: 1 addition & 1 deletion packages/api/test/user.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ describe('GET /user/pins', () => {

assert(res.ok)
const body = await res.json()
assert.equal([...new Set(body.results.map(x => x.pin.authKey))].length, 2)
assert.equal(body.count, 8)
})
})

Expand Down
17 changes: 13 additions & 4 deletions packages/db/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1168,6 +1168,10 @@ export class DBClient {
async listPsaPinRequests (authKey, opts = {}) {
const match = opts?.match || 'exact'
const limit = opts?.limit || 10
/**
* @type {Array.<string>|undefined}
*/
let statuses

let query = this._client
.from(psaPinRequestTableName)
Expand All @@ -1190,12 +1194,17 @@ export class DBClient {
query = query.range(rangeFrom, rangeTo - 1)
}

if (!opts.cid && !opts.name && !opts.statuses) {
query = query.eq('content.pins.status', 'Pinned')
// If not specified we default to pinned only if no other filters are provided.
// While slightly inconsistent, that's the current expectation.
// This is being discussed in https://github.com/ipfs-shipyard/pinning-service-compliance/issues/245
if (!opts.cid && !opts.name && !opts.meta && !opts.statuses) {
statuses = ['Pinned']
} else {
statuses = opts.statuses
}

if (opts.statuses) {
query = query.in('content.pins.status', opts.statuses)
if (statuses) {
query = query.in('content.pins.status', statuses)
}

if (opts.cid) {
Expand Down

0 comments on commit 8b1c1d3

Please sign in to comment.