diff --git a/app/allowlist.go b/app/allowlist.go index da8f6c1..5b4b5fb 100644 --- a/app/allowlist.go +++ b/app/allowlist.go @@ -481,8 +481,8 @@ func findConstraint(i *Integration, callerID, pth, method string) (RequestConstr allowlists.RLock() callers := allowlists.m[i.Name] - // NOTE: We check wildcard callers too incase an allowlist has both defined callers - // and a fallback wildcard caller. + // NOTE: wildcard callers are used for anonymous requests and when callerID + // does not have an explicit allowlist entry. wildcard, hasWildcard := callers["*"] c, ok := callers[callerID] allowlists.RUnlock() @@ -501,8 +501,13 @@ func findConstraint(i *Integration, callerID, pth, method string) (RequestConstr } } } + // Identified callers with explicit entries should not fall back to wildcard + // rules when their own rules do not match. + if callerID != "*" && callerID != "" { + return RequestConstraint{}, false + } } - if hasWildcard { + if hasWildcard && (!ok || callerID == "*" || callerID == "") { if len(wildcard.Capabilities) > 0 { wildcard = integrationplugins.ExpandCapabilities(i.Name, []CallerConfig{wildcard})[0] for ri := range wildcard.Rules { diff --git a/app/allowlist_test.go b/app/allowlist_test.go index ba4addd..2bd9ac8 100644 --- a/app/allowlist_test.go +++ b/app/allowlist_test.go @@ -213,11 +213,46 @@ func TestFindConstraintWildcard(t *testing.T) { if _, ok := findConstraint(integ, "xyz", "/wild", http.MethodGet); !ok { t.Fatal("expected wildcard constraint") } + if _, ok := findConstraint(integ, "abc", "/wild", http.MethodGet); ok { + t.Fatal("unexpected wildcard fallback for configured caller") + } if _, ok := findConstraint(integ, "xyz", "/none", http.MethodGet); ok { t.Fatal("unexpected match for unknown path") } } +func TestFindConstraintWildcardFallbackSemantics(t *testing.T) { + allowlists.Lock() + allowlists.m = make(map[string]map[string]CallerConfig) + allowlists.Unlock() + + if err := SetAllowlist("fc-fallback", []CallerConfig{ + {ID: "alice", Rules: []CallRule{{Path: "/alice-only", Methods: map[string]RequestConstraint{"GET": {}}}}}, + {ID: "*", Rules: []CallRule{{Path: "/wild-only", Methods: map[string]RequestConstraint{"GET": {}}}}}, + }); err != nil { + t.Fatalf("failed to set allowlist: %v", err) + } + + integ := &Integration{Name: "fc-fallback"} + + // Explicit caller matches explicit rules. + if _, ok := findConstraint(integ, "alice", "/alice-only", http.MethodGet); !ok { + t.Fatal("expected explicit caller rule match") + } + // Explicit caller must not fall back to wildcard. + if _, ok := findConstraint(integ, "alice", "/wild-only", http.MethodGet); ok { + t.Fatal("unexpected wildcard fallback for explicit caller") + } + // Unknown caller should use wildcard fallback. + if _, ok := findConstraint(integ, "bob", "/wild-only", http.MethodGet); !ok { + t.Fatal("expected wildcard fallback for unknown caller") + } + // Anonymous caller should use wildcard fallback. + if _, ok := findConstraint(integ, "*", "/wild-only", http.MethodGet); !ok { + t.Fatal("expected wildcard fallback for anonymous caller") + } +} + func TestSetAllowlistDuplicateCaller(t *testing.T) { allowlists.Lock() allowlists.m = make(map[string]map[string]CallerConfig)