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

Strange behaviour when creating a policy for ACP #2340

Open
renyuneyun opened this issue Feb 25, 2024 · 2 comments
Open

Strange behaviour when creating a policy for ACP #2340

renyuneyun opened this issue Feb 25, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@renyuneyun
Copy link

Search terms you've used

  • ACP
  • getResourcePolicyAll
  • getPolicyUrlAll

Bug description

I'm following the official document to create a policy (rule) in ACP. The policy was successfully created, and seems to be interpreted by Solid (CSS). However, if I look into the .acr document, things are not as expected.

There are three strange behaviours:

  1. Unnamed nodes from existing policy are unwrapped and given a blank node name (a very long one rather than a compact one);
  2. It creates a new node (#defaultAccessControl) for acp:AccessControl instead of using an existing one (because this line refers to a function which should reuse an existing acp:AccessControl);
  3. The created new node does not have a acp:AccessControl for its type.

To Reproduce

  1. Create a .acr with some policy (e.g. the MRE below)
  2. Follow the steps in the official document to create a new access control policy for it

Minimal reproduction

See this gist for the example .acr file: https://gist.github.com/renyuneyun/834eb5ee542e06a2cc3ee1a57712ca3f.
To reproduce, remove , <#defaultAccessControl> at line 6 and remove line 25-31, and put it to the .acr file.

Expected result

  1. Unnamed nodes stay the same;
  2. Existing acp:AccessControl node being used;
  3. (Or) the created new node has acp:AccessControl as its type.

Actual result

See comment under the gist for the actual .acr file after the operation.

Environment

  System:
    OS: Linux 6.6 Arch Linux
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
    Memory: 1.90 GB / 15.40 GB
    Container: Yes
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 21.6.1 - ~/node_modules/.bin/node
    Yarn: 1.22.21 - /usr/bin/yarn
    npm: 10.4.0 - /usr/bin/npm
  Browsers:
    Chromium: 122.0.6261.57
  npmPackages:
    @comunica/query-sparql: ^2.8.1 => 2.10.0 
    @inrupt/solid-client: ^2.0.0 => 2.0.0 
    @inrupt/solid-client-authn-browser: ^2.0.0 => 2.0.0 
    @inrupt/vocab-common-rdf: ^1.0.5 => 1.0.5 
    @inrupt/vocab-solid: ^1.0.4 => 1.0.4 
    @mdi/font: ^7.1.96 => 7.2.96 
    @renyuneyun/solid-helper: ^0.0.3 => 0.0.3 
    @rushstack/eslint-patch: ^1.1.4 => 1.3.2 
    @tsconfig/node20: ^20.1.0 => 20.1.0 
    @types/n3: ^1.10.4 => 1.16.0 
    @types/node: ^20.4.4 => 20.4.4 
    @vitejs/plugin-vue: ^4.0.0 => 4.2.3 
    @vue/eslint-config-prettier: ^8.0.0 => 8.0.0 
    @vue/eslint-config-typescript: ^11.0.0 => 11.0.3 
    @vue/tsconfig: ^0.4.0 => 0.4.0 
    eslint: ^8.22.0 => 8.45.0 
    eslint-plugin-vue: ^9.3.0 => 9.15.1 
    n3: ^1.16.3 => 1.17.2 
    npm-run-all: ^4.1.5 => 4.1.5 
    path-browserify: ^1.0.1 => 1.0.1 
    pinia: ^2.1.4 => 2.1.4 
    prettier: ^3.0.0 => 3.0.0 
    solid-helper-vue: ^0.1.4 => 0.1.4 
    typescript: ^5.1.6 => 5.1.6 
    vite: ^4.0.0 => 4.4.6 
    vue: ^3.3.4 => 3.3.4 
    vue-tsc: ^1.0.12 => 1.8.6 
    vuetify: ^3.3.9 => 3.3.9 
  npmGlobalPackages:
    @quasar/cli: 2.3.0
    @renyuneyun/solid-helper: 0.0.3
    @renyuneyun/parse-link-header-ts: 1.0.1
    orchestrator-app: 0.1.0
    solid-helper-vue: 0.2.0
    solid-shell: 2.0.8
    vercel: 32.5.5

Additional information

@renyuneyun renyuneyun added the bug Something isn't working label Feb 25, 2024
@NSeydoux
Copy link
Contributor

Hi @renyuneyun , thanks for reaching out.

I understand your concerns, but I think it is correct that a given Access Control Resource may have several Access Controls, and having the acp:AccessControl type is implicit. For instance, it is omitted in https://solidproject.org/TR/acp#example-access-control.

Is it accurate to say that the graph isn't isomorphic to what you would prefer, but it is semantically equivalent and results in access policies being applied accurately?

@renyuneyun
Copy link
Author

renyuneyun commented Feb 27, 2024

Hi @NSeydoux , thanks for looking into this.

Yes, you are right, it is semantically equivalent, and the policies are applied accurately.

I understand that they may have implicit types, and actually would be more inclined to not requiring an explicit type (because it should be inferrable in principle). Although, 1) the solid-client library actually assumes explicit typing and relies on that heavily for ACP; 2) for the manipulation, I would believe it is a better idea to explicitly type every new node the library creates.

Apart from that, the main issue I see is the (seemingly) inconsistency between the code and the actual behaviour, particularly the item 2 and this line of the library (which is called by acp_ess_2.setResourcePolicy()). The code seems to suggest that no new acp:AccessControl node should be created, while in reality it creates a new node (without explicitly giving it a type acp:AccessControl), and named it #defaultAccessControl.
The behaviour seems to be more aligned with this addPolicyUrl() function in policy/addPolicyUrl.ts, rather than the imported control.ts#addPolicyUrl().

So, again, this routes back to my question (in the other recent issues) on the relation between these same-name similar-purpose different-implementation functions under different files / directories, and the possibility of them messing up the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants