Permissions pane causes 401 with PUT on CSS

[RubenVerborgh]

When using the permissions pane to change an ACL file hosted on CSS, SolidOS will send a PUT request without an Authorization header, causing CSS to reply with 401.

In constrast, opening that same ACL file in the source pane, will generate the correct PUT request with the Authorization header, resulting in a 2xx.

This can be replicated with the Mashlib recipe.

[timbl]

Looks like the wrong fetch being called

[timbl]

But strange that NSS doesn't have the problem

[timbl]

It uses UpdateManager.put which uses webOperation which uses Fetcher._fetch which should be good if you are logged in .. nothing obvious

[bourgeoa]

I can reproduce the 401 error with SolidOS web app on https://solidweb.me (CSS)

[bourgeoa]

The PUT do not use Session.fetch() but browser-ponyfill fetch.

[RubenVerborgh]

The PUT do not use Session.fetch() but browser-ponyfill fetch.

What line is that?

This is a bug in the code; the reason it works with NSS is (non-standard) cookies.

[jeff-zucker]

AFAIK, the only place in the SolidOS stack where browser-ponyfill.fetch is used is in @inrupt/solid-client-authn-core/dist/login/oidc/redirectHandler/redirectHandler.js in this line :

    const jwksResponse = await (0, cross_fetch_1.fetch)(jwksIri);  

In other words, it is part of Inrupt's login redirect process and only peripherally related to PUT and, unless I'm mistaken, is an Inrupt issue. I'm not sure the browser-ponyfill fetch is even related to this permissions pane issue.

[bourgeoa]

@timbl @SharonStrats The code https://github.com/solid/solid-ui/blob/eb82ec38b28b2e2feb872492149f48b6507ebf42/src/acl/access-controller.ts#L217 is not using the solidLogic store but is creating a new graph : newAClGraph = graph() with I suppose a non authenticated fetch. The code organization seems to need 2 stores to be able to duplicate the graph in certain situations.

I do not succeed to create a duplicate of the authenticate store. Could you look at it. With this code I still have a 401

