Skip to content

fix(connect): clear wanport upon disabling remote access #1624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 25, 2025
Merged

fix(connect): clear wanport upon disabling remote access #1624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6acba27
fix(connect): clear `wanport` upon disabling remote access
pujitm Aug 25, 2025
06be9be
don't add wanport to LAN FQDN
pujitm Aug 25, 2025
0d4cf9f
don't getOrThrow connect.config.wanport
pujitm Aug 25, 2025
af061fa
Revert "don't add wanport to LAN FQDN"
pujitm Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 107 additions & 6 deletions packages/unraid-api-plugin-connect/src/__test__/url-resolver.service.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ describe('UrlResolverService', () => {
describe('getServerIps', () => {
it('should return empty arrays when store is not loaded', () => {
(mockConfigService.get as Mock).mockReturnValue(null);
(mockConfigService.getOrThrow as Mock).mockReturnValue(null);

const result = service.getServerIps();

Expand Down Expand Up @@ -144,7 +143,7 @@ describe('UrlResolverService', () => {
nginx: {
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: '2001:db8::1',
lanIp6: 'ipv6.unraid.local',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: testCase.sslEnabled,
Expand Down Expand Up @@ -205,8 +204,9 @@ describe('UrlResolverService', () => {
},
};

(mockConfigService.get as Mock).mockReturnValue(mockStore);
(mockConfigService.getOrThrow as Mock).mockReturnValue(443);
(mockConfigService.get as Mock)
.mockReturnValueOnce(mockStore)
.mockReturnValueOnce(443);

const result = service.getServerIps();

Expand Down Expand Up @@ -259,6 +259,106 @@ describe('UrlResolverService', () => {
expect(wanFqdnUrl).toBeDefined();
expect(wanFqdnUrl?.ipv4?.toString()).toBe('https://wan.unraid.net/');
});
it('should handle invalid WAN port values gracefully', () => {
const testCases = [
{ port: null, description: 'null port' },
{ port: undefined, description: 'undefined port' },
{ port: '', description: 'empty string port' },
{ port: 'invalid', description: 'non-numeric port' },
{ port: 0, description: 'zero port' },
{ port: -1, description: 'negative port' },
{ port: 65536, description: 'port above valid range' },
{ port: 1.5, description: 'non-integer port' },
];

testCases.forEach(({ port, description }) => {
const mockStore = {
emhttp: {
nginx: {
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: 'ipv6.unraid.local',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: true,
sslMode: 'yes',
httpPort: 80,
httpsPort: 443,
fqdnUrls: [
{
interface: 'WAN',
id: null,
fqdn: 'wan.unraid.net',
isIpv6: false,
},
],
},
},
};

(mockConfigService.get as Mock)
.mockReturnValueOnce(mockStore)
.mockReturnValueOnce(port);

const result = service.getServerIps();

// Should fallback to nginx.httpsPort (443) for WAN FQDN URLs
const wanFqdnUrl = result.urls.find(
(url) => url.type === URL_TYPE.WAN && url.name === 'FQDN WAN'
);
expect(wanFqdnUrl).toBeDefined();
expect(wanFqdnUrl?.ipv4?.toString()).toBe('https://wan.unraid.net/');
expect(result.errors).toHaveLength(0);
});
});

it('should use valid WAN port when provided', () => {
const testCases = [
{ port: 1, expected: 'https://wan.unraid.net:1/' },
{ port: 8080, expected: 'https://wan.unraid.net:8080/' },
{ port: 65535, expected: 'https://wan.unraid.net:65535/' },
{ port: '3000', expected: 'https://wan.unraid.net:3000/' }, // string that parses to valid number
];

testCases.forEach(({ port, expected }) => {
const mockStore = {
emhttp: {
nginx: {
defaultUrl: 'https://default.unraid.net',
lanIp: '192.168.1.1',
lanIp6: 'ipv6.unraid.local',
lanName: 'unraid.local',
lanMdns: 'unraid.local',
sslEnabled: true,
sslMode: 'yes',
httpPort: 80,
httpsPort: 443,
fqdnUrls: [
{
interface: 'WAN',
id: null,
fqdn: 'wan.unraid.net',
isIpv6: false,
},
],
},
},
};

(mockConfigService.get as Mock)
.mockReturnValueOnce(mockStore)
.mockReturnValueOnce(port);

const result = service.getServerIps();

const wanFqdnUrl = result.urls.find(
(url) => url.type === URL_TYPE.WAN && url.name === 'FQDN WAN'
);
expect(wanFqdnUrl).toBeDefined();
expect(wanFqdnUrl?.ipv4?.toString()).toBe(expected);
expect(result.errors).toHaveLength(0);
});
});
});

describe('getRemoteAccessUrl', () => {
Expand Down Expand Up @@ -287,8 +387,9 @@ describe('UrlResolverService', () => {
},
};

(mockConfigService.get as Mock).mockReturnValue(mockStore);
(mockConfigService.getOrThrow as Mock).mockReturnValue(443);
(mockConfigService.get as Mock)
.mockReturnValueOnce(mockStore)
.mockReturnValueOnce(443);

const result = service.getRemoteAccessUrl();

Expand Down
22 changes: 21 additions & 1 deletion packages/unraid-api-plugin-connect/src/network/url-resolver.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,25 @@ export class UrlResolverService {
}, []);
}

/**
* Validates and sanitizes a WAN port value.
*
* @param rawPort - The raw port value from configuration
* @returns A valid port number between 1-65535, or undefined if invalid
*/
private validateWanPort(rawPort: unknown): number | undefined {
if (rawPort == null || rawPort === '') {
return undefined;
}

const port = Number(rawPort);
if (!Number.isInteger(port) || port < 1 || port > 65535) {
return undefined;
}

return port;
}

/**
* Resolves all available server access URLs from the nginx configuration.
* This is the main method of the service that aggregates all possible access URLs.
Expand All @@ -260,7 +279,8 @@ export class UrlResolverService {
}

const { nginx } = store.emhttp;
const wanport = this.configService.getOrThrow('connect.config.wanport', { infer: true });
const rawWanPort = this.configService.get('connect.config.wanport', { infer: true });
const wanport = this.validateWanPort(rawWanPort);

if (!nginx || Object.keys(nginx).length === 0) {
return { urls: [], errors: [new Error('Nginx Not Loaded')] };
Expand Down
24 changes: 17 additions & 7 deletions packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,17 +216,28 @@ export class ConnectSettingsService {
input.forwardType
);

this.configService.set('connect.config.wanaccess', input.accessType === WAN_ACCESS_TYPE.ALWAYS);
if (input.forwardType === WAN_FORWARD_TYPE.STATIC) {
this.configService.set('connect.config.wanport', input.port);
// when forwarding with upnp, the upnp service will clear & set the wanport as necessary
}
// Currently, Dynamic Remote Access (WAN_ACCESS_TYPE.DYNAMIC) is not enabled,
// so we treat it as disabled for this condition.
const wanaccessEnabled = input.accessType === WAN_ACCESS_TYPE.ALWAYS;

this.configService.set(
'connect.config.upnpEnabled',
input.forwardType === WAN_FORWARD_TYPE.UPNP
wanaccessEnabled && input.forwardType === WAN_FORWARD_TYPE.UPNP
);

if (wanaccessEnabled && input.forwardType === WAN_FORWARD_TYPE.STATIC) {
this.configService.set('connect.config.wanport', input.port);
// when forwarding with upnp, the upnp service will clear & set the wanport as necessary
}

this.configService.set('connect.config.wanaccess', wanaccessEnabled);
// do the wanaccess port-override last; it should have the highest precedence
if (!wanaccessEnabled) {
this.configService.set('connect.config.wanport', null);
}

// Use the dynamic remote access service to handle the transition
// currently disabled; this call ensures correct migration behavior.
await this.remoteAccess.enableDynamicRemoteAccess({
type: dynamicRemoteAccessType,
allowedUrl: {
Expand All @@ -238,7 +249,6 @@ export class ConnectSettingsService {
});

await this.networkService.reloadNetworkStack();

return true;
}

Expand Down
Loading