From 308ec650c857fbda4cda6a6afbdab6f599473f55 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Mon, 4 Jul 2022 12:01:54 +0200 Subject: [PATCH] server: pass IQBC to authorizer Before, the option InterQueryCache(...) passed to the authorizer's config had set it to `nil` -- it wasn't set up yet. Now, the ordering allows for caching in the system authz policies. Fixes #4829. Signed-off-by: Stephan Renatus --- server/server.go | 6 ++-- server/server_test.go | 72 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/server/server.go b/server/server.go index e2141c155e..bf74190de6 100644 --- a/server/server.go +++ b/server/server.go @@ -158,8 +158,6 @@ func New() *Server { // from s.Listeners(). func (s *Server) Init(ctx context.Context) (*Server, error) { s.initRouters() - s.Handler = s.initHandlerAuth(s.Handler) - s.DiagnosticHandler = s.initHandlerAuth(s.DiagnosticHandler) txn, err := s.store.NewTransaction(ctx, storage.WriteParams) if err != nil { @@ -182,6 +180,10 @@ func (s *Server) Init(ctx context.Context) (*Server, error) { s.interQueryBuiltinCache = iCache.NewInterQueryCache(s.manager.InterQueryBuiltinCacheConfig()) s.manager.RegisterCacheTrigger(s.updateCacheConfig) + // authorizer, if configured, needs the iCache to be set up already + s.Handler = s.initHandlerAuth(s.Handler) + s.DiagnosticHandler = s.initHandlerAuth(s.DiagnosticHandler) + return s, s.store.Commit(ctx, txn) } diff --git a/server/server_test.go b/server/server_test.go index 042cdc4562..859fb92a8e 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -16,6 +16,7 @@ import ( "reflect" "sort" "strings" + "sync/atomic" "testing" "time" @@ -3545,6 +3546,73 @@ func TestAuthorization(t *testing.T) { } } +func TestAuthorizationUsesInterQueryCache(t *testing.T) { + + ctx := context.Background() + store := inmem.New() + m, err := plugins.New([]byte{}, "test", store) + if err != nil { + panic(err) + } + + if err := m.Start(ctx); err != nil { + panic(err) + } + + txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams) + + var c uint64 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddUint64(&c, 1) + fmt.Fprintf(w, `{"count": %d}`, c) + })) + + authzPolicy := fmt.Sprintf(`package system.authz + +default allow := false + +allow { + resp := http.send({ + "method": "GET", "url": "%[1]s/foo", + "force_cache": true, + "force_json_decode": true, + "force_cache_duration_seconds": 60, + }) + + resp.body.count == 1 +} +`, ts.URL) + t.Log(authzPolicy) + + if err := store.UpsertPolicy(ctx, txn, "test", []byte(authzPolicy)); err != nil { + t.Fatal(err) + } + + if err := store.Commit(ctx, txn); err != nil { + t.Fatal(err) + } + + server, err := New(). + WithAddresses([]string{":8182"}). + WithStore(store). + WithManager(m). + WithAuthorization(AuthorizationBasic). + Init(ctx) + + if err != nil { + t.Fatal(err) + } + + for i := 0; i < 5; i++ { + req1, err := http.NewRequest(http.MethodGet, "http://localhost:8182/health", nil) + if err != nil { + t.Fatal(err) + } + + validateAuthorizedRequest(t, server, req1, http.StatusOK) + } +} + func validateAuthorizedRequest(t *testing.T, s *Server, req *http.Request, exp int) { t.Helper() @@ -3553,7 +3621,7 @@ func validateAuthorizedRequest(t *testing.T, s *Server, req *http.Request, exp i // First check the main router s.Handler.ServeHTTP(r, req) if r.Code != exp { - t.Fatalf("(Default Handler) Expected %v but got: %v", exp, r) + t.Errorf("(Default Handler) Expected %v but got: %v", exp, r) } r = httptest.NewRecorder() @@ -3561,7 +3629,7 @@ func validateAuthorizedRequest(t *testing.T, s *Server, req *http.Request, exp i // Ensure that auth happens for the diagnostic handler as well s.DiagnosticHandler.ServeHTTP(r, req) if r.Code != exp { - t.Fatalf("(Diagnostic Handler) Expected %v but got: %v", exp, r) + t.Errorf("(Diagnostic Handler) Expected %v but got: %v", exp, r) } }