Skip to content

Commit

Permalink
Auth: Mask errors returned by OpenFGADatastore (#14446)
Browse files Browse the repository at this point in the history
While working on #14085 I set up
a new fine-grained TLS identity and issued the following commands as
that identity, without any permissions yet (I forgot I'd changed my
default remote):
```
$ lxc auth group create tmp
Error: Forbidden
$ lxc auth group permission add tmp server admin
Error: Failed to check OpenFGA relation: No such entity "/1.0/auth/groups/tmp"
```
Creating the group failed, this is correct behaviour. 

When attempting to add a permission to the non-existent group, the
request failed (correct) but the OpenFGA Authorization driver returned
the above error. This is incorrect.

This PR checks if the error returned by a `Check` request on the
embedded OpenFGA server is a `Not Found` error and returns a generic not
found error. This makes errors returned by the authorizer consistent. We
are masking all not found errors returned before access control
decisions are made to prevent discovery. After this change, the same
command returns:
```
$ lxc auth group permission add tmp server admin
Error: Not Found
```
  • Loading branch information
tomponline authored Nov 12, 2024
2 parents db47f43 + 54b59a9 commit 183b46e
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions lxd/auth/drivers/openfga.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,12 @@ func (e *embeddedOpenFGA) CheckPermission(ctx context.Context, entityURL *api.UR
l.Debug("Checking OpenFGA relation")
resp, err := e.server.Check(ctx, req)
if err != nil {
// If we have a not found error from the underlying OpenFGADatastore we should mask it to make requests consistent.
// (all not found errors returned before an access control decision is made are masked to prevent discovery).
if api.StatusErrorCheck(err, http.StatusNotFound) {
return api.NewGenericStatusError(http.StatusNotFound)
}

// Attempt to extract the internal error. This allows bubbling errors up from the OpenFGA datastore implementation.
// (Otherwise we just get "rpc error (4000): Internal Server Error" or similar which isn't useful).
var openFGAInternalError openFGAErrors.InternalError
Expand All @@ -275,6 +281,12 @@ func (e *embeddedOpenFGA) CheckPermission(ctx context.Context, entityURL *api.UR
l.Debug("Checking OpenFGA relation")
resp, err := e.server.Check(ctx, req)
if err != nil {
// If we have a not found error from the underlying OpenFGADatastore we should mask it to make requests consistent.
// (all not found errors returned before an access control decision is made are masked to prevent discovery).
if api.StatusErrorCheck(err, http.StatusNotFound) {
return api.NewGenericStatusError(http.StatusNotFound)
}

// Attempt to extract the internal error. This allows bubbling errors up from the OpenFGA datastore implementation.
// (Otherwise we just get "rpc error (4000): Internal Server Error" or similar which isn't useful).
var openFGAInternalError openFGAErrors.InternalError
Expand Down

0 comments on commit 183b46e

Please sign in to comment.