From 53c8731bda0b46a4656535c7d7f8c5b8de7ca986 Mon Sep 17 00:00:00 2001 From: jay-dee7 Date: Sat, 25 Nov 2023 22:30:22 +0530 Subject: [PATCH] fix: Middleware logging Signed-off-by: jay-dee7 --- auth/basic_auth.go | 12 ++++++++++-- auth/jwt_middleware.go | 6 +++++- auth/oci_token.go | 39 +++++++++++++++++++++++++++++---------- auth/permissions.go | 14 ++++++++++++-- router/middlewares.go | 30 +++++++++++++++++++++--------- 5 files changed, 77 insertions(+), 24 deletions(-) diff --git a/auth/basic_auth.go b/auth/basic_auth.go index d7729e74..c06f054a 100644 --- a/auth/basic_auth.go +++ b/auth/basic_auth.go @@ -139,14 +139,22 @@ func (a *auth) SkipBasicAuth(ctx echo.Context) bool { // if Authorization header contains JWT, we skip basic auth and perform a JWT validation if ok := a.checkJWT(authHeader, ctx.Request().Cookies()); ok { - a.logger.Debug().Bool("skip_basic_auth", true).Str("method", ctx.Request().Method).Str("path", ctx.Request().URL.RequestURI()).Send() + a.logger.Debug(). + Bool("skip_basic_auth", true). + Str("method", ctx.Request().Method). + Str("path", ctx.Request().URL.RequestURI()). + Send() return true } readOp := ctx.Request().Method == http.MethodHead || ctx.Request().Method == http.MethodGet // if it's a read operation on a public repository, we skip auth requirement if readOp && repo != nil && repo.Visibility == types.RepositoryVisibilityPublic { - a.logger.Debug().Bool("skip_basic_auth", true).Str("method", ctx.Request().Method).Str("path", ctx.Request().URL.RequestURI()).Send() + a.logger.Debug(). + Bool("skip_basic_auth", true). + Str("method", ctx.Request().Method). + Str("path", ctx.Request().URL.RequestURI()). + Send() return true } diff --git a/auth/jwt_middleware.go b/auth/jwt_middleware.go index 473252e8..6f1b13c9 100644 --- a/auth/jwt_middleware.go +++ b/auth/jwt_middleware.go @@ -39,7 +39,11 @@ func (a *auth) JWT() echo.MiddlewareFunc { skip := readOp && repo.Visibility == types.RepositoryVisibilityPublic if skip { - a.logger.Debug().Bool("skip_jwt_middleware", true).Str("method", ctx.Request().Method).Str("path", ctx.Request().URL.RequestURI()).Send() + a.logger.Debug(). + Bool("skip_jwt_middleware", true). + Str("method", ctx.Request().Method). + Str("path", ctx.Request().URL.RequestURI()). + Send() } return skip diff --git a/auth/oci_token.go b/auth/oci_token.go index a3bcc638..4d54bb24 100644 --- a/auth/oci_token.go +++ b/auth/oci_token.go @@ -128,11 +128,11 @@ func (a *auth) Token(ctx echo.Context) error { scopes, err := ParseOCITokenPermissionRequest(ctx.Request().URL) if err != nil { - echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{ - "error": err.Error(), - "message": "invalid scope provided", + registryErr := common.RegistryErrorResponse(registry.RegistryErrorCodeUnknown, "invalid scope provided", echo.Map{ + "error": err.Error(), }) - a.logger.Log(ctx, err).Send() + echoErr := ctx.JSONBlob(http.StatusBadRequest, registryErr.Bytes()) + a.logger.Log(ctx, registryErr).Send() return echoErr } @@ -141,9 +141,16 @@ func (a *auth) Token(ctx echo.Context) error { if isPullRequest { repo, repoErr := a.registryStore.GetRepositoryByNamespace(ctx.Request().Context(), scopes[0].Name) if repoErr != nil { - common.RegistryErrorResponse(registry.RegistryErrorCodeNameInvalid, "requested resource does not exist on the registry", echo.Map{ - "error": repoErr.Error(), - }) + registryErr := common.RegistryErrorResponse( + registry.RegistryErrorCodeNameInvalid, + "requested resource does not exist on the registry", + echo.Map{ + "error": repoErr.Error(), + }, + ) + echoErr := ctx.JSONBlob(http.StatusBadRequest, registryErr.Bytes()) + a.logger.Log(ctx, registryErr).Send() + return echoErr } if repo.Visibility == types.RepositoryVisibilityPublic { @@ -163,7 +170,14 @@ func (a *auth) Token(ctx echo.Context) error { if authHeader != "" && len(scopes) != 0 { token, authErr := a.tryBasicAuthFlow(ctx, scopes) if authErr != nil { - echoErr := ctx.NoContent(http.StatusUnauthorized) + registryErr := common.RegistryErrorResponse( + registry.RegistryErrorCodeUnauthorized, + "authentication failed", + echo.Map{ + "error": authErr.Error(), + }, + ) + echoErr := ctx.JSONBlob(http.StatusUnauthorized, registryErr.Bytes()) a.logger.Log(ctx, authErr).Send() return echoErr } @@ -177,8 +191,13 @@ func (a *auth) Token(ctx echo.Context) error { return echoErr } - err = ctx.NoContent(http.StatusUnauthorized) - a.logger.Log(ctx, err).Send() + registryErr := common.RegistryErrorResponse( + registry.RegistryErrorCodeUnauthorized, + "authentication failed", + nil, + ) + err = ctx.JSONBlob(http.StatusUnauthorized, registryErr.Bytes()) + a.logger.Log(ctx, registryErr).Send() return err } diff --git a/auth/permissions.go b/auth/permissions.go index ab8d29a2..c12838d0 100644 --- a/auth/permissions.go +++ b/auth/permissions.go @@ -77,8 +77,18 @@ func (a *auth) populateUserFromPermissionsCheck(ctx echo.Context) error { func (a *auth) RepositoryPermissionsMiddleware() echo.MiddlewareFunc { return func(handler echo.HandlerFunc) echo.HandlerFunc { return func(ctx echo.Context) error { - a.populateUserFromPermissionsCheck(ctx) - a.populateUserFromPermissionsCheck(ctx) + if err := a.populateUserFromPermissionsCheck(ctx); err != nil { + registryErr := common.RegistryErrorResponse( + registry.RegistryErrorCodeDenied, + "invalid user credentials", + echo.Map{ + "error": err.Error(), + }, + ) + echoErr := ctx.JSONBlob(http.StatusBadRequest, registryErr.Bytes()) + a.logger.Log(ctx, err).Send() + return echoErr + } // handle skipping scenarios if ctx.QueryParam("offline_token") == "true" { a.logger.Log(ctx, nil).Bool("skipping_middleware", true).Str("request_type", "offline_token").Send() diff --git a/router/middlewares.go b/router/middlewares.go index a3ae8d9f..abd51295 100644 --- a/router/middlewares.go +++ b/router/middlewares.go @@ -24,9 +24,13 @@ func registryNamespaceValidator(logger telemetry.Logger) echo.MiddlewareFunc { namespace := username + "/" + imageName if username == "" || imageName == "" || !nsRegex.MatchString(namespace) { - registryErr := common.RegistryErrorResponse(registry.RegistryErrorCodeNameInvalid, "invalid user namespace", echo.Map{ - "error": "the required format for namespace is /", - }) + registryErr := common.RegistryErrorResponse( + registry.RegistryErrorCodeNameInvalid, + "invalid user namespace", + echo.Map{ + "error": "the required format for namespace is /", + }, + ) echoErr := ctx.JSONBlob(http.StatusBadRequest, registryErr.Bytes()) logger.Log(ctx, echoErr).Send() return echoErr @@ -47,9 +51,13 @@ func registryReferenceOrTagValidator(logger telemetry.Logger) echo.MiddlewareFun ref := ctx.Param("reference") if ref != "" && !refRegex.MatchString(ref) { - registryErr := common.RegistryErrorResponse(registry.RegistryErrorCodeTagInvalid, "reference/tag does not match the required format", echo.Map{ - "error": fmt.Sprintf("reference/tag must match the following regex: %s", refRegex.String()), - }) + registryErr := common.RegistryErrorResponse( + registry.RegistryErrorCodeTagInvalid, + "reference/tag does not match the required format", + echo.Map{ + "error": fmt.Sprintf("reference/tag must match the following regex: %s", refRegex.String()), + }, + ) echoErr := ctx.JSONBlob(http.StatusBadRequest, registryErr.Bytes()) logger.Log(ctx, registryErr).Send() @@ -68,9 +76,13 @@ func progagatRepository(store registry_store.RegistryStore, logger telemetry.Log user, ok := ctx.Get(string(types.UserContextKey)).(*types.User) if !ok { - registryErr := common.RegistryErrorResponse(registry.RegistryErrorCodeUnauthorized, "Unauthorized", echo.Map{ - "error": "User is not found in request context", - }) + registryErr := common.RegistryErrorResponse( + registry.RegistryErrorCodeUnauthorized, + "Unauthorized", + echo.Map{ + "error": "User is not found in request context", + }, + ) echoErr := ctx.JSONBlob(http.StatusBadRequest, registryErr.Bytes()) logger.Log(ctx, registryErr).Send() return echoErr