Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NSS test-suite and spec compliant #1557

Merged
merged 6 commits into from
Jan 26, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
37 changes: 27 additions & 10 deletions lib/acl-checker.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'
/* eslint-disable node/no-deprecated-api */

const { dirname } = require('path')
const rdf = require('rdflib')
const debug = require('./debug').ACL
const debugCache = require('./debug').cache
Expand Down Expand Up @@ -40,7 +41,7 @@ class ACLChecker {

// Returns a fulfilled promise when the user can access the resource
// in the given mode, or rejects with an HTTP error otherwise
bourgeoa marked this conversation as resolved.
Show resolved Hide resolved
async can (user, mode) {
async can (user, mode, method = 'GET', resourceExists = true) {
const cacheKey = `${mode}-${user}`
if (this.aclCached[cacheKey]) {
return this.aclCached[cacheKey]
Expand All @@ -64,10 +65,10 @@ class ACLChecker {
resource = rdf.sym(this.resource.substring(0, this.resource.length - this.suffix.length))
}
// If the slug is an acl, reject
if (this.isAcl(this.slug)) {
/* if (this.isAcl(this.slug)) {
this.aclCached[cacheKey] = Promise.resolve(false)
return this.aclCached[cacheKey]
}
} */
const directory = acl.isContainer ? rdf.sym(ACLChecker.getDirectory(acl.acl)) : null
const aclFile = rdf.sym(acl.acl)
const agent = user ? rdf.sym(user) : null
Expand All @@ -84,7 +85,19 @@ class ACLChecker {
// FIXME: https://github.com/solid/acl-check/issues/23
// console.error(e.message)
}
const accessDenied = aclCheck.accessDenied(acl.graph, resource, directory, aclFile, agent, modes, agentOrigin, trustedOrigins, originTrustedModes)
let accessDenied = aclCheck.accessDenied(acl.graph, resource, directory, aclFile, agent, modes, agentOrigin, trustedOrigins, originTrustedModes)

// For create and update HTTP methods
if ((method === 'PUT' || method === 'PATCH' || method === 'COPY') && directory) {
// if resource and acl have same parent container,
// and resource do not exists then accessTo Append from parent is required
bourgeoa marked this conversation as resolved.
Show resolved Hide resolved
if (directory.value === dirname(aclFile.value) + '/' && !resourceExists) {
const accessDeniedAccessTo = aclCheck.accessDenied(acl.graph, directory, null, aclFile, agent, [ACL('Append')], agentOrigin, trustedOrigins, originTrustedModes)
const accessResult = !accessDenied && !accessDeniedAccessTo
accessDenied = accessResult ? false : accessDenied || accessDeniedAccessTo
// debugCache('accessDenied result ' + accessDenied)
}
}
if (accessDenied && user) {
this.messagesCached[cacheKey].push(HTTPError(403, accessDenied))
} else if (accessDenied) {
Expand All @@ -96,6 +109,7 @@ class ACLChecker {

async getError (user, mode) {
const cacheKey = `${mode}-${user}`
// TODO ?? add to can: req.method and resourceExists. Actually all tests pass
this.aclCached[cacheKey] = this.aclCached[cacheKey] || this.can(user, mode)
const isAllowed = await this.aclCached[cacheKey]
return isAllowed ? null : this.messagesCached[cacheKey].reduce((prevMsg, msg) => msg.status > prevMsg.status ? msg : prevMsg, { status: 0 })
Expand Down Expand Up @@ -206,6 +220,8 @@ function fetchLocalOrRemote (mapper, serverUri) {
try {
({ path, contentType } = await mapper.mapUrlToFile({ url }))
} catch (err) {
// delete from cache
delete temporaryCache[url]
throw new HTTPError(404, err)
}
// Read the file from disk
Expand All @@ -220,10 +236,10 @@ function fetchLocalOrRemote (mapper, serverUri) {
}
return async function fetch (url, graph = rdf.graph()) {
if (!temporaryCache[url]) {
debugCache('Populating cache', url)
// debugCache('Populating cache', url)
temporaryCache[url] = {
timer: setTimeout(() => {
debugCache('Expunging from cache', url)
// debugCache('Expunging from cache', url)
delete temporaryCache[url]
if (Object.keys(temporaryCache).length === 0) {
debugCache('Cache is empty again')
Expand All @@ -232,7 +248,7 @@ function fetchLocalOrRemote (mapper, serverUri) {
promise: doFetch(url)
}
}
debugCache('Cache hit', url)
// debugCache('Cache hit', url)
const { body, contentType } = await temporaryCache[url].promise
// Parse the file as Turtle
rdf.parse(body, graph, url, contentType)
Expand All @@ -248,7 +264,8 @@ function lastSlash (string, pos = string.length) {
module.exports = ACLChecker
module.exports.DEFAULT_ACL_SUFFIX = DEFAULT_ACL_SUFFIX

// Used in the unit tests:
module.exports.clearAclCache = function () {
temporaryCache = {}
// Used in ldp and the unit tests:
module.exports.clearAclCache = function (url) {
if (url) delete temporaryCache[url]
else temporaryCache = {}
}
12 changes: 4 additions & 8 deletions lib/handlers/allow.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
module.exports = allow

const path = require('path')
// const path = require('path')
const ACL = require('../acl-checker')
const debug = require('../debug.js').ACL
// const error = require('../http-error')

function allow (mode, checkPermissionsForDirectory) {
function allow (mode) {
return async function allowHandler (req, res, next) {
const ldp = req.app.locals.ldp || {}
if (!ldp.webid) {
Expand All @@ -20,11 +21,6 @@ function allow (mode, checkPermissionsForDirectory) {
? res.locals.path
: req.path

// Check permissions of the directory instead of the file itself.
if (checkPermissionsForDirectory) {
resourcePath = path.dirname(resourcePath)
}

// Check whether the resource exists
let stat
try {
Expand All @@ -49,7 +45,7 @@ function allow (mode, checkPermissionsForDirectory) {

// Ensure the user has the required permission
const userId = req.session.userId
const isAllowed = await req.acl.can(userId, mode)
const isAllowed = await req.acl.can(userId, mode, req.method, stat)
if (isAllowed) {
return next()
}
Expand Down
5 changes: 5 additions & 0 deletions lib/handlers/delete.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ async function handler (req, res, next) {
next()
} catch (err) {
debug('DELETE -- Failed to delete: ' + err)

// method DELETE not allowed
if (err.status === 405) {
res.set('allow', 'OPTIONS, HEAD, GET, PATCH, POST, PUT')
}
next(err)
}
}
3 changes: 2 additions & 1 deletion lib/handlers/error-pages.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ function renderLoginRequired (req, res, err) {
*/
function renderNoPermission (req, res, err) {
const currentUrl = util.fullUrlForReq(req)
const webId = req.session.userId
let webId = ''
if (req.session) webId = req.session.userId
debug(`Display no-permission for ${currentUrl}`)
res.statusMessage = err.message
res.status(403)
Expand Down
40 changes: 14 additions & 26 deletions lib/handlers/patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,13 @@ module.exports = handler

const bodyParser = require('body-parser')
const fs = require('fs')
const mkdirp = require('fs-extra').mkdirp
const debug = require('../debug').handlers
const error = require('../http-error')
const $rdf = require('rdflib')
const crypto = require('crypto')
const overQuota = require('../utils').overQuota
const getContentType = require('../utils').getContentType
const withLock = require('../lock')
const { promisify } = require('util')
const pathUtility = require('path')

// Patch parsers by request body content type
const PATCH_PARSERS = {
Expand All @@ -31,13 +28,17 @@ async function patchHandler (req, res, next) {
// Obtain details of the target resource
const ldp = req.app.locals.ldp
let path, contentType
let resourceExists = true
try {
// First check if the file already exists
({ path, contentType } = await ldp.resourceMapper.mapUrlToFile({ url: req }))
} catch (err) {
// If the file doesn't exist, request one to be created with the default content type
({ path, contentType } = await ldp.resourceMapper.mapUrlToFile(
{ url: req, createIfNotExists: true, contentType: DEFAULT_FOR_NEW_CONTENT_TYPE }))
// check if a folder with same name exists
await ldp.checkItemName(req)
resourceExists = false
}
const { url } = await ldp.resourceMapper.mapFileToUrl({ path, hostname: req.hostname })
const resource = { path, contentType, url }
Expand All @@ -56,26 +57,10 @@ async function patchHandler (req, res, next) {

// Parse the patch document and verify permissions
const patchObject = await parsePatch(url, patch.uri, patch.text)
await checkPermission(req, patchObject)

// Check to see if folder exists
const pathSplit = path.split('/')
const directory = pathSplit.slice(0, pathSplit.length - 1).join('/')
if (!fs.existsSync(directory)) {
const firstDirectoryRecursivelyCreated = await promisify(mkdirp)(directory)
if (ldp.live) {
// Get parent for the directory made
const parentDirectoryPath =
pathUtility.dirname(firstDirectoryRecursivelyCreated) + pathUtility.sep
// Get the url for the parent
const parentDirectoryUrl = (await ldp.resourceMapper.mapFileToUrl({
path: parentDirectoryPath,
hostname: req.hostname
})).url
// Update websockets
ldp.live(new URL(parentDirectoryUrl).pathname)
}
}
await checkPermission(req, patchObject, resourceExists)

// Create the enclosing directory, if necessary
await ldp.createDirectory(path, req.hostname)

// Patch the graph and write it back to the file
const result = await withLock(path, async () => {
Expand Down Expand Up @@ -133,28 +118,31 @@ function readGraph (resource) {
}

// Verifies whether the user is allowed to perform the patch on the target
async function checkPermission (request, patchObject) {
async function checkPermission (request, patchObject, resourceExists) {
// If no ACL object was passed down, assume permissions are okay.
if (!request.acl) return Promise.resolve(patchObject)
// At this point, we already assume append access,
// as this can be checked upfront before parsing the patch.
// Now that we know the details of the patch,
// we might need to perform additional checks.
let modes = []
// acl:default Write is required for create
if (!resourceExists) modes = ['Write']
const { acl, session: { userId } } = request
// Read access is required for DELETE and WHERE.
// If we would allows users without read access,
// they could use DELETE or WHERE to trigger 200 or 409,
// and thereby guess the existence of certain triples.
// DELETE additionally requires write access.
if (patchObject.delete) {
// ACTUALLY Read not needed by solid/test-suite only Write
modes = ['Read', 'Write']
// checks = [acl.can(userId, 'Read'), acl.can(userId, 'Write')]
} else if (patchObject.where) {
modes = ['Read']
modes = modes.concat(['Read'])
// checks = [acl.can(userId, 'Read')]
}
const allowed = await Promise.all(modes.map(mode => acl.can(userId, mode)))
const allowed = await Promise.all(modes.map(mode => acl.can(userId, mode, request.method, resourceExists)))
const allAllowed = allowed.reduce((memo, allowed) => memo && allowed, true)
if (!allAllowed) {
const errors = await Promise.all(modes.map(mode => acl.getError(userId, mode)))
Expand Down
9 changes: 5 additions & 4 deletions lib/handlers/put.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@ async function handler (req, res, next) {
return putStream(req, res, next)
}

// TODO could be renamed as putResource (it now covers container and non-container)
async function putStream (req, res, next, stream = req) {
const ldp = req.app.locals.ldp
try {
debug('test ' + req.get('content-type') + getContentType(req.headers))
await ldp.put(req, stream, getContentType(req.headers))
debug('succeded putting the file')

debug('succeded putting the file/folder')
res.sendStatus(201)
return next()
} catch (err) {
debug('error putting the file:' + err.message)
err.message = 'Can\'t write file: ' + err.message
debug('error putting the file/folder:' + err.message)
err.message = 'Can\'t write file/folder: ' + err.message
return next(err)
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/ldp-middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function LdpMiddleware (corsSettings) {
router.post('/*', allow('Append'), post)
router.patch('/*', allow('Append'), patch)
router.put('/*', allow('Write'), put)
router.delete('/*', allow('Write', true), del)
router.delete('/*', allow('Write'), del)

return router
}
Loading