-
Notifications
You must be signed in to change notification settings - Fork 27
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 ability to call custom GET/POST command via synadm script #11
Comments
Also will be good to show response text in prettified JSON, instead of plaintext like in raw |
Did you check the latest upstream version (master branch)? Have you seen the --output/-o option? Actually pretty printed json is supported since day one :-) It was called --json and now is called --output pprint. |
As for this suggestion: Yes nice idea and I will think about it but actually what do you think about my preferred idea: You seem to have a specific idea of what admin API calls you are missing. It is pretty easy to implement a missing one. So please file a pull-request with the one you are missing most and I will happily try to assist with reviewing your code :-) The goal of synadm is to at some point support everything the Synapse admin APIs offer. So implementing one available API after the other is the first priority. Any help with that is greatly appreciated! The second priority is supporting regular matrix calls too. One of the next features will be to let the admin user login by password prompt or token environment variable, so if a user prefers it that way, the token has not to be saved in the configuration. User login is a regular matrix client API and to support it there needs to be some foundational work done first. At the moment top priority is getting the latest bugs fixed that were introduced by latest PR. So please go ahead and try out the latest version and open issues when you find anything that fails or is not working as before! That would help a lot! Thanks and keep those suggestions and issue reports coming! Appreciated :-) |
+1 on this. I was chatting about this earlier on #synapse:matrix.org as well. I propose It would be a great stopgap when the official cli ui is not implemented for some new feature. Bonus points if you can make the matrix api documentation writers to adopt the tool instead of curl (though now i can't find examples of curl usage in any such documentation). |
I will think about it but it seems to be more work and thinking compared to supporting a new regular admin API, which as already mentioned, currently should be my main priority. If any of you would send a PR draft we could work on it together. That would be awesome! :-) |
@amstan I like The first part of the URI is in the configuration already, currently only for the admin api but as mentioned synadm will make use of the regular matrix client api in the future, so it will be a configuration item and should be used. See here, soon there will be an option called "Matrix Client path" or similar:
You have a good suggestion how the command could be constructed? My only idea would be something like:
or better make dedicated commands? (but that's kind of even more ugly ;-))
As mentioned already: I'd welcome a PR implementing this but won't find the time any time soon to code it myself. Thanks in advance if you'd really consider to submit it. |
@amstan still waiting for your pr draft 😅 |
Part of this has been implemented in branch Note that this is a convenience feature for admins trying to help users and not trying to replace existing Matrix API CLI tools! Since I recently also implemented a command for "user token fetching API": Data can be read in from file or stdin:
or from CLI directly:
A Matrix token can be read in interactivly:
Set on CLI:
Or even read from an environment variable:
The current help of the new command should describe all the possibilities:
@amstan and @MurzNN: Please help me get this feature stable and do some testing and playing around with it. Also review of the help text's english and clarity of descriptions is appreciated. That would help a lot. Thanks in advance |
Above mentioned has been released in this version: https://github.com/JOJ0/synadm/releases/tag/v0.30 |
I am closing the issue because the requested feature already has been implemented |
Sorry, please reopen. It only is implemented for matrix calls but not for admin api calls 😀 some details pn the initial ideas here: #11 (comment) |
The admin API is not exactly documentation for the end-sysadmins, it's mainly for developers.
My answer to that is don't touch the "path" part of the URL, it gets annoying just trying to figure out how the code behaves if it touches any variables. Why? The
To re-iterate over my answer (differently this time), don't distinguish between the Matrix API calls and the synapse admin API calls and don't manipulate the URL. Single command to call any endpoint, that's it. The just my two cents. |
I absolutely agree that it doesn't help a synadm user at all to have to copy and paste an endpoint URL and then having to remove Some concerns anyway:
Lines 97 to 111 in bda3057
I didn't think through entirely whether your idea would break this ^ possibility or if it could happily co-exist!! Appologiez if some thought might occur random, I just wanted to finally answer to this and hopefully find a concept together that works out and doesn't break existing concepts too much. That said, this command still is a very advanced one and we could probably change some concepts without bothering such advanced users too much if we improve it. At least I think that this command probably isn't used in scripts too often, but we can never know.... |
I just jecallled that somewhere @Ascurius and me came to the conclusion that I just can't find anywhere the discussion .... 🤷 Update: Found it: #62 (comment), Damnit that is too obvious of a name for that issue LOL |
Re: Tokens keeping secrets a secret on the shell is... probably unexpectedly difficult. There's shell history and process arguments, and to hide those without using standard input to accept tokens is quite complicated. Look at this: https://unix.stackexchange.com/questions/439497/is-there-a-way-to-pass-sensitive-data-in-bash-using-a-prompt-for-any-command So, when implementing one-off token usage, there really should be a way to accept standard input as a method to pass tokens (and also don't echo the token). Just something to remember when implementing. |
?
root context since you're able to do anything (matrix or synapse admin) with it.
i'm not sure if I understand, but then I came up with another concern: what if the API the user is calling disallows authorization? I guess just a flag to not give the authorization header should be enough. |
Too lazy for reading now but I will once we start a reimplementation. Thanks for the article. We had disussed quite some of these security related topics in #synadm back then before I implemented
Exactly, we should provide that! We have that for We have a way of interactive prompt as well as using a shell env var called MTOKEN, the latter probably being not a good idea to use. We can only provide the tools and if we are kind provide a word of warning that using shell variables with secrets is tricky. |
Sorry for speking complicated ;-) Yes I actually ment: An existing user that knows about The most logic way would be to move this raw command to the root context. Yes, but could break workflows or scripts of users...
Maybe my statement is bollocks but I think I was thinking something like this: Sometimes I would send a call to a Matrix API where I don't want to send my admin token, but now that I read the docs of but your idea is also a thing/feature we should provide: not sending a token at all! could be useful. |
Actually, I don't think it's that cumbersome to use curl. Here's the equivalent command with curl:
The differences between these commands are:
And that's it. Pretty much everything else including the request type, request body and URL path is the same. Practically no other differences there. What if some user demands more curl like options in synadm? Because then, we're just gonna start re-implementing curl. The benefits (tiny, microscopic when compared) don't outweigh the cost (curl has hundreds of options, been developed over decades, and has a man page that's 6229 lines long). This suggestion now feels quite out of scope to me, because synadm hits synapse admin APIs. It probably shouldn't be the next curl re-implementation, or a HTTP client tool. synadm isn't that, nor should it be that. |
synadm should be a convenient helper to a Synapse admin. Actually I personally dont care too much if we have this featurr or not. I still think that using that curl command you provide here is supertedious and havin synadm do it, could be just a little easier I suggest we ask the original submitter whether they're still interested in the feature. @MurzNN what do you say? |
I still think it's very handy and I'd like to have it. I just started an implementation. Also I'll use that opportunity to adress a problem we often faced already: Sharing common things between commands. |
Will be good to use same
synadm
tool not only for pre-defined API calls, but for custom too, for not to create custom scripts for each call.For example, allow something like this:
where make default argument value for
type
asGET
,body
={}
.This way will be much more comfortable, than composing full
curl
command in cli manually and remember your token each time.Especially this is useful at now, when some of popular API calls are not implemented in build-in commands.
The text was updated successfully, but these errors were encountered: