Auth: Add storage volume and bucket location to URL in access check

[markylaing]

This PR fixes fine grained authorization for storage volumes and storage buckets in clustered LXD.

When listing volumes, we can use the Location field of the storage volume or bucket to perform an access check.

When performing an action on a single storage volume, we need to perform the following checks:

1. Is the pool containing the volume remote? If so, the target query parameter may have been used to target a particular cluster member, but should not be used in the URL for the volume (volumes in remote pools do not have a location).
2. Is the volume located on another cluster member? If so, and the target parameter is set, use the target parameter as the location. If so, and the target parameter is unset, use the cluster member name as the location. If not, use the server name as the location.

To prevent extra queries from being performed when a request is forwarded to another node. Each node will assume that forwarded requests have already reached their intended destination. This is done by checking for request.CtxForwardedProtocol in the request context (I tried this originally by setting the target parameter but this broke storage volume migrations).

Closes #13365

[markylaing]

I've added a few comments on things I noticed while working on this that might be worth revisiting.

[tomponline]

Please can you rebase

[markylaing]

@tomponline rebased and ready for review

[tomponline]

When performing an action on a single storage volume, we need to perform the following checks:

1. Is the pool containing the volume remote? If so, the `target` query parameter may have been used to target a particular cluster member, but should not be used in the URL for the volume (volumes in remote pools do not have a location).

2. Is the volume located on another cluster member? If so, and the target parameter is set, use the target parameter as the location. If so, and the target parameter is unset, use the cluster member name as the location. If not, use the server name as the location.

Before reviewing further I just wanted to check and understand this statement, as it seems over complicated to me for an access check of a volume (which in my understanding ultimately boils down to loading the volume record to get its ID before doing an access check on that resource).

Or is this just the existing logic that is used to load a specific volume already?

[markylaing]

When performing an action on a single storage volume, we need to perform the following checks:

1. Is the pool containing the volume remote? If so, the `target` query parameter may have been used to target a particular cluster member, but should not be used in the URL for the volume (volumes in remote pools do not have a location).

2. Is the volume located on another cluster member? If so, and the target parameter is set, use the target parameter as the location. If so, and the target parameter is unset, use the cluster member name as the location. If not, use the server name as the location.

Before reviewing further I just wanted to check and understand this statement, as it seems over complicated to me for an access check of a volume (which in my understanding ultimately boils down to loading the volume record to get its ID before doing an access check on that resource).

Or is this just the existing logic that is used to load a specific volume already?

This is the logic that is used to figure out where to forward the request. Coming back to this I think it can probably be simplified by writing a custom query and bypassing all this complication.

The main issue is that there can be multiple volumes with the same name, just located on different members.

[markylaing]

@tomponline summarising here my remarks about this PR in the core daily stand up:

As far as fine-grained auth is concerned, we can call CheckPermission on a URL if that URL represents a single unique resource. We are using cluster.GetEntityReferenceFromURL to do this in the OpenFGA storage driver.

For storage buckets and volumes, this represents a challenge because bucket/volumes can have the same name if they are in a local pool and located on different cluster members. Additionally, a target query parameter can be provided if the caller wants to make the request to a specific member, regardless of the actual location of the bucket/volume.

There are a few cases to consider:

1. Volume in remote pool, target parameter provided.
2. Volume in remote pool, target parameter not provided.
3. Volume in local pool, target parameter provided.
4. Volume in local pool, target parameter not provided.

To catch all these cases, we need to*:

1. Load the storage pool and check if it is remote or not.
2. If it is remote:
   i. Omit the target parameter from the URL used in the access check.
3. If it is local:
   i. If the target parameter was provided, use it in the URL for the access check.
   ii. Otherwise, find the node that the volume is located on and use that member name in the access check.

*For all of these cases, we additionally need to check for the effective project of the bucket/volume and substitute the project parameter if it differs from that of the request.

In figuring out all of these steps, we've already done a lot of the work that would normally be done in the handler for forwarding requests to appropriate nodes. This PR adds the effective project, the storage pool, and forwarding node info (name & address) to the request context so that the handlers don't need to perform these queries again. Additionally, if the request has already been forwarded, the query for forwarding node info is skipped (we assume the other node has already correctly handled this).

I think this PR is the way forward because it overall reduces the number of queries (by skipping the remote volume check when it's a forwarded request) but does not change the queries themselves. (We can optimise these later if it becomes necessary).

[markylaing]

@tomponline have updated this to address your comments. Ready for review when you have some time. Thanks.

[tomponline]

Please can you rebase this

[markylaing]

Please can you rebase this

Yep done.

[tomponline]

Thanks! This pattern of extracting fields common to several API endpoints and loading the DB resource and storing them into the context seems like a good way of avoiding additional DB queries when performing access checks as well as removing code duplication in the API endpoint handlers themselves.