diff --git a/management/internals/modules/reverseproxy/reverseproxy.go b/management/internals/modules/reverseproxy/reverseproxy.go index a832d799e..3adb0b982 100644 --- a/management/internals/modules/reverseproxy/reverseproxy.go +++ b/management/internals/modules/reverseproxy/reverseproxy.go @@ -213,7 +213,7 @@ func (r *ReverseProxy) ToProtoMapping(operation Operation, authToken string, oid Host: target.Host, Path: path, } - if target.Port > 0 { + if target.Port > 0 && !isDefaultPort(target.Protocol, target.Port) { targetURL.Host = net.JoinHostPort(targetURL.Host, strconv.Itoa(target.Port)) } @@ -267,6 +267,12 @@ func operationToProtoType(op Operation) proto.ProxyMappingUpdateType { } } +// isDefaultPort reports whether port is the standard default for the given scheme +// (443 for https, 80 for http). +func isDefaultPort(scheme string, port int) bool { + return (scheme == "https" && port == 443) || (scheme == "http" && port == 80) +} + func (r *ReverseProxy) FromAPIRequest(req *api.ReverseProxyRequest, accountID string) { r.Name = req.Name r.Domain = req.Domain diff --git a/management/internals/modules/reverseproxy/reverseproxy_test.go b/management/internals/modules/reverseproxy/reverseproxy_test.go index bb00f8e84..9fe1eeb43 100644 --- a/management/internals/modules/reverseproxy/reverseproxy_test.go +++ b/management/internals/modules/reverseproxy/reverseproxy_test.go @@ -1,10 +1,13 @@ package reverseproxy import ( + "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/shared/management/proto" ) func validProxy() *ReverseProxy { @@ -79,3 +82,142 @@ func TestValidate_MultipleTargetsOneInvalid(t *testing.T) { assert.Contains(t, err.Error(), "target 1") assert.Contains(t, err.Error(), "empty target_id") } + +func TestIsDefaultPort(t *testing.T) { + tests := []struct { + scheme string + port int + want bool + }{ + {"http", 80, true}, + {"https", 443, true}, + {"http", 443, false}, + {"https", 80, false}, + {"http", 8080, false}, + {"https", 8443, false}, + {"http", 0, false}, + {"https", 0, false}, + } + for _, tt := range tests { + t.Run(fmt.Sprintf("%s/%d", tt.scheme, tt.port), func(t *testing.T) { + assert.Equal(t, tt.want, isDefaultPort(tt.scheme, tt.port)) + }) + } +} + +func TestToProtoMapping_PortInTargetURL(t *testing.T) { + oidcConfig := OIDCValidationConfig{} + + tests := []struct { + name string + protocol string + host string + port int + wantTarget string + }{ + { + name: "http with default port 80 omits port", + protocol: "http", + host: "10.0.0.1", + port: 80, + wantTarget: "http://10.0.0.1/", + }, + { + name: "https with default port 443 omits port", + protocol: "https", + host: "10.0.0.1", + port: 443, + wantTarget: "https://10.0.0.1/", + }, + { + name: "port 0 omits port", + protocol: "http", + host: "10.0.0.1", + port: 0, + wantTarget: "http://10.0.0.1/", + }, + { + name: "non-default port is included", + protocol: "http", + host: "10.0.0.1", + port: 8080, + wantTarget: "http://10.0.0.1:8080/", + }, + { + name: "https with non-default port is included", + protocol: "https", + host: "10.0.0.1", + port: 8443, + wantTarget: "https://10.0.0.1:8443/", + }, + { + name: "http port 443 is included", + protocol: "http", + host: "10.0.0.1", + port: 443, + wantTarget: "http://10.0.0.1:443/", + }, + { + name: "https port 80 is included", + protocol: "https", + host: "10.0.0.1", + port: 80, + wantTarget: "https://10.0.0.1:80/", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rp := &ReverseProxy{ + ID: "test-id", + AccountID: "acc-1", + Domain: "example.com", + Targets: []Target{ + { + TargetId: "peer-1", + TargetType: TargetTypePeer, + Host: tt.host, + Port: tt.port, + Protocol: tt.protocol, + Enabled: true, + }, + }, + } + pm := rp.ToProtoMapping(Create, "token", oidcConfig) + require.Len(t, pm.Path, 1, "should have one path mapping") + assert.Equal(t, tt.wantTarget, pm.Path[0].Target) + }) + } +} + +func TestToProtoMapping_DisabledTargetSkipped(t *testing.T) { + rp := &ReverseProxy{ + ID: "test-id", + AccountID: "acc-1", + Domain: "example.com", + Targets: []Target{ + {TargetId: "peer-1", TargetType: TargetTypePeer, Host: "10.0.0.1", Port: 8080, Protocol: "http", Enabled: false}, + {TargetId: "peer-2", TargetType: TargetTypePeer, Host: "10.0.0.2", Port: 9090, Protocol: "http", Enabled: true}, + }, + } + pm := rp.ToProtoMapping(Create, "token", OIDCValidationConfig{}) + require.Len(t, pm.Path, 1) + assert.Equal(t, "http://10.0.0.2:9090/", pm.Path[0].Target) +} + +func TestToProtoMapping_OperationTypes(t *testing.T) { + rp := validProxy() + tests := []struct { + op Operation + want proto.ProxyMappingUpdateType + }{ + {Create, proto.ProxyMappingUpdateType_UPDATE_TYPE_CREATED}, + {Update, proto.ProxyMappingUpdateType_UPDATE_TYPE_MODIFIED}, + {Delete, proto.ProxyMappingUpdateType_UPDATE_TYPE_REMOVED}, + } + for _, tt := range tests { + t.Run(string(tt.op), func(t *testing.T) { + pm := rp.ToProtoMapping(tt.op, "", OIDCValidationConfig{}) + assert.Equal(t, tt.want, pm.Type) + }) + } +} diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index bcd67da1b..f74950760 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -2961,7 +2961,7 @@ components: description: Backend ip or domain for this target port: type: integer - description: Backend port for this target + description: Backend port for this target. Use 0 or omit to use the scheme default (80 for http, 443 for https). enabled: type: boolean description: Whether this target is enabled