mirror/api
1
0
Fork 0
mirror of https://github.com/unraid/api.git synced 2025-12-31 13:39:52 -06:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
Resolve #1615 -- lingering wanport caused issue with LAN Access via
Connect, because those URL's are constructed using `wanport`, but since
WAN access would be disabled, NGINX would not listen on the port.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* WAN access settings now correctly gate related options; UPnP only
enables when WAN access is Always.
* Static WAN port is applied only when eligible and is cleared when WAN
access is disabled to avoid unintended overrides.
* Dynamic remote access migration uses sanitized URLs to prevent
propagation of user-provided addressing.

* **Chores**
* Minor ordering and formatting adjustments to reflect updated
precedence and clarify behavior.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Pujit Mehrotra
2025-08-25 13:21:48 -04:00
committed by GitHub
parent aa588883cc
commit 9df6a3f5eb
3 changed files with 145 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

113
packages/unraid-api-plugin-connect/src/__test__/url-resolver.service.test.ts
View File

@@ -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();
@@ -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,
@@ -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();
@@ -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', () => {
@@ -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();

22
packages/unraid-api-plugin-connect/src/network/url-resolver.service.ts
View File

@@ -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.
@@ -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')] };

24
packages/unraid-api-plugin-connect/src/unraid-connect/connect-settings.service.ts
View File

@@ -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) {
// 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',
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.upnpEnabled',
input.forwardType === WAN_FORWARD_TYPE.UPNP
);
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: {
@@ -238,7 +249,6 @@ export class ConnectSettingsService {
});
await this.networkService.reloadNetworkStack();
return true;
}