hashicorp / hashicorp/consul

Improve Permission denied errors by outputting the permision that is missing

Open
#7,640 2 comments 1 reaction 0 assignees View on GitHub
type/enhancement
Dominant language
Go
Stars
30.1k
Forks
4.6k
Avg merge
2d 6h
Merged PRs (30d)
43

Description

Right now when a request is denied due to lack of permissions, we end up returning just a blanket `Permission denied` message. We can greatly improve this with adding extra context about which permissions were missing and in some cases why.

Take for example this location: https://github.com/hashicorp/consul/blob/0e51f8941493902eb4365fba25a24fc816a4e28b/agent/acl.go#L128

Here we are returning a permission denied error for missing `service:write` on the target service. We could change this line to the following:

```go
return acl.PermissionDeniedWithCause("Missing service:write for new service %s", service.CompoundServiceName().String())
```

This would require that function in the ACL package to exist which is easily done:

```go
func PermissionDeniedWithCause(msg string, args ...interface{}) PermissionDeniedError {
return PermissionDeniedError{fmt.Sprintf(msg, args...)}
}
```

We can take advantage of the fact that the `acl.PermissionDeniedError` already has a `Cause` field. We just need to use it and make it easy to do so in the code by adding the helper function to also invoke `Sprintf`.

I have done a little bit of this when debugging some ACL permission denials but we should do this more broadly. Besides the small change to the `acl` package I think the rest of the changes are just going to be a bunch of single line changes to how we return those errors everywhere. There might also be tests that are doing equality checks against the string output by the `Error` method that need updating.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.