synadm icon indicating copy to clipboard operation
synadm copied to clipboard

Add possibility to pass token via environment variable

Open Lykos153 opened this issue 2 years ago • 8 comments

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

Lykos153 avatar Apr 14 '22 13:04 Lykos153

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

https://github.com/JOJ0/synadm/blob/c827c9b3e738d7081c4fb283550985bdaea7534d/synadm/cli/matrix.py#L102

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

Ascurius avatar Apr 19 '22 12:04 Ascurius

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...
[...]

Lykos153 avatar Apr 19 '22 12:04 Lykos153

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

Ascurius avatar Apr 19 '22 12:04 Ascurius

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.

JOJ0 avatar Apr 19 '22 12:04 JOJ0

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.

Ascurius avatar Apr 19 '22 16:04 Ascurius

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 avatar Apr 20 '22 13:04 JOJ0

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

JOJ0 avatar Apr 20 '22 13:04 JOJ0

@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 avatar Apr 20 '22 16:04 Ascurius