search

inrupt / solid-client-js

Library for accessing data and managing permissions on data stored in a Solid Pod
https://docs.inrupt.com/developer-tools/javascript/client-libraries/
MIT License
230 stars 37 forks source link

set...Access does not update existing rules, adding triples #1019

Closed jaxoncreed closed 3 years ago

jaxoncreed commented 3 years ago

Search terms you've used

"ACL"

Bug description

setAgentDefaultAccess, setAgentResourceAccess, setPublicDefaultAccess, and setPublicResourceAccess do not modify rules if they already exist. The documentation claims that If rules already exist for the Agent in the given ACL, in the returned ACL, they are replaced by the new rules., but this does not happen in practice.

To Reproduce

  1. Create a folder

  2. Define this function for setting ACLs

    export async function setAcl(
url: string,
webIdAccess: Record<string, Access>
): Promise<void> {
// Fetch the SolidDataset and its associated ACLs, if available:
const myDatasetWithAcl = await getSolidDatasetWithAcl(url, {
fetch: (info, init) => {
  return fetcher(info, init)
},
});

// Obtain the SolidDataset's own ACL, if available,
// or initialise a new one, if possible:
if (
!hasResourceAcl(myDatasetWithAcl) &&
!hasAccessibleAcl(myDatasetWithAcl)
) {
throw new HttpError(
  "The current user does not have permission to change access rights to this Resource.",
  403
);
}

let resourceAcl = createAcl(myDatasetWithAcl);
// Give someone Control access to the given Resource:
Object.entries(webIdAccess).forEach(([webId, access]) => {
if (webId === "public") {
  resourceAcl = setPublicDefaultAccess(resourceAcl, access);
  resourceAcl = setPublicResourceAccess(resourceAcl, access);
} else {
  resourceAcl = setAgentDefaultAccess(resourceAcl, webId, access);
  resourceAcl = setAgentResourceAccess(resourceAcl, webId, access);
}
});

// Now save the ACL:
await saveAclFor(myDatasetWithAcl, resourceAcl, {
fetch: (info, init) => {
  return fetcher(info, init);
},
});
}

  3. Run the following

    await setAcl("https://pod.example/folder/", {
"https://jackson.solidcommunity.net/profile/card#me": {
"read": true,
"write": true,
"append": true,
"control": true,
},
"public": {
"read": true,
"write": true,
"append": true,
"control": false,
}
});

  4. That will cause the ACL to look like

    
@prefix : <#>.
@prefix n0: <http://www.w3.org/ns/auth/acl#>.
@prefix c: </profile/card#>.
@prefix tes: <./>.
@prefix n1: <http://xmlns.com/foaf/0.1/>.

:16200627777229416622674554878 a n0:Authorization; n0:agent c:me; n0:default tes:; n0:mode n0:Control, n0:Read, n0:Write. :16200627777242747520251968627 a n0:Authorization; n0:accessTo tes:; n0:agent c:me; n0:mode n0:Control, n0:Read, n0:Write. :16200627777336972880372581038 a n0:Authorization; n0:agentClass n1:Agent; n0:default tes:; n0:mode n0:Read, n0:Write. :16200627777355729480738631167 a n0:Authorization; n0:accessTo tes:; n0:agentClass n1:Agent; n0:mode n0:Read, n0:Write.

