Skip to content

Commit

Permalink
server: check old policy path for bundle ownership (#4847)
Browse files Browse the repository at this point in the history
Before, we'd only check if the NEW policy path was owned by a bundle. Now,
we'll also check if the to-be-updated policy is owned by a bundle. If so,
return an error.

Fixes #4846

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
(cherry picked from commit b2bf19f)
  • Loading branch information
srenatus committed Jul 8, 2022
1 parent 22641e5 commit 2819143
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
15 changes: 10 additions & 5 deletions server/server.go
Expand Up @@ -1982,7 +1982,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
vars := mux.Vars(r)

path, err := url.PathUnescape(vars["path"])
id, err := url.PathUnescape(vars["path"])
if err != nil {
writer.ErrorString(w, http.StatusBadRequest, types.CodeInvalidParameter, err)
return
Expand Down Expand Up @@ -2010,7 +2010,12 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {
return
}

if bs, err := s.store.GetPolicy(ctx, txn, path); err != nil {
if err := s.checkPolicyIDScope(ctx, txn, id); err != nil && !storage.IsNotFound(err) {
s.abortAuto(ctx, txn, w, err)
return
}

if bs, err := s.store.GetPolicy(ctx, txn, id); err != nil {
if !storage.IsNotFound(err) {
s.abortAuto(ctx, txn, w, err)
return
Expand All @@ -2026,7 +2031,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {
}

m.Timer(metrics.RegoModuleParse).Start()
parsedMod, err := ast.ParseModule(path, string(buf))
parsedMod, err := ast.ParseModule(id, string(buf))
m.Timer(metrics.RegoModuleParse).Stop()

if err != nil {
Expand Down Expand Up @@ -2057,7 +2062,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {
return
}

modules[path] = parsedMod
modules[id] = parsedMod

c := ast.NewCompiler().
SetErrorLimit(s.errLimit).
Expand All @@ -2075,7 +2080,7 @@ func (s *Server) v1PoliciesPut(w http.ResponseWriter, r *http.Request) {

m.Timer(metrics.RegoModuleCompile).Stop()

if err := s.store.UpsertPolicy(ctx, txn, path, buf); err != nil {
if err := s.store.UpsertPolicy(ctx, txn, id, buf); err != nil {
s.abortAuto(ctx, txn, w, err)
return
}
Expand Down
7 changes: 7 additions & 0 deletions server/server_test.go
Expand Up @@ -1843,6 +1843,13 @@ func TestBundleScope(t *testing.T) {
code: http.StatusBadRequest,
resp: `{"code": "invalid_parameter", "message": "path a/b is owned by bundle \"test-bundle\""}`,
},
{
method: "PUT",
path: "/policies/someid",
body: `package other.path`,
code: http.StatusBadRequest,
resp: `{"code": "invalid_parameter", "message": "path x/y/z is owned by bundle \"test-bundle\""}`,
},
{
method: "DELETE",
path: "/policies/someid",
Expand Down

0 comments on commit 2819143

Please sign in to comment.