Popup gives Error: Popup login is not implemented yet

[jaxoncreed]

Describe the bug
When a config with { popUp: true } is provided, the popup flow doesn't trigger. The following error is printed: Error: Popup login is not implemented yet.

To Reproduce
Steps to reproduce the behavior:

1. Run the example at https://github.com/inrupt/solid-client-authn-js/tree/master/packages/browser/examples/single/bundle
2. Replace src/App.js with the code I wrote here: https://gist.github.com/jaxoncreed/0e9453f03488f43d9ed382593e82fc66
3. Try to log in using the "Popup Login" button.

Expected behavior
A popup window should appear and should be directed to the issuer's login page. Upon login, the window closes and the session triggers an onLogin event.

Environment

System:
    OS: macOS 10.15.6
    CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
    Memory: 1.27 GB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 14.4.0 - /usr/local/bin/node
    npm: 6.14.4 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Browsers:
    Chrome: 86.0.4240.80
    Firefox: 81.0
    Safari: 14.0
  npmPackages:
    @babel/core: ^7.8.6 => 7.10.5 
    @babel/preset-env: ^7.8.6 => 7.10.4 
    @babel/preset-react: ^7.8.3 => 7.10.4 
    @inrupt/solid-client-authn-browser: file://../../../ => 0.2.1 
    babel-loader: ^8.0.6 => 8.1.0 
    html-loader: ^0.5.5 => 0.5.5 
    html-webpack-plugin: ^3.2.0 => 3.2.0 
    react: ^16.13.0 => 16.13.1 
    react-dom: ^16.13.0 => 16.13.1 
    regenerator-runtime: ^0.13.3 => 0.13.7 
    webpack: ^4.41.6 => 4.44.0 
    webpack-cli: ^3.3.11 => 3.3.12 
    webpack-dev-server: ^3.10.3 => 3.11.0 
  npmGlobalPackages:
    @inrupt/generator-solid-react: 0.7.2
    @inrupt/solid-auth-fetcher: 0.0.6
    expo-cli: 3.27.4
    lerna: 3.22.1
    npm: 6.14.4
    ochat-api: 1.0.0
    redis-commander: 0.7.0
    static-server: 2.2.1
    typescript: 3.9.5
    yo: 3.1.1

Additional context
It looks like the functionality for popups was removed at some point (https://github.com/inrupt/solid-client-authn-js/blob/master/packages/browser/src/login/popUp/PopUpLoginHandler.ts#L65). Probably because it wouldn't work very well with closures. I expect that this will be solved along with #423. The popup window will trigger a login then close, causing the auth token to be lost but retaining the refresh token. Then the parent window would go through the refresh token flow to obtain an auth token for itself.

I need this for ESS compatibility. Unlike NSS, ESS will not accept hashes in its redirect URLs. So, if you log into https://jackson.inrupt.net/profile/card#me and want to get redirect there instead of https://jackson.inrupt.net/profile/card, ESS will complain. However, this same problem wouldn't happen in the Popup window flow.

[NSeydoux]

Indeed, the support for popup login has been currently removed, and we are considering our options to add it back. As you mention, the necessity to have inter-process communication between the popup and the browser window challenges the way the token is stored never to be exposed in the current implementation. I've seen some best practices documents frowning upon popup login in general, so we'll see what tradeoff can be made and if it is worth it.

[jaxoncreed]

Hmm. If popup is discouraged, I could work around that.

[jaxoncreed]

Though, for the data-browser, without popup login, if you have navigated into the folder structure without logging in and you want to then login, it's going to pop you back at the root of that page instead of redirecting you back to the thing you were looking at ☹️

[jaxoncreed]

Tim is saying that he'd like login to open in a new tab rather than a popup.

[Vinnl]

The way the data browser works is that it does actually display the resource you were looking at. However, you're probably using the folder pane, which allows you to drill down into the folder represented by the current URL - but the folder pane doesn't maintain its state across refreshes, nor does it represent it in the URL.

I'm afraid that whether it's in a new tab or in a popup doesn't matter in terms of avoiding the inter-process communication, so @NSeydoux's comment above would still apply.

[timbl]

I'm not saying I want it in a new tab, unless that is what it takes for the to log in securely and preserve the client DOM state. A popup would work, or a temporary pull down area which has its own URL bar and so it clearly a different site. From the UX point of view, an iframe wold be great -- though it would not be secure as would be forgeable.

It's not just a question of which folders are exposed in the folder view. In general in solid-ui and solid-panes the view state is only stored in the DOM. You open something a preferences drawer or a detail view, and it is inserted in the DOM with no record being kept in any JS state variable (or on the POD). When if is closed, it just disappears. And because in solid you can do anything to anything, you might be filling in a form, and for that have open up your contacts to check on a person'd details, then for that have to open a chat with them, all nested within the same view. So when the chat asks you to log in to further, you have to be able to do so, and then go back to the whole complex user view of their stuff.

From the user flow point of view, it would be best if the process of logging in, just like that of checking a fact with someone in a chat, and like the process of picking a person, are all processes which insert some space into the UI to do what they need to do, and then vanish, leaving the users with a sub-problem solved and back in the super problem, if you like. So the login functionality needs to work like that too.

[jaxoncreed]

Generally, when it comes to databrowser implementation it comes down to a choice between 3 things:

• Provide some kind of way to log in without redirecting the current window in this library (it could be popup or a new tab). This would take extra work on the part of the solid-client-authn-js team.
• Just accept that the databrowser will no longer maintain its state after you login. This would be inconvenient if you're browsing something nested in the folder pane or you're looking at something in the pane.
• Before redirecting save the state of the application so it can be loaded back later. This would take a lot of work because the databrowser doesn't currently save its state anywhere and doing this would require a rearchitecture.

Let me know if I charactarized that correctly.

[Vinnl]

I'd add to the first bullet that it's not just extra work, but that we don't yet know how to do this securely. Your suggestion at #423 (comment) looks promising and might, I think, be able to help in this regard, but we have yet to investigate whether that satisfies the security requirements. So fingers crossed :)

[NSeydoux]

Also, doing a full loggin in an iframe is technically impossible in a lot of cases, because logging endpoints such as Google's actively prevent that to protect against phishing.

I think your three points capture well the tradeoff @jaxoncreed, with the caveat brought by @Vinnl. Returning the token back to the main window from the popup would require some work, as it does not fit with the current way the token is handled for security reasons, so that would effectively mean that login to the data browser cannot be done securely. Combined with the fact that the modular architecture makes it possible to have scripts from all over the place running together in the page, that might be a big concern. However, I did not have the time yet to look into the alternatives you suggest @jaxoncreed, maybe they could resolve the issue. Otherwise, on a short term, the only option that looks achievable to me is not to maintain state, which makes a terrible user experience...

[josephguillaume]

While maintaining DOM state might not be possible, persisting state through the authentication process should be possible but isn't implemented yet (#950).

At the moment it seems even that is not possible - URL parameters or hash links would not survive the login process? (and it is needed at every page refresh #423)

Edit: Sorry, just realised URL params do persist through the redirect URL.