Skip to content

fix(gql-api): remove GqlCustomsGuard from select resolvers#18798

Merged
chenba merged 1 commit intomainfrom
FXA-11209
May 1, 2025
Merged

fix(gql-api): remove GqlCustomsGuard from select resolvers#18798
chenba merged 1 commit intomainfrom
FXA-11209

Conversation

@chenba
Copy link
Copy Markdown
Contributor

@chenba chenba commented May 1, 2025

Because:

  • we are doubling the ip based counts in customs when both gql-api and auth-server (in response to gql-api) check customs on an action
  • a lot has changed since GqlCustomsGuard was added as some sort of preventive measure to denial-of-service from service call fan-out
    • we've added gql allowlist
    • we use Cloud Armor for ip based rate limiting
  • when GqlCustomsGuard rate limits a user, they see a generic error on the frontend

This commit:

  • removes GqlCustomsGuard if the resolver calls an auth-server API endpoint that also checks customs

Because:
 - we are doubling the ip based counts in customs when _both_ gql-api
   and auth-server (in response to gql-api) check customs on an action
 - a lot has changed since GqlCustomsGuard was added as some sort of
   preventive measure to denial-of-service from service call fan-out
   - we've added gql allowlist
   - we use Cloud Armor for ip based rate limiting
 - when GqlCustomsGuard rate limits a user, they see a generic error on
   the frontend

This commit:
 - removes GqlCustomsGuard if the resolver calls an auth-server API
   endpoint that also checks customs
@chenba chenba requested a review from a team as a code owner May 1, 2025 16:51
Copy link
Copy Markdown
Contributor

@vbudhram vbudhram left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find!

@chenba chenba merged commit ebbbc9e into main May 1, 2025
20 checks passed
@chenba chenba deleted the FXA-11209 branch May 1, 2025 18:15
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.

2 participants