WordPress / wordpress-playground

Run WordPress in the browser via WebAssembly PHP
https://w.org/playground/
GNU General Public License v2.0
1.62k stars 248 forks source link

Secure client-side storage on playground.wordpress.net to avoid going through the GitHub connection flow so often #1395

Open adamziel opened 4 months ago

adamziel commented 4 months ago

Authorizing GitHub is a painful and annoying process. We don't store the token anywhere so you have to do it every. single. time. In the process, you're losing the running Playground instance. Let's find a secure way to store that token for later.

The problem

Suppose we authorized GitHub and wanted to store the OAuth token for later. How can we do that securely?

If the "browser app" (index.html) stores it in localStorage, a clever attacker could use the fact that WordPress (remote.html) is running on the same origin and craft a Blueprint to simply steal it from localStorage. The same goes for IndexedDB, OPFS, and probably all forms of client-side storage.

If the "browser app" stores is on the server, it would mean sending an authorized fetch() request to save or expose the OAuth token. The fetch() would be authorized client based on a cookie, a GET or POST parameter, or a custom header. In each scenario, WordPress could issue the same fetch and steal the OAuth token.

Possible solutions

Expose a client-side storage only to index.html and no other page

Above I said there's no safe client-side storage option, but now I want to challenge that.

For once, I wonder if there's a way of storing credentials in a service worker in a way that would only be accessible via index.html but not remote.html.

const client = await self.clients.get(event.clientId);
client.frameType === 'top-level'  // or 'auxiliary', 'none', 'nested'
client.clientType === 'window' // or 'worker', 'sharedWorker', 'all'
client.url

Putting it all together, perhaps the service worker could provide a set of /kv/get, /kv/set, /kv/delete endpoints that would only respond to fetch() requests from top-level index.html pages, but not to ones made by WordPress – either the iframed one or a top-level one running via "open in a new tab".

This solution would require:

Even then, it would be risky as a single missed assumption could expose that storage to

Therefore, let's put index.html and remote.html on different domains.

Separate origins for 'the browser app' and remote.html

Either:

  1. Leave index.html on https://playground.wordpress.net and move remote.html to something like https://api.playground.wordpress.net.
  2. Leave remote.html on https://playground.wordpress.net and move index.html to something like https://app.playground.wordpress.net.

UX-wise, I like the option number 1 better as typing https://playground.wordpress.net feels more natural than https://api.playground.wordpress.net.

Here's what we'd need to consider before the split:

The Privacy Sandbox rabbit hole is relevant. If we do separate domains but need to mark them as "same site" for any reason, we could probably do that.

Any other options?

If you have any other, potentially simpler ideas, let's discuss them!

cc @bgrgicak @dmsnell @brandonpayton FYI @henriqueiamarino @sarahnorris @beafialho

bgrgicak commented 4 months ago

Why is the current solution bad? GitHub access is extremely sensitive in some cases and asking for auth every time is acceptable for me personally.

If not storing the data is more secure could we optimize the current auth flow to make it easy to use?

@adamziel you mentioned a redirect that causes us to lose data, could we avoid losing the data? We could try to use Window: postMessage(), open the auth flow in a new tab, and send the token back to the original tab without reloading it. Alternatively, we could explore how to store the state and restore it after redirecting, but that sounds more complicated.

adamziel commented 4 months ago

could we avoid losing the data?

Oh, I like that! It would make the repeated OAuth flow unnoticeable. The only problem to solve now is – how to enable initiating and concluding the flow from index.html but not from remote.html.

adamziel commented 4 months ago

I wonder if this popup would do the trick:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Popup</title>
    <script>
        window.addEventListener('message', (event) => {
            if (event.origin !== window.location.origin) return; // Ensure same origin
            if (event.source !== window.opener) return; // Ensure it's from the opener

            // Verify the opener's URL to ensure it's index.html
            try {
                const openerUrl = new URL(window.opener.location.href);
                if (openerUrl.pathname !== '/index.html') return; // Ensure it's index.html
            } catch (e) {
                return; // In case of any error, reject the message
            }

            if (event.data === 'Hello from index.html') {
                console.log('Message from index.html:', event.data);
                event.source.postMessage('Hello from popup.html', event.origin);
            }
        });

        function sendMessageToIndex() {
            if (window.opener) {
                window.opener.postMessage('Hello from popup.html', window.location.origin);
            }
        }
    </script>
</head>
<body>
    <button onclick="sendMessageToIndex()">Send Message to Index</button>
</body>
</html>
bgrgicak commented 4 months ago

This should work. If window.opener causes issues, we can do something like this https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage#examples

brandonpayton commented 4 months ago

Why is the current solution bad? GitHub access is extremely sensitive in some cases and asking for auth every time is acceptable for me personally.

@bgrgicak Thank you for bringing this up. It is a good observation, and I find auth-each-time not only acceptable but desirable compared to storing credentials.

In addition, would it be possible to:

If not storing the data is more secure could we optimize the current auth flow to make it easy to use?

❤️

Possible solutions

Separate origins for 'the browser app' and remote.html

I wonder if this is a good idea to do eventually, regardless of what we do with GitHub auth flow.

On a related note, it might be good to ask adjacent security experts for a general audit of Playground on the web. It would be interesting to see if there any concerns we haven't considered.