import { store } from '../logic'
......
public save (): Promise<void> {
       const newAClGraph = store
......
       const updater = newAClGraph?.updater // || new UpdateManager(newAClGraph)

[jeff-zucker]

@bourgeoa the code there creates a new store (newACLGraph) and an upadater for it without specifying a fetch, Which means you are correct that it uses the default cross-fetch. So we don't need to do anything with the store, just with the fetcher for the updater. This can be done by this sequence :

  import {fetcher} from 'rdflib';
  const newACLGraph = graph() 
  const fetcher = fetcher(newACLGraph,{fetch:this.store.fetcher._fetch})
const updater ...

That will create a new store and an updater for it that uses the fetch from the existing store. I believe that is the desired effect.

[bourgeoa]

Adding the authorized fetch enable creation and edit on creation. when trying to edit an existing ACL the 401 error comes again. Weird. When the error is there there is no way to remove the 401 error except by reloading the page.

[bourgeoa]

@angelo-v

Adding the authorized fetch enable creation and edit on creation. when trying to edit an existing ACL the 401 error comes again. Weird. When the error is there there is no way to remove the 401 error except by reloading the page.

I haven't created a branch in solid-ui for that. The only think I did is add here https://github.com/solid/solid-ui/blob/48f3ebed3572bc099bfcd5b770fd5a3f4d738795/src/acl/access-controller.ts#L230

    newAClGraph.fetcher = fetcher(newAClGraph, { fetch: this.store.fetcher._fetch })

This add an authenticated fetch to the newAClGraph. I use the tryMoveAuth branch, but the problem should not be branch depending. It existed before.

My analyse is that for any reason loading an existing resource ACL to the store does load in a store with no-authenticated.fetch

[bourgeoa]

@aveltens found the problem https://github.com/solid/solid-ui/blob/48f3ebed3572bc099bfcd5b770fd5a3f4d738795/src/acl/access-groups.ts#L71

This is creating a node-fetch fetcher Just commenting out solves the issue.

[bourgeoa]

@RubenVerborgh a test-version of mashlib@1.7.20-401 includes the PATCH resolving this issue.

[RubenVerborgh]

Super, thanks!

[timbl]

When those bits were written, we used to think about making a Fetcher. In fact we had the shortcut fetcher() to do it easily. Now we just make a LiveStore which has automatically has a Fetcher and an UpdateManager attached to it. Should we change the default _fetch for any new Fetcher?

[jeff-zucker]

I believe so, yes. It should check the window.solidFetch to see if the user set a global fetch method and then check the existing _fetch and use that. I can't see any advantage to ignoring the user's definition of a fetch method and lots of reasons to honor it.

[timbl]

I wonder why the problem did not show up on NSS.

[RubenVerborgh]

I wonder why the problem did not show up on NSS.

Cookie-based auth is also active (which CSS and ESS don't use because of security concerns).

[jeff-zucker]

@RubenVerborgh and @timbl - I don't believe it has to do with cookies. If you try the script below, you see that I login to NSS and then do an unauthenticated fetch and get back a 401 even though the cookie is active.

<!DOCTYPE html><html><head>
  <meta charset="UTF-8" />
</head><body>
  <span id="webId"></span>
  <input id="oidc" value="https://solidcommunity.net" style="width:24em" />
  <button id="loginButton">Login</button>
  <button id="logoutButton">Logout</button>

<script src="https://cdn.jsdelivr.net/npm/@inrupt/solid-client-authn-browser@1.11.2/dist/solid-client-authn.bundle.js">
</script>
<script>

const iscan = solidClientAuthentication;
var session = iscan.getDefaultSession();

const loginButton = document.querySelector("#loginButton");
const logoutButton = document.querySelector("#logoutButton");
const webIdArea = document.querySelector("#webId");
loginButton.onclick = ()=> { 
  return iscan.login({
    oidcIssuer: document.getElementById("oidc").value,
    redirectUrl: window.location.href,
    clientName: "Minimal login/logout"
  });
};
logoutButton.onclick = async ()=> { 
  await session.logout();
  showLoginStatus();
};
async function handleRedirectAfterLogin() {
  await iscan.handleIncomingRedirect();
  showLoginStatus();
}
async function showLoginStatus() {
  session = iscan.getDefaultSession();
  if (session.info.isLoggedIn) {
    loginButton.style.display = "none";
    logoutButton.style.display = "inline-block";
    webId.innerHTML = `Logged in as ${session.info.webId}`;
    const privateThing='https://jeff-zucker.solidcommunity.net/private/testput.txt';
    let response = await window.fetch(privateThing,{
      method:"PUT",
      headers: {"content-type":"text/plain"},
      body: "some words",
    })
    webId.innerHTML+="Response to un-authenticated PUT : "+response.status;
  }
  else {
    loginButton.style.display =  "inline-block";
    logoutButton.style.display = "none";
    webId.innerHTML = `Not logged in.`;
  }
}
handleRedirectAfterLogin();
</script></body></html>

[RubenVerborgh]

    let response = await window.fetch(privateThing,{
      method:"PUT",
      headers: {"content-type":"text/plain"},
      body: "some words",
    })

is missing credentials: "include" which rdflib used to set at some point

[jeff-zucker]

Same results when I add that. In any case, NSS does NOT accept its cookie as proof of login when it receives an unauthenticated PUT. Unless I'm mistaken, this means the cookie is not the reason NSS fails to give a 401 when CSS gave one.

On Mon, Feb 21, 2022 at 3:08 PM Ruben Verborgh @.***> wrote:

let response = await window.fetch(privateThing,{
  method:"PUT",
  headers: {"content-type":"text/plain"},
  body: "some words",
})

is missing credentials: "include" which rdflib used to set at some point

— Reply to this email directly, view it on GitHub https://github.com/solid/solidos/issues/113#issuecomment-1047286630, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKVJCJCDYID7A7AJZECBDDDU4LAXDANCNFSM5M3LTXAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

[bourgeoa]

closed with https://github.com/solid/solid-ui/pull/476/files