moving guard against setting both SSH configs to the RPC calls indivi…

…dually
gravitational · Dec 5, 2024 · 0d932f2 · 0d932f2
1 parent b897de1
commit 0d932f2
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 17 deletions.
diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go
@@ -2060,6 +2060,10 @@ func (g *GRPCServer) CreateRole(ctx context.Context, req *authpb.CreateRoleReque
 		return nil, trace.Wrap(err)
 	}
 
+	if req.Role.GetOptions().SSHPortForwarding != nil && req.Role.GetOptions().PortForwarding != nil {
+		return nil, trace.BadParameter("options define both 'port_forwarding' and 'ssh_port_forwarding', only one can be set")
+	}
+
 	if err = services.ValidateRole(req.Role); err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -2087,6 +2091,10 @@ func (g *GRPCServer) UpdateRole(ctx context.Context, req *authpb.UpdateRoleReque
 		return nil, trace.Wrap(err)
 	}
 
+	if req.Role.GetOptions().SSHPortForwarding != nil && req.Role.GetOptions().PortForwarding != nil {
+		return nil, trace.BadParameter("options define both 'port_forwarding' and 'ssh_port_forwarding', only one can be set")
+	}
+
 	if err = services.ValidateRole(req.Role); err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -2114,6 +2122,10 @@ func (g *GRPCServer) UpsertRoleV2(ctx context.Context, req *authpb.UpsertRoleReq
 		return nil, trace.Wrap(err)
 	}
 
+	if req.Role.GetOptions().SSHPortForwarding != nil && req.Role.GetOptions().PortForwarding != nil {
+		return nil, trace.BadParameter("options define both 'port_forwarding' and 'ssh_port_forwarding', only one can be set")
+	}
+
 	if err = services.ValidateRole(req.Role); err != nil {
 		return nil, trace.Wrap(err)
 	}

diff --git a/lib/services/role.go b/lib/services/role.go
@@ -255,10 +255,6 @@ func ValidateRole(r types.Role, opts ...validateRoleOption) error {
 		opt(&options)
 	}
 
-	if r.GetOptions().SSHPortForwarding != nil && r.GetOptions().PortForwarding != nil {
-		return trace.BadParameter("options define both 'port_forwarding' and 'ssh_port_forwarding', only one can be set")
-	}
-
 	if err := CheckAndSetDefaults(r); err != nil {
 		return trace.Wrap(err)
 	}

diff --git a/lib/services/role_test.go b/lib/services/role_test.go
@@ -832,19 +832,6 @@ func TestValidateRole(t *testing.T) {
 				},
 			},
 		},
-		{
-			name: "invalid port forwarding config",
-			spec: types.RoleSpecV6{
-				Options: types.RoleOptions{
-					PortForwarding:    types.NewBoolOption(true),
-					SSHPortForwarding: &types.SSHPortForwarding{},
-				},
-				Allow: types.RoleConditions{
-					Logins: []string{`{{external["http://schemas.microsoft.com/ws/2008/06/identity/claims/windowsaccountname"]}}`},
-				},
-			},
-			expectError: trace.BadParameter("options define both 'port_forwarding' and 'ssh_port_forwarding', only one can be set"),
-		},
 		{
 			name: "invalid role condition login syntax",
 			spec: types.RoleSpecV6{