mui / toolpad

Toolpad: Full stack components and low-code builder for dashboards and internal apps.
https://mui.com/toolpad/
MIT License
1.27k stars 282 forks source link

Should't show query content to client #1052

Closed WangLarry closed 2 years ago

WangLarry commented 2 years ago

Duplicates

Latest version

Current behavior 😯

'app/[appId]/[version]' entry is ssr, which call loadRenderTree. It load dom that contain query node's attributes.query.value, which is perhaps contain secret infos. So we should hidden it for client.

             "j2t3hpa": {
            "id": "j2t3hpa",
            "name": "query4",
            "type": "query",
            "parentId": "6k1pc4c",
            "attributes": {
              "query": {
                "type": "const",
                **"value": {
                  "module": "export default async function ({ params }: ToolpadFunctionEvent) {\n  console.info('Executing function with params:', params);\n  const url = new URL('https://gist.githubusercontent.com/saniyusuf/406b843afdfb9c6a86e25753fe2761f4/raw/523c324c7fcc36efab8224f9ebb7556c09b69a14/Film.JSON');\n  url.searchParams.set('timestamp', String(Date.now()));\n  const response = await fetch(String(url));\n  if (!response.ok) {\n    throw new Error(`HTTP ${response.status}: ${response.statusText}`);\n  }\n  return response.json();\n}"
                }**
              },
              "dataSource": {
                "type": "const",
                "value": "function"
              },
              "connectionId": {
                "type": "const",
                "value": null
              }
            },
            "parentProp": "queries",
            "parentIndex": "a5"
          },

Expected behavior πŸ€”

No response

Steps to reproduce πŸ•Ή

Steps:

1. 2. 3. 4.

Context πŸ”¦

No response

Your environment 🌎

No response

apedroferreira commented 2 years ago

Thanks @WangLarry ! @Janpot would know better but I'm pretty sure that it's not yet secure to use secrets in Toolpad if the application can be accessed by people you don't trust. That functionality is in the plans though!

Janpot commented 2 years ago

@WangLarry Would you be interested in opening a pull request?

To give you a starting point, the following should work:

index e3b73f85..7e0234ca 100644
--- a/packages/toolpad-app/src/appDom.ts
+++ b/packages/toolpad-app/src/appDom.ts
@@ -14,7 +14,7 @@ import { ConnectionStatus, AppTheme } from './types';
 import { omit, update, updateOrCreate } from './utils/immutability';
 import { camelCase, generateUniqueString, removeDiacritics } from './utils/strings';
 import { ExactEntriesOf, Maybe } from './utils/types';
-import { filterValues } from './utils/collections';
+import { mapProperties } from './utils/collections';

 export const RESERVED_NODE_PROPERTIES = [
   'id',
@@ -603,6 +603,23 @@ export function setNodeProp<Node extends AppDomNode, Prop extends BindableProps<
   });
 }

+function setNamespacedProp<
+  Node extends AppDomNode,
+  Namespace extends PropNamespaces<Node>,
+  Prop extends keyof Node[Namespace] & string,
+>(node: Node, namespace: Namespace, prop: Prop, value: Node[Namespace][Prop] | null): Node {
+  if (value) {
+    return update(node, {
+      [namespace]: updateOrCreate((node as Node)[namespace], {
+        [prop]: value,
+      } as any) as Partial<Node[Namespace]>,
+    } as Partial<Node>);
+  }
+  return update(node, {
+    [namespace]: omit(node[namespace], prop) as Partial<Node[Namespace]>,
+  } as Partial<Node>);
+}
+
 export function setNodeNamespacedProp<
   Node extends AppDomNode,
   Namespace extends PropNamespaces<Node>,
@@ -614,22 +631,9 @@ export function setNodeNamespacedProp<
   prop: Prop,
   value: Node[Namespace][Prop] | null,
 ): AppDom {
-  if (value) {
-    return update(dom, {
-      nodes: update(dom.nodes, {
-        [node.id]: update(dom.nodes[node.id], {
-          [namespace]: updateOrCreate((dom.nodes[node.id] as Node)[namespace], {
-            [prop]: value,
-          } as any) as Partial<Node[Namespace]>,
-        } as Partial<Node>),
-      }),
-    });
-  }
   return update(dom, {
     nodes: update(dom.nodes, {
-      [node.id]: update(node, {
-        [namespace]: omit(node[namespace], prop) as Partial<Node[Namespace]>,
-      } as Partial<Node>),
+      [node.id]: setNamespacedProp(node, namespace, prop, value),
     }),
   });
 }
@@ -863,16 +867,31 @@ export interface RenderTree {
   nodes: RenderTreeNodes;
 }

+const frontendNodes = new Set<string>(RENDERTREE_NODES);
+function createRenderTreeNode(node: AppDomNode): RenderTreeNode | null {
+  if (!frontendNodes.has(node.type)) {
+    return null;
+  }
+
+  if (isQuery(node) || isMutation(node)) {
+    node = setNamespacedProp(node, 'attributes', 'query', null);
+  }
+
+  return node as RenderTreeNode;
+}
+
 /**
  * We need to make sure no secrets end up in the frontend html, so let's only send the
  * nodes that we need to build frontend, and that we know don't contain secrets.
  * TODO: Would it make sense to create a separate datastructure that represents the render tree?
  */
 export function createRenderTree(dom: AppDom): RenderTree {
-  const frontendNodes = new Set<string>(RENDERTREE_NODES);
   return {
     ...dom,
-    nodes: filterValues(dom.nodes, (node) => frontendNodes.has(node.type)) as RenderTreeNodes,
+    nodes: mapProperties(dom.nodes, ([id, node]) => {
+      const rendernode = createRenderTreeNode(node);
+      return rendernode ? [id, rendernode] : null;
+    }),
   };
 }
WangLarry commented 2 years ago

Sorry, I am not familiar with pull request, and my local repo is in a mess.

Janpot commented 2 years ago

Ok, no worries.I'm opening https://github.com/mui/mui-toolpad/pull/1155