From ef116091423d8ccd2c8921bb4a51311e6501a58f Mon Sep 17 00:00:00 2001 From: Stefan Benz <46600784+stebenz@users.noreply.github.com> Date: Tue, 28 Nov 2023 16:56:29 +0100 Subject: [PATCH] fix: add https status to activity log (#6978) * fix: add https status to activity log * create prerelease * create RC * pass info from gateway to grpc server * fix: update releaserc to create RC version * cleanup --------- Co-authored-by: Livio Spring --- cmd/start/start.go | 2 - internal/activity/activity.go | 39 +++++++------------ .../api/grpc/client/middleware/activity.go | 20 ++++++++++ internal/api/grpc/server/gateway.go | 11 +++++- .../server/middleware/activity_interceptor.go | 24 +++++++++--- internal/api/grpc/session/v2/session.go | 4 -- internal/api/oidc/auth_request.go | 14 +++---- internal/api/oidc/op.go | 1 + internal/api/saml/provider.go | 1 + internal/api/saml/storage.go | 2 +- internal/command/session.go | 3 ++ 11 files changed, 75 insertions(+), 46 deletions(-) create mode 100644 internal/api/grpc/client/middleware/activity.go diff --git a/cmd/start/start.go b/cmd/start/start.go index 03651dc3e8..519772ef7c 100644 --- a/cmd/start/start.go +++ b/cmd/start/start.go @@ -319,8 +319,6 @@ func startAPIs( oidcPrefixes := []string{"/.well-known/openid-configuration", "/oidc/v1", "/oauth/v2"} // always set the origin in the context if available in the http headers, no matter for what protocol router.Use(middleware.OriginHandler) - // adds used HTTPPathPattern and RequestMethod to context - router.Use(middleware.ActivityHandler) systemTokenVerifier, err := internal_authz.StartSystemTokenVerifierFromConfig(http_util.BuildHTTP(config.ExternalDomain, config.ExternalPort, config.ExternalSecure), config.SystemAPIUsers) if err != nil { return err diff --git a/internal/activity/activity.go b/internal/activity/activity.go index 1f89d8cd2b..b62b3094c5 100644 --- a/internal/activity/activity.go +++ b/internal/activity/activity.go @@ -2,7 +2,9 @@ package activity import ( "context" + "strconv" + "github.com/grpc-ecosystem/grpc-gateway/v2/runtime" "github.com/zitadel/logging" "github.com/zitadel/zitadel/internal/api/authz" @@ -12,6 +14,9 @@ import ( const ( Activity = "activity" + + PathKey = "zitadel-activity-path" + RequestMethodKey = "zitadel-activity-request-method" ) type TriggerMethod int @@ -44,7 +49,8 @@ func (t TriggerMethod) String() string { } } -func TriggerHTTP(ctx context.Context, orgID, userID string, trigger TriggerMethod) { +// Trigger is used to log a specific events for a user (e.g. session or oidc token creation) +func Trigger(ctx context.Context, orgID, userID string, trigger TriggerMethod) { ai := info.ActivityInfoFromContext(ctx) triggerLog( authz.GetInstance(ctx).InstanceID(), @@ -56,47 +62,29 @@ func TriggerHTTP(ctx context.Context, orgID, userID string, trigger TriggerMetho ai.Path, ai.RequestMethod, "", - authz.GetCtxData(ctx).SystemMemberships != nil, - ) -} - -func TriggerGRPC(ctx context.Context, orgID, userID string, trigger TriggerMethod) { - ai := info.ActivityInfoFromContext(ctx) - // GRPC call the method is contained in the HTTP request path - method := ai.Path - triggerLog( - authz.GetInstance(ctx).InstanceID(), - orgID, - userID, - http_utils.ComposedOrigin(ctx), - trigger, - method, "", - ai.RequestMethod, - ai.GRPCStatus.String(), authz.GetCtxData(ctx).SystemMemberships != nil, ) } func TriggerGRPCWithContext(ctx context.Context, trigger TriggerMethod) { ai := info.ActivityInfoFromContext(ctx) - // GRPC call the method is contained in the HTTP request path - method := ai.Path triggerLog( authz.GetInstance(ctx).InstanceID(), authz.GetCtxData(ctx).OrgID, authz.GetCtxData(ctx).UserID, http_utils.ComposedOrigin(ctx), trigger, - method, - "", + ai.Method, + ai.Path, ai.RequestMethod, - ai.GRPCStatus.String(), + strconv.Itoa(int(ai.GRPCStatus)), + strconv.Itoa(runtime.HTTPStatusFromCode(ai.GRPCStatus)), authz.GetCtxData(ctx).SystemMemberships != nil, ) } -func triggerLog(instanceID, orgID, userID, domain string, trigger TriggerMethod, method, path, requestMethod, status string, isSystemUser bool) { +func triggerLog(instanceID, orgID, userID, domain string, trigger TriggerMethod, method, path, requestMethod, grpcStatus, httpStatus string, isSystemUser bool) { logging.WithFields( "instance", instanceID, "org", orgID, @@ -105,7 +93,8 @@ func triggerLog(instanceID, orgID, userID, domain string, trigger TriggerMethod, "trigger", trigger.String(), "method", method, "path", path, - "grpcStatus", status, + "grpcStatus", grpcStatus, + "httpStatus", httpStatus, "requestMethod", requestMethod, "isSystemUser", isSystemUser, ).Info(Activity) diff --git a/internal/api/grpc/client/middleware/activity.go b/internal/api/grpc/client/middleware/activity.go new file mode 100644 index 0000000000..fd676523c4 --- /dev/null +++ b/internal/api/grpc/client/middleware/activity.go @@ -0,0 +1,20 @@ +package middleware + +import ( + "context" + + "google.golang.org/grpc" + "google.golang.org/grpc/metadata" + + "github.com/zitadel/zitadel/internal/activity" + "github.com/zitadel/zitadel/internal/api/info" +) + +func UnaryActivityClientInterceptor() grpc.UnaryClientInterceptor { + return func(ctx context.Context, method string, req, reply any, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error { + activityInfo := info.ActivityInfoFromContext(ctx) + ctx = metadata.AppendToOutgoingContext(ctx, activity.PathKey, activityInfo.Path) + ctx = metadata.AppendToOutgoingContext(ctx, activity.RequestMethodKey, activityInfo.RequestMethod) + return invoker(ctx, method, req, reply, cc, opts...) + } +} diff --git a/internal/api/grpc/server/gateway.go b/internal/api/grpc/server/gateway.go index 6c506ea34f..a4b89d5fca 100644 --- a/internal/api/grpc/server/gateway.go +++ b/internal/api/grpc/server/gateway.go @@ -96,7 +96,10 @@ func CreateGatewayWithPrefix( runtimeMux := runtime.NewServeMux(serveMuxOptions...) opts := []grpc.DialOption{ grpc.WithTransportCredentials(grpcCredentials(tlsConfig)), - grpc.WithUnaryInterceptor(client_middleware.DefaultTracingClient()), + grpc.WithChainUnaryInterceptor( + client_middleware.DefaultTracingClient(), + client_middleware.UnaryActivityClientInterceptor(), + ), } connection, err := dial(ctx, port, opts) if err != nil { @@ -120,7 +123,10 @@ func CreateGateway( port, []grpc.DialOption{ grpc.WithTransportCredentials(grpcCredentials(tlsConfig)), - grpc.WithUnaryInterceptor(client_middleware.DefaultTracingClient()), + grpc.WithChainUnaryInterceptor( + client_middleware.DefaultTracingClient(), + client_middleware.UnaryActivityClientInterceptor(), + ), }) if err != nil { return nil, err @@ -176,6 +182,7 @@ func addInterceptors( handler = http_mw.CORSInterceptor(handler) handler = http_mw.RobotsTagHandler(handler) handler = http_mw.DefaultTelemetryHandler(handler) + handler = http_mw.ActivityHandler(handler) // For some non-obvious reason, the exhaustedCookieInterceptor sends the SetCookie header // only if it follows the http_mw.DefaultTelemetryHandler handler = exhaustedCookieInterceptor(handler, accessInterceptor, queries) diff --git a/internal/api/grpc/server/middleware/activity_interceptor.go b/internal/api/grpc/server/middleware/activity_interceptor.go index a75108328f..79a3930c2d 100644 --- a/internal/api/grpc/server/middleware/activity_interceptor.go +++ b/internal/api/grpc/server/middleware/activity_interceptor.go @@ -6,6 +6,7 @@ import ( "strings" "google.golang.org/grpc" + "google.golang.org/grpc/metadata" "github.com/zitadel/zitadel/internal/activity" "github.com/zitadel/zitadel/internal/api/grpc/errors" @@ -14,17 +15,13 @@ import ( func ActivityInterceptor() grpc.UnaryServerInterceptor { return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { + ctx = activityInfoFromGateway(ctx).SetMethod(info.FullMethod).IntoContext(ctx) resp, err := handler(ctx, req) if isResourceAPI(info.FullMethod) { code, _, _, _ := errors.ExtractCaosError(err) ctx = ainfo.ActivityInfoFromContext(ctx).SetGRPCStatus(code).IntoContext(ctx) activity.TriggerGRPCWithContext(ctx, activity.ResourceAPI) } - if strings.HasPrefix(info.FullMethod, "/zitadel.session.v1.SessionService/") { - code, _, _, _ := errors.ExtractCaosError(err) - ctx = ainfo.ActivityInfoFromContext(ctx).SetGRPCStatus(code).IntoContext(ctx) - activity.TriggerGRPCWithContext(ctx, activity.SessionAPI) - } return resp, err } } @@ -42,3 +39,20 @@ func isResourceAPI(method string) bool { return strings.HasPrefix(method, prefix) }) } + +func activityInfoFromGateway(ctx context.Context) *ainfo.ActivityInfo { + info := ainfo.ActivityInfoFromContext(ctx) + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return info + } + path := md.Get(activity.PathKey) + if len(path) != 1 { + return info + } + requestMethod := md.Get(activity.RequestMethodKey) + if len(requestMethod) != 1 { + return info + } + return info.SetPath(path[0]).SetRequestMethod(requestMethod[0]) +} diff --git a/internal/api/grpc/session/v2/session.go b/internal/api/grpc/session/v2/session.go index 25e0c2af58..ff99c1ae5f 100644 --- a/internal/api/grpc/session/v2/session.go +++ b/internal/api/grpc/session/v2/session.go @@ -11,7 +11,6 @@ import ( "github.com/muhlemmer/gu" - "github.com/zitadel/zitadel/internal/activity" "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/api/grpc/object/v2" "github.com/zitadel/zitadel/internal/command" @@ -352,9 +351,6 @@ func (s *Server) checksToCommand(ctx context.Context, checks *session.Checks) ([ if err != nil { return nil, err } - - // trigger activity log for session for user - activity.TriggerHTTP(ctx, user.ResourceOwner, user.ID, activity.SessionAPI) sessionChecks = append(sessionChecks, command.CheckUser(user.ID, user.ResourceOwner)) } if password := checks.GetPassword(); password != nil { diff --git a/internal/api/oidc/auth_request.go b/internal/api/oidc/auth_request.go index 1d4cad0c32..fa1e919f6b 100644 --- a/internal/api/oidc/auth_request.go +++ b/internal/api/oidc/auth_request.go @@ -198,7 +198,7 @@ func (o *OPStorage) CreateAccessToken(ctx context.Context, req op.TokenRequest) userOrgID = authReq.UserOrgID case *AuthRequestV2: // trigger activity log for authentication for user - activity.TriggerHTTP(ctx, "", authReq.CurrentAuthRequest.UserID, activity.OIDCAccessToken) + activity.Trigger(ctx, "", authReq.CurrentAuthRequest.UserID, activity.OIDCAccessToken) return o.command.AddOIDCSessionAccessToken(setContextUserSystem(ctx), authReq.GetID()) } @@ -213,7 +213,7 @@ func (o *OPStorage) CreateAccessToken(ctx context.Context, req op.TokenRequest) } // trigger activity log for authentication for user - activity.TriggerHTTP(ctx, userOrgID, req.GetSubject(), activity.OIDCAccessToken) + activity.Trigger(ctx, userOrgID, req.GetSubject(), activity.OIDCAccessToken) return resp.TokenID, resp.Expiration, nil } @@ -225,11 +225,11 @@ func (o *OPStorage) CreateAccessAndRefreshTokens(ctx context.Context, req op.Tok switch tokenReq := req.(type) { case *AuthRequestV2: // trigger activity log for authentication for user - activity.TriggerHTTP(ctx, "", tokenReq.GetSubject(), activity.OIDCRefreshToken) + activity.Trigger(ctx, "", tokenReq.GetSubject(), activity.OIDCRefreshToken) return o.command.AddOIDCSessionRefreshAndAccessToken(setContextUserSystem(ctx), tokenReq.GetID()) case *RefreshTokenRequestV2: // trigger activity log for authentication for user - activity.TriggerHTTP(ctx, "", tokenReq.GetSubject(), activity.OIDCRefreshToken) + activity.Trigger(ctx, "", tokenReq.GetSubject(), activity.OIDCRefreshToken) return o.command.ExchangeOIDCSessionRefreshAndAccessToken(setContextUserSystem(ctx), tokenReq.OIDCSessionWriteModel.AggregateID, refreshToken, tokenReq.RequestedScopes) } @@ -258,7 +258,7 @@ func (o *OPStorage) CreateAccessAndRefreshTokens(ctx context.Context, req op.Tok } // trigger activity log for authentication for user - activity.TriggerHTTP(ctx, userOrgID, req.GetSubject(), activity.OIDCRefreshToken) + activity.Trigger(ctx, userOrgID, req.GetSubject(), activity.OIDCRefreshToken) return resp.TokenID, token, resp.Expiration, nil } @@ -288,7 +288,7 @@ func (o *OPStorage) TokenRequestByRefreshToken(ctx context.Context, refreshToken return nil, err } // trigger activity log for authentication for user - activity.TriggerHTTP(ctx, "", oidcSession.UserID, activity.OIDCRefreshToken) + activity.Trigger(ctx, "", oidcSession.UserID, activity.OIDCRefreshToken) return &RefreshTokenRequestV2{OIDCSessionWriteModel: oidcSession}, nil } @@ -298,7 +298,7 @@ func (o *OPStorage) TokenRequestByRefreshToken(ctx context.Context, refreshToken } // trigger activity log for use of refresh token for user - activity.TriggerHTTP(ctx, tokenView.ResourceOwner, tokenView.UserID, activity.OIDCRefreshToken) + activity.Trigger(ctx, tokenView.ResourceOwner, tokenView.UserID, activity.OIDCRefreshToken) return RefreshTokenRequestFromBusiness(tokenView), nil } diff --git a/internal/api/oidc/op.go b/internal/api/oidc/op.go index e615efabcf..defa5fdcdc 100644 --- a/internal/api/oidc/op.go +++ b/internal/api/oidc/op.go @@ -145,6 +145,7 @@ func NewServer( userAgentCookie, http_utils.CopyHeadersToContext, accessHandler.HandleIgnorePathPrefixes(ignoredQuotaLimitEndpoint(config.CustomEndpoints)), + middleware.ActivityHandler, )) return server, nil diff --git a/internal/api/saml/provider.go b/internal/api/saml/provider.go index 644fe2886a..e11c281871 100644 --- a/internal/api/saml/provider.go +++ b/internal/api/saml/provider.go @@ -65,6 +65,7 @@ func NewProvider( userAgentCookie, accessHandler.HandleIgnorePathPrefixes(ignoredQuotaLimitEndpoint(conf.ProviderConfig)), http_utils.CopyHeadersToContext, + middleware.ActivityHandler, ), provider.WithCustomTimeFormat("2006-01-02T15:04:05.999Z"), } diff --git a/internal/api/saml/storage.go b/internal/api/saml/storage.go index 2156a347dc..cad83d86a5 100644 --- a/internal/api/saml/storage.go +++ b/internal/api/saml/storage.go @@ -151,7 +151,7 @@ func (p *Storage) SetUserinfoWithUserID(ctx context.Context, applicationID strin setUserinfo(user, userinfo, attributes, customAttributes) // trigger activity log for authentication for user - activity.TriggerHTTP(ctx, user.ResourceOwner, user.ID, activity.SAMLResponse) + activity.Trigger(ctx, user.ResourceOwner, user.ID, activity.SAMLResponse) return nil } diff --git a/internal/command/session.go b/internal/command/session.go index c62b6ccee8..fefb24ddda 100644 --- a/internal/command/session.go +++ b/internal/command/session.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "github.com/zitadel/zitadel/internal/activity" "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" @@ -226,6 +227,8 @@ func (s *SessionCommands) OTPEmailChecked(ctx context.Context, checkedAt time.Ti } func (s *SessionCommands) SetToken(ctx context.Context, tokenID string) { + // trigger activity log for session for user + activity.Trigger(ctx, s.sessionWriteModel.UserResourceOwner, s.sessionWriteModel.UserID, activity.SessionAPI) s.eventCommands = append(s.eventCommands, session.NewTokenSetEvent(ctx, s.sessionWriteModel.aggregate, tokenID)) }