Skip to content
This repository has been archived by the owner on Dec 18, 2024. It is now read-only.

Add support for access tokens to cli #477

Merged
merged 5 commits into from
Feb 28, 2023

Conversation

argerus
Copy link
Contributor

@argerus argerus commented Feb 10, 2023

Adds support for access tokens to databroker-cli.

Included are a number of other improvements. Some necessary for adding this support, some not. E.g.

  • Add support for command line arguments
  • Add initial support for subcommands
  • Start moving GRPC related parts into client.rs
  • A number of small updates to the interface

@argerus argerus force-pushed the feature/cli_access_token branch 12 times, most recently from 231282e to f4f9507 Compare February 16, 2023 15:53
* Add support for authorization Bearer token
* Add argument for JWT public key
* Validate JWT access token when authorization enabled
@argerus argerus force-pushed the feature/cli_access_token branch from f4f9507 to fcaa92e Compare February 17, 2023 10:36
Copy link
Contributor

@lukasmittag lukasmittag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works for me

@argerus argerus force-pushed the feature/cli_access_token branch from fcaa92e to e6f3c2c Compare February 20, 2023 09:02
@SebastianSchildt
Copy link
Contributor

I could get this to run

cargo run --bin databroker -- --port 55556 --metadata ../data/vss-core/vss_release_3.0.json --jwt-public-key ../kuksa_certificates/jwt/jwt.key.pub

But then I guess I met some documentation issues with client

     Running `/Users/scs2rng/Documents/Dev/kuksa.val/target/debug/databroker-cli --server 'http://127.0.0.1:55556' --token-file ../kuksa_certificates/jwt/super-admin.json.token`

  ⠀⠀⠀⢀⣤⣶⣾⣿⢸⣿⣿⣷⣶⣤⡀
  ⠀⠀⣴⣿⡿⠋⣿⣿⠀⠀⠀⠈⠙⢿⣿⣦⠀
  ⠀⣾⣿⠋⠀⠀⣿⣿⠀⠀⣶⣿⠀⠀⠙⣿⣷   
  ⣸⣿⠇⠀⠀⠀⣿⣿⠠⣾⡿⠃⠀⠀⠀⠸⣿⣇⠀⠀⣶⠀⣠⡶⠂⠀⣶⠀⠀⢰⡆⠀⢰⡆⢀⣴⠖⠀⢠⡶⠶⠶⡦⠀⠀⠀⣰⣶⡀
  ⣿⣿⠀⠀⠀⠀⠿⢿⣷⣦⡀⠀⠀⠀⠀⠀⣿⣿⠀⠀⣿⢾⣏⠀⠀⠀⣿⠀⠀⢸⡇⠀⢸⡷⣿⡁⠀⠀⠘⠷⠶⠶⣦⠀⠀⢠⡟⠘⣷
  ⢹⣿⡆⠀⠀⠀⣿⣶⠈⢻⣿⡆⠀⠀⠀⢰⣿⡏⠀⠀⠿⠀⠙⠷⠄⠀⠙⠷⠶⠟⠁⠀⠸⠇⠈⠻⠦⠀⠐⠷⠶⠶⠟⠀⠠⠿⠁⠀⠹⠧
  ⠀⢿⣿⣄⠀⠀⣿⣿⠀⠀⠿⣿⠀⠀⣠⣿⡿
  ⠀⠀⠻⣿⣷⡄⣿⣿⠀⠀⠀⢀⣠⣾⣿⠟    databroker-cli                
  ⠀⠀⠀⠈⠛⠇⢿⣿⣿⣿⣿⡿⠿⠛⠁     v0.3.1                        

Successfully connected to http://127.0.0.1:55556/
sdv.databroker.v1 > get Vehicle.Speed 
[get]  OK  
Vehicle.Speed: ( NotAvailable )
sdv.databroker.v1 > feed Vehicle.Speed 200
unknown command
sdv.databroker.v1 > ?
unknown command
sdv.databroker.v1 > set
unknown command
sdv.databroker.v1 > help

  connect [URI]            Connect to server
  get <PATH> [[PATH] ...]  Get signal value(s)
  actuate <PATH> <VALUE>   Actuate signal
  subscribe <QUERY>        Subscribe to signals using a query
  metadata [FILTER]        Fetch metadata. Provide FILTER to view metadata of signals matching filter.
  provide <PATH> <VALUE>   Provide (publish) a signal value
  token <TOKEN>            Use TOKEN as access token
  token-file <FILE>        Use content of FILE as access token
  help                     You're looking at it.
  quit       
sdv.databroker.v1 > provide Vehicle.Speed 200
[provide]  OK  
sdv.databroker.v1 > actuate Vehicle.Body.Trunk.
Front.  Rear.
sdv.databroker.v1 > actuate Vehicle.Body.Trunk.Rear.Is
IsLocked  IsOpen
sdv.databroker.v1 > actuate Vehicle.Body.Trunk.Rear.IsOpen 
Usage: actuate <PATH> <VALUE>
sdv.databroker.v1 > actuate Vehicle.Body.Trunk.Rear.IsOpen true
[actuate]  Unauthenticated  No auth token provided
sdv.databroker.v1 > 

It see sdv.databroker.v1, I think that is not a good prompt, Because this should not matter when using the client.. if it does, how would I switch to the "new" API?

I would suggest, that unknown command command be extended with sth like see "help" for a list of available commands

