Improve private scenes error pages (#40)

* Improve 401 and 404 errors for private scenes (fix #39 )

* fix share link to scenes

* try to only set the www-authenticate header on non-interactive sessions

source/client/ui/explorer/ShareMenu.ts

@@ -51,7 +51,7 @@ export default class ShareMenu extends Popup
         this.position = "center";
         this.modal = true;
         let match = /\/scenes\/([^/]+).*?/.exec(window.location.href); 
-        let u = new URL((match?`/scenes/${match[1]}/view`:""), window.location.href);
+        let u = new URL((match?`/ui/scenes/${match[1]}/view`:""), window.location.href);
         u.searchParams.set("lang", ELanguageType[language.ins.language.value]);
         this.url = u.toString();
     }

source/server/auth/UserManager.ts

@@ -297,12 +297,12 @@ export default class UserManager {
     return (await this.db.get(`
       SELECT COALESCE(
         json_extract(access, '$.' || $uid),
-        ${0 < uid? `json_extract(access, '$.1'),`:""}
+        ${((0 < uid) ? `json_extract(access, '$.1'),`:"")}
         json_extract(access, '$.0')
       ) AS access
       FROM scenes
       WHERE scene_name = $scene
-    `, {$scene:scene, $uid:uid.toString(10)}))?.access;
+    `, {$scene:scene, $uid:uid.toString(10)}))?.access ?? null;
   }
 
   /**

source/server/integration.test.ts

@@ -52,7 +52,7 @@ describe("Web Server Integration", function(){
         await request(this.server).get("/scenes/foo/models/foo.glb").expect(200);
       });
       it("can't create a model", async function(){
-        await request(this.server).put("/scenes/bar/models/bar.glb").expect(401);
+        await request(this.server).put("/scenes/foo/models/bar.glb").expect(401);
       });
       it("can't fetch user list", async function(){
         await request(this.server).get("/api/v1/users")
@@ -191,8 +191,8 @@ describe("Web Server Integration", function(){
       .set("Accept", "")
       .expect(200);
     });
-    it("can't edit other people's models", async function(){
-      await this.agent.put("/scenes/foo/models/foo.glb").send("foo\n").expect(401);
+    it("can't edit other people's models (obfuscated as 404)", async function(){
+      await this.agent.put("/scenes/foo/models/foo.glb").send("foo\n").expect(404);
     });
     it.skip("can be granted permissions", async function(){
 

source/server/routes/api/v1/scenes/scene/get.test.ts

@@ -36,10 +36,10 @@ describe("GET /api/v1/scenes/:scene", function(){
       .expect("Content-Type", "application/json; charset=utf-8");
     });
 
-    it("is access-protected", async function(){
+    it("is access-protected (obfuscated as 404)", async function(){
       await userManager.grant("foo", "default", "none");
       await request(this.server).get("/api/v1/scenes/foo")
-      .expect(401);
+      .expect(404);
     });
   });
 

source/server/routes/scenes/get/file.test.ts

@@ -37,13 +37,13 @@ describe("GET /scenes/:scene/:filename(.*)", function(){
     .expect("foo\n");
   });
 
-  it("can't get a private scene's file", async function(){
+  it("can't get a private scene's file (obfuscated as 404)", async function(){
     let scene_id = await vfs.createScene("foo", {"0":"none", [user.uid]: "admin"});
     await vfs.writeDoc("{}", scene_id, user.uid);
     await vfs.writeFile(dataStream(), {scene: "foo", mime:"model/gltf-binary", name: "models/foo.glb", user_id: user.uid});
 
     await request(this.server).get("/scenes/foo/models/foo.glb")
-    .expect(401);
+    .expect(404);
   });
 
   it("can get an owned scene's file", async function(){

source/server/server.ts

@@ -205,7 +205,10 @@ export default async function createServer(config = defaultConfig) :Promise<expr
     }
     let code = (error instanceof HTTPError )? error.code : 500;
 
-    if(code === 401){
+    if(code === 401 && !(req.get("Accept")?.startsWith("text/html") && req.get("User-Agent")?.startsWith("Mozilla"))){
+      //We try to NOT send the header for browser requests because we prefer the HTML login to the browser's popup
+      //Browser tends to prefer text/html and always send Mozilla/5.0 at the beginning of their user-agent
+      //If someone has customized their headers, they'll get the ugly popup and live with it.
       res.set("WWW-Authenticate", "Basic realm=\"authenticated access\"");
     }
 

source/server/templates/error.hbs

@@ -1,8 +1,45 @@
+<corpus-navbar>
+  <nav-link href="/ui/scenes/" class="nav-link">Collection</nav-link>
+  <nav-link href="https://ethesaurus.holusion.com" class="nav-link">Documentation</nav-link>
+
+  <div class="divider"></div>
+  <user-button class="nav-link">Connexion</user-button>
+</corpus-navbar>
+<main  class="error-main">
 {{#with error}}
-<h1>Error :{{code}}</h1>
+<h1 class="error-title">Error :{{code}}</h1>
 
-<p>{{message}}</p>
+<p class="message">{{message}}</p>
 
-<pre><code>{{stack}}</code></pre>
+<pre class="stack"><code>{{stack}}</code></pre>
 
-{{/with}}
+{{/with}}
+</main>
+<modal-dialog></modal-dialog>
+
+<style>
+  main.error-main{
+    color:var(--color-light);
+    margin: auto;
+    max-width:1200px;
+    min-height: 60vh;
+
+    display:flex;
+    align-items: center;
+    flex-direction: column;
+    justify-content: center;
+  }
+  main.error-main .error-title{
+    color:var(--color-secondary);
+  }
+  main.error-main .message{
+    font-weight: bold;
+  }
+  main.error-main .stack{
+    background:var(--color-medium);
+    max-width:82ch;
+    padding: 1ch;
+  }
+</style>
+
+<script src="/js/corpus.js"></script>

source/server/utils/locals.ts

@@ -4,7 +4,7 @@ import {basename, dirname} from "path";
 import User, { SafeUser } from "../auth/User";
 import UserManager, { AccessType, AccessTypes } from "../auth/UserManager";
 import Vfs, { GetFileParams } from "../vfs";
-import { BadRequestError, ForbiddenError, HTTPError, InternalError, UnauthorizedError } from "./errors";
+import { BadRequestError, ForbiddenError, HTTPError, InternalError, NotFoundError, UnauthorizedError } from "./errors";
 
 export interface AppLocals extends Record<string, any>{
   port :number;
@@ -72,11 +72,20 @@ function _perms(check:number,req :Request, res :Response, next :NextFunction){
   if(isAdministrator) return next();
 
   let userManager = getUserManager(req);
-  (res.locals.access? Promise.resolve(res.locals.access):
-    userManager.getAccessRights(scene, uid).then(a=>{ res.locals.access = a; return a })
+  (res.locals.access? 
+    Promise.resolve(res.locals.access) :
+    userManager.getAccessRights(scene, uid)
   ).then( access => {
-    if(check <= AccessTypes.indexOf(access)) next();
-    else next(new UnauthorizedError(`user does not have ${AccessTypes[check]} rights on ${scene}`));
+    res.locals.access = access;
+    const lvl = AccessTypes.indexOf(access);
+    if(check <= lvl){
+      next();
+    } else if(req.method === "GET" || lvl <= AccessTypes.indexOf("none")){
+      next(new NotFoundError(`Can't find scene ${scene}. It may be private or not exist entirely.`))
+    } else {
+      //User has insuficient level but can read the scene
+      next(new UnauthorizedError(`user does not have ${AccessTypes[check]} rights on ${scene}`));
+    }
   }, next);
 }
 

source/ui/state/router.ts

@@ -86,6 +86,7 @@ export function router<T extends Constructor<LitElement>>(baseClass:T) : T & Con
 
       if(!this.inPath(url.pathname) && this.path != "/") return; //Return to bubble up the stack
       ev.stopPropagation(); //Handle the route change
+      ev.preventDefault();
 
       if(!url.pathname.endsWith("/")) url.pathname += "/";
       window.history.pushState({},"", url);
@@ -129,12 +130,17 @@ export function navigate(that :HTMLElement,href ?:string|URL, queries?:Record<st
       else url.searchParams.set(key, value.toString());
     }
   }
-  console.debug("Navigate to :", url.toString(), "with queries : ", queries);
-  (that ?? this).dispatchEvent(new CustomEvent("navigate", {
+  const unhandled = (that ?? this).dispatchEvent(new CustomEvent("navigate", {
     detail: {href: url},
     bubbles: true,
     composed: true
   }));
+
+  if(unhandled){
+    window.location.href = url.toString();
+  }else{
+    console.debug("Navigate to :", url.toString(), "with queries : ", queries);
+  }
 }
 
 export declare class Link{