OpenFeature: Fix ofrep path and stack id check (#107884)

* OpenFeature: Fix ofrep path and stack id check

* Use namespace instead of stackId in eval ctx

* Apply review feedback

* Update pkg/registry/apis/ofrep/register.go

Co-authored-by: Stephanie Hingtgen <stephanie.hingtgen@grafana.com>

* Update pkg/registry/apis/ofrep/register.go

Co-authored-by: Dave Henderson <dave.henderson@grafana.com>

---------

Co-authored-by: Stephanie Hingtgen <stephanie.hingtgen@grafana.com>
Co-authored-by: Dave Henderson <dave.henderson@grafana.com>
pull/108043/head
Tania 1 week ago committed by GitHub
parent 2cd0be3cbd
commit f9f04c2a55
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 3
      conf/defaults.ini
  2. 41
      pkg/registry/apis/ofrep/register.go

@ -1994,8 +1994,7 @@ provider = static
[feature_toggles.openfeature.context] [feature_toggles.openfeature.context]
# This is EXPERIMENTAL. Please, do not use this section # This is EXPERIMENTAL. Please, do not use this section
# instance = "grafana" # foo = bar
# version = 11.0.0
[date_formats] [date_formats]
# For information on what formatting patterns that are supported https://momentjs.com/docs/#/displaying/ # For information on what formatting patterns that are supported https://momentjs.com/docs/#/displaying/

@ -32,6 +32,8 @@ var _ builder.APIGroupVersionProvider = (*APIBuilder)(nil)
const ofrepPath = "/ofrep/v1/evaluate/flags" const ofrepPath = "/ofrep/v1/evaluate/flags"
const namespaceMismatchMsg = "rejecting request with namespace mismatch"
var groupVersion = schema.GroupVersion{ var groupVersion = schema.GroupVersion{
Group: "features.grafana.app", Group: "features.grafana.app",
Version: "v0alpha1", Version: "v0alpha1",
@ -126,7 +128,7 @@ func (b *APIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.APIRoutes {
return &builder.APIRoutes{ return &builder.APIRoutes{
Namespace: []builder.APIRouteHandler{ Namespace: []builder.APIRouteHandler{
{ {
Path: "ofrep/v1/evaluate/flags/", Path: "ofrep/v1/evaluate/flags",
Spec: &spec3.PathProps{ Spec: &spec3.PathProps{
Post: &spec3.Operation{ Post: &spec3.Operation{
OperationProps: spec3.OperationProps{ OperationProps: spec3.OperationProps{
@ -225,8 +227,8 @@ func (b *APIBuilder) GetAPIRoutes(gv schema.GroupVersion) *builder.APIRoutes {
func (b *APIBuilder) oneFlagHandler(w http.ResponseWriter, r *http.Request) { func (b *APIBuilder) oneFlagHandler(w http.ResponseWriter, r *http.Request) {
if !b.validateNamespace(r) { if !b.validateNamespace(r) {
b.logger.Error("stackId in evaluation context does not match requested namespace") b.logger.Error(namespaceMismatchMsg)
http.Error(w, "stackId in evaluation context does not match requested namespace", http.StatusUnauthorized) http.Error(w, namespaceMismatchMsg, http.StatusUnauthorized)
return return
} }
@ -255,8 +257,8 @@ func (b *APIBuilder) oneFlagHandler(w http.ResponseWriter, r *http.Request) {
func (b *APIBuilder) allFlagsHandler(w http.ResponseWriter, r *http.Request) { func (b *APIBuilder) allFlagsHandler(w http.ResponseWriter, r *http.Request) {
if !b.validateNamespace(r) { if !b.validateNamespace(r) {
b.logger.Error("stackId in evaluation context does not match requested namespace") b.logger.Error(namespaceMismatchMsg)
http.Error(w, "stackId in evaluation context does not match requested namespace", http.StatusUnauthorized) http.Error(w, namespaceMismatchMsg, http.StatusUnauthorized)
return return
} }
@ -279,25 +281,25 @@ func writeResponse(statusCode int, result any, logger log.Logger, w http.Respons
} }
} }
func (b *APIBuilder) stackIdFromEvalCtx(body []byte) int64 { func (b *APIBuilder) namespaceFromEvalCtx(body []byte) string {
// Extract stackID from request body without consuming it // Extract namespace from request body without consuming it
var evalCtx struct { var evalCtx struct {
Context struct { Context struct {
StackID int64 `json:"stackId"` // TODO -- replace with namespace "stackId" ONLY makes sense in cloud Namespace string `json:"namespace"`
} `json:"context"` } `json:"context"`
} }
if err := json.Unmarshal(body, &evalCtx); err != nil { if err := json.Unmarshal(body, &evalCtx); err != nil {
b.logger.Debug("Failed to unmarshal evaluation context", "error", err, "body", string(body)) b.logger.Debug("Failed to unmarshal evaluation context", "error", err, "body", string(body))
return 0 return ""
} }
if evalCtx.Context.StackID <= 0 { if evalCtx.Context.Namespace == "" {
b.logger.Debug("Invalid or missing stackId in evaluation context", "stackId", evalCtx.Context.StackID) b.logger.Debug("namespace missing from evaluation context", "namespace", evalCtx.Context.Namespace)
return 0 return ""
} }
return evalCtx.Context.StackID return evalCtx.Context.Namespace
} }
// isAuthenticatedRequest returns true if the request is authenticated // isAuthenticatedRequest returns true if the request is authenticated
@ -309,7 +311,7 @@ func (b *APIBuilder) isAuthenticatedRequest(r *http.Request) bool {
return user.GetIdentityType() != "" return user.GetIdentityType() != ""
} }
// validateNamespace checks if the stackId in the evaluation context matches the namespace in the request // validateNamespace checks if the namespace in the evaluation context matches the namespace in the request
func (b *APIBuilder) validateNamespace(r *http.Request) bool { func (b *APIBuilder) validateNamespace(r *http.Request) bool {
// Extract namespace from request context or URL path // Extract namespace from request context or URL path
var namespace string var namespace string
@ -324,13 +326,7 @@ func (b *APIBuilder) validateNamespace(r *http.Request) bool {
namespace = mux.Vars(r)["namespace"] namespace = mux.Vars(r)["namespace"]
} }
info, err := types.ParseNamespace(namespace) // Extract namespace from feature flag evaluation context
if err != nil {
b.logger.Error("Error parsing namespace", "error", err)
return false
}
// Extract stackId from feature flag evaluation context
body, err := io.ReadAll(r.Body) body, err := io.ReadAll(r.Body)
if err != nil { if err != nil {
b.logger.Error("Error reading evaluation request body", "error", err) b.logger.Error("Error reading evaluation request body", "error", err)
@ -338,8 +334,9 @@ func (b *APIBuilder) validateNamespace(r *http.Request) bool {
} }
r.Body = io.NopCloser(bytes.NewBuffer(body)) r.Body = io.NopCloser(bytes.NewBuffer(body))
evalCtxNamespace := b.namespaceFromEvalCtx(body)
// "default" namespace case can only occur in on-prem grafana // "default" namespace case can only occur in on-prem grafana
if b.stackIdFromEvalCtx(body) == info.StackID { if (namespace == "default" && evalCtxNamespace == "") || (evalCtxNamespace == namespace) {
return true return true
} }

Loading…
Cancel
Save