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

Rate Limit API Calls #391

Closed
wants to merge 17 commits into from
Closed

Rate Limit API Calls #391

wants to merge 17 commits into from

Conversation

nik-dange
Copy link
Member

Description

Closes #349

Changes

  • Added rate limiter middleware that applies to all routes

Type of Change

  • Patch (non-breaking change/bugfix)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to not work as
    expected)
  • Documentation (A change to a README/description)
  • Continuous Integration/DevOps Change (Related to deployment steps, continuous integration
    workflows, linting, etc.)
  • Other: (Fill In)

If you've selected Patch, Minor, or Major as your change type, make sure to bump the version before merging in package.json!

Testing

I have tested that my changes fully resolve the linked issue ...

  • locally.
  • on the testing API/testing database.
  • with appropriate Postman routes. Screenshots are included below.

Checklist

  • I have performed a self-review of my own code.
  • I have followed the style guidelines of this project.
  • I have appropriately edited the API version in the package.json file.
  • My changes produce no new warnings.

Screenshots

Please include a screenshot of your Postman testing passing successfully.

nik-dange and others added 12 commits December 19, 2023 09:20
* attendences from user uuid

* lint and bugfix

* check same user

* controller factory changes

* lint fixes

* unit test for get attendance by uuid

* lint

* add permision

* add types

* add everything else

* rename migrtion

* test when permision is off

* lint

* forgor to add

* change permission name and fix logic a bit

* rename permission, change patch user

* lint fix

* lint fix

* oops

* check user exists

* lint

* new ci

---------

Co-authored-by: Max Weng <[email protected]>
* started migration

* almost completed the query

* change MerchandiseItemModel picture to pictures

* renamed Merchandise to MercahndiseItems

* generated a new model for the pictures

* fixed syntax error with migration

* understand and fix casting error

* edited requests and created some todos

* return position == 0 instead of first picture in array

* migration temp fix

* this one line of code would attach pictures to the collection so that the frontend can display the first picture

* edited some todos to implement a new idea

* edited the service

* created a repository for photos

* Completed the photo create route

* completed the photo deletion route

* getting started with seeding

* make sure the index is consistent

* removed the current file name from url for security purpose

* quick linting

* edited seeding to ensure correctness

* update MerchFactory item for photo support

* refactor and renaming variables

* wrote outline for test and rewrote a method

* fix error

* edits

* removing some junk code

* the error is playing hide and seek with me

* im such a genius

* removing partial debug msgs

* edits

* fixed the order item test

* finished creating tests and pass all tests

* fixed some error

* I CHATGPTED THE SQL AND IT WORKED

* fixed linting error

* edit migration number order

* clean up some unused variables

* renamed picture to uploadedPhoto and photo to merchPhoto for clarity, added some documentation

* remove magic number

* slight seeding edit

* removed position logic

* clarify cascading quetsion

* fixed cascade

* clean up

* clarify seeding data structure

* link fix

* change position in request to string because form data does not accept number

* throw error if position is not a number

* updated deletion logic to delete from s3 first

* link fix

* default url logic fix for positions no longer being 0

* Get another user's event attendance (#358)

* attendences from user uuid

* lint and bugfix

* check same user

* controller factory changes

* lint fixes

* unit test for get attendance by uuid

* lint

* add permision

* add types

* add everything else

* rename migrtion

* test when permision is off

* lint

* forgor to add

* change permission name and fix logic a bit

* rename permission, change patch user

* lint fix

* lint fix

* oops

* check user exists

* lint

* rename tests

* public profile change

* change user model

* lint

* tests

* lint

* updated api version

---------

Co-authored-by: Nikhil Dange <[email protected]>

* staging deployment workflow (#381)

* bumped my migration file number

* bumped my migration file number v2

* remove local settings.json change

* added edge case for migration up

* lint

* lint

---------

Co-authored-by: Max Weng <[email protected]>
Co-authored-by: Nikhil Dange <[email protected]>
Co-authored-by: Nikhil Dange <[email protected]>
* migration

* Schema and response classes and interfaces

* validators and naming

* Merch Collection Photo Repository

* repository and models stuff

* api request bug

* renaming and bugfix

* renaming and stuff

* types

* add/delete collection photo, edit collection fix service

* naming and documentation

* migration name fix

* delete controller

* renaming and shifting types for consistency

* factory and start seed

* bugfixes for postman test

* tests

* weird tests stuff

* deletion logic change, cascade delete

* sort fix

* ling

* naming and some position logic

* merge stuff

* seed data change

* test change

* remove automatically end of the list

* lint stuff

* build and lint fix

* lint

* more lint

* fix migration
Copy link

Thanks for contributing!
If you've made changes to the API's functionality, please make sure to bump the package
version—see this guide to semantic versioning for details—and
document those changes as appropriate.

@nik-dange
Copy link
Member Author

Regarding the numbers:

What do we want for max requests in a certain time frame, specifically for Express Check-in & User Login? Do we also want to look at rate limiting other routes (while maybe not necessary, would be nice since the code exists)?

@farisashai
Copy link
Member

does this rate limit all event related endpoints on the api? If so, can you check with alex and Sean if it breaks the portal ui in any way from something simple like refreshing the events page nonstop

@nik-dange
Copy link
Member Author

nik-dange commented Jan 29, 2024

Oops, ignore line 14 in RateLimiter.ts + any changes in EventController.ts for now; that was just me experimenting with various ways of applying different limits to different endpoints. In theory, we can customize the numbers for different endpoints, or not limit any endpoints at all (in the case of something like fetching events, if it messes with the UI)

nik-dange and others added 5 commits January 31, 2024 16:48
@nik-dange
Copy link
Member Author

Accidentally included a bunch of other changes in this PR–nuking it for a cleaner one

@nik-dange nik-dange closed this Feb 15, 2024
@nik-dange nik-dange mentioned this pull request Feb 15, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rate limit user registration and express checkin routes
4 participants