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

Add possibility to pass token via environment variable #62

Open
Lykos153 opened this issue Apr 14, 2022 · 8 comments
Open

Add possibility to pass token via environment variable #62

Lykos153 opened this issue Apr 14, 2022 · 8 comments
Labels
feature Feature request, not a core feature

Comments

@Lykos153
Copy link
Contributor

So it doesn't have to be stored on disk in plaintext but can be retrieved from eg. pass

@Ascurius
Copy link
Collaborator

Hi Lykos, this functionality is already implemented for the matrix raw command which can access the token via the environment variable MTOKEN.

"--token", "-t", type=str, envvar='MTOKEN', show_default=True,

Maybe I missunderstood your request. Could you please clarify your use case and any errors that you may have encountered.

@Lykos153
Copy link
Contributor Author

Lykos153 commented Apr 19, 2022

That's great! However, for every other command (eg. synadm user list) I still need to have the access token of an admin account configured in the config file in plaintext:

MTOKEN=$(pass my-admin-token) synadm user list
ERROR Config entry missing: token
Running configurator...
[...]

@Ascurius
Copy link
Collaborator

Alright, now I understand your request. I will have a look at this in due time.

@JOJ0
Copy link
Owner

JOJ0 commented Apr 19, 2022

Thhanks for the request @Lykos153, this is on my personal "would be nice to have" list for a long time already. As you both figured out already we have it implemented in matrix subcommand but not on regular admin api commands.
Please have a look at the precedence rules for token reading in the matrix raw command: https://synadm.readthedocs.io/en/latest/synadm.cli.matrix.html#synadm-matrix-raw

We should respect these rules and code this featur as similar as possible or even try to share code.

@Ascurius
Copy link
Collaborator

We should respect these rules and code this featur as similar as possible or even try to share code.

I agree with you. To be consistent, I would suggest to use the --token option as implemented in matrix raw command, so we can "copy-paste" this option to other commands. Let me know what you think about this approach and if you have any other ideas on how to implement this option.

@JOJ0
Copy link
Owner

JOJ0 commented Apr 20, 2022

Well it's not too easy I realize just now....

In that case I think it makes sense to have --token an option of synadm main command directly, and actually when we think about it, the synadm matrix subcommand should have that option directly as well. Changin that breaks existing behaviour but I think it would be worth it to streamline usability of both matrix and regular commands

@JOJ0
Copy link
Owner

JOJ0 commented Apr 20, 2022

Still there would be one caveat with my proposal: The existing matrix login command would not make sense with that --token option and would just ignore it. Is that bad? I could live with it and would definitely prefer having one --token option for all matrix commands

@Ascurius
Copy link
Collaborator

@JOJ0 I agree that it would be better to add the --token option to the root command of synadm. Here, I think the advantage that we only have to implement the option once in the root command outweighs the potential disadvantages or any complications it may cause with the matrix login command.

@Ascurius Ascurius added the feature Feature request, not a core feature label Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request, not a core feature
Projects
None yet
Development

No branches or pull requests

3 participants