From f6ec9744059518b82e8b33ef1a2f7bc94cdb8773 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 6 Dec 2023 02:21:23 -0500 Subject: [PATCH 1/5] Save Work progress Signed-off-by: Leo Li --- pkg/auth/utils.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkg/auth/utils.go b/pkg/auth/utils.go index 81e7145a8f2..b6519220949 100644 --- a/pkg/auth/utils.go +++ b/pkg/auth/utils.go @@ -17,6 +17,7 @@ limitations under the License. package auth import ( + "context" "fmt" "net/http" "strings" @@ -62,3 +63,19 @@ func GetJWTExpiry(token string) (time.Time, error) { return claims.Expiry.Time(), nil } + +// VerifyJWTFromRequest will verify the incoming request contains the correct JWT token +func VerifyJWTFromRequest (ctx context.Context, tokenVerifier OIDCTokenVerifier, r *http.Request, audience string, response http.ResponseWriter) error { + token := GetJWTFromHeader(r.Header) + if token == "" { + return fmt.Errorf("no JWT token found in request") + } + + if _,err := tokenVerifier.VerifyJWT(ctx, token, audience); err != nil { + response.WriteHeader(http.StatusUnauthorized) + return fmt.Errorf("failed to verify JWT: %w", err) + + } + + return nil +} \ No newline at end of file From 2067497d61e0fd0cf447bd824e5c2a1e58ed93d5 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Tue, 12 Dec 2023 00:48:44 -0500 Subject: [PATCH 2/5] Use the newly wrapped function Signed-off-by: Leo Li --- pkg/auth/utils.go | 20 +++++++++++++------- pkg/broker/filter/filter_handler.go | 12 +++--------- pkg/broker/ingress/ingress_handler.go | 19 +++---------------- pkg/channel/event_receiver.go | 17 +++-------------- 4 files changed, 22 insertions(+), 46 deletions(-) diff --git a/pkg/auth/utils.go b/pkg/auth/utils.go index b6519220949..3d421dd9fb0 100644 --- a/pkg/auth/utils.go +++ b/pkg/auth/utils.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net/http" + nethttp "net/http" "strings" "time" @@ -65,17 +66,22 @@ func GetJWTExpiry(token string) (time.Time, error) { } // VerifyJWTFromRequest will verify the incoming request contains the correct JWT token -func VerifyJWTFromRequest (ctx context.Context, tokenVerifier OIDCTokenVerifier, r *http.Request, audience string, response http.ResponseWriter) error { +func VerifyJWTFromRequest(ctx context.Context, tokenVerifier *OIDCTokenVerifier, r *http.Request, audience string, response http.ResponseWriter) (http.ResponseWriter, error) { token := GetJWTFromHeader(r.Header) if token == "" { - return fmt.Errorf("no JWT token found in request") + response.WriteHeader(nethttp.StatusUnauthorized) + return response, fmt.Errorf("no JWT token found in request") } - if _,err := tokenVerifier.VerifyJWT(ctx, token, audience); err != nil { - response.WriteHeader(http.StatusUnauthorized) - return fmt.Errorf("failed to verify JWT: %w", err) + if audience == "" { + response.WriteHeader(nethttp.StatusInternalServerError) + return response, fmt.Errorf("no audience is provided") + } + if _, err := tokenVerifier.VerifyJWT(ctx, token, audience); err != nil { + response.WriteHeader(http.StatusUnauthorized) + return response, fmt.Errorf("failed to verify JWT: %w", err) } - return nil -} \ No newline at end of file + return response, nil +} diff --git a/pkg/broker/filter/filter_handler.go b/pkg/broker/filter/filter_handler.go index 9acb91a3845..509403fdc21 100644 --- a/pkg/broker/filter/filter_handler.go +++ b/pkg/broker/filter/filter_handler.go @@ -193,16 +193,10 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { features := feature.FromContext(ctx) if features.IsOIDCAuthentication() { h.logger.Debug("OIDC authentication is enabled") - token := auth.GetJWTFromHeader(request.Header) - if token == "" { - h.logger.Warn(fmt.Sprintf("No JWT in %s header provided while feature %s is enabled", auth.AuthHeaderKey, feature.OIDCAuthentication)) - writer.WriteHeader(http.StatusUnauthorized) - return - } - if _, err := h.tokenVerifier.VerifyJWT(ctx, token, FilterAudience); err != nil { - h.logger.Warn("no valid JWT provided", zap.Error(err)) - writer.WriteHeader(http.StatusUnauthorized) + writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, FilterAudience, writer) + if err != nil { + h.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return } diff --git a/pkg/broker/ingress/ingress_handler.go b/pkg/broker/ingress/ingress_handler.go index 89111289cae..902ddf77e5f 100644 --- a/pkg/broker/ingress/ingress_handler.go +++ b/pkg/broker/ingress/ingress_handler.go @@ -228,22 +228,9 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { if features.IsOIDCAuthentication() { h.Logger.Debug("OIDC authentication is enabled") - if broker.Status.Address.Audience == nil { - h.Logger.Warn(fmt.Sprintf("Audience of broker %s/%s must not be nil, while feature %s is enabled", broker.Name, broker.Namespace, feature.OIDCAuthentication)) - writer.WriteHeader(http.StatusInternalServerError) - return - } - - token := auth.GetJWTFromHeader(request.Header) - if token == "" { - h.Logger.Warn(fmt.Sprintf("No JWT in %s header provided while feature %s is enabled", auth.AuthHeaderKey, feature.OIDCAuthentication)) - writer.WriteHeader(http.StatusUnauthorized) - return - } - - if _, err := h.tokenVerifier.VerifyJWT(ctx, token, *broker.Status.Address.Audience); err != nil { - h.Logger.Warn("no valid JWT provided", zap.Error(err)) - writer.WriteHeader(http.StatusUnauthorized) + writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, *broker.Status.Address.Audience, writer) + if err != nil { + h.Logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return } diff --git a/pkg/channel/event_receiver.go b/pkg/channel/event_receiver.go index f39e8facc32..7600fe4388c 100644 --- a/pkg/channel/event_receiver.go +++ b/pkg/channel/event_receiver.go @@ -250,23 +250,12 @@ func (r *EventReceiver) ServeHTTP(response nethttp.ResponseWriter, request *neth features := feature.FromContext(ctx) if features.IsOIDCAuthentication() { r.logger.Debug("OIDC authentication is enabled") - - token := auth.GetJWTFromHeader(request.Header) - if token == "" { - r.logger.Warn(fmt.Sprintf("No JWT in %s header provided while feature %s is enabled", auth.AuthHeaderKey, feature.OIDCAuthentication)) - response.WriteHeader(nethttp.StatusUnauthorized) - return - } - - if _, err := r.tokenVerifier.VerifyJWT(ctx, token, r.audience); err != nil { - r.logger.Warn("no valid JWT provided", zap.Error(err)) - response.WriteHeader(nethttp.StatusUnauthorized) + response, err = auth.VerifyJWTFromRequest(ctx, r.tokenVerifier, request, r.audience, response) + if err != nil { + r.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return } - r.logger.Debug("Request contained a valid JWT. Continuing...") - } else { - r.logger.Debug("OIDC authentication is disabled") } err = r.receiverFunc(request.Context(), channel, *event, utils.PassThroughHeaders(request.Header)) From e87690d7415265b67500c778687b18b3cc7a1b5f Mon Sep 17 00:00:00 2001 From: Leo Li Date: Tue, 12 Dec 2023 10:34:09 -0500 Subject: [PATCH 3/5] Pass the audience string pointer instead Signed-off-by: Leo Li --- pkg/auth/utils.go | 6 +++--- pkg/broker/filter/filter_handler.go | 4 +++- pkg/broker/ingress/ingress_handler.go | 2 +- pkg/channel/event_receiver.go | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/auth/utils.go b/pkg/auth/utils.go index 3d421dd9fb0..84dbe53c0c7 100644 --- a/pkg/auth/utils.go +++ b/pkg/auth/utils.go @@ -66,19 +66,19 @@ func GetJWTExpiry(token string) (time.Time, error) { } // VerifyJWTFromRequest will verify the incoming request contains the correct JWT token -func VerifyJWTFromRequest(ctx context.Context, tokenVerifier *OIDCTokenVerifier, r *http.Request, audience string, response http.ResponseWriter) (http.ResponseWriter, error) { +func VerifyJWTFromRequest(ctx context.Context, tokenVerifier *OIDCTokenVerifier, r *http.Request, audience *string, response http.ResponseWriter) (http.ResponseWriter, error) { token := GetJWTFromHeader(r.Header) if token == "" { response.WriteHeader(nethttp.StatusUnauthorized) return response, fmt.Errorf("no JWT token found in request") } - if audience == "" { + if audience == nil { response.WriteHeader(nethttp.StatusInternalServerError) return response, fmt.Errorf("no audience is provided") } - if _, err := tokenVerifier.VerifyJWT(ctx, token, audience); err != nil { + if _, err := tokenVerifier.VerifyJWT(ctx, token, *audience); err != nil { response.WriteHeader(http.StatusUnauthorized) return response, fmt.Errorf("failed to verify JWT: %w", err) } diff --git a/pkg/broker/filter/filter_handler.go b/pkg/broker/filter/filter_handler.go index 509403fdc21..f0c94a75146 100644 --- a/pkg/broker/filter/filter_handler.go +++ b/pkg/broker/filter/filter_handler.go @@ -194,7 +194,9 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { if features.IsOIDCAuthentication() { h.logger.Debug("OIDC authentication is enabled") - writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, FilterAudience, writer) + audience := FilterAudience + + writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, &audience, writer) if err != nil { h.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/pkg/broker/ingress/ingress_handler.go b/pkg/broker/ingress/ingress_handler.go index 902ddf77e5f..4acb8e60936 100644 --- a/pkg/broker/ingress/ingress_handler.go +++ b/pkg/broker/ingress/ingress_handler.go @@ -228,7 +228,7 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { if features.IsOIDCAuthentication() { h.Logger.Debug("OIDC authentication is enabled") - writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, *broker.Status.Address.Audience, writer) + writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, broker.Status.Address.Audience, writer) if err != nil { h.Logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/pkg/channel/event_receiver.go b/pkg/channel/event_receiver.go index 7600fe4388c..46ca17e2cb7 100644 --- a/pkg/channel/event_receiver.go +++ b/pkg/channel/event_receiver.go @@ -250,7 +250,7 @@ func (r *EventReceiver) ServeHTTP(response nethttp.ResponseWriter, request *neth features := feature.FromContext(ctx) if features.IsOIDCAuthentication() { r.logger.Debug("OIDC authentication is enabled") - response, err = auth.VerifyJWTFromRequest(ctx, r.tokenVerifier, request, r.audience, response) + response, err = auth.VerifyJWTFromRequest(ctx, r.tokenVerifier, request, &r.audience, response) if err != nil { r.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return From 7ef6909c97e73c13d670c398967c50c5c639c9c1 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Tue, 12 Dec 2023 10:54:02 -0500 Subject: [PATCH 4/5] Remove the redundant return Signed-off-by: Leo Li --- pkg/auth/utils.go | 10 +++++----- pkg/broker/filter/filter_handler.go | 2 +- pkg/broker/ingress/ingress_handler.go | 2 +- pkg/channel/event_receiver.go | 2 +- .../auth/features/oidc/addressable_oidc_conformance.go | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/auth/utils.go b/pkg/auth/utils.go index 84dbe53c0c7..0b5d97caa91 100644 --- a/pkg/auth/utils.go +++ b/pkg/auth/utils.go @@ -66,22 +66,22 @@ func GetJWTExpiry(token string) (time.Time, error) { } // VerifyJWTFromRequest will verify the incoming request contains the correct JWT token -func VerifyJWTFromRequest(ctx context.Context, tokenVerifier *OIDCTokenVerifier, r *http.Request, audience *string, response http.ResponseWriter) (http.ResponseWriter, error) { +func VerifyJWTFromRequest(ctx context.Context, tokenVerifier *OIDCTokenVerifier, r *http.Request, audience *string, response http.ResponseWriter) error { token := GetJWTFromHeader(r.Header) if token == "" { response.WriteHeader(nethttp.StatusUnauthorized) - return response, fmt.Errorf("no JWT token found in request") + return fmt.Errorf("no JWT token found in request") } if audience == nil { response.WriteHeader(nethttp.StatusInternalServerError) - return response, fmt.Errorf("no audience is provided") + return fmt.Errorf("no audience is provided") } if _, err := tokenVerifier.VerifyJWT(ctx, token, *audience); err != nil { response.WriteHeader(http.StatusUnauthorized) - return response, fmt.Errorf("failed to verify JWT: %w", err) + return fmt.Errorf("failed to verify JWT: %w", err) } - return response, nil + return nil } diff --git a/pkg/broker/filter/filter_handler.go b/pkg/broker/filter/filter_handler.go index f0c94a75146..908fe656e3b 100644 --- a/pkg/broker/filter/filter_handler.go +++ b/pkg/broker/filter/filter_handler.go @@ -196,7 +196,7 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { audience := FilterAudience - writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, &audience, writer) + err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, &audience, writer) if err != nil { h.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/pkg/broker/ingress/ingress_handler.go b/pkg/broker/ingress/ingress_handler.go index 4acb8e60936..09a96a5a85d 100644 --- a/pkg/broker/ingress/ingress_handler.go +++ b/pkg/broker/ingress/ingress_handler.go @@ -228,7 +228,7 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { if features.IsOIDCAuthentication() { h.Logger.Debug("OIDC authentication is enabled") - writer, err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, broker.Status.Address.Audience, writer) + err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, broker.Status.Address.Audience, writer) if err != nil { h.Logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/pkg/channel/event_receiver.go b/pkg/channel/event_receiver.go index 46ca17e2cb7..9fabec42923 100644 --- a/pkg/channel/event_receiver.go +++ b/pkg/channel/event_receiver.go @@ -250,7 +250,7 @@ func (r *EventReceiver) ServeHTTP(response nethttp.ResponseWriter, request *neth features := feature.FromContext(ctx) if features.IsOIDCAuthentication() { r.logger.Debug("OIDC authentication is enabled") - response, err = auth.VerifyJWTFromRequest(ctx, r.tokenVerifier, request, &r.audience, response) + err = auth.VerifyJWTFromRequest(ctx, r.tokenVerifier, request, &r.audience, response) if err != nil { r.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/test/auth/features/oidc/addressable_oidc_conformance.go b/test/auth/features/oidc/addressable_oidc_conformance.go index a5e7378b6c8..2424f6a9843 100644 --- a/test/auth/features/oidc/addressable_oidc_conformance.go +++ b/test/auth/features/oidc/addressable_oidc_conformance.go @@ -49,8 +49,8 @@ func AddressableOIDCTokenConformance(gvr schema.GroupVersionResource, kind, name Features: []*feature.Feature{ addressableRejectInvalidAudience(gvr, kind, name), addressableRejectCorruptedSignature(gvr, kind, name), - addressableRejectExpiredToken(gvr, kind, name), - addressableAllowsValidRequest(gvr, kind, name), + //addressableRejectExpiredToken(gvr, kind, name), + //addressableAllowsValidRequest(gvr, kind, name), }, } From f8cb477d506eaeda1af7f486e9a05b96e7edae63 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Thu, 14 Dec 2023 01:29:03 -0500 Subject: [PATCH 5/5] Move the function under `OIDCTokenVerifier` Signed-off-by: Leo Li --- pkg/auth/token_verifier.go | 21 +++++++++++++++++ pkg/auth/utils.go | 23 ------------------- pkg/broker/filter/filter_handler.go | 2 +- pkg/broker/ingress/ingress_handler.go | 2 +- pkg/channel/event_receiver.go | 2 +- .../oidc/addressable_oidc_conformance.go | 4 ++-- 6 files changed, 26 insertions(+), 28 deletions(-) diff --git a/pkg/auth/token_verifier.go b/pkg/auth/token_verifier.go index a37f8060012..5571b67f2b1 100644 --- a/pkg/auth/token_verifier.go +++ b/pkg/auth/token_verifier.go @@ -151,6 +151,27 @@ func (c *OIDCTokenVerifier) getKubernetesOIDCDiscovery() (*openIDMetadata, error return openIdConfig, nil } +// VerifyJWTFromRequest will verify the incoming request contains the correct JWT token +func (tokenVerifier *OIDCTokenVerifier) VerifyJWTFromRequest(ctx context.Context, r *http.Request, audience *string, response http.ResponseWriter) error { + token := GetJWTFromHeader(r.Header) + if token == "" { + response.WriteHeader(http.StatusUnauthorized) + return fmt.Errorf("no JWT token found in request") + } + + if audience == nil { + response.WriteHeader(http.StatusInternalServerError) + return fmt.Errorf("no audience is provided") + } + + if _, err := tokenVerifier.VerifyJWT(ctx, token, *audience); err != nil { + response.WriteHeader(http.StatusUnauthorized) + return fmt.Errorf("failed to verify JWT: %w", err) + } + + return nil +} + type openIDMetadata struct { Issuer string `json:"issuer"` JWKSURI string `json:"jwks_uri"` diff --git a/pkg/auth/utils.go b/pkg/auth/utils.go index 0b5d97caa91..81e7145a8f2 100644 --- a/pkg/auth/utils.go +++ b/pkg/auth/utils.go @@ -17,10 +17,8 @@ limitations under the License. package auth import ( - "context" "fmt" "net/http" - nethttp "net/http" "strings" "time" @@ -64,24 +62,3 @@ func GetJWTExpiry(token string) (time.Time, error) { return claims.Expiry.Time(), nil } - -// VerifyJWTFromRequest will verify the incoming request contains the correct JWT token -func VerifyJWTFromRequest(ctx context.Context, tokenVerifier *OIDCTokenVerifier, r *http.Request, audience *string, response http.ResponseWriter) error { - token := GetJWTFromHeader(r.Header) - if token == "" { - response.WriteHeader(nethttp.StatusUnauthorized) - return fmt.Errorf("no JWT token found in request") - } - - if audience == nil { - response.WriteHeader(nethttp.StatusInternalServerError) - return fmt.Errorf("no audience is provided") - } - - if _, err := tokenVerifier.VerifyJWT(ctx, token, *audience); err != nil { - response.WriteHeader(http.StatusUnauthorized) - return fmt.Errorf("failed to verify JWT: %w", err) - } - - return nil -} diff --git a/pkg/broker/filter/filter_handler.go b/pkg/broker/filter/filter_handler.go index 908fe656e3b..e493c35b17c 100644 --- a/pkg/broker/filter/filter_handler.go +++ b/pkg/broker/filter/filter_handler.go @@ -196,7 +196,7 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { audience := FilterAudience - err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, &audience, writer) + err = h.tokenVerifier.VerifyJWTFromRequest(ctx, request, &audience, writer) if err != nil { h.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/pkg/broker/ingress/ingress_handler.go b/pkg/broker/ingress/ingress_handler.go index 09a96a5a85d..c0aa4938b39 100644 --- a/pkg/broker/ingress/ingress_handler.go +++ b/pkg/broker/ingress/ingress_handler.go @@ -228,7 +228,7 @@ func (h *Handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { if features.IsOIDCAuthentication() { h.Logger.Debug("OIDC authentication is enabled") - err = auth.VerifyJWTFromRequest(ctx, h.tokenVerifier, request, broker.Status.Address.Audience, writer) + err = h.tokenVerifier.VerifyJWTFromRequest(ctx, request, broker.Status.Address.Audience, writer) if err != nil { h.Logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/pkg/channel/event_receiver.go b/pkg/channel/event_receiver.go index 9fabec42923..1e3ff3c2cc7 100644 --- a/pkg/channel/event_receiver.go +++ b/pkg/channel/event_receiver.go @@ -250,7 +250,7 @@ func (r *EventReceiver) ServeHTTP(response nethttp.ResponseWriter, request *neth features := feature.FromContext(ctx) if features.IsOIDCAuthentication() { r.logger.Debug("OIDC authentication is enabled") - err = auth.VerifyJWTFromRequest(ctx, r.tokenVerifier, request, &r.audience, response) + err = r.tokenVerifier.VerifyJWTFromRequest(ctx, request, &r.audience, response) if err != nil { r.logger.Warn("Error when validating the JWT token in the request", zap.Error(err)) return diff --git a/test/auth/features/oidc/addressable_oidc_conformance.go b/test/auth/features/oidc/addressable_oidc_conformance.go index 2424f6a9843..a5e7378b6c8 100644 --- a/test/auth/features/oidc/addressable_oidc_conformance.go +++ b/test/auth/features/oidc/addressable_oidc_conformance.go @@ -49,8 +49,8 @@ func AddressableOIDCTokenConformance(gvr schema.GroupVersionResource, kind, name Features: []*feature.Feature{ addressableRejectInvalidAudience(gvr, kind, name), addressableRejectCorruptedSignature(gvr, kind, name), - //addressableRejectExpiredToken(gvr, kind, name), - //addressableAllowsValidRequest(gvr, kind, name), + addressableRejectExpiredToken(gvr, kind, name), + addressableAllowsValidRequest(gvr, kind, name), }, }