-
Notifications
You must be signed in to change notification settings - Fork 462
internal/appsec: request blocking based on user ID #1706
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
Conversation
5cde1f1
to
bb27190
Compare
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.
Peer review comments
d9fb6dd
to
c4dbc67
Compare
c4dbc67
to
87f3fb6
Compare
appsec/appsec.go
Outdated
"google.golang.org/grpc/codes" | ||
"google.golang.org/grpc/status" |
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 is a problem: we need to keep our dependency tree as lean as possible.
Here is an example of the resulting issue if not: an HTTP-only service will suddenly depend on gRPC (and all its sub-dependencies).
The reason why this is currently imported isn't valid as we discussed previously: the exact reason of the blocking will have to be split into separate sub-packages of this one (ie. public httpsec and grpcsec packages). But from now on, I suggest you to keep it simple and focused on our main goal here: returning an error without details means blocking.
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.
Side note: net/http
is considered acceptable because it lives in the standard library.
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.
956681d cleans this up. Everything is abstracted under the error
interface and set within grpc/http dependant packages. I also changed the gRPC start operation to directly hold an error rather that a status code since status codes are communicated through errors anyway. The result is much cleaner and simpler - while we wait to refactor the actions system.
SetUser() can be used to associate a user ID to a tracer. With the appsec version, an error is returned to indicate whether requests attributed to this user ID should be blocked.
03d8b04
to
da5abdd
Compare
bc3da20
to
6fe5fc5
Compare
e857a5e
to
b5b9601
Compare
b5b9601
to
88d7881
Compare
Co-authored-by: Julio Guerra <julio@datadog.com>
What does this PR do?
This change adds a new SDK to the appsec top level package,
SetUser()
.SetUser()
wraps the existingtracer.SetUser()
and performs a WAF call to check if the user ID passed to
SetUser
is on a block list, in which case an error is returned,instructing the SDK user to block the request execution as soon as possible.
Additionally:
ASMUserBlocking
, and registered during remote config initialization by appsecTrackUserLoginSuccessEvent()
now usesappsec.SetUser()
and returns an error if the user should be blockedMotivation
This is a planned feature.
Describe how to test/QA your changes
The change includes integration tests to test both the high level API usage (in the appsec package) and
the internal mechanics (internal/appsec), as well as the gRPC integration (contrib/google.golang.org/grpc).
Reviewer's Checklist
Triage
milestone is set.