Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

createToken.py: Allow specification of the JWT key file and output file #676

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

landgraf
Copy link
Contributor

Current version of the script assumes that jwt key file is located in the same directory as the script itself and token is written into the same directory as json specification. This is not convinient for binary distribution with key files placed under /etc and script under bin or libexec dirs.
Allowing specification of key file and output files location fixes this and application token can be generated with [1] while keeping backward compatibility and existing behaviour (single argument to specify json policy definition)

[1]
/usr/libexec/kuksa-createToken.py /usr/share/kuksa-val/jwt/all-read-write.json --key /etc/pki/kuksa/jwt.key --output /etc/xdg/AGL/%i/%i.token

@argerus
Copy link
Contributor

argerus commented Sep 27, 2023

The script supports multiple input files. So the output would need to either be a directory, or somehow specify how multiple input files would be named.

If I understand correctly, the code as suggested would cause the (single) output file to just be overwritten for every input file?


output_filename = input_filename[:-5] if input_filename.endswith(".json") else input_filename
output_filename += ".token"

Copy link
Contributor

Choose a reason for hiding this comment

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

The failed build is caused by trailing blanks on this line

@landgraf
Copy link
Contributor Author

landgraf commented Oct 1, 2023

The script supports multiple input files. So the output would need to either be a directory, or somehow specify how multiple input files would be named.

If I understand correctly, the code as suggested would cause the (single) output file to just be overwritten for every input file?

This is valid point. I'll add some logic to fix that.

@SebastianSchildt
Copy link
Contributor

  1. @erikbosch is this still relevant, it seems only blocked by linter issues

  2. Is OP @landgraf still alive?

If 1==true and 2==false I migh just push another commit fixing the linter issues

@erikbosch
Copy link
Contributor

@SebastianSchildt - we have as part of our migration to eclipse-kuksa copied this script and related data to https://github.com/eclipse-kuksa/kuksa-common/tree/main/jwt , but kept it here as well as both databroker and KUKSA Server references them. However the scripts does not need to exist in multiple locations, so if we should merge it then it should as I see it go to kuksa-common (and the scripts possibly removed here).

From a functional perspective - there is a problem as indicated by the last comments from John and Landgraf, the script loops over multiple input parameters but supports only one output parameter. This should better be fixed or at least marked/documented as a limitation

@landgraf
Copy link
Contributor Author

  1. @erikbosch is this still relevant, it seems only blocked by linter issues
  2. Is OP @landgraf still alive?

If 1==true and 2==false I migh just push another commit fixing the linter issues

@landgraf is still alive :) I'm not working on the project anymore though.
#676 (comment) is valid point and have to be fixed. I've fixed this issue but apparently forgot to push.

Current version of the script assumes that jwt key file is located in
the same directory as the script itself and token is written into the
same directory as json specification. This is not convinient for binary
distribution with key files placed under /etc and script under bin or
libexec dirs.
Allowing specification of key file and output files location fixes this
and application token can be generated with [1] while keeping backward
compatibility and existing behaviour (single argument to specify json
policy definition)
Error out if both output option and multiple input files have been
specified

[1]
/usr/libexec/kuksa-createToken.py /usr/share/kuksa-val/jwt/all-read-write.json --key /etc/pki/kuksa/jwt.key --output /etc/xdg/AGL/%i/%i.token
Copy link
Contributor

@erikbosch erikbosch left a comment

Choose a reason for hiding this comment

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

Looks good.

@SebastianSchildt - we need to agree on how to handle this improvement vs the migration to eclipse-kuksa. A possible workflow could be:

@erikbosch erikbosch merged commit 4077045 into eclipse:master Mar 1, 2024
6 checks passed
erikbosch added a commit to boschglobal/kuksa-common that referenced this pull request Mar 1, 2024
erikbosch added a commit to boschglobal/kuksa-common that referenced this pull request Mar 1, 2024
erikbosch added a commit to boschglobal/kuksa-common that referenced this pull request Mar 4, 2024
SebastianSchildt pushed a commit to eclipse-kuksa/kuksa-common that referenced this pull request Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants