Skip to content

Commit a8a0857

Browse files
bourgeoaTallTed
andauthored
Make NSS test-suite and spec compliant (#1557)
* adds create container with PUT and more (#1543) * delete root returns 405 * delete root/rootAcl throw 405 * no POST for auxiliary resources * no resource and container with same name * fake commit * reverse fake commit * fake * reverse fake * fake * reverse fake * delete pod * cleaning * cleaning * some more cleaning * content-type must * cleaning * content-type do not default * cleaning * update CRUD checks to solid specifications * PATCH check filename is not a folder * cleaning * pubsub on parent * add PUT for container * cleaning * create container with PUT (#1545) * delete root/rootAcl * no POST for auxiliary resources * no resource and container with same name except '/' * content-type is a MUST (is it a must for create container ?) * content-type do not default * update CRUD checks to solid specifications * pubsub on parent * add PUT for container * function createDirectory() for PUT PATCH * delete acl from cache if not exists * PUT do not need req.headers link (#1547) and typos * make NSS test-suite compliant (#1554) * PUT do not need req.headers link * make NSS test-suite compliant * typings : Update lib/acl-checker.js Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com> * typings : Update lib/acl-checker.js Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com> Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
1 parent 44e9ce7 commit a8a0857

File tree

14 files changed

+374
-141
lines changed

14 files changed

+374
-141
lines changed

lib/acl-checker.js

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict'
22
/* eslint-disable node/no-deprecated-api */
33

4+
const { dirname } = require('path')
45
const rdf = require('rdflib')
56
const debug = require('./debug').ACL
67
const debugCache = require('./debug').cache
@@ -39,8 +40,8 @@ class ACLChecker {
3940
}
4041

4142
// Returns a fulfilled promise when the user can access the resource
42-
// in the given mode, or rejects with an HTTP error otherwise
43-
async can (user, mode) {
43+
// in the given mode; otherwise, rejects with an HTTP error
44+
async can (user, mode, method = 'GET', resourceExists = true) {
4445
const cacheKey = `${mode}-${user}`
4546
if (this.aclCached[cacheKey]) {
4647
return this.aclCached[cacheKey]
@@ -64,10 +65,10 @@ class ACLChecker {
6465
resource = rdf.sym(this.resource.substring(0, this.resource.length - this.suffix.length))
6566
}
6667
// If the slug is an acl, reject
67-
if (this.isAcl(this.slug)) {
68+
/* if (this.isAcl(this.slug)) {
6869
this.aclCached[cacheKey] = Promise.resolve(false)
6970
return this.aclCached[cacheKey]
70-
}
71+
} */
7172
const directory = acl.isContainer ? rdf.sym(ACLChecker.getDirectory(acl.acl)) : null
7273
const aclFile = rdf.sym(acl.acl)
7374
const agent = user ? rdf.sym(user) : null
@@ -84,7 +85,19 @@ class ACLChecker {
8485
// FIXME: https://github.com/solid/acl-check/issues/23
8586
// console.error(e.message)
8687
}
87-
const accessDenied = aclCheck.accessDenied(acl.graph, resource, directory, aclFile, agent, modes, agentOrigin, trustedOrigins, originTrustedModes)
88+
let accessDenied = aclCheck.accessDenied(acl.graph, resource, directory, aclFile, agent, modes, agentOrigin, trustedOrigins, originTrustedModes)
89+
90+
// For create and update HTTP methods
91+
if ((method === 'PUT' || method === 'PATCH' || method === 'COPY') && directory) {
92+
// if resource and acl have same parent container,
93+
// and resource does not exist, then accessTo Append from parent is required
94+
if (directory.value === dirname(aclFile.value) + '/' && !resourceExists) {
95+
const accessDeniedAccessTo = aclCheck.accessDenied(acl.graph, directory, null, aclFile, agent, [ACL('Append')], agentOrigin, trustedOrigins, originTrustedModes)
96+
const accessResult = !accessDenied && !accessDeniedAccessTo
97+
accessDenied = accessResult ? false : accessDenied || accessDeniedAccessTo
98+
// debugCache('accessDenied result ' + accessDenied)
99+
}
100+
}
88101
if (accessDenied && user) {
89102
this.messagesCached[cacheKey].push(HTTPError(403, accessDenied))
90103
} else if (accessDenied) {
@@ -96,6 +109,7 @@ class ACLChecker {
96109

97110
async getError (user, mode) {
98111
const cacheKey = `${mode}-${user}`
112+
// TODO ?? add to can: req.method and resourceExists. Actually all tests pass
99113
this.aclCached[cacheKey] = this.aclCached[cacheKey] || this.can(user, mode)
100114
const isAllowed = await this.aclCached[cacheKey]
101115
return isAllowed ? null : this.messagesCached[cacheKey].reduce((prevMsg, msg) => msg.status > prevMsg.status ? msg : prevMsg, { status: 0 })
@@ -206,6 +220,8 @@ function fetchLocalOrRemote (mapper, serverUri) {
206220
try {
207221
({ path, contentType } = await mapper.mapUrlToFile({ url }))
208222
} catch (err) {
223+
// delete from cache
224+
delete temporaryCache[url]
209225
throw new HTTPError(404, err)
210226
}
211227
// Read the file from disk
@@ -220,10 +236,10 @@ function fetchLocalOrRemote (mapper, serverUri) {
220236
}
221237
return async function fetch (url, graph = rdf.graph()) {
222238
if (!temporaryCache[url]) {
223-
debugCache('Populating cache', url)
239+
// debugCache('Populating cache', url)
224240
temporaryCache[url] = {
225241
timer: setTimeout(() => {
226-
debugCache('Expunging from cache', url)
242+
// debugCache('Expunging from cache', url)
227243
delete temporaryCache[url]
228244
if (Object.keys(temporaryCache).length === 0) {
229245
debugCache('Cache is empty again')
@@ -232,7 +248,7 @@ function fetchLocalOrRemote (mapper, serverUri) {
232248
promise: doFetch(url)
233249
}
234250
}
235-
debugCache('Cache hit', url)
251+
// debugCache('Cache hit', url)
236252
const { body, contentType } = await temporaryCache[url].promise
237253
// Parse the file as Turtle
238254
rdf.parse(body, graph, url, contentType)
@@ -248,7 +264,8 @@ function lastSlash (string, pos = string.length) {
248264
module.exports = ACLChecker
249265
module.exports.DEFAULT_ACL_SUFFIX = DEFAULT_ACL_SUFFIX
250266

251-
// Used in the unit tests:
252-
module.exports.clearAclCache = function () {
253-
temporaryCache = {}
267+
// Used in ldp and the unit tests:
268+
module.exports.clearAclCache = function (url) {
269+
if (url) delete temporaryCache[url]
270+
else temporaryCache = {}
254271
}

lib/handlers/allow.js

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
module.exports = allow
22

3-
const path = require('path')
3+
// const path = require('path')
44
const ACL = require('../acl-checker')
55
const debug = require('../debug.js').ACL
6+
// const error = require('../http-error')
67

7-
function allow (mode, checkPermissionsForDirectory) {
8+
function allow (mode) {
89
return async function allowHandler (req, res, next) {
910
const ldp = req.app.locals.ldp || {}
1011
if (!ldp.webid) {
@@ -20,11 +21,6 @@ function allow (mode, checkPermissionsForDirectory) {
2021
? res.locals.path
2122
: req.path
2223

23-
// Check permissions of the directory instead of the file itself.
24-
if (checkPermissionsForDirectory) {
25-
resourcePath = path.dirname(resourcePath)
26-
}
27-
2824
// Check whether the resource exists
2925
let stat
3026
try {
@@ -49,7 +45,7 @@ function allow (mode, checkPermissionsForDirectory) {
4945

5046
// Ensure the user has the required permission
5147
const userId = req.session.userId
52-
const isAllowed = await req.acl.can(userId, mode)
48+
const isAllowed = await req.acl.can(userId, mode, req.method, stat)
5349
if (isAllowed) {
5450
return next()
5551
}

lib/handlers/delete.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ async function handler (req, res, next) {
1313
next()
1414
} catch (err) {
1515
debug('DELETE -- Failed to delete: ' + err)
16+
17+
// method DELETE not allowed
18+
if (err.status === 405) {
19+
res.set('allow', 'OPTIONS, HEAD, GET, PATCH, POST, PUT')
20+
}
1621
next(err)
1722
}
1823
}

lib/handlers/error-pages.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ function renderLoginRequired (req, res, err) {
147147
*/
148148
function renderNoPermission (req, res, err) {
149149
const currentUrl = util.fullUrlForReq(req)
150-
const webId = req.session.userId
150+
let webId = ''
151+
if (req.session) webId = req.session.userId
151152
debug(`Display no-permission for ${currentUrl}`)
152153
res.statusMessage = err.message
153154
res.status(403)

lib/handlers/patch.js

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,13 @@ module.exports = handler
44

55
const bodyParser = require('body-parser')
66
const fs = require('fs')
7-
const mkdirp = require('fs-extra').mkdirp
87
const debug = require('../debug').handlers
98
const error = require('../http-error')
109
const $rdf = require('rdflib')
1110
const crypto = require('crypto')
1211
const overQuota = require('../utils').overQuota
1312
const getContentType = require('../utils').getContentType
1413
const withLock = require('../lock')
15-
const { promisify } = require('util')
16-
const pathUtility = require('path')
1714

1815
// Patch parsers by request body content type
1916
const PATCH_PARSERS = {
@@ -31,13 +28,17 @@ async function patchHandler (req, res, next) {
3128
// Obtain details of the target resource
3229
const ldp = req.app.locals.ldp
3330
let path, contentType
31+
let resourceExists = true
3432
try {
3533
// First check if the file already exists
3634
({ path, contentType } = await ldp.resourceMapper.mapUrlToFile({ url: req }))
3735
} catch (err) {
3836
// If the file doesn't exist, request one to be created with the default content type
3937
({ path, contentType } = await ldp.resourceMapper.mapUrlToFile(
4038
{ url: req, createIfNotExists: true, contentType: DEFAULT_FOR_NEW_CONTENT_TYPE }))
39+
// check if a folder with same name exists
40+
await ldp.checkItemName(req)
41+
resourceExists = false
4142
}
4243
const { url } = await ldp.resourceMapper.mapFileToUrl({ path, hostname: req.hostname })
4344
const resource = { path, contentType, url }
@@ -56,26 +57,10 @@ async function patchHandler (req, res, next) {
5657

5758
// Parse the patch document and verify permissions
5859
const patchObject = await parsePatch(url, patch.uri, patch.text)
59-
await checkPermission(req, patchObject)
60-
61-
// Check to see if folder exists
62-
const pathSplit = path.split('/')
63-
const directory = pathSplit.slice(0, pathSplit.length - 1).join('/')
64-
if (!fs.existsSync(directory)) {
65-
const firstDirectoryRecursivelyCreated = await promisify(mkdirp)(directory)
66-
if (ldp.live) {
67-
// Get parent for the directory made
68-
const parentDirectoryPath =
69-
pathUtility.dirname(firstDirectoryRecursivelyCreated) + pathUtility.sep
70-
// Get the url for the parent
71-
const parentDirectoryUrl = (await ldp.resourceMapper.mapFileToUrl({
72-
path: parentDirectoryPath,
73-
hostname: req.hostname
74-
})).url
75-
// Update websockets
76-
ldp.live(new URL(parentDirectoryUrl).pathname)
77-
}
78-
}
60+
await checkPermission(req, patchObject, resourceExists)
61+
62+
// Create the enclosing directory, if necessary
63+
await ldp.createDirectory(path, req.hostname)
7964

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

135120
// Verifies whether the user is allowed to perform the patch on the target
136-
async function checkPermission (request, patchObject) {
121+
async function checkPermission (request, patchObject, resourceExists) {
137122
// If no ACL object was passed down, assume permissions are okay.
138123
if (!request.acl) return Promise.resolve(patchObject)
139124
// At this point, we already assume append access,
140125
// as this can be checked upfront before parsing the patch.
141126
// Now that we know the details of the patch,
142127
// we might need to perform additional checks.
143128
let modes = []
129+
// acl:default Write is required for create
130+
if (!resourceExists) modes = ['Write']
144131
const { acl, session: { userId } } = request
145132
// Read access is required for DELETE and WHERE.
146133
// If we would allows users without read access,
147134
// they could use DELETE or WHERE to trigger 200 or 409,
148135
// and thereby guess the existence of certain triples.
149136
// DELETE additionally requires write access.
150137
if (patchObject.delete) {
138+
// ACTUALLY Read not needed by solid/test-suite only Write
151139
modes = ['Read', 'Write']
152140
// checks = [acl.can(userId, 'Read'), acl.can(userId, 'Write')]
153141
} else if (patchObject.where) {
154-
modes = ['Read']
142+
modes = modes.concat(['Read'])
155143
// checks = [acl.can(userId, 'Read')]
156144
}
157-
const allowed = await Promise.all(modes.map(mode => acl.can(userId, mode)))
145+
const allowed = await Promise.all(modes.map(mode => acl.can(userId, mode, request.method, resourceExists)))
158146
const allAllowed = allowed.reduce((memo, allowed) => memo && allowed, true)
159147
if (!allAllowed) {
160148
const errors = await Promise.all(modes.map(mode => acl.getError(userId, mode)))

lib/handlers/put.js

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,18 @@ async function handler (req, res, next) {
1818
return putStream(req, res, next)
1919
}
2020

21+
// TODO could be renamed as putResource (it now covers container and non-container)
2122
async function putStream (req, res, next, stream = req) {
2223
const ldp = req.app.locals.ldp
2324
try {
25+
debug('test ' + req.get('content-type') + getContentType(req.headers))
2426
await ldp.put(req, stream, getContentType(req.headers))
25-
debug('succeded putting the file')
26-
27+
debug('succeded putting the file/folder')
2728
res.sendStatus(201)
2829
return next()
2930
} catch (err) {
30-
debug('error putting the file:' + err.message)
31-
err.message = 'Can\'t write file: ' + err.message
31+
debug('error putting the file/folder:' + err.message)
32+
err.message = 'Can\'t write file/folder: ' + err.message
3233
return next(err)
3334
}
3435
}

lib/ldp-middleware.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ function LdpMiddleware (corsSettings) {
2626
router.post('/*', allow('Append'), post)
2727
router.patch('/*', allow('Append'), patch)
2828
router.put('/*', allow('Write'), put)
29-
router.delete('/*', allow('Write', true), del)
29+
router.delete('/*', allow('Write'), del)
3030

3131
return router
3232
}

0 commit comments

Comments
 (0)