emit user_origin for local login event
#52124
Conversation
| @@ -271,6 +271,11 @@ func (r *Reporter) run(ctx context.Context) { | |||
|
|
|||
| return record | |||
| } | |||
| userRecordWithOrigin := func(userName string, v1AlphaUserKind prehogv1alpha.UserKind, v1AlphaUserOrigin prehogv1alpha.UserOrigin) *prehogv1.UserActivityRecord { | |||
| record := userRecord(userName, v1AlphaUserKind) | |||
| record.UserOrigin = prehogv1.UserOrigin(v1AlphaUserOrigin) | |||
There was a problem hiding this comment.
Casting enum types here and below. I've put a note on the proto definition to keep the enum values in sync. Open for suggestion to implement explicit conversion.
| @@ -161,7 +168,8 @@ func (a *Server) emitAuthAuditEvent(ctx context.Context, props authAuditProps) e | |||
| Success: true, | |||
| }, | |||
| UserMetadata: apievents.UserMetadata{ | |||
| User: props.username, | |||
| User: props.username, | |||
| UserOrigin: props.userOrigin, | |||
There was a problem hiding this comment.
The integer value will be reflected in the UI audit event. Not sold out on the alternative to use string field for audit event and the use int for the usage event. Open for suggestions.
I noticed we emit integer value for user_kind field.
| @@ -136,6 +141,7 @@ func (a *Server) authenticateUserLogin(ctx context.Context, req authclient.Authe | |||
| clientMetadata: req.ClientMetadata, | |||
| mfaDevice: mfaDev, | |||
| checker: checker, | |||
| userOrigin: userOrigin, | |||
There was a problem hiding this comment.
Is it possible to emit userOrigin on failure?
a.emitAuthAuditEven reused is invoked in multiple places, but right now the userOrigin is only set here in the successful flow.
There was a problem hiding this comment.
Not possible, only successful attempts are passed to the usage reporter. See https://github.com/gravitational/teleport/blob/master/lib/usagereporter/teleport/audit.go#L62C2-L67C1.
| userOrigin := apievents.UserOriginFromOriginLabel(user.Origin()) | ||
| if userOrigin == apievents.UserOrigin_USER_ORIGIN_UNSPECIFIED { | ||
| userOrigin = apievents.UserOriginFromUserType(user.GetUserType()) | ||
| } |
There was a problem hiding this comment.
| userOrigin := apievents.UserOriginFromOriginLabel(user.Origin()) | |
| if userOrigin == apievents.UserOrigin_USER_ORIGIN_UNSPECIFIED { | |
| userOrigin = apievents.UserOriginFromUserType(user.GetUserType()) | |
| } | |
| userOrigin := getUserOrgin(user) |
or even
emitAuthAuditEvent(ctx, authAuditProps{
...
userOrigin: getUserOrgin(user),
...
}| @@ -271,6 +271,17 @@ func (r *Reporter) run(ctx context.Context) { | |||
|
|
|||
| return record | |||
| } | |||
| userRecordWithOrigin := func(userName string, v1AlphaUserKind prehogv1alpha.UserKind, v1AlphaUserOrigin prehogv1alpha.UserOrigin) *prehogv1.UserActivityRecord { | |||
There was a problem hiding this comment.
| userRecordWithOrigin := func(userName string, v1AlphaUserKind prehogv1alpha.UserKind, v1AlphaUserOrigin prehogv1alpha.UserOrigin) *prehogv1.UserActivityRecord { | |
| userRecordWithOrigin := func(userName string, kind prehogv1alpha.UserKind, origin prehogv1alpha.UserOrigin) *prehogv1.UserActivityRecord { |
There was a problem hiding this comment.
Agree the naming could be better but if you see line above, the userRecordWithOrigin just wraps the userRecord function with origin label. And the userRecord uses the same function param names v1AlphaUserKind. So imo its rather ideal to reuse the same param names rather than having two different terminology within a same method.
| record := userRecord(userName, v1AlphaUserKind) | ||
| if v1AlphaUserOrigin == prehogv1alpha.UserOrigin_USER_ORIGIN_UNSPECIFIED { | ||
| // Ignore unspecified value cause the request may be coming from an | ||
| // older auth that does not process user_origin. | ||
| return record | ||
| } | ||
| // Allow overriding origin value with a new value. | ||
| record.UserOrigin = prehogv1.UserOrigin(v1AlphaUserOrigin) | ||
| return record |
There was a problem hiding this comment.
Why not just:
| record := userRecord(userName, v1AlphaUserKind) | |
| if v1AlphaUserOrigin == prehogv1alpha.UserOrigin_USER_ORIGIN_UNSPECIFIED { | |
| // Ignore unspecified value cause the request may be coming from an | |
| // older auth that does not process user_origin. | |
| return record | |
| } | |
| // Allow overriding origin value with a new value. | |
| record.UserOrigin = prehogv1.UserOrigin(v1AlphaUserOrigin) | |
| return record | |
| record := userRecord(userName, v1AlphaUserKind) | |
| record.UserOrigin = prehogv1.UserOrigin(v1AlphaUserOrigin) | |
| return record |
There was a problem hiding this comment.
Because an older auth might override correct origin value set by the latest auth so the skipping USER_ORIGIN_UNSPECIFIED saves us from that.
| } | ||
| } | ||
|
|
||
| // UserOriginFromUserType converts API origin label value to UserOrigin. |
There was a problem hiding this comment.
| // UserOriginFromUserType converts API origin label value to UserOrigin. | |
| // UserOriginFromOriginLabel converts API origin label value to UserOrigin. |
As part of Extend user activity report events with Identity events we want to emit user origin in user login event to distinguish user originated from Identity Governance related integration. This PR updates local user login event to include
user_originvalue and have them emitted in theUserActivityRecordand theUserLoginevent.To pass the user origin value to the audit and usage event, new
user_loginfield has been added toUserMetadatatype.The
UserMetadatais included in many user related audit event, includingUserLogintype that is emitted during local login. We set theuser_loginvalue only for theUserLoginevent.The origin label of the user resource metadata
teleport.dev/originis used to retrieve the origin value. If the label is not set, then we fall back to user type, which can belocalorsso.changelog: Updates local user login audit event to include
user_originfield.Part of https://github.com/gravitational/teleport.e/issues/5946