kemko/nomad
1
0
Fork 0
mirror of https://github.com/kemko/nomad.git synced 2026-01-04 01:15:43 +03:00
Code Issues Packages Projects Releases Wiki Activity

acl: remove unused nil ACL object handling (#20150)

Browse Source 
As of #18754 which shipped in Nomad 1.7, we no longer need to nil-check the
object returned by `ResolveACL` if there's no error return, because in the case
where ACLs are disabled we return a special "ACLs disabled" ACL object. Checking
nil is not a bug but should be discouraged because it opens us up to future bugs
that would bypass ACLs.

While working on an unrelated feature @lindleywhite discovered that we missed
removing the nil check from several endpoints with our semgrep linter. This
changeset fixes that.

Co-Author: Lindley <lindley@hashicorp.com>
This commit is contained in:
Tim Gross
2024-03-18 10:04:51 -04:00
committed by GitHub
parent 695bb7ffcf
commit 1cbddfa8ce
2 changed files with 46 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
nomad/acl_endpoint.go
View File

@@ -94,9 +94,9 @@ func (a *ACL) UpsertPolicies(args *structs.ACLPolicyUpsertRequest, reply *struct
defer metrics.MeasureSince([]string{"nomad", "acl", "upsert_policies"}, time.Now())
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -143,9 +143,9 @@ func (a *ACL) DeletePolicies(args *structs.ACLPolicyDeleteRequest, reply *struct
defer metrics.MeasureSince([]string{"nomad", "acl", "delete_policies"}, time.Now())
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -632,9 +632,9 @@ func (a *ACL) UpsertTokens(args *structs.ACLTokenUpsertRequest, reply *structs.A
defer metrics.MeasureSince([]string{"nomad", "acl", "upsert_tokens"}, time.Now())
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -786,9 +786,9 @@ func (a *ACL) DeleteTokens(args *structs.ACLTokenDeleteRequest, reply *structs.G
defer metrics.MeasureSince([]string{"nomad", "acl", "delete_tokens"}, time.Now())
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -864,9 +864,9 @@ func (a *ACL) ListTokens(args *structs.ACLTokenListRequest, reply *structs.ACLTo
defer metrics.MeasureSince([]string{"nomad", "acl", "list_tokens"}, time.Now())
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -1019,9 +1019,9 @@ func (a *ACL) GetTokens(args *structs.ACLTokenSetRequest, reply *structs.ACLToke
defer metrics.MeasureSince([]string{"nomad", "acl", "get_tokens"}, time.Now())
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -1232,9 +1232,9 @@ func (a *ACL) ExpireOneTimeTokens(args *structs.OneTimeTokenExpireRequest, reply
// Check management level permissions
if a.srv.config.ACLEnabled {
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
}
@@ -1282,9 +1282,9 @@ func (a *ACL) UpsertRoles(
}
// Only management level permissions can create ACL roles.
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -1426,9 +1426,9 @@ func (a *ACL) DeleteRolesByID(
}
// Only management level permissions can create ACL roles.
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -1867,9 +1867,9 @@ func (a *ACL) UpsertAuthMethods(
}
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -1970,9 +1970,9 @@ func (a *ACL) DeleteAuthMethods(
}
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -2059,10 +2059,9 @@ func (a *ACL) GetAuthMethod(
defer metrics.MeasureSince([]string{"nomad", "acl", "get_auth_method_name"}, time.Now())
// Resolve the token and ensure it has some form of permissions.
acl, err := a.srv.ResolveACL(args)
if err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -2224,9 +2223,9 @@ func (a *ACL) UpsertBindingRules(
}
// Check management level permissions
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -2351,9 +2350,9 @@ func (a *ACL) DeleteBindingRules(
}
// Check management level permissions.
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -2393,9 +2392,9 @@ func (a *ACL) ListBindingRules(
defer metrics.MeasureSince([]string{"nomad", "acl", "list_binding_rules"}, time.Now())
// Check management level permissions.
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -2452,9 +2451,9 @@ func (a *ACL) GetBindingRules(
defer metrics.MeasureSince([]string{"nomad", "acl", "get_rules"}, time.Now())
// Check management level permissions.
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}
@@ -2507,9 +2506,9 @@ func (a *ACL) GetBindingRule(
defer metrics.MeasureSince([]string{"nomad", "acl", "get_binding_rule"}, time.Now())
// Check management level permissions.
if acl, err := a.srv.ResolveACL(args); err != nil {
if aclObj, err := a.srv.ResolveACL(args); err != nil {
return err
} else if acl == nil || !acl.IsManagement() {
} else if !aclObj.IsManagement() {
return structs.ErrPermissionDenied
}

24
nomad/operator_endpoint.go
View File

@@ -365,11 +365,11 @@ func (op *Operator) AutopilotGetConfiguration(args *structs.GenericRequest, repl
}
// This action requires operator read access.
rule, err := op.srv.ResolveACL(args)
aclObj, err := op.srv.ResolveACL(args)
if err != nil {
return err
}
if rule != nil && !rule.AllowOperatorRead() {
if aclObj != nil && !aclObj.AllowOperatorRead() {
return structs.ErrPermissionDenied
}
@@ -400,11 +400,11 @@ func (op *Operator) AutopilotSetConfiguration(args *structs.AutopilotSetConfigRe
}
// This action requires operator write access.
rule, err := op.srv.ResolveACL(args)
aclObj, err := op.srv.ResolveACL(args)
if err != nil {
return err
}
if rule != nil && !rule.AllowOperatorWrite() {
if aclObj != nil && !aclObj.AllowOperatorWrite() {
return structs.ErrPermissionDenied
}
@@ -443,11 +443,11 @@ func (op *Operator) ServerHealth(args *structs.GenericRequest, reply *structs.Op
}
// This action requires operator read access.
rule, err := op.srv.ResolveACL(args)
aclObj, err := op.srv.ResolveACL(args)
if err != nil {
return err
}
if rule != nil && !rule.AllowOperatorRead() {
if aclObj != nil && !aclObj.AllowOperatorRead() {
return structs.ErrPermissionDenied
}
@@ -478,10 +478,10 @@ func (op *Operator) SchedulerSetConfiguration(args *structs.SchedulerSetConfigRe
}
// This action requires operator write access.
rule, err := op.srv.ResolveACL(args)
aclObj, err := op.srv.ResolveACL(args)
if err != nil {
return err
} else if rule != nil && !rule.AllowOperatorWrite() {
} else if aclObj != nil && !aclObj.AllowOperatorWrite() {
return structs.ErrPermissionDenied
}
@@ -532,10 +532,10 @@ func (op *Operator) SchedulerGetConfiguration(args *structs.GenericRequest, repl
}
// This action requires operator read access.
rule, err := op.srv.ResolveACL(args)
aclObj, err := op.srv.ResolveACL(args)
if err != nil {
return err
} else if rule != nil && !rule.AllowOperatorRead() {
} else if aclObj != nil && !aclObj.AllowOperatorRead() {
return structs.ErrPermissionDenied
}
@@ -803,10 +803,10 @@ func (op *Operator) UpgradeCheckVaultWorkloadIdentity(
}
// This action requires operator read access.
rule, err := op.srv.ResolveACL(args)
aclObj, err := op.srv.ResolveACL(args)
if err != nil {
return err
} else if rule != nil && !rule.AllowOperatorRead() {
} else if aclObj != nil && !aclObj.AllowOperatorRead() {
return structs.ErrPermissionDenied
}