SolidOS/solid-logic

Authentication error: hash fragment= not a valid redirect URL

Closed this issue · 6 comments

josephguillaume commented

This may or may not be related to SolidOS/solidos#200, but clearly appears to be an error in how solidos is interacting with recent changes in the authentication library.

  1. Go to a URL with a hash fragment, e.g. https://josephguillaume.solidcommunity.net/public/index.ttl#this

  2. Try to login

  3. Page shows an alert with error message: "https://josephguillaume.solidcommunity.net/public/index.ttl#this is not a valid redirect URL, it is either a malformed IRI or it includes a hash fragment"

It appears this message originates from here:
https://github.com/inrupt/solid-client-authn-js/blob/5bc1113604576dcff61b9284f8b736472bc42f4d/packages/node/src/ClientAuthentication.ts#L66

And is a result of a design change by which the authentication library no longer normalises redirect urls
inrupt/solid-client-authn-js#2643

I would guess that solid-logic needs to be updated to normalise the url before passing it in, but I haven't confirmed (apologies if this issue is in the wrong repo)

The possible link to SolidOS/solidos#200 is that the example in question ends up with a hash fragment for "state". I'm not sure where that's coming from, but it seems possible that error could be related to the same change in the authentication library.

bourgeoa commented

@josephguillaume Thanks for the issue you can make a PR in the redirectUrl branch.

josephguillaume commented

I wish I could, but I'll see if I can at least work on what the solution should be.

removeOidcQueryParam is now no longer applied to redirectUrl when it is passed in by the client
inrupt/solid-client-authn-js@baac030#diff-84bdced58167dbe812f9b691108262a1590dff1c5df9263f6cd5a9dfa4b6d77aR80
I don't think this is the issue

The only other normalisation I could find that might have been removed in that PR is

redirectUrl: options.redirectUrl
        ? new URL(options.redirectUrl).href
        : undefined

inrupt/solid-client-authn-js@baac030#diff-5e0ce0c5dff0fa7fb28da12e83474b4bc88c930ca57525bea273411789596f6fL64

To reinstate the previous behaviour and close this issue, that's probably sufficient, but given that solid-logic is now responsible for normalising and this is now an explicit design choice, it feels like redirecting to the current page without a hash is rather arbitrary.
At the very least we'd want some tests to make the expected behaviour explicit.

Shouldn't mashlib just consistently redirect to it's homepage as is common with SPAs (i.e. the redirectUrl is always the same), and then use existing functionality to reinstate the correct URI?

josephguillaume commented

Turns out the specification of the redirectUrl actually occurs in solid-ui
https://github.com/SolidOS/solid-ui/blob/6a9639ed3ed2cd0a96d3c6cd7446c3147bfd3582/src/login/login.ts#L517

The handling of restoration of state also appears to be spread across both solid-ui and solid-logic.

I'm not touching this, sorry.

bourgeoa commented

@timea-solid I'm not sure the above PR's are correctly resolving the problem. Could you check ?

bourgeoa commented

NSS tested locally with Watch on both PR's
Issues seemed resolved.

  • 👍1
  • ❤️1