Skip to content

Commit

Permalink
fix signing key template processing dropping allow (#3390)
Browse files Browse the repository at this point in the history
Scoped signing keys allow for optional values in allow rules
If an allow rule therefore gets removed because a tag is not present,
the removal needs to be compensated by adding in a deny >

Signed-off-by: Matthias Hanel <mh@synadia.com>
  • Loading branch information
matthiashanel committed Aug 23, 2022
1 parent 4dd4b42 commit a43c1e3
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
12 changes: 12 additions & 0 deletions server/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,10 @@ func processUserPermissionsTemplate(lim jwt.UserPermissionLimits, ujwt *jwt.User
}
return emittedList, nil
}

subAllowWasNotEmpty := len(lim.Permissions.Sub.Allow) > 0
pubAllowWasNotEmpty := len(lim.Permissions.Pub.Allow) > 0

var err error
if lim.Permissions.Sub.Allow, err = applyTemplate(lim.Permissions.Sub.Allow, false); err != nil {
return jwt.UserPermissionLimits{}, err
Expand All @@ -523,6 +527,14 @@ func processUserPermissionsTemplate(lim jwt.UserPermissionLimits, ujwt *jwt.User
} else if lim.Permissions.Pub.Deny, err = applyTemplate(lim.Permissions.Pub.Deny, true); err != nil {
return jwt.UserPermissionLimits{}, err
}

// if pub/sub allow were not empty, but are empty post template processing, add in a "deny >" to compensate
if subAllowWasNotEmpty && len(lim.Permissions.Sub.Allow) == 0 {
lim.Permissions.Sub.Deny.Add(">")
}
if pubAllowWasNotEmpty && len(lim.Permissions.Pub.Allow) == 0 {
lim.Permissions.Pub.Deny.Add(">")
}
return lim, nil
}

Expand Down
4 changes: 3 additions & 1 deletion server/jwt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4416,8 +4416,10 @@ func TestJwtTemplates(t *testing.T) {
test(resLim.Pub.Deny, []string{"foo1.acc1", "foo1.acc2", "foo2.acc1", "foo2.acc2"})

require_True(t, len(resLim.Sub.Allow) == 0)
require_True(t, len(resLim.Sub.Deny) == 1)
require_True(t, len(resLim.Sub.Deny) == 2)
require_Contains(t, resLim.Sub.Deny[0], fmt.Sprintf("foo.myname.%s.accname.%s.bar", upub, aPub))
// added in to compensate for sub allow not resolving
require_Contains(t, resLim.Sub.Deny[1], ">")

lim.Pub.Deny.Add("{{tag(NOT_THERE)}}")
_, err = processUserPermissionsTemplate(lim, uclaim, acc)
Expand Down

0 comments on commit a43c1e3

Please sign in to comment.