-
Notifications
You must be signed in to change notification settings - Fork 438
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
appsec: differentiate user login and user set event #2956
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,36 +7,58 @@ package usersec | |
|
||
import ( | ||
"context" | ||
"sync" | ||
|
||
"gopkg.in/DataDog/dd-trace-go.v1/appsec/events" | ||
"gopkg.in/DataDog/dd-trace-go.v1/internal/appsec/dyngo" | ||
"gopkg.in/DataDog/dd-trace-go.v1/internal/log" | ||
) | ||
|
||
const errorLog = ` | ||
appsec: user login monitoring ignored: could not find the http handler instrumentation metadata in the request context: | ||
the request handler is not being monitored by a middleware function or the provided context is not the expected request context | ||
the request handler is not being monitored by a middleware function or the provided context is not the expected request context. | ||
If the user has been blocked using remote rules, blocking will still be enforced but it will not be reported. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes you are right, I removed it |
||
` | ||
|
||
var errorLogOnce sync.Once | ||
|
||
type ( | ||
// UserEventType is the type of user event, such as a successful login or a failed login or any other authenticated request. | ||
UserEventType int | ||
|
||
// UserLoginOperation type representing a call to appsec.SetUser(). It gets both created and destroyed in a single | ||
// call to ExecuteUserIDOperation | ||
UserLoginOperation struct { | ||
dyngo.Operation | ||
EventType UserEventType | ||
} | ||
// UserLoginOperationArgs is the user ID operation arguments. | ||
UserLoginOperationArgs struct{} | ||
UserLoginOperationArgs struct { | ||
} | ||
|
||
// UserLoginOperationRes is the user ID operation results. | ||
UserLoginOperationRes struct { | ||
UserID string | ||
SessionID string | ||
Success bool | ||
} | ||
) | ||
|
||
func StartUserLoginOperation(ctx context.Context, args UserLoginOperationArgs) (*UserLoginOperation, *error) { | ||
parent, _ := dyngo.FromContext(ctx) | ||
op := &UserLoginOperation{Operation: dyngo.NewOperation(parent)} | ||
const ( | ||
// UserLoginSuccess is the event type for a successful user login, when a new session or JWT is created. | ||
UserLoginSuccess UserEventType = iota | ||
// UserLoginFailure is the event type for a failed user login, when the user ID is not found or the password is incorrect. | ||
UserLoginFailure | ||
// UserSet is the event type for a user ID operation that is not a login, such as any authenticated request made by the user. | ||
UserSet | ||
) | ||
|
||
func StartUserLoginOperation(ctx context.Context, eventType UserEventType, args UserLoginOperationArgs) (*UserLoginOperation, *error) { | ||
parent, ok := dyngo.FromContext(ctx) | ||
if !ok { // Nothing will be reported in this case, but we can still block so we don't return | ||
errorLogOnce.Do(func() { log.Error(errorLog) }) | ||
} | ||
|
||
op := &UserLoginOperation{Operation: dyngo.NewOperation(parent), EventType: eventType} | ||
var err error | ||
dyngo.OnData(op, func(e *events.BlockingSecurityEvent) { err = e }) | ||
dyngo.StartOperation(op, args) | ||
|
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.
appsec.SetUser doing a fake user login event is a bad design idea.
generally speaking, all those fake operations are coming from the time we didn't have EmitData and I believe they should be replaced by that to simplify everything.
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.
It's not really a "fake" user login operation, it's just badly named so I renamed it.
We can't use EmitData yet because we are missing context on the listener side of things but it could also be useful if we want to do some autoinstrumentation on auth frameworks later.