5. Now, run `setAcl` again as follows:
```typescript
await setAcl("https://pod.example/folder/", {
  "https://jackson.solidcommunity.net/profile/card#me": {
    "read": true,
    "write": true,
    "append": true,
    "control": true,
  },
  "public": {
    "read": true,
    "write": true,
    "append": true,
    "control": false,
  },
  "https://jackson2.inrupt.net/profile/card#me": {
    "read": true,
    "write": true,
    "append": true,
    "control": true,
  }
});
  1. The ACL now looks like: 
    
@prefix : <#>.
@prefix n0: <http://www.w3.org/ns/auth/acl#>.
@prefix c: </profile/card#>.
@prefix tes: <./>.
@prefix n1: <http://xmlns.com/foaf/0.1/>.
@prefix c0: <https://jackson2.inrupt.net/profile/card#>.

:16200627777229416622674554878 a n0:Authorization; n0:agent c:me; n0:default tes:; n0:mode n0:Control, n0:Read, n0:Write. :16200627777242747520251968627 a n0:Authorization; n0:accessTo tes:; n0:agent c:me; n0:mode n0:Control, n0:Read, n0:Write. :16200627777336972880372581038 a n0:Authorization; n0:agentClass n1:Agent; n0:default tes:; n0:mode n0:Read, n0:Write. :16200627777355729480738631167 a n0:Authorization; n0:accessTo tes:; n0:agentClass n1:Agent; n0:mode n0:Read, n0:Write. :16200630181165886215561149628 a n0:Authorization; n0:agent c:me; n0:default tes:; n0:mode n0:Control, n0:Read, n0:Write. :162006301811724778766789878337 a n0:Authorization; n0:accessTo tes:; n0:agent c:me; n0:mode n0:Control, n0:Read, n0:Write. :16200630181229927508212065623 a n0:Authorization; n0:agent c0:me; n0:default tes:; n0:mode n0:Read, n0:Write. :16200630181237677687525331884 a n0:Authorization; n0:accessTo tes:; n0:agent c0:me; n0:mode n0:Read, n0:Write. :162006301813334106972307351047 a n0:Authorization; n0:agentClass n1:Agent; n0:default tes:; n0:mode n0:Read, n0:Write. :162006301813808167874828088695 a n0:Authorization; n0:accessTo tes:; n0:agentClass n1:Agent; n0:mode n0:Read, n0:Write.


Notice that instead of ignoring the authorization rule for `c:me` and `n1:Agent`, it duplicated them. This can lead to a lot of unnecessary triples and was the reason for inrupt.net crashing last week.

### Expected result

When `setAgentResourceAccess` (or friends) is called with a WebId that already exists, it will simply update (or ignore if they are identical) existing rules rather than adding new ones.

### Actual result

New rules are added.

### Environment

$ npx envinfo --system --npmPackages --binaries --npmGlobalPackages --browsers System: OS: macOS 11.3 CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz Memory: 119.89 MB / 16.00 GB Shell: 5.8 - /bin/zsh Binaries: Node: 15.2.1 - /usr/local/bin/node npm: 7.0.10 - /usr/local/bin/npm Watchman: 4.9.0 - /usr/local/bin/watchman Browsers: Chrome: 90.0.4430.93 Firefox: 88.0 Safari: 14.1 npmPackages: @inrupt/solid-auth-fetcher: jaxoncreed/solid-auth-fetcher-fork#master => 0.0.6 @inrupt/solid-client: ^1.0.0 => 1.6.1 @inrupt/solid-client-authn-browser: ^1.0.0 => 1.8.0 @rdfjs/fetch: ^2.1.0 => 2.1.0 @types/bluebird: ^3.5.33 => 3.5.33 @types/clownface: ^1.0.1 => 1.2.0 @types/cookie-parser: ^1.4.2 => 1.4.2 @types/cors: ^2.8.7 => 2.8.10 @types/express: ^4.17.8 => 4.17.11 @types/express-session: ^1.17.0 => 1.17.3 @types/ioredis: ^4.17.8 => 4.26.0 @types/jest: ^26.0.14 => 26.0.22 @types/jsonschema: ^1.1.1 => 1.1.1 @types/mongodb: ^3.6.0 => 3.6.12 @types/multer: ^1.4.5 => 1.4.5 @types/node-cron: ^2.0.3 => 2.0.3 @types/node-fetch: ^2.5.7 => 2.5.10 @types/parse-link-header: ^1.0.0 => 1.0.0 @types/rdfjsdataset: ^1.0.4 => 1.0.4 @types/rdfjsfetch: ^2.0.2 => 2.0.2 @types/rdfjs__parser-n3: ^1.1.3 => 1.1.3 @types/socket.io: ^2.1.11 => 2.1.13 @types/url-parse: ^1.4.3 => 1.4.3 @types/uuid: ^8.3.0 => 8.3.0 @types/web-push: ^3.3.0 => 3.3.0 @types/ws: ^7.4.0 => 7.4.1 @typescript-eslint/eslint-plugin: ^4.2.0 => 4.22.0 @typescript-eslint/parser: ^4.2.0 => 4.22.0 bluebird: ^3.7.2 => 3.7.2 body-parser: ^1.19.0 => 1.19.0 clownface: ^1.0.0 => 1.2.0 cookie: ^0.4.0 => 0.4.1 cookie-parser: ^1.4.5 => 1.4.5 cors: ^2.8.5 => 2.8.5 dotenv: ^8.2.0 => 8.2.0 eslint: ^7.10.0 => 7.25.0 eslint-config-prettier: ^6.12.0 => 6.15.0 eslint-plugin-prettier: ^3.1.4 => 3.4.0 expo-server-sdk: ^3.6.0 => 3.6.0 express: 5.0.0-alpha.8 => 5.0.0-alpha.8 express-session: ^1.17.1 => 1.17.1 ioredis: ^4.19.2 => 4.26.0 jest: ^26.4.2 => 26.6.3 jsonschema: ^1.4.0 => 1.4.0 jsonwebtoken: ^8.5.1 => 8.5.1 mongodb: ^3.6.3 => 3.6.6 multer: ^1.4.2 => 1.4.2 n3: ^1.6.4 => 1.9.0 node-cron: ^2.0.3 => 2.0.3 node-fetch: ^2.6.1 => 2.6.1 onesignal-node: ^3.2.0 => 3.2.1 prettier: ^2.1.2 => 2.2.1 socket.io: ^3.0.3 => 3.1.2 socket.io-client: ^3.0.3 => 3.1.3 ts-node: ^9.0.0 => 9.1.1 ts-node-dev: ^1.0.0-pre.63 => 1.1.6 typescript: ^4.1.2 => 4.2.4 url-parse: ^1.4.7 => 1.5.1 uuid: ^8.3.2 => 8.3.2 web-push: ^3.4.4 => 3.4.4 ws: ^7.4.0 => 7.4.5 npmGlobalPackages: @paciolan/remote-component: 0.0.0-semantic-versioning @paciolan/remote-module-loader: 1.0.0-symantec-versioning @solid/oidc-auth-manager: 0.24.1 @solid/oidc-op: 0.11.1 componentsjs-generator: 1.6.0 dts-gen: 0.6.0 expo-cli: 4.0.16 http-server: 0.12.3 jest: 26.6.3 nodemon: 2.0.6 npm-check-updates: 10.2.2 npm: 6.14.9 ochat-api: 1.0.0 oidc-provider: 6.29.8 react-dom: 16.13.1 react: 16.13.1 ts-node-dev: 1.0.0 ts-node: 9.0.0 typescript: 4.1.2

Vinnl commented 3 years ago

Thanks for reporting this @jaxoncreed. One thing that stands out to me is that your setAcl function is always creating a new ACL with createAcl, even though you've already verified that one exists with hasResourceAcl. Could you try replacing the call to createAcl with getResourceAcl and report back if that fixes it? Thanks!

jaxoncreed commented 3 years ago

Yep that was it! Sorry about that. This worked:

let resourceAcl = hasResourceAcl(myDatasetWithAcl) ?
    getResourceAcl(myDatasetWithAcl) :
    createAcl(myDatasetWithAcl);
Vinnl commented 3 years ago

Good to hear, thanks for reporting back! I'll close this ticket then, feel free to open a new one if you encounter further issues.