-
Notifications
You must be signed in to change notification settings - Fork 66
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
API-34434 Reject non-gateway requests to some APIs #15751
Conversation
80bf091
to
ec7f286
Compare
ec7f286
to
6da7609
Compare
# In deployed environments, the `source` parameter is filled in by the gateway - we only | ||
# want to accept requests that come through the gateway: | ||
raise Common::Exceptions::Unauthorized if Rails.env.production? && params[:source].blank? | ||
end |
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.
This code looks good at first glance, but have you confirmed that we definitely receive a source
parameter? I checked our controller logs in Datadog and didn't see it listed alongside our params like id
, icn
, receiptDate
, etc., which concerned me. Is it there but just being excluded from our logs?
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.
@cilestin told me that the source parameter is added automatically by Kong, but I don't know how to confirm that for sure
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.
@caseywilliams Let's put this behind a feature flag so that we can change it live should things go wonky. Default to false and we can turn it on in different envs to ensure Prod isn't affected until we've tested it.
6da7609
to
8f63b84
Compare
# In deployed environments, the `source` parameter is filled in by the gateway - we only | ||
# want to accept requests that come through the gateway: | ||
raise Common::Exceptions::Unauthorized if Rails.env.production? && params[:source].blank? | ||
end |
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.
@caseywilliams Let's put this behind a feature flag so that we can change it live should things go wonky. Default to false and we can turn it on in different envs to ensure Prod isn't affected until we've tested it.
8f63b84
to
b1829d7
Compare
b1829d7
to
b462e3b
Compare
b462e3b
to
b4c53ba
Compare
Summary
Updates APIs in the
appeals_api
,va_forms
, andvba_documents
modules so that they return an Unauthorized error if the request did not come through the gateway (only applies in production mode)Related issue(s)
Testing done
What areas of the site does it impact?
ApplicationController
s are affectedAcceptance criteria