search

bolkedebruin / rdpgw

Remote Desktop Gateway in Go for deploying on Linux/BSD/Kubernetes
Apache License 2.0
698 stars 115 forks source link

Append extra settings in RDP file #58

Closed KoltesDigital closed 1 year ago

KoltesDigital commented 1 year ago

This patch allows assigning extra RDP settings in rdpgw.yaml. They are simply appended in the generated file, or they overwrite values which are already set by the usual behavior. In particular, one can remove a setting key if the value is the default one.

Optionally, users may also be able to add extra settings in query parameters, e.g. /connect?host=X.X.X.X:3389&use multimon=1, which ultimately take precedence.

For backward-compatibility reason, the patch does not remove ClientConfig's NetworkAutoDetect, BandwidthAutoDetect and ConnectionType, as well as hardcoded SmartSizing and BitmapCacheSize. However it makes them obsolete.

bolkedebruin commented 1 year ago

Thanks. I'm taking a look. My intention is to allow loading a default.rdp file that can be customized by query vales and then by required settings (e.g. openid needs a PAA token). So this is a good start :-)

KoltesDigital commented 1 year ago

I'll use my branch in the meantime, but I'd be happy to help you improve the project overall :)

If I may add my two cents:

Loading a default .rdp file would mean parsing it, whereas for now only YAML is deserialized thanks to an external lib, and RDP is generated. Adding a parser would require more code, more tests. Moreover config will then be split among two files with two different formats. As I've now deployed the patched Docker image in "production" (some users from a very small team will begin to use it tomorrow I guess), I'm satisfied with the config: for instance in rdpgw.yaml under Client.ExtraSettings I now have authentication level: 2, and it would have been authentication level:i:2 in the default .rdp file, it's almost the same.

For this PR at first I was thinking about a simple map, but when I started I learnt about the tag system in Go. Its use in rdp.go is clever! However I found cumbersome to split all the fields into multiple structs. For each public function, a private one must be called five times, thus indexing each struct. It seems to me that still, a .rdp file is a flat list of key value pairs. I suggest you consider merging them all together into one struct, this should greatly simplify the handling of RDP format, and this may sharpen your eye about providing default key-values.

KoltesDigital commented 1 year ago

Hey. Have you made progress on that?

bolkedebruin commented 1 year ago

This is now fixed in the latest version