Add option to disable api - #108
Conversation
|
Thanks for submitting this PR, I should be able to find some time next week to review it |
There was a problem hiding this comment.
The code changes in serve.go look fine, but if I remember correctly there's currently no support for adding more than one client to the server config. Additional clients have to be added dynamically (using the API) and are not preserved between server restarts.
So I'm not sure what benefit these changes give compared to just using the --simple flag instead, which already disables the API and supports a single client. It seems like for this to actually support the new functionality you describe the configure command would also need to be updated to allow multiple clients to be defined in the server config. And/or the add client command would need a new flag or sub-command that causes new client data to be written to a server config, requiring the server to be manually restarted with the new config.
| if !c.disableApi { | ||
| c.allowedIPs = append(c.allowedIPs, c.apiAddr) | ||
| } |
There was a problem hiding this comment.
This just prevents the API address from being added to the client config files, right? If so the user could potentially just guess it based on the source code add it back manually.
Since this would not really "disable" the API, I would prefer the flag here for the configure command be named something more accurate like "ignore-api" and the description/help updated to reflect what it actually does.
I have a usecase where I want to give users a wireguard config without them being able to use the api, therefore an option to disable it would be great.