ScottLogic / kdb-boothroyd

a prototype re-implementation of KDB Studio using JavaScript and Electron
Apache License 2.0
5 stars 2 forks source link

Ability to use alternative authentication mechanisms #14

Open ihull opened 3 years ago

OiNutter commented 2 years ago

I've added in username and password auth on feature/authentication. It looks like node-q doesn't support other authentication stuff but I think we can support TLS and also SSH key auth by wrapping the node-q socket in a TLS Socket call. I just need to get a q server running that requires it so I can test this out.

OiNutter commented 2 years ago

We now have TLS support in place. I expect SSH support would require managing 2 socket connections, one to the server that's ssh protected and then another to the database using the first as a tunnel.

gyorokpeter commented 2 years ago

From the MS perspective, the extra auth method is just a preprocessor that takes the parameters (host, port, user, password) and returns a similar tuple that is then actually used for the connection.

During my first attempt to implement this in vscode-q, I could change the upstream code in a way that adding the new auth method would be a matter of adding a new file, a new import statement to an existing file, and the necessary dependencies to package.json. The author of vscode-q then offered an alternative solution that relies on vscode-specific features to find the plugin, but that's only because of his own preferences so I'm open for suggestions on how auth methods could be integrated in this project.

OiNutter commented 2 years ago

Ok, so if i understand you correctly, you just want to be able to add in a custom file that does some fiddling with the parameters and returns them as to the connection string? So if for instance you encrypted your creds by reversing them, (super secure I know) you're preprocessor would do that?

That certainly sounds doable, I'd need to give some thought to how to handle that from a UI perspective but a start point might be to just look for a file on the filesystem, if it exists import it, if not carry on as normal. Then the user just needs to copy it into the correct location.

ColinEberhardt commented 2 years ago

a start point might be to just look for a file on the filesystem

Perhaps have kdb-studio-2 use a simple settings.json file? There are likely other things we might want to configure in future.

OiNutter commented 2 years ago

Seems a reasonable plan. That would specify the path to the custom auth module. Also gives us a good point to layer a ui on top of.

gyorokpeter commented 2 years ago

Another aspect is that the whole thing should be zero-config. Effectively, we should be able to do some changes to the code and files, and then mark the whole thing as read-only, then the users can only use the read-only version. Writing config to the registry, APPDATA etc. is possible. However storing anything related to the custom auth in a config file suggests that the end-user would have to do something to make it work which we want to avoid. Unless you mean the settings.json would be populated from an initial copy that is included in the read-only version so we could make it work by default. Similarly, the user should not have to do anything for every single connection to make it use the custom auth.

If we don't hold the user's hand in this, some will definitely slip through and fail to enable the necessary settings, which means they will be unable to use the app, and then come to us with their support requests, wasting our time pointing them to the docs where it says you have to manually enable kerberos.

OiNutter commented 2 years ago

@gyorokpeter ok, this is sounding more complicated than I first thought. Am I right in thinking you want to be able to do a custom build from source with your auth file included, rather than this being something configurable in a release version?

OiNutter commented 2 years ago

Actually this might not be as bad as I think. We already store our server records in electron's userData folder, which is just the app specific folder under APPDATA. So we could have a settings.json file in there that specifies a path to a custom auth plugin file. Then by the sounds of it you can just write those 2 files to the directory on behalf of the user and make them read only so even if we build a gui it can't change it. Would that work? Saves you guys having to build code yourselves each time we do a release and mean's we have a flexible solution for other use cases?

gyorokpeter commented 2 years ago

Sounds good, as long as the customization is easy to automate (e.g. just copying files and editing lines that match a pattern).

OiNutter commented 2 years ago

I've started some work on this on the https://github.com/ScottLogic/kdb-studio-2/tree/feature/settings-json branch. I'm going to add a gui in to enable editing of the customAuthPlugin setting, but disable that if the settings.json file is marked as readonly. You'll need to create a settings.json file in the APPDATA folder for the app and add a customAuthPlugin key, which is an array of file paths (so you can chain plugins if necessary).

{
  "customAuthPlugin": [
    "./path/to/plugin1.js",
    "./path/to/plugin2.js"
  ]
}

Examples of the plugins can be found in tests/fixtures. Let me know if this would work for your use case?

gyorokpeter commented 2 years ago

Who is "you" in "You'll need to create a settings.json file in the APPDATA"? If that's the end user, that will lead to the problem that we are trying to avoid: at least some users will slip through the cracks and not do it, and then come waste our time with support requests because they can't be bothered to read the docs or search for the solution.

OiNutter commented 2 years ago

For the sake of this, it would be a system admin like yourself creating it. You said you were able to put files into the APPDATA folder on behalf of the end user so my expectation would be you would create a readonly file there that would have the required values. An actual user wouldn't need to create it manually as it would be created for them if it didn't exist. The intention is if it's not readonly, they are able to edit values via the GUI to account for users outside of enterprise ecosystems such as yours.

gyorokpeter commented 2 years ago

Well I guess if there is no better idea then there could be a wrapper script on our side that checks if the config file exists and if not then it creates it from the read-only copy, but I don't like this solution because it still opens up the user to messing up their own config, although they will have to be trying in this case. (Still issues could occur when migrating to a newer version or a new PC where the user needs to migrate their settings.)

The reason I brought up APPDATA was only a tangent to explain that even though the release itself is read-only, it will not run in an environment where it is stripped of any write permissions, so it will still be able to write to places like TEMP and APPDATA as well as any other place the user can normally write to. But I think APPDATA is better used for actual user preferences and state like the window size, server list, fonts etc. However we are not able to "put files into the APPDATA on behalf of the end user". The app itself would have to do it, either via code built into the app itself or a wrapper script. I still think a robust solution would have the auth plugin baked in as part of the build. I'm not aware of how or if other companies use this feature in KDB Studio, whether it's optional there or not, whether they need more than one auth plugin etc. It would be helpful to get some data points on this.

mcleo-d commented 2 years ago

Updates as discussed between @gyorokpeter and @ColinEberhardt during KDB meeting https://github.com/finos/kdb/issues/59. There should be a local config to disable communications to external servers where this can be a banking infrastructure concern.