Also, why is "provide" working, but "actuate" not, is that intended?

Generally, I think READMEs for databroker to be extended/updated to mention these features.

Should also give at least a small example how to use it with databroker-cli (with warning that it is not fully implemented and maybe also mentioning kuksa-client (py) not yet supported)

@argerus
Copy link
Contributor Author

argerus commented Feb 21, 2023

Also, why is "provide" working, but "actuate" not, is that intended?

That's a bug, will push an update.

It see sdv.databroker.v1, I think that is not a good prompt, Because this should not matter when using the client.. if it does, how would I switch to the "new" API?

The intention is to add support for kuksa.val.v1 and the ability to switch between them.

Generally, I think READMEs for databroker to be extended/updated to mention these features.

Agreed

@argerus argerus force-pushed the feature/cli_access_token branch from e6f3c2c to 0ca1b37 Compare February 21, 2023 21:38
Copy link
Contributor

@SebastianSchildt SebastianSchildt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ : Actuate working now

I still think "Unknown command" should give a hint to type help

This documentation
https://github.com/eclipse/kuksa.val/tree/master/kuksa_databroker
is outdated:

Need to have updated command line aprameters (and may one sentence for each where not obvious) here : https://github.com/eclipse/kuksa.val/tree/master/kuksa_databroker#test-the-databroker

It should at leat repair the cli examples here https://github.com/eclipse/kuksa.val/tree/master/kuksa_databroker#test-the-databroker

And update the quickstart hee https://github.com/eclipse/kuksa.val/blob/master/doc/quickstart.md
(not adding authorisation just changing "feed->provide" in the examples of databroker-cli

* Add support for command line arguments
* Add initial support for subcommands
* Start moving GRPC related parts into client.rs
* A number of small updates to the interface
@argerus argerus force-pushed the feature/cli_access_token branch 2 times, most recently from f95a456 to 256939b Compare February 26, 2023 23:44
@argerus argerus force-pushed the feature/cli_access_token branch from 256939b to 30962e1 Compare February 26, 2023 23:57
@argerus
Copy link
Contributor Author

argerus commented Feb 26, 2023

I still think "Unknown command" should give a hint to type help

Done

Need to have updated command line aprameters (and may one sentence for each where not obvious) here : https://github.com/eclipse/kuksa.val/tree/master/kuksa_databroker#test-the-databroker

Updated it, not sure I got it all.

It should at leat repair the cli examples here https://github.com/eclipse/kuksa.val/tree/master/kuksa_databroker#test-the-databroker

Done as well, I think.. (I reverted renaming of commands for now as well)

Copy link
Contributor

@SebastianSchildt SebastianSchildt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I seem unlucky, I am still facing issues:

sdv.databroker.v1 > lol
Unknown command. See `help` for a list of available commands.
sdv.databroker.v1 > help

  connect [URI]            Connect to server
  get <PATH> [[PATH] ...]  Get signal value(s)
  set <PATH> <VALUE>       Set actuator signal
  subscribe <QUERY>        Subscribe to signals with QUERY
  feed <PATH> <VALUE>      Publish signal value
  metadata [PATTERN]       Fetch metadata. Provide PATTERN to list metadata of signals matching pattern.
  token <TOKEN>            Use TOKEN as access token
  token-file <FILE>        Use content of FILE as access token
  help                     You're looking at it.
  quit                     Quit

sdv.databroker.v1 > get Vehicle.ADAS.CruiseControl.IsEnabled 
[get]  OK  
Vehicle.ADAS.CruiseControl.IsEnabled: ( NotAvailable )
sdv.databroker.v1 > feed Vehicle.ADAS.CruiseControl.IsEnabled
Usage: feed <PATH> <VALUE>
sdv.databroker.v1 > feed Vehicle.ADAS.CruiseControl.IsEnabled true
No metadata available for Vehicle.ADAS.CruiseControl.IsEnabled. Needed to determine data type for serialization.
sdv.databroker.v1 > feed Vehicle.Speed 200
No metadata available for Vehicle.Speed. Needed to determine data type for serialization.
sdv.databroker.v1 > metadata Vehicle.ADAS.CruiseControl.IsEnabled
[metadata]  OK  
Path                                 Entry type Data type
Vehicle.ADAS.CruiseControl.IsEnabled Actuator   Bool     
sdv.databroker.v1 > 

@argerus
Copy link
Contributor Author

argerus commented Feb 27, 2023

sdv.databroker.v1 > feed Vehicle.Speed 200
No metadata available for Vehicle.Speed. Needed to determine data type for serialization.

I seem unlucky, I am still facing issues:

No, you just put a spotlight on a usability issue (that wasn't there before adding authorization).

I'm assuming you started unauthenticated, which means the metadata couldn't be fetched on startup. The lack of metadata is communicated, but since this behaviour is new it's not obvious why it's missing. It could have been remedied by running metadata and trying again.

But I don't see why you should need to run that manually. Pushed an additional commit adding automatic fetching of metadata after setting a new access token.

Copy link
Contributor

@SebastianSchildt SebastianSchildt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally 🌵

@SebastianSchildt SebastianSchildt merged commit 71faaba into eclipse:master Feb 28, 2023
This was referenced Feb 28, 2023
@erikbosch erikbosch deleted the feature/cli_access_token branch February 27, 2024 11:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants