Conversation
|
@pozylon what do you think? and importable google wallet pass All imprts are with optional dependencies |
ea3fb7b to
ac07016
Compare
pozylon
left a comment
There was a problem hiding this comment.
Please check my comments, there is various topics that needs a bit polishing. After that's done we need to check again how we can make the createPDFTicketRenderer actually useful to create customized tickets.
459d3fb to
860030e
Compare
pozylon
left a comment
There was a problem hiding this comment.
Comes together nicely, now we need a factory patter for the pdf ticket renderer so someone can easily set logo, image, colors and such stuff to actually generate tickets that resemble a custom brand.
packages/ticketing/src/pdf-tickets/defaultTicketReceiptRenderer.ts
Outdated
Show resolved
Hide resolved
42b25ec to
820fd08
Compare
|
I haven't tested the apple wallet pass, please do on your end if you can |
288bab5 to
517fda9
Compare
|
It's hard to test this functionality as the ticketing example does currently not use the new functions, i'd expect something like this in the ticketing example: renderOrderPDF should be typed to something like this: Goal is to give users of the ticketing module a sane default to at least generate usable tickets. If the document generated is unusable it doesn't make sense to give helpers. It should actually print general tickets like in the theater or gastro projects but with a much more generalized design. User can do:
|
pozylon
left a comment
There was a problem hiding this comment.
See my other comment, although the way you structured it comes together well, the actual "value" for the developer is not leveraged
3cfe981 to
edf309b
Compare
15788ec to
db21131
Compare
|
@Mikearaya i have rebased this branch on master and did some code cleanups and added default initialization logic to the ticketing example with claude so make sure to hard reset before continuing your work. When done, we should be able to cleanup the Theater im Hof Repository substantially leveraging the factory functions to implement the simple ticket case. Please make sure the tests pass in the ticketing example and use that to test ticketing stuff. Somehow we need to be able to easily test the processes there so it should seed a product that is a ticket and do some checkout and everything. |
979ebb1 to
d291269
Compare
|
Added few more tests and fixed existing ones |
d291269 to
9b12b50
Compare
| fastify.route({ | ||
| url: `${APPLE_WALLET_WEBSERVICE_PATH}*`, | ||
| method: ['GET', 'POST', 'DELETE'], | ||
| handler: appleWalletHandler, |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 12 days ago
In general, the fix is to attach rate limiting middleware to the Fastify routes whose handlers perform authorization or other expensive operations. In Fastify applications this is commonly done with the @fastify/rate-limit plugin, which can be configured globally or scoped per-route/per-prefix. To avoid changing existing behavior, we should add a modest, configurable rate limit just to the Apple Wallet webservice routes defined here. This will constrain the number of requests per client IP in a given window while leaving the handlers’ logic, URLs, and methods unchanged.
The best targeted fix within this file is to register the @fastify/rate-limit plugin in a scoped way for the Apple Wallet routes by using fastify.register with a prefix equal to APPLE_WALLET_WEBSERVICE_PATH, and then defining the Apple Wallet route(s) inside that scoped instance with rate-limit options. However, we are constrained to only modify the given snippet and not change imports other than adding well-known packages, and we currently register that route directly on fastify. A minimal and clear approach is:
- Import
@fastify/rate-limit. - Register it near the top of the default export with a configuration appropriate for authorization endpoints (for example, 60 requests per minute per IP) and, to avoid affecting unrelated routes unexpectedly, apply the rate limit per-route via route-level
configor plugin options. - Attach the rate limit specifically to the Apple Wallet route by adding a
configproperty specifying rate-limit settings.
Concretely in packages/ticketing/src/fastify.ts:
- Add
import rateLimit from '@fastify/rate-limit';alongside the other imports. - Inside the exported default function, before defining routes, ensure the plugin is registered with the Fastify instance using
fastify.register(rateLimit, { global: false });so that no routes are rate-limited unless explicitly configured. - Update the Apple Wallet route definition to include a
configblock such as:This keeps behavior for other routes the same while enforcing a sensible default for authorization-heavy Apple Wallet endpoints.config: { rateLimit: { max: 60, timeWindow: '1 minute', }, }
| @@ -5,6 +5,7 @@ | ||
| import googleWalletHandler from './mobile-tickets/google-handler-fastify.ts'; | ||
| import printTicketsHandler from './pdf-tickets/print-handler-fastify.ts'; | ||
| import appleWalletHandler from './mobile-tickets/apple-handler-fastify.ts'; | ||
| import rateLimit from '@fastify/rate-limit'; | ||
|
|
||
| export default (fastify: FastifyInstance) => { | ||
| if (ticketingRoutes.length === 0) return; | ||
| @@ -14,6 +15,9 @@ | ||
| UNCHAINED_PDF_PRINT_HANDLER_PATH = '/rest/print_tickets', | ||
| } = process.env; | ||
|
|
||
| // Register rate limiting plugin with opt-in (non-global) configuration | ||
| fastify.register(rateLimit, { global: false }); | ||
|
|
||
| fastify.route({ | ||
| url: `${UNCHAINED_PDF_PRINT_HANDLER_PATH}`, | ||
| method: 'GET', | ||
| @@ -24,6 +28,12 @@ | ||
| url: `${APPLE_WALLET_WEBSERVICE_PATH}*`, | ||
| method: ['GET', 'POST', 'DELETE'], | ||
| handler: appleWalletHandler, | ||
| config: { | ||
| rateLimit: { | ||
| max: 60, | ||
| timeWindow: '1 minute', | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| fastify.route({ |
| @@ -41,7 +41,8 @@ | ||
| "@unchainedshop/core-worker": "^4.6.0", | ||
| "@unchainedshop/events": "^4.6.0", | ||
| "@unchainedshop/logger": "^4.6.0", | ||
| "@unchainedshop/mongodb": "^4.6.0" | ||
| "@unchainedshop/mongodb": "^4.6.0", | ||
| "@fastify/rate-limit": "^10.3.0" | ||
| }, | ||
| "peerDependencies": { | ||
| "@parse/node-apn": "^7.0.1", |
| Package | Version | Security advisories |
| @fastify/rate-limit (npm) | 10.3.0 | None |
|
I think this is not needed anymore because it is part of master already |
No description provided.