mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-07 16:40:21 +09:00
policy/v2: match SaaS wording for group nesting rejection
SaaS rejects any group-in-group reference (chain, cycle, or self-reference) with `groups["X"]: "Y": group members cannot be recursive`. headscale rejected the same input but the message surfaced as a generic JSON parse error from the unmarshal layer. Groups.UnmarshalJSON now scans the raw map in descending alphabetical order and reports the first group whose member is itself a group, mirroring the (X, Y) pair SaaS picks (deepest non-leaf parent first). Drop the now-unused ErrNestedGroups sentinel and update the existing group-in-group test row plus add chain, cycle, and self-cycle rows.
This commit is contained in:
@@ -106,21 +106,6 @@ var sshRejectSkipReasons = map[string]string{
|
||||
"ssh-e1": "domain validation: headscale has no 'associated tailnet domains' concept",
|
||||
"ssh-e2": "domain validation: headscale has no 'associated tailnet domains' concept",
|
||||
"ssh-malformed-user-localpart-multi-glob": "domain validation: headscale has no 'associated tailnet domains' concept (same gap as ssh-b4/d1/e1/e2)",
|
||||
|
||||
// GROUP_NESTING_ERROR_BODY (3 tests)
|
||||
//
|
||||
// SaaS rejects any group-in-group reference (cycle, chain,
|
||||
// self-cycle) with the structured message
|
||||
// `groups["X"]: "Y": group members cannot be recursive`.
|
||||
// headscale rejects too but the error surfaces as a generic
|
||||
// `parsing policy: parsing policy from bytes: json: unable to
|
||||
// unmarshal …` because the group resolver fails before the
|
||||
// validation phase that would emit a specific message. Wire the
|
||||
// resolver's "group references a group" detection to produce the
|
||||
// SaaS-style structured error so the body matches.
|
||||
"ssh-group-nested-cycle": "group nesting rejected with different error body (parse error vs structured 'group members cannot be recursive')",
|
||||
"ssh-group-nested-three-deep": "group nesting rejected with different error body (parse error vs structured 'group members cannot be recursive')",
|
||||
"ssh-group-nested-two-deep": "group nesting rejected with different error body (parse error vs structured 'group members cannot be recursive')",
|
||||
}
|
||||
|
||||
// TestSSHDataCompat is a data-driven test that loads all ssh-*.hujson test
|
||||
|
||||
@@ -54,6 +54,7 @@ var (
|
||||
ErrSSHActionInvalid = errors.New("is not a valid action")
|
||||
ErrSSHDestinationHostAlias = errors.New("invalid dst")
|
||||
ErrTagNameMustStartWithLetter = errors.New("tag names must start with a letter, after 'tag:'")
|
||||
ErrGroupMembersCannotBeRecursive = errors.New("group members cannot be recursive")
|
||||
)
|
||||
|
||||
// SSH check period constants per Tailscale docs:
|
||||
@@ -104,7 +105,6 @@ var (
|
||||
ErrGroupNotDefined = errors.New("group not defined in policy")
|
||||
ErrInvalidGroupMember = errors.New("invalid group member type")
|
||||
ErrGroupValueNotArray = errors.New("group value must be an array of users")
|
||||
ErrNestedGroups = errors.New("nested groups are not allowed")
|
||||
ErrInvalidHostIP = errors.New("hostname contains invalid IP address")
|
||||
ErrTagNotDefined = errors.New("tag not found")
|
||||
ErrAutoApproverNotAlias = errors.New("auto approver is not an alias")
|
||||
@@ -1388,6 +1388,27 @@ func (g *Groups) UnmarshalJSON(b []byte) error {
|
||||
}
|
||||
}
|
||||
|
||||
// SaaS rejects any group-in-group reference (cycle, chain,
|
||||
// self-cycle) with `groups["X"]: "Y": group members cannot be
|
||||
// recursive`. Iterate keys in descending alphabetical order so
|
||||
// the reported (X, Y) pair matches the SaaS engine, which
|
||||
// reports the deepest non-leaf parent first.
|
||||
keys := make([]string, 0, len(rawGroups))
|
||||
for k := range rawGroups {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
|
||||
slices.Sort(keys)
|
||||
slices.Reverse(keys)
|
||||
|
||||
for _, key := range keys {
|
||||
for _, u := range rawGroups[key] {
|
||||
if isGroup(u) {
|
||||
return fmt.Errorf("groups[%q]: %q: %w", key, u, ErrGroupMembersCannotBeRecursive)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
*g = make(Groups)
|
||||
|
||||
for key, value := range rawGroups {
|
||||
@@ -1400,10 +1421,6 @@ func (g *Groups) UnmarshalJSON(b []byte) error {
|
||||
|
||||
err := username.Validate()
|
||||
if err != nil {
|
||||
if isGroup(u) {
|
||||
return fmt.Errorf("%w: found %q inside %q", ErrNestedGroups, u, group)
|
||||
}
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
@@ -407,8 +407,50 @@ func TestUnmarshalPolicy(t *testing.T) {
|
||||
},
|
||||
}
|
||||
`,
|
||||
// wantErr: `username must contain @, got: "group:inner"`,
|
||||
wantErr: `nested groups are not allowed: found "group:inner" inside "group:example"`,
|
||||
wantErr: `groups["group:example"]: "group:inner": group members cannot be recursive`,
|
||||
},
|
||||
{
|
||||
// SaaS reports the deepest non-leaf parent first: for
|
||||
// the three-deep chain `a -> b -> c -> user`, the
|
||||
// reported pair is `b -> c` rather than `a -> b`.
|
||||
name: "group-nested-three-deep",
|
||||
input: `
|
||||
{
|
||||
"groups": {
|
||||
"group:a": ["group:b"],
|
||||
"group:b": ["group:c"],
|
||||
"group:c": ["thor@example.org"],
|
||||
},
|
||||
}
|
||||
`,
|
||||
wantErr: `groups["group:b"]: "group:c": group members cannot be recursive`,
|
||||
},
|
||||
{
|
||||
// Cycle `a <-> b`: reported as `b -> a` so the body
|
||||
// matches SaaS exactly.
|
||||
name: "group-nested-cycle",
|
||||
input: `
|
||||
{
|
||||
"groups": {
|
||||
"group:a": ["group:b"],
|
||||
"group:b": ["group:a"],
|
||||
},
|
||||
}
|
||||
`,
|
||||
wantErr: `groups["group:b"]: "group:a": group members cannot be recursive`,
|
||||
},
|
||||
{
|
||||
// Self-cycle: the same group appears as its own
|
||||
// member. Same wording.
|
||||
name: "group-nested-self-cycle",
|
||||
input: `
|
||||
{
|
||||
"groups": {
|
||||
"group:a": ["group:a"],
|
||||
},
|
||||
}
|
||||
`,
|
||||
wantErr: `groups["group:a"]: "group:a": group members cannot be recursive`,
|
||||
},
|
||||
{
|
||||
name: "invalid-addr",
|
||||
|
||||
Reference in New Issue
Block a user