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.

Sign up for GitHub

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

Jump to bottom

feat: token items #353

Merged
merged 19 commits into from
Feb 4, 2025
Merged

feat: token items #353

merged 19 commits into from
Feb 4, 2025

Conversation

micahg
Copy link
Owner

@micahg micahg commented Jan 4, 2025 •
edited
Loading

feat: store tokens separate from overlay

github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 5, 2025
View reviewed changes
packages/api/src/config/express.ts
jwtCheck,
sceneTokenInstanceValidator(),
schemaErrorCheck,
getSceneTokenInstance,

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a database access
Loading
, but is not rate-limited.

Copilot Autofix

AI 3 months ago

To fix the problem, we need to introduce rate limiting to the Express application. The best way to do this is by using the express-rate-limit middleware. This middleware allows us to set a maximum number of requests that a client can make within a specified time window. We will apply this rate limiter to the specific route handler that performs the database access.

  1. Install the express-rate-limit package if it is not already installed.
  2. Import the express-rate-limit package in the packages/api/src/config/express.ts file.
  3. Create a rate limiter instance with appropriate configuration.
  4. Apply the rate limiter to the getSceneTokenInstance route handler.
Suggested changeset 2
packages/api/src/config/express.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/api/src/config/express.ts b/packages/api/src/config/express.ts
--- a/packages/api/src/config/express.ts
+++ b/packages/api/src/config/express.ts
@@ -1,3 +1,3 @@
 import * as os from "os";
-
+import * as rateLimit from "express-rate-limit";
 import { log } from "../utils/logger";
@@ -64,2 +64,7 @@
 
+const limiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 100, // limit each IP to 100 requests per windowMs
+});
+
 /**
@@ -280,2 +285,3 @@
     jwtCheck,
+    limiter,
     sceneTokenInstanceValidator(),
EOF
@@ -1,3 +1,3 @@
import * as os from "os";

import * as rateLimit from "express-rate-limit";
import { log } from "../utils/logger";
@@ -64,2 +64,7 @@

const limiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 requests per windowMs
});

/**
@@ -280,2 +285,3 @@
jwtCheck,
limiter,
sceneTokenInstanceValidator(),
packages/api/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/api/package.json b/packages/api/package.json
--- a/packages/api/package.json
+++ b/packages/api/package.json
@@ -73,3 +73,4 @@
     "winston": "^3.11.0",
-    "ws": "^8.17.1"
+    "ws": "^8.17.1",
+    "express-rate-limit": "^7.5.0"
   }
EOF
@@ -73,3 +73,4 @@
"winston": "^3.11.0",
"ws": "^8.17.1"
"ws": "^8.17.1",
"express-rate-limit": "^7.5.0"
}
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.5.0 None
Copilot is powered by AI and may make mistakes. Always verify output.
packages/api/src/config/express.ts
jwtCheck,
deleteTokenInstanceValidator(),
schemaErrorCheck,
deleteTokenInstance,

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a database access
Loading
, but is not rate-limited.

Copilot Autofix

AI 3 months ago

To fix the problem, we need to introduce rate limiting to the Express application. The best way to do this is by using the express-rate-limit package, which allows us to set up a rate limiter and apply it to the relevant routes. This will help prevent denial-of-service attacks by limiting the number of requests that can be made to the deleteTokenInstance route within a specified time window.

We will:

  1. Install the express-rate-limit package.
  2. Import the package in the packages/api/src/config/express.ts file.
  3. Set up a rate limiter with appropriate configuration.
  4. Apply the rate limiter to the deleteTokenInstance route.
Suggested changeset 2
packages/api/src/config/express.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/api/src/config/express.ts b/packages/api/src/config/express.ts
--- a/packages/api/src/config/express.ts
+++ b/packages/api/src/config/express.ts
@@ -5,2 +5,3 @@
 import { Express, NextFunction } from "express";
+import rateLimit from "express-rate-limit";
 import * as bodyParser from "body-parser";
@@ -68,2 +69,7 @@
  */
+const limiter = rateLimit({
+  windowMs: 15 * 60 * 1000, // 15 minutes
+  max: 100, // limit each IP to 100 requests per windowMs
+});
+
 function copyAuthParam(
@@ -287,2 +293,3 @@
     jwtCheck,
+    limiter,
     deleteTokenInstanceValidator(),
EOF
@@ -5,2 +5,3 @@
import { Express, NextFunction } from "express";
import rateLimit from "express-rate-limit";
import * as bodyParser from "body-parser";
@@ -68,2 +69,7 @@
*/
const limiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // limit each IP to 100 requests per windowMs
});

function copyAuthParam(
@@ -287,2 +293,3 @@
jwtCheck,
limiter,
deleteTokenInstanceValidator(),
packages/api/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/api/package.json b/packages/api/package.json
--- a/packages/api/package.json
+++ b/packages/api/package.json
@@ -73,3 +73,4 @@
     "winston": "^3.11.0",
-    "ws": "^8.17.1"
+    "ws": "^8.17.1",
+    "express-rate-limit": "^7.5.0"
   }
EOF
@@ -73,3 +73,4 @@
"winston": "^3.11.0",
"ws": "^8.17.1"
"ws": "^8.17.1",
"express-rate-limit": "^7.5.0"
}
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.5.0 None
Copilot is powered by AI and may make mistakes. Always verify output.
micahg added 11 commits January 4, 2025 20:05
@micahg
feat: add missing tests for token instance deletion
b3e8586
@micahg
feat: send token instance to server
1cfc397
@micahg
feat: unified scene interface, scene token loading
03e987b
@micahg
feat: fix excessive renders (#357)
57479b7 
* looks like we can do better on the rendering loop

* steps toward avoiding repaints by detecting the right overlay version

* feat: separate things from background and overlay

* chore: minimize differences
@micahg
fix: getting ready for drawing tokens
d8366d5
@micahg
feat: token items load bitmap (#358)
9a550a6 
* feat: tokens now display but only until something happens
@micahg
Feat/token items drawable placement (#360)
13146b7 
* feat: fix the token location when rendering existing token instances

* feat: work towards drawables

* feat: draw working for token

* feat: scale working

* fix: get rid of rendering glitch on token placement

* feat: rotating tokens works (but no tokens with scene rotation)

* feat: rotational placement with scene rotation

* chore: cleanup

* fix: correct placement glitches
@micahg
Feat/token items table state (#361)
b8b52f7 
* feat: refactor to add token instances to table state

* chore: make launch a little more descriptive

* feat: visible tokens for frontend

* fix: work with angle token instance in unit tests
@micahg
feat: create visible or invisible tokens (#362)
c334b99 
* feat: create remote branch

* feat: set visibility on instance token creation

* fix: whoops

* asdf

* feat: conversion not done.

* feat: start with hydrated token instance

* chore: store asset and token in hydrated instance

* fix: use asset in remote display component.

* chore: cleanup dependencies

* chore: cleanup
@micahg
fix: visible no longer part of token (unit test)
ee03b57
@micahg
chore: code review cleanup
3dab1aa
@micahg micahg merged commit ca6afc5 into main Feb 4, 2025
2 of 3 checks passed
@micahg micahg deleted the feat/token-items branch February 4, 2025 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@micahg