Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cookies do not persist across different windows #36

Closed
simonw opened this issue Sep 1, 2021 · 5 comments
Closed

Cookies do not persist across different windows #36

simonw opened this issue Sep 1, 2021 · 5 comments
Labels
electron-wrapper Features that go in the Node.js/Electron code

Comments

@simonw
Copy link
Owner

simonw commented Sep 1, 2021

Noticed this after I resurrected the --root option from #32.

@simonw simonw added the electron-wrapper Features that go in the Node.js/Electron code label Sep 1, 2021
@simonw
Copy link
Owner Author

simonw commented Sep 1, 2021

Here's the resurrection diff:

diff --git a/main.js b/main.js
index cc3a0da..e38d7cc 100644
--- a/main.js
+++ b/main.js
@@ -29,9 +29,13 @@ class DatasetteServer {
     this.process = null;
   }
   async startOrRestart() {
-    const datasette_bin = await this.ensureDatasetteInstalled();
+    const python_bin = await this.ensureDatasetteInstalled();
     const args = [
+      "-u", // Unbuffered, to ensure process.stdin gets data
+      "-m",
+      "datasette",
       "--memory",
+      "--root",
       "--port",
       this.port,
       "--version-note",
@@ -40,12 +44,30 @@ class DatasetteServer {
     if (this.process) {
       this.process.kill();
     }
+    const re = new RegExp('.*(http://[^/]+/-/auth-token\\?token=\\w+).*');
+    let serverStarted = false;
+    let authURL = null;
     return new Promise((resolve, reject) => {
-      const process = cp.spawn(datasette_bin, args);
+      const process = cp.spawn(python_bin, args, {stdio: 'pipe'});
       this.process = process;
+      process.stdout.on("data", (data) => {
+        console.log("stdout", data.toString());
+        const m = re.exec(data);
+        if (m) {
+          authURL = m[1];
+          if (serverStarted) {
+            resolve(authURL);
+          }
+        }
+      });
       process.stderr.on("data", (data) => {
+        console.log("stderr", data.toString());
         if (/Uvicorn running/.test(data)) {
-          resolve(`http://localhost:${this.port}/`);
+          console.log("Uvicorn is running");
+          serverStarted = true;
+          if (authURL) {
+            resolve(authURL);
+          }
         }
       });
       this.process.on("error", (err) => {
@@ -76,7 +98,7 @@ class DatasetteServer {
     const venv_dir = path.join(datasette_app_dir, "venv");
     const datasette_binary = path.join(venv_dir, "bin", "datasette");
     if (fs.existsSync(datasette_binary)) {
-      return datasette_binary;
+      return path.join(venv_dir, "bin", "python3.9");
     }
     if (!fs.existsSync(datasette_app_dir)) {
       await mkdir(datasette_app_dir);
@@ -91,7 +113,7 @@ class DatasetteServer {
       "datasette-app-support>=0.1.2",
     ]);
     await new Promise((resolve) => setTimeout(resolve, 500));
-    return datasette_binary;
+    return path.join(venv_dir, "bin", "python3.9");
   }
 }

@simonw
Copy link
Owner Author

simonw commented Sep 1, 2021

Documentation: https://www.electronjs.org/docs/api/cookies

It looks like cookies are per-browser-window by default, but you can establish a session and pass session: thatSession to those browser windows when you create them.

@simonw
Copy link
Owner Author

simonw commented Sep 2, 2021

I'm having enormous trouble fixing this. It may be a bug? electron/electron#7268

Here's my most recent attempt that didn't work:

diff --git a/main.js b/main.js
index cc3a0da..227849a 100644
--- a/main.js
+++ b/main.js
@@ -1,4 +1,11 @@
-const { app, Menu, BrowserWindow, dialog, shell } = require("electron");
+const {
+  app,
+  Menu,
+  BrowserWindow,
+  dialog,
+  session,
+  shell,
+} = require("electron");
 const request = require("electron-request");
 const path = require("path");
 const cp = require("child_process");
@@ -11,15 +18,21 @@ const util = require("util");
 const execFile = util.promisify(cp.execFile);
 const mkdir = util.promisify(fs.mkdir);
 
-function postConfigure(mainWindow) {
-  mainWindow.webContents.on("will-navigate", function (event, reqUrl) {
+function postConfigure(win) {
+  win.webContents.on("will-navigate", function (event, reqUrl) {
     let requestedHost = new url.URL(reqUrl).host;
-    let currentHost = new url.URL(mainWindow.webContents.getURL()).host;
+    let currentHost = new url.URL(win.webContents.getURL()).host;
     if (requestedHost && requestedHost != currentHost) {
       event.preventDefault();
       shell.openExternal(reqUrl);
     }
   });
+  win.on('did-start-navigation', function() {
+    session.defaultSession.cookies.flushStore();
+  });
+  win.on('did-navigate', function() {
+    session.defaultSession.cookies.flushStore();
+  });
 }
 
 class DatasetteServer {
@@ -29,9 +42,13 @@ class DatasetteServer {
     this.process = null;
   }
   async startOrRestart() {
-    const datasette_bin = await this.ensureDatasetteInstalled();
+    const python_bin = await this.ensureDatasetteInstalled();
     const args = [
+      "-u", // Unbuffered, to ensure process.stdin gets data
+      "-m",
+      "datasette",
       "--memory",
+      "--root",
       "--port",
       this.port,
       "--version-note",
@@ -40,12 +57,30 @@ class DatasetteServer {
     if (this.process) {
       this.process.kill();
     }
+    const re = new RegExp(".*(http://[^/]+/-/auth-token\\?token=\\w+).*");
+    let serverStarted = false;
+    let authURL = null;
     return new Promise((resolve, reject) => {
-      const process = cp.spawn(datasette_bin, args);
+      const process = cp.spawn(python_bin, args, { stdio: "pipe" });
       this.process = process;
+      process.stdout.on("data", (data) => {
+        console.log("stdout", data.toString());
+        const m = re.exec(data);
+        if (m) {
+          authURL = m[1].replace('auth-token', 'auth-token-persistent');
+          if (serverStarted) {
+            resolve(authURL);
+          }
+        }
+      });
       process.stderr.on("data", (data) => {
+        console.log("stderr", data.toString());
         if (/Uvicorn running/.test(data)) {
-          resolve(`http://localhost:${this.port}/`);
+          console.log("Uvicorn is running");
+          serverStarted = true;
+          if (authURL) {
+            resolve(authURL);
+          }
         }
       });
       this.process.on("error", (err) => {
@@ -76,7 +111,7 @@ class DatasetteServer {
     const venv_dir = path.join(datasette_app_dir, "venv");
     const datasette_binary = path.join(venv_dir, "bin", "datasette");
     if (fs.existsSync(datasette_binary)) {
-      return datasette_binary;
+      return path.join(venv_dir, "bin", "python3.9");
     }
     if (!fs.existsSync(datasette_app_dir)) {
       await mkdir(datasette_app_dir);
@@ -91,7 +126,7 @@ class DatasetteServer {
       "datasette-app-support>=0.1.2",
     ]);
     await new Promise((resolve) => setTimeout(resolve, 500));
-    return datasette_binary;
+    return path.join(venv_dir, "bin", "python3.9");
   }
 }
 
@@ -115,12 +150,16 @@ function windowOpts() {
   let opts = {
     width: 800,
     height: 600,
+    webPreferences: {
+      partition: "persist:sharecookies"
+    }
   };
   if (BrowserWindow.getFocusedWindow()) {
     const pos = BrowserWindow.getFocusedWindow().getPosition();
     opts.x = pos[0] + 22;
     opts.y = pos[1] + 22;
   }
+  console.log(opts);
   return opts;
 }
 
@@ -130,9 +169,8 @@ function createWindow() {
   let mainWindow = null;
 
   mainWindow = new BrowserWindow({
-    width: 800,
-    height: 600,
-    show: false,
+    ...windowOpts(),
+    ...{ show: false },
   });
   mainWindow.loadFile("loading.html");
   mainWindow.once("ready-to-show", () => {
@@ -293,11 +331,19 @@ function createWindow() {
             },
           ],
         },
+        {
+          label: 'Debug',
+          submenu: [{
+            label: 'Open DevTools',
+            click() {
+              BrowserWindow.getFocusedWindow().webContents.openDevTools();
+            }
+          }]
+        }
       ]);
       Menu.setApplicationMenu(menu);
     }
   );
-  // mainWindow.webContents.openDevTools()
 }
 
 app.whenReady().then(() => {

@simonw
Copy link
Owner Author

simonw commented Sep 2, 2021

I also modified datasette-app-support to set those cookies with an expiry date so they wouldn't be treated as session cookies - diff for that is:

diff --git a/datasette_app_support/__init__.py b/datasette_app_support/__init__.py
index 4a97927..e5c01cd 100644
--- a/datasette_app_support/__init__.py
+++ b/datasette_app_support/__init__.py
@@ -1,10 +1,11 @@
 from datasette.database import Database
-from datasette.utils.asgi import Response
+from datasette.utils.asgi import Response, Forbidden
 from datasette.utils import sqlite3
 from datasette import hookimpl
 import json
 import os
 import pathlib
+import secrets
 
 
 @hookimpl
@@ -51,6 +52,26 @@ async def open_database_file(request, datasette):
     return Response.json({"ok": True})
 
 
+async def auth_token_persistent(request, datasette):
+    token = request.args.get("token") or ""
+    if not datasette._root_token:
+        raise Forbidden("Root token has already been used")
+    if secrets.compare_digest(token, datasette._root_token):
+        datasette._root_token = None
+        response = Response.redirect(datasette.urls.instance())
+        response.set_cookie(
+            "ds_actor", datasette.sign({"a": {"id": "root"}}, "actor"),
+            expires=364 * 24 * 60 * 60
+        )
+        print(response._set_cookie_headers)
+        return response
+    else:
+        raise Forbidden("Invalid token")
+
+
 @hookimpl
 def register_routes():
-    return [(r"^/-/open-database-file$", open_database_file)]
+    return [
+        (r"^/-/open-database-file$", open_database_file),
+        (r"^/-/auth-token-persistent$", auth_token_persistent)
+    ]

@simonw
Copy link
Owner Author

simonw commented Sep 2, 2021

I've spent enough time on this - I'm going to assume that each BrowserWindow cannot share cookies (or there are bugs that prevent them from doing that) and consider an alternative approach in a separate ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron-wrapper Features that go in the Node.js/Electron code
Projects
None yet
Development

No branches or pull requests

1 participant