access/universal functions throw error instead of creating .acl when no .acl exists

[ianconsolata]

Search terms you've used

setAccess, setAccessFor

Bug description

The documentation states

If the Resource did not have an ACL yet, a new one will be initialised. This means that changes to the ACL of a parent Container can no longer affect access people have to this Resource, although existing access will be preserved. However, in practice, when trying to setAccessFor a newly created resource with no .acl file the function throws an error because it receives a 404.

To Reproduce

1. Call setAccessFor(reousourceUrl, "public", { read: true }, options) on a resource with no .acl

Expected result

The .acl file should be initialized, and set with the requested access.

Actual result

An error message is thrown with the following message:

Error: Fetching the metadata of the Resource at https://ian.staging.mysilio.me/spaces/home/public.ttl.acl failed: [404] [Not Found].

The error bubbles up from from responseToResourceInfo : https://github.com/inrupt/solid-client-js/blob/4b9960014a5d5e6ccaab79988feba79407167fdc/src/resource/resource.ts#L84, which seems to throw on all not ok responses (including 404s).

That function is used in the getReourceInfoWithAcr call that happens on the first line of setPublicAccess (called by setAccessFor: https://github.com/inrupt/solid-client-js/blob/4b9960014a5d5e6ccaab79988feba79407167fdc/src/access/universal_v1.ts#L457

getResourceInfoWithAcr calls getResourceInfo which calls responseToResourceInfo which throws. This happens on the first line of all the access setters, so I am not sure if any of them correctly implement the .acl creation described in the documentation.

Environment

System:
    OS: macOS 11.6.5
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 7.10 GB / 32.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.14.0 - ~/.nvm/versions/node/v16.14.0/bin/node
    npm: 8.3.1 - ~/.nvm/versions/node/v16.14.0/bin/npm
  Browsers:
    Brave Browser: 96.1.33.106
    Chrome: 99.0.4844.84
    Firefox: 98.0.2
    Safari: 15.3
  npmPackages:
    @babel/core: ^7.12.3 => 7.14.3
    @inrupt/solid-client: ^1.19.0 => 1.19.0
    @inrupt/solid-client-authn-browser: ^1.11.2 => 1.11.2
    @inrupt/vocab-common-rdf: ^1.0.3 => 1.0.3
    @inrupt/vocab-solid-common: ^0.7.5 => 0.7.5
    @size-limit/preset-small-lib: ^4.8.0 => 4.10.3
    @storybook/addon-essentials: ^6.0.28 => 6.2.9
    @storybook/addon-info: ^5.3.21 => 5.3.21
    @storybook/addon-links: ^6.0.28 => 6.1.1
    @storybook/addons: ^6.0.28 => 6.1.1
    @storybook/react: ^6.0.28 => 6.2.9
    @testing-library/react-hooks: ^3.4.2 => 3.4.2
    @types/jest: ^25.2.3 => 25.2.3
    @types/react: ^17.0.30 => 17.0.30
    @types/react-dom: ^17.0.9 => 17.0.9
    @types/url-parse: ^1.4.3 => 1.4.3
    babel-loader: ^8.2.1 => 8.2.2
    dequal: ^2.0.2 => 2.0.2
    molid: ^0.3.0 => 0.3.0
    react: ^17.0.1 => 17.0.2
    react-dom: ^17.0.2 => 17.0.2
    react-is: ^17.0.2 => 17.0.2
    react-test-renderer: ^17.0.2 => 17.0.2
    size-limit: ^4.8.0 => 4.10.3
    swr: ^1.0.1 => 1.0.1
    ts-jest: ^25.5.1 => 25.5.1
    tsdx: ^0.14.1 => 0.14.1
    tslib: ^2.3.1 => 2.3.1
    typedoc: ^0.22.6 => 0.22.6
    typescript: ^4.4.4 => 4.4.4
    url-parse: ^1.4.7 => 1.5.1
    whatwg-fetch: ^3.5.0 => 3.5.0
  npmGlobalPackages:
    corepack: 0.10.0
    npm: 8.3.1
    yalc: 1.0.0-pre.53
    ```

[ThisIsMissEm]

We’ve opened it as an issue to look into, though it may take us a little bit to get to as we’re working on finalising a major release. It appears to be a WAC based issue, and we generally test more against ACP instead. Thank you for reporting the issue! 😄

[ThisIsMissEm]

Just a heads up, the setAccessFor API was deprecated and removed: https://github.com/inrupt/solid-client-js/blob/main/CHANGELOG.md#1190---2022-02-09 — but the bug you're talking about may still exist on setPublicAccess and setAgentAccess

[travis]

just chiming in to say that I have seen this bug with setPublicAccess against CSS - believe this is indeed a WAC issue.

I think we can work around this be using the WAC-specific APIs for now, but it means our app (https://github.com/mysilio-co/garden) will not support ACP-based Solid servers, which is a bummer. I poked around and it seems like the issue is that getAcrUrl is expectinggetAclServerResourceInfo to return null in this case but an error is being thrown further down the stack by responseToResourceInfo.

there is a bit of logic inside responseToResourceInfo to ignore authentication errors, but I don't think it works for 404s and in any case TypeScript won't let me pass the right option to setPublicAccess so this is blocking our ability to use this API

[renyuneyun]

Just wanted to add more info: This bug also applies to NSS (solidcommunity.net in my case). I'm using setAgentAccess.

[ianconsolata]

@ThisIsMissEm I would like to raise the priority on this issue. It has remain unaddressed for almost a year, and it makes the universal access module almost useless on NSS and CSS. As far as I can tell, this bug exists on all universal access functions (i.e. setAgentAccess, setPublicAccess). If the acl files do not exist, these functions cannot be used to set access for any resources. These functions need to be able to understand that when an acl files does not exist, they should create one with the appropriate permissions based on the arguments passed to the function.

[ThisIsMissEm]

Hi @ianconsolata we're aware, we do have this ticket, but it's been blocked behind some much more pressing work (such as node.js 18 support, and fixing our CI/CD infrastructure); If you think you can fix it, and write a unit test or end to end test for it, that might be helpful. Sorry we've not been quicker to resolve this issue.

[travis]

I feel compelled to chime in and just note that this is an incredibly disappointing response. This is a critical bug that essentially prevents these libraries from being used with both of the non-Inrupt POD providers in anything but a trivial way.

I've been hopeful for years that Inrupt would play a key role in organizing a community around Solid, and thought this library signaled a commitment to providing at least the basic building blocks of that community - that is, a high quality, well tested foundational library that provides at least basic storage and authz functionality against the main POD implementations, but the lack of prioritization of a critical bug like this feels like a very clear signal that Inrupt doesn't care about anything but Pod Spaces and developers who want to build on it.

I won't speak for @ianconsolata but at this point I don't have the time or emotional energy to fix bugs in your code when it doesn't seem clear to me that support for CSS will be provided in a timely or earnest way by the maintainers of this library.

Apologies if my frustration here comes off too sharply, but I am very frustrated.

[VirginiaBalseiro]

@ianconsolata : This has been fixed in v1.27.1. Would you mind testing and confirming the issue is resolved? Thanks!

[Vinnl]

I've just encountered this in v1.30.0. Looking at my stack trace, and comparing that to the fix in #1942, I don't think that that try..catch block was added to the correct file - it looks like it was added to the ACP-specific, rather than the universal, APIs. I think it might need to go in this file? https://github.com/inrupt/solid-client-js/blob/main/src/universal/getAclServerResourceInfo.ts

[jaxoncreed]

I've also encountered this in v1.30.0.

[NSeydoux]

Thanks for reopening, we'll look into this

[NSeydoux]

v1.30.2 should resolve this, but do reopen if it isn't the case!

[Vinnl]

Confirming that it appears to be resolved for me, thank you!