-
Notifications
You must be signed in to change notification settings - Fork 62
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
Support multiple, configurable client ids to protect API limits. #18677
Conversation
…ts-api into btsss/multi-client-ids
…ts-api into btsss/multi-client-ids
…ts-api into btsss/multi-client-ids
…ts-api into btsss/multi-client-ids
Generated by 🚫 Danger |
Backend-review-group approval confirmed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this mostly makes sense.... I do have some comments, and am curious about next steps re: authenticated sessions per user in addition to per client (but I understand this PR is only in re: the client aspect).
…ts-api into btsss/multi-client-ids
…ts-api into btsss/multi-client-ids
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I see a few places where you commented out the before do
call to allow the token call to go through, not sure if you were leaving the commented code there for a reason or if it can be removed, but not gonna block you for that, so still approved. 🎉
This work enables different integrations to send their own client number so the upstream API can differentiate traffic and enforce rate limits appropriately.
It looks like this:
It works like this:
Closes department-of-veterans-affairs/va.gov-team#91982