Auth: Add project query parameter to URLs in authorizer #13317

markylaing · 2024-04-11T15:55:41Z

In our implementation of (github.com/openfga/openfga/pkg/storage).OpenFGADatastore, entity URLs are expected to include the project query parameter even if it is the default project. However, LXD typically displays URLs without the project query parameter when a resource is in the default project (with the exception of permissions, where we have been explicit).

This PR ensures that the project query parameter is added to URLs inside the Authorizer so that this doesn't need to be done elsewhere in the codebase (such as in project.FilterUsedBy which has now been simplified).

Closes #13072

markylaing · 2024-07-05T15:40:12Z

@tomponline This is ready for review when you have time. Thanks.

shared/entity/type_test.go

lxd/auth/drivers/tls.go

lxd/auth/drivers/openfga.go

lxd/project/permissions.go

tomponline

One question on tests and a few nits around "err" in logging.

Previously this was just returned in the path arguments. Signed-off-by: Mark Laing <[email protected]>

So that `URL` is the inverse of `ParseURL`, we should explicitly ignore the `projectName` argument when creating the URL of a project. Signed-off-by: Mark Laing <[email protected]>

We now expect the project name to be returned from ParseURL. Additionally, we now test that the normalised URL is what we expect. Signed-off-by: Mark Laing <[email protected]>

…ents. We can now use the value of `projectName` returned from ParseURL. Signed-off-by: Mark Laing <[email protected]>

…uments. We can now use the value of `projectName` returned from ParseURL. Signed-off-by: Mark Laing <[email protected]>

This commit regenerates the entity URLs that are passed into `CheckPermission` and the `PermissionChecker` returned by `GetPermissionChecker` to enforce that the project parameter is added to the URL even if it is "default". This is expected by the underlying `storage.OpenFGADatastore` implementation. Signed-off-by: Mark Laing <[email protected]>

The entity URLs returned by `projectUsedBy` included the project query parameter even when the project was "default". Signed-off-by: Mark Laing <[email protected]>

We no longer need to enforce that the project query parameter is set in the URLs that are passed into Authorizer methods, nor do we need to strip the project query parameter from the URL. Signed-off-by: Mark Laing <[email protected]>

All LXD entity URLs are parsed into a project, a location, and a slice of path arguments. Previously when parsing a project URL, the project name was empty because it is present in the path arguments. Now that the project name is being returned, we need our entity SQL queries to match. Signed-off-by: Mark Laing <[email protected]>

tomponline

Thanks!

markylaing requested a review from tomponline as a code owner April 11, 2024 15:55

markylaing self-assigned this Apr 11, 2024

markylaing marked this pull request as draft April 11, 2024 15:56

markylaing force-pushed the authorizer-add-project branch from ad1d4e6 to a4fa4d3 Compare April 24, 2024 12:29

markylaing force-pushed the authorizer-add-project branch from a4fa4d3 to eb099b5 Compare July 5, 2024 15:15

markylaing marked this pull request as ready for review July 5, 2024 15:16

markylaing changed the title ~~Auth: Add project query parameter to URLs in authorizer.~~ Auth: Add project query parameter to URLs in authorizer Jul 5, 2024

markylaing force-pushed the authorizer-add-project branch from 685eb8f to e030027 Compare July 9, 2024 09:21