Use entries iterator that handles permissions correctly

[rafaeling]

New expected behaviour

1. Client remains as it was, just pre-commit style was updated.
2. Databroker responses works as follow:

   • getValue(path): return value or error if there is any.

   • getValues(path1, path2, ... , n) return all path values if there are no errors, if there is/are any error/errors it would return no partial values and just its corresponding error for each error path.

   • getValue(wildcard): return value or error if there is any but no partial values

   • getValues(wildcard1, wildcard2, ... , n) return all wildcard values if there are no errors, if there is/are any error/errors it would return no partial values of any wildcard and just its corresponding error for each error wildcard.

   • getValues(wildcard1, path2, ... , n) return all path/wildcard values if there are no errors, if there is/are any error/errors it would return no partial values and just its corresponding error for each error path/wildcard.

   • getMetaData(wildcard or path) return values without permission errors.

(Outdated)
How wildards and request errors are handled?

• When a wildcard pattern is requested and there are permission errors for some items, the response will include entries for which permission was granted, along with just one permission error indicating that "access was denied for some entries" -> Lazy developers should find out which those entries are
• If a wildcard pattern is requested, but it doesn't match any items, the response will consist of a single error, "No entries found for the provided path."
• When a request is made for an entry that doesn't exist, the response will also contain a single error, "No entries found for the provided path."
• In cases where a request is made for a value for which the requester lacks permission, the response will include one error indicating "Permission Denied" for the access attempt.

[rafaeling]

A separate PR should be created to fix the actuator filter on kuksa-client for getTargetValue method

[argerus]

Code wise, I don't have anything of substance to add.

I haven't tested the actual behaviour that much though. I did try testing this though:

1. Start databroker with authorization enabled.
2. Start kuksa-client using jwt token jwt/read-vehicle-speed.token.
3. Run: getValues Vehicle.Speed
4. Run: getValues Vehicle.Speed Vehicle.Service.* Vehicle.Asd

The first command correctly returns:

{
    "path": "Vehicle.Speed",
    "value": {
        "value": 10.0,
        "timestamp": "2023-09-20T08:51:12.047766+00:00"
    }
}

But the second command returns:

{
    "error": {
        "code": 403,
        "reason": "forbidden",
        "message": "Permission denied for some entries"
    },
    "errors": [
        {
            "path": "Vehicle.Service.*",
            "error": {
                "code": 403,
                "reason": "forbidden",
                "message": "Permission denied for some entries"
            }
        },
        {
            "path": "Vehicle.Asd",
            "error": {
                "code": 404,
                "reason": "not_found",
                "message": "No entries found for the provided path"
            }
        }
    ]
}

which I guess is wrong, as i doesn't include what was in the first response.

Not sure if this is a bug in the client or this implementation (or my understanding of how it should work).

[lukasmittag]

Works as expected. Tried:

Test Client> authorize ../jwt/provide-vehicle-speed.token 
"Authenticated"

Test Client> getValues Vehicle.Speed Vehicle.PowerOptimizeLevel 
[
    {
        "path": "Vehicle.Speed"
    },
    {
        "path": "Vehicle.PowerOptimizeLevel",
        "error": {
            "code": 403,
            "reason": "forbidden",
            "message": "Permission denied"
        }
    }
]

Test Client> setValue Vehicle.Speed 12
OK

Test Client> getValues Vehicle.Speed Vehicle.PowerOptimizeLevel 
[
    {
        "path": "Vehicle.Speed",
        "value": {
            "value": 12.0,
            "timestamp": "2023-09-21T07:23:20.029219+00:00"
        }
    },
    {
        "path": "Vehicle.PowerOptimizeLevel",
        "error": {
            "code": 403,
            "reason": "forbidden",
            "message": "Permission denied"
        }
    }
]

Test Client> getValue Vehicle.Speed
[
    {
        "path": "Vehicle.Speed",
        "value": {
            "value": 12.0,
            "timestamp": "2023-09-21T07:23:20.029219+00:00"
        }
    }
]

Test Client> getValue Vehicle.Power
PowerOptimizeLevel    Powertrain.           
Test Client> getValue Vehicle.PowerOptimizeLevel 
{
    "error": {
        "code": 403,
        "reason": "forbidden",
        "message": "Permission denied"
    },
    "errors": [
        {
            "path": "Vehicle.PowerOptimizeLevel",
            "error": {
                "code": 403,
                "reason": "forbidden",
                "message": "Permission denied"
            }
        }
    ]
}

Test Client> getMetaData Vehicle.PowerOptimizeLevel 
{
    "error": {
        "code": 403,
        "reason": "forbidden",
        "message": "Permission denied"
    },
    "errors": [
        {
            "path": "Vehicle.PowerOptimizeLevel",
            "error": {
                "code": 403,
                "reason": "forbidden",
                "message": "Permission denied"
            }
        }
    ]
}

Test Client> getMetaData Vehicle.Speed 
[
    {
        "path": "Vehicle.Speed",
        "metadata": {
            "data_type": "FLOAT",
            "entry_type": "SENSOR",
            "description": "Vehicle speed."
        }
    }
]

Just one remark: Have we agreed upon that metadata needs permission now too?

[SebastianSchildt]

My understanding was, last discussion went to having metaData free for all for now, or has that changed? Espiecially wouldnt this break "Learning" with "*" what data is available?

[rafaeling]

My understanding was, last discussion went to having metaData free for all for now, or has that changed? Espiecially wouldnt this break "Learning" with "*" what data is available?

It is a bug, getMetaData should work anyways, have to fix it, thanks Lukas :)

[rafaeling]

PR description was updated with the new behaviour -> #654 (comment)

Discussions regarding modifications to the gRPC interface for metadata, as well as new features for separating wildcard and paths, will be moved to a new issue.

python interface changes reverted

[argerus]

This is perhaps overly pedantic, but the server is still returning "partial success" and silently ignoring permission errors.

from kuksa_client.grpc import VSSClient, EntryRequest, View, Field

read_vehicle_speed_token = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJsb2NhbCBkZXYiLCJpc3MiOiJjcmVhdGVUb2tlbi5weSIsImF1ZCI6WyJrdWtzYS52YWwiXSwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjE3NjcyMjU1OTksInNjb3BlIjoicmVhZDpWZWhpY2xlLlNwZWVkIn0.QyaKO-vjjzeimAAyUMjlM_jAvhlmYVguzXp76jAnW9UUiWowrXeYTa_ERVzZqxz21txq1QQmee8WtCO978w95irSRugTmlydWNjS6xPGAOCan6TEXBryrmR5FSmm6fPNrgLPhFEfcqFIQm07B286JTU98s-s2yeb6bJRpm7gOzhH5klCLxBPFYNncwdqqaT6aQD31xOcq4evooPOoV5KZFKI9WKkzOclDvto6TCLYIgnxu9Oey_IqyRAkDHybeFB7LR-1XexweRjWKleXGijpNweu6ATsbPakeVIfJ6k3IN-oKNvP6nawtBg7GeSEFYNksUUCVrIg8b0_tZi3c3E114MvdiDe7iWfRd5SDjO1f8nKiqDYy9P-hwejHntsaXLZbWSs_GNbWmi6J6pwgdM4XI3x4vx-0Otsb4zZ0tOmXjIL_tRsKA5dIlSBYEpIZq6jSXgQLN2_Ame0i--gGt4jTg1sYJHqE56BKc74HqkcvrQAZrZcoeqiKkwKcy6ofRpIQ57ghooZLkJ8BLWEV_zJgSffHBX140EEnMg1z8GAhrsbGvJ29TmXGmYyLrAhyTj_L6aNCZ1XEkbK0Yy5pco9sFGRm9nKTeFcNICogPuzbHg4xsGX_SZgUmla8Acw21AAXwuFY60_aFDUlCKZh93Z2SAruz1gfNIL43I0L8-wfw"

client = VSSClient(host="127.0.0.1", port=55555)
client.connect()
client.authorize(token=read_vehicle_speed_token)

# This will give no error / exception. I.e. permission denied is silently ignored and
# all metadata is returned + the value for the one thing we have permission for ("Vehicle.Speed").
resp1 = client.get([EntryRequest(path="Vehicle", view=View.UNSPECIFIED, fields=[Field.VALUE, Field.METADATA])])

# Causes an exception
# kuksa_client.grpc.VSSClientError: ({'code': 403, 'reason': 'forbidden', 'message': 'Permission denied for some entries'}, [{'path': 'Vehicle', 'error': {'code': 403, 'reason': 'forbidden', 'message': 'Permission denied for some entries'}}])
resp2 = client.get([EntryRequest(path="Vehicle", view=View.UNSPECIFIED, fields=[Field.VALUE])])

PS
Unrelated, but the python client just returns None if one fails to call connect before making a call.

client = VSSClient(host="127.0.0.1", port=55555)
client.authorize(token=read_vehicle_speed_token)

# client.get(...) returns None

and fails to use the access token provided to it if client.connect() is called after client.authorize(...)

client = VSSClient(host="127.0.0.1", port=55555)
client.authorize(token=read_vehicle_speed_token)
client.connect()

# client.get(...) fails with
# kuksa_client.grpc.VSSClientError: ({'code': 16, 'reason': 'unauthenticated', 'message': 'Invalid auth token'}, [])

[SebastianSchildt]

I tested the behaviors lined out, and those work for me as I expect.

for Johns first comment: If several things are requested, i.e. in that case Metadata and Value, would it be possible @rafaeling to also just deny the whole request, i.e. that always "deny" wins, i.e. if requesting value, target and metaData for a single data point, but you miss e.g. read rights, it will just deny the whole request? or is this hard with how this is currently architected?

The other comment, i.e. returning none on get when not connected, might be worth a separate issue. Not ideal, but let's not fix it here.

I think, that the token is not automatically used after "connect" is maybe ok, because as I see it, connect() resets the whole state client side, and I understand authorize more like a function, and not an inherent property fo the client object. I know this is not how it is in reality, but for waht you expect @argerus I would maybe expect the function not be called "authorize", but rather set_authorization_token or something like that, but same -> Sounds like material for a separate issue

[rafaeling]

This is perhaps overly pedantic, but the server is still returning "partial success" and silently ignoring permission errors.

from kuksa_client.grpc import VSSClient, EntryRequest, View, Field

read_vehicle_speed_token = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJsb2NhbCBkZXYiLCJpc3MiOiJjcmVhdGVUb2tlbi5weSIsImF1ZCI6WyJrdWtzYS52YWwiXSwiaWF0IjoxNTE2MjM5MDIyLCJleHAiOjE3NjcyMjU1OTksInNjb3BlIjoicmVhZDpWZWhpY2xlLlNwZWVkIn0.QyaKO-vjjzeimAAyUMjlM_jAvhlmYVguzXp76jAnW9UUiWowrXeYTa_ERVzZqxz21txq1QQmee8WtCO978w95irSRugTmlydWNjS6xPGAOCan6TEXBryrmR5FSmm6fPNrgLPhFEfcqFIQm07B286JTU98s-s2yeb6bJRpm7gOzhH5klCLxBPFYNncwdqqaT6aQD31xOcq4evooPOoV5KZFKI9WKkzOclDvto6TCLYIgnxu9Oey_IqyRAkDHybeFB7LR-1XexweRjWKleXGijpNweu6ATsbPakeVIfJ6k3IN-oKNvP6nawtBg7GeSEFYNksUUCVrIg8b0_tZi3c3E114MvdiDe7iWfRd5SDjO1f8nKiqDYy9P-hwejHntsaXLZbWSs_GNbWmi6J6pwgdM4XI3x4vx-0Otsb4zZ0tOmXjIL_tRsKA5dIlSBYEpIZq6jSXgQLN2_Ame0i--gGt4jTg1sYJHqE56BKc74HqkcvrQAZrZcoeqiKkwKcy6ofRpIQ57ghooZLkJ8BLWEV_zJgSffHBX140EEnMg1z8GAhrsbGvJ29TmXGmYyLrAhyTj_L6aNCZ1XEkbK0Yy5pco9sFGRm9nKTeFcNICogPuzbHg4xsGX_SZgUmla8Acw21AAXwuFY60_aFDUlCKZh93Z2SAruz1gfNIL43I0L8-wfw"

client = VSSClient(host="127.0.0.1", port=55555)
client.connect()
client.authorize(token=read_vehicle_speed_token)

# This will give no error / exception. I.e. permission denied is silently ignored and
# all metadata is returned + the value for the one thing we have permission for ("Vehicle.Speed").
resp1 = client.get([EntryRequest(path="Vehicle", view=View.UNSPECIFIED, fields=[Field.VALUE, Field.METADATA])])

Good you found it but, what response should be expected here? An error or the metadata?

[argerus]

Sounds like material for a separate issue

Yes, definitely not in this PR.

Good you found it but, what response should be expected here? An error or the metadata?

An error. Partial success are:

1. Not supported by kuksa-client.
2. Confusing in general.