Add type-safe URL override mechanism for RDP options (#182)

Introduce a generic, allow-list-gated way for the /connect endpoint to
accept RDP setting overrides via URL query parameters. Operators opt in
via client.rdpoverridablekeys; absent that allow-list, URL-driven
overrides are rejected with 400.

Override values are routed through rdp.Builder.ApplyOverrides, which
matches query keys against the rdp struct tags of RdpSettings and
validates per Go field type. Overridden fields are tracked so explicit
values always serialize even when they equal the field default. The
override pass runs before authoritative server-controlled fields
(gateway address, access token, full address, username) so those
always win.

This replaces the per-option string-splice approach considered in #181:
multimon now works via ?usemultimon=1 against an operator allow-list
containing "use multimon", and any other RDP key follows the same path
without bespoke handler code.

README.md

@@ -287,6 +287,45 @@ and RDP file will download to your desktop. This file can be opened by one
 of the remote desktop clients and it will try to connect to the gateway and
 desktop host behind it.
 
+### Overriding RDP options from the URL
+The `/connect` endpoint can apply caller-supplied RDP setting overrides from
+URL query parameters when (and only when) the operator explicitly opts in.
+This lets users vary settings such as multi-monitor, audio mode or
+clipboard redirection without maintaining separate RDP templates.
+
+The query parameter name is the underlying RDP key with whitespace removed
+and lowercased. For example:
+
+| RDP key (in the file) | Query parameter |
+|-----------------------|-----------------|
+| `use multimon`        | `usemultimon`   |
+| `audiomode`           | `audiomode`     |
+| `redirectclipboard`   | `redirectclipboard` |
+| `screen mode id`      | `screenmodeid`  |
+
+Values are validated against the field type: booleans accept `0`/`1` or
+`true`/`false`, integers must parse as base-10 numbers, strings are taken
+verbatim. Unknown query parameters are ignored so the existing `host`,
+etc. continue to work.
+
+To enable, list the keys you want to expose under `client.rdpoverridablekeys`
+in your YAML configuration (or via the `RDPGW_CLIENT__RDPOVERRIDABLEKEYS`
+environment variable as a space-separated list):
+
+```yaml
+client:
+  rdpoverridablekeys:
+    - use multimon
+    - audiomode
+```
+
+With the snippet above, `https://your-gateway/connect?usemultimon=1` adds
+`use multimon:i:1` to the generated RDP file. Keys that are not on the
+allow-list are rejected with `400 Bad Request`. Authoritative settings
+controlled by the gateway (gateway address, access token, full address,
+username) always win over URL parameters even if an operator adds those
+keys to the allow-list — but adding sensitive keys is still discouraged.
+
 ## Integration
 The gateway exposes an endpoint for the verification of user tokens at
 https://yourserver/tokeninfo . The query parameter is 'access_token' so

cmd/rdpgw/config/configuration.go

@@ -108,6 +108,11 @@ type ClientConfig struct {
 	NoUsername       bool   `koanf:"nousername"`
 	SigningCert      string `koanf:"signingcert"`
 	SigningKey       string `koanf:"signingkey"`
+	// RdpOverridableKeys is the operator allow-list of RDP setting keys that
+	// the /connect endpoint may override from URL query parameters. Empty
+	// disables URL-based overrides. Entries are normalized (lowercase, no
+	// whitespace), so "use multimon" and "usemultimon" are equivalent.
+	RdpOverridableKeys []string `koanf:"rdpoverridablekeys"`
 }
 
 func ToCamel(s string) string {

cmd/rdpgw/main.go

@@ -109,9 +109,10 @@ func main() {
 		Hosts:            conf.Server.Hosts,
 		HostSelection:    conf.Server.HostSelection,
 		RdpOpts: web.RdpOpts{
-			UsernameTemplate: conf.Client.UsernameTemplate,
-			SplitUserDomain:  conf.Client.SplitUserDomain,
-			NoUsername:       conf.Client.NoUsername,
+			UsernameTemplate:   conf.Client.UsernameTemplate,
+			SplitUserDomain:    conf.Client.SplitUserDomain,
+			NoUsername:         conf.Client.NoUsername,
+			OverridableRdpKeys: conf.Client.RdpOverridableKeys,
 		},
 		GatewayAddress: url,
 		TemplateFile:   conf.Client.Defaults,

cmd/rdpgw/rdp/rdp.go

@@ -95,6 +95,7 @@ type RdpSettings struct {
 type Builder struct {
 	Settings RdpSettings
 	Metadata mapstructure.Metadata
+	forced   map[string]struct{}
 }
 
 func NewBuilder() *Builder {
@@ -137,15 +138,92 @@ func NewBuilderFromFile(filename string) (*Builder, error) {
 func (rb *Builder) String() string {
 	var sb strings.Builder
 
-	addStructToString(rb.Settings, rb.Metadata, &sb)
+	rb.addStructToString(rb.Settings, &sb)
 
 	return sb.String()
 }
 
-func addStructToString(st interface{}, metadata mapstructure.Metadata, sb *strings.Builder) {
+// NormalizeRdpKey returns the canonical, URL-friendly form of an rdp tag
+// (lowercase, no whitespace). Used to look up fields by query parameter name
+// and to compare entries in the operator-supplied allow-list. For example,
+// "use multimon", "Use Multimon" and "usemultimon" all normalize to
+// "usemultimon".
+func NormalizeRdpKey(s string) string {
+	s = strings.ToLower(strings.TrimSpace(s))
+	return strings.ReplaceAll(s, " ", "")
+}
+
+// ApplyOverrides applies a set of caller-supplied RDP option overrides onto
+// the Builder's settings. Each input key is normalized via NormalizeRdpKey and
+// matched against the rdp struct tags of RdpSettings. Keys that do not match
+// any field are ignored (so passing the entire request query is safe). Keys
+// that match a field but are not present in allowed return an error: the
+// allow-list is opt-in by design and empty by default.
+//
+// Values are validated against the field's Go type: bools accept 0/1 or
+// true/false; ints must parse as base-10 integers; strings are taken
+// verbatim. Overridden fields are tracked so they always serialize, even when
+// the new value equals the field's default.
+func (rb *Builder) ApplyOverrides(values map[string][]string, allowed []string) error {
+	if len(values) == 0 {
+		return nil
+	}
+
+	allow := make(map[string]struct{}, len(allowed))
+	for _, k := range allowed {
+		allow[NormalizeRdpKey(k)] = struct{}{}
+	}
+
+	s := structs.New(&rb.Settings)
+	byKey := make(map[string]*structs.Field, len(s.Fields()))
+	for _, f := range s.Fields() {
+		tag := f.Tag("rdp")
+		if tag == "" {
+			continue
+		}
+		byKey[NormalizeRdpKey(tag)] = f
+	}
+
+	if rb.forced == nil {
+		rb.forced = make(map[string]struct{})
+	}
+
+	for k, vs := range values {
+		nk := NormalizeRdpKey(k)
+		f, ok := byKey[nk]
+		if !ok {
+			continue
+		}
+		if _, ok := allow[nk]; !ok {
+			return fmt.Errorf("rdp option %q is not allowed to be overridden", k)
+		}
+		if len(vs) == 0 {
+			continue
+		}
+		v := strings.TrimSpace(vs[0])
+		if v == "" {
+			return fmt.Errorf("rdp option %q has empty value", k)
+		}
+		if err := setRdpFieldFromString(f, v); err != nil {
+			return fmt.Errorf("invalid value for rdp option %q: %w", k, err)
+		}
+		rb.forced[f.Name()] = struct{}{}
+	}
+	return nil
+}
+
+func (rb *Builder) isForced(f *structs.Field) bool {
+	if rb.forced == nil {
+		return false
+	}
+	_, ok := rb.forced[f.Name()]
+	return ok
+}
+
+func (rb *Builder) addStructToString(st interface{}, sb *strings.Builder) {
 	s := structs.New(st)
 	for _, f := range s.Fields() {
-		if isZero(f) && !isSet(f, metadata) {
+		if isZero(f) && !isSet(f, rb.Metadata) && !rb.isForced(f) {
 			continue
 		}
 		sb.WriteString(f.Tag("rdp"))
@@ -251,3 +329,27 @@ func setVariable(f *structs.Field, v string) error {
 		return errors.New("invalid field type")
 	}
 }
+
+func setRdpFieldFromString(f *structs.Field, v string) error {
+	switch f.Kind() {
+	case reflect.String:
+		return f.Set(v)
+	case reflect.Int:
+		i, err := strconv.Atoi(v)
+		if err != nil {
+			return fmt.Errorf("expected integer, got %q", v)
+		}
+		return f.Set(i)
+	case reflect.Bool:
+		switch strings.ToLower(v) {
+		case "0", "false":
+			return f.Set(false)
+		case "1", "true":
+			return f.Set(true)
+		default:
+			return fmt.Errorf("expected 0/1 or true/false, got %q", v)
+		}
+	default:
+		return fmt.Errorf("unsupported rdp field kind %s", f.Kind())
+	}
+}

cmd/rdpgw/rdp/rdp_test.go

@@ -44,3 +44,164 @@ func TestLoadFile(t *testing.T) {
 		t.Fatalf("LoadFile failed: %v", err)
 	}
 }
+
+func TestNormalizeRdpKey(t *testing.T) {
+	cases := map[string]string{
+		"use multimon": "usemultimon",
+		"USE MULTIMON": "usemultimon",
+		"  Use Multimon  ": "usemultimon",
+		"audiomode":     "audiomode",
+		"screen mode id": "screenmodeid",
+	}
+	for in, want := range cases {
+		if got := NormalizeRdpKey(in); got != want {
+			t.Errorf("NormalizeRdpKey(%q) = %q, want %q", in, got, want)
+		}
+	}
+}
+
+func TestApplyOverrides_AllowedBoolApplies(t *testing.T) {
+	b := NewBuilder()
+	err := b.ApplyOverrides(
+		map[string][]string{"usemultimon": {"1"}},
+		[]string{"use multimon"},
+	)
+	if err != nil {
+		t.Fatalf("ApplyOverrides returned error: %v", err)
+	}
+	if !b.Settings.UseMultimon {
+		t.Errorf("UseMultimon = false, want true")
+	}
+	if !strings.Contains(b.String(), "use multimon:i:1"+CRLF) {
+		t.Errorf("rendered file does not contain use multimon:i:1, got:\n%s", b.String())
+	}
+}
+
+func TestApplyOverrides_AllowListNormalizes(t *testing.T) {
+	// allow-list given with the human/rdp form; query uses URL-friendly form.
+	b := NewBuilder()
+	err := b.ApplyOverrides(
+		map[string][]string{"USEMULTIMON": {"1"}},
+		[]string{"Use Multimon"},
+	)
+	if err != nil {
+		t.Fatalf("ApplyOverrides returned error: %v", err)
+	}
+	if !strings.Contains(b.String(), "use multimon:i:1"+CRLF) {
+		t.Errorf("expected normalized match to apply override, got:\n%s", b.String())
+	}
+}
+
+func TestApplyOverrides_DefaultValueStillSerializes(t *testing.T) {
+	// UseMultimon defaults to false; the bare struct would skip serialization.
+	// An explicit override of "0" must still emit `use multimon:i:0` so that
+	// the operator's intent is signaled to the client.
+	b := NewBuilder()
+	err := b.ApplyOverrides(
+		map[string][]string{"usemultimon": {"0"}},
+		[]string{"use multimon"},
+	)
+	if err != nil {
+		t.Fatalf("ApplyOverrides returned error: %v", err)
+	}
+	if !strings.Contains(b.String(), "use multimon:i:0"+CRLF) {
+		t.Errorf("expected use multimon:i:0 to render despite matching default, got:\n%s", b.String())
+	}
+}
+
+func TestApplyOverrides_RejectedWhenNotInAllowList(t *testing.T) {
+	b := NewBuilder()
+	err := b.ApplyOverrides(
+		map[string][]string{"usemultimon": {"1"}},
+		nil, // empty allow-list
+	)
+	if err == nil {
+		t.Fatalf("ApplyOverrides accepted a key with empty allow-list")
+	}
+}
+
+func TestApplyOverrides_UnknownKeysIgnored(t *testing.T) {
+	// Query strings carry unrelated params (host=, etc.); these must not
+	// trip an error.
+	b := NewBuilder()
+	err := b.ApplyOverrides(
+		map[string][]string{"host": {"example.com"}, "totally-bogus": {"x"}},
+		[]string{"use multimon"},
+	)
+	if err != nil {
+		t.Errorf("ApplyOverrides should ignore unknown keys, got error: %v", err)
+	}
+}
+
+func TestApplyOverrides_BoolValidation(t *testing.T) {
+	cases := []struct {
+		v       string
+		wantErr bool
+	}{
+		{"0", false}, {"1", false},
+		{"true", false}, {"false", false},
+		{"TRUE", false},
+		{"yes", true}, {"2", true}, {"", true}, {"abc", true},
+	}
+	for _, c := range cases {
+		b := NewBuilder()
+		err := b.ApplyOverrides(
+			map[string][]string{"usemultimon": {c.v}},
+			[]string{"use multimon"},
+		)
+		if (err != nil) != c.wantErr {
+			t.Errorf("value %q: got err=%v, wantErr=%v", c.v, err, c.wantErr)
+		}
+	}
+}
+
+func TestApplyOverrides_IntValidation(t *testing.T) {
+	b := NewBuilder()
+	if err := b.ApplyOverrides(
+		map[string][]string{"audiomode": {"2"}},
+		[]string{"audiomode"},
+	); err != nil {
+		t.Fatalf("ApplyOverrides returned error: %v", err)
+	}
+	if b.Settings.AudioMode != 2 {
+		t.Errorf("AudioMode = %d, want 2", b.Settings.AudioMode)
+	}
+	b2 := NewBuilder()
+	if err := b2.ApplyOverrides(
+		map[string][]string{"audiomode": {"hello"}},
+		[]string{"audiomode"},
+	); err == nil {
+		t.Errorf("expected int parse error for value 'hello'")
+	}
+}
+
+func TestApplyOverrides_StringField(t *testing.T) {
+	b := NewBuilder()
+	if err := b.ApplyOverrides(
+		map[string][]string{"alternateshell": {"explorer.exe"}},
+		[]string{"alternate shell"},
+	); err != nil {
+		t.Fatalf("ApplyOverrides returned error: %v", err)
+	}
+	if !strings.Contains(b.String(), "alternate shell:s:explorer.exe"+CRLF) {
+		t.Errorf("expected alternate shell to be set, got:\n%s", b.String())
+	}
+}
+
+func TestApplyOverrides_EmptyValueRejected(t *testing.T) {
+	b := NewBuilder()
+	err := b.ApplyOverrides(
+		map[string][]string{"usemultimon": {""}},
+		[]string{"use multimon"},
+	)
+	if err == nil {
+		t.Errorf("expected error for empty value")
+	}
+}
+
+func TestApplyOverrides_NilValuesNoOp(t *testing.T) {
+	b := NewBuilder()
+	if err := b.ApplyOverrides(nil, []string{"use multimon"}); err != nil {
+		t.Errorf("ApplyOverrides(nil) should be a no-op, got: %v", err)
+	}
+}

cmd/rdpgw/web/web.go

@@ -72,6 +72,13 @@ type RdpOpts struct {
 	UsernameTemplate string
 	SplitUserDomain  bool
 	NoUsername       bool
+	// OverridableRdpKeys is the operator-supplied allow-list of RDP setting
+	// keys that the /connect endpoint may override from URL query parameters.
+	// Empty (the default) disables URL-based RDP overrides entirely. Each
+	// entry is matched against the rdp struct tag of an RdpSettings field
+	// after normalization (lowercase, no whitespace), so "use multimon",
+	// "Use Multimon" and "usemultimon" are equivalent.
+	OverridableRdpKeys []string
 }
 
 type Handler struct {
@@ -484,6 +491,16 @@ func (h *Handler) HandleDownload(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// Apply URL-driven RDP option overrides (e.g. ?usemultimon=1) before
+	// the server-controlled fields below, so authoritative gateway/auth
+	// settings always win regardless of what an operator put on the
+	// allow-list.
+	if err := d.ApplyOverrides(r.URL.Query(), h.rdpOpts.OverridableRdpKeys); err != nil {
+		log.Printf("rejected rdp override for user %s: %s", id.UserName(), err)
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
 	if !h.rdpOpts.NoUsername {
 		d.Settings.Username = render
 		if domain != "" {

cmd/rdpgw/web/web_test.go

@@ -182,6 +182,200 @@ func TestHandler_HandleDownload(t *testing.T) {
 
 }
 
+func TestHandler_HandleDownload_RdpOverrides(t *testing.T) {
+	tests := []struct {
+		name        string
+		query       string
+		allow       []string
+		template    string
+		wantStatus  int
+		wantContain []string // substrings expected in body when status==200
+		wantMissing []string
+	}{
+		{
+			name:        "allowed bool override applies",
+			query:       "?usemultimon=1",
+			allow:       []string{"use multimon"},
+			wantStatus:  http.StatusOK,
+			wantContain: []string{"use multimon:i:1\r\n"},
+		},
+		{
+			name:        "allowed bool override emits default value explicitly",
+			query:       "?usemultimon=0",
+			allow:       []string{"use multimon"},
+			wantStatus:  http.StatusOK,
+			wantContain: []string{"use multimon:i:0\r\n"},
+		},
+		{
+			name:        "override beats template",
+			query:       "?usemultimon=0",
+			allow:       []string{"use multimon"},
+			template:    "use multimon:i:1\r\n",
+			wantStatus:  http.StatusOK,
+			wantContain: []string{"use multimon:i:0\r\n"},
+			wantMissing: []string{"use multimon:i:1\r\n"},
+		},
+		{
+			name:       "no allow list disables overrides",
+			query:      "?usemultimon=1",
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:       "key not in allow list is rejected",
+			query:      "?audiomode=2",
+			allow:      []string{"use multimon"},
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:       "invalid bool value rejected",
+			query:      "?usemultimon=hello",
+			allow:      []string{"use multimon"},
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:       "invalid int value rejected",
+			query:      "?audiomode=loud",
+			allow:      []string{"audiomode"},
+			wantStatus: http.StatusBadRequest,
+		},
+		{
+			name:        "allow list normalizes (caller uses url-friendly form)",
+			query:       "?usemultimon=1",
+			allow:       []string{"USE MULTIMON"},
+			wantStatus:  http.StatusOK,
+			wantContain: []string{"use multimon:i:1\r\n"},
+		},
+		{
+			name:        "unrelated query params are ignored",
+			query:       "?host=10.0.0.1:3389&usemultimon=1",
+			allow:       []string{"use multimon"},
+			wantStatus:  http.StatusOK,
+			wantContain: []string{"use multimon:i:1\r\n"},
+		},
+		{
+			name:        "string field override applies",
+			query:       "?alternateshell=explorer.exe",
+			allow:       []string{"alternate shell"},
+			wantStatus:  http.StatusOK,
+			wantContain: []string{"alternate shell:s:explorer.exe\r\n"},
+		},
+		{
+			name:        "url override cannot escape authoritative server fields",
+			query:       "?username=evil@attacker",
+			allow:       []string{"username"}, // operator footgun: still must not leak past server set
+			wantStatus:  http.StatusOK,
+			wantContain: []string{"username:s:" + testuser + "\r\n"},
+			wantMissing: []string{"username:s:evil@attacker\r\n"},
+		},
+	}
+
+	u, _ := url.Parse(gateway)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req, err := http.NewRequest("GET", "/connect"+tt.query, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			rr := httptest.NewRecorder()
+			id := identity.NewUser()
+			id.SetUserName(testuser)
+			id.SetAuthenticated(true)
+			req = identity.AddToRequestCtx(id, req)
+
+			var templateFile string
+			if tt.template != "" {
+				f, err := os.CreateTemp("", "rdp")
+				if err != nil {
+					t.Fatal(err)
+				}
+				defer os.Remove(f.Name())
+				if _, err := f.WriteString(tt.template); err != nil {
+					t.Fatal(err)
+				}
+				if err := f.Close(); err != nil {
+					t.Fatal(err)
+				}
+				templateFile = f.Name()
+			}
+
+			c := Config{
+				HostSelection:     "roundrobin",
+				Hosts:             hosts,
+				PAATokenGenerator: paaTokenMock,
+				GatewayAddress:    u,
+				RdpOpts:           RdpOpts{OverridableRdpKeys: tt.allow},
+				TemplateFile:      templateFile,
+			}
+			h := c.NewHandler()
+			http.HandlerFunc(h.HandleDownload).ServeHTTP(rr, req)
+
+			if rr.Code != tt.wantStatus {
+				t.Fatalf("status: got %d, want %d (body=%q)", rr.Code, tt.wantStatus, rr.Body.String())
+			}
+			if rr.Code != http.StatusOK {
+				return
+			}
+			body := rr.Body.String()
+			for _, s := range tt.wantContain {
+				if !strings.Contains(body, s) {
+					t.Errorf("body missing %q\nbody=\n%s", s, body)
+				}
+			}
+			for _, s := range tt.wantMissing {
+				if strings.Contains(body, s) {
+					t.Errorf("body contains forbidden %q\nbody=\n%s", s, body)
+				}
+			}
+		})
+	}
+}
+
+func TestHandler_HandleSignedDownload_RdpOverrideApplies(t *testing.T) {
+	// The override must take effect on the signed path too: ApplyOverrides
+	// runs on the same Builder used to render the signed content.
+	req, err := http.NewRequest("GET", "/connect?usemultimon=1", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rr := httptest.NewRecorder()
+	id := identity.NewUser()
+	id.SetUserName(testuser)
+	id.SetAuthenticated(true)
+	req = identity.AddToRequestCtx(id, req)
+
+	u, _ := url.Parse(gateway)
+	c := Config{
+		HostSelection:     "roundrobin",
+		Hosts:             hosts,
+		PAATokenGenerator: paaTokenMock,
+		GatewayAddress:    u,
+		RdpOpts:           RdpOpts{OverridableRdpKeys: []string{"use multimon"}},
+	}
+	h := c.NewHandler()
+
+	fs := afero.NewMemMapFs()
+	if err := genKeypair(fs); err != nil {
+		t.Fatalf("could not generate key pair: %s", err)
+	}
+	signer, err := rdpsign.New("test.crt", "test.key", rdpsign.WithFs(fs))
+	if err != nil {
+		t.Fatalf("could not create signer: %s", err)
+	}
+	h.rdpSigner = signer
+
+	http.HandlerFunc(h.HandleDownload).ServeHTTP(rr, req)
+	if rr.Code != http.StatusOK {
+		t.Fatalf("status: got %d, want 200 (body=%q)", rr.Code, rr.Body.String())
+	}
+	body := rr.Body.String()
+	if !strings.Contains(body, "use multimon:i:1\r\n") {
+		t.Errorf("signed body missing use multimon:i:1\nbody=\n%s", body)
+	}
+	if !strings.Contains(body, "signature:s:") {
+		t.Errorf("signed body missing signature\nbody=\n%s", body)
+	}
+}
+
 func TestHandler_HandleSignedDownload(t *testing.T) {
 	req, err := http.NewRequest("GET", "/connect", nil)
 	if err != nil {