Skip to content

Commit a4ff3c4

Browse files
authored
fix: insecure routes not working for SSO (#1587)
1 parent 1e0a54d commit a4ff3c4

File tree

7 files changed

+80
-37
lines changed

7 files changed

+80
-37
lines changed

.github/workflows/create-docusaurus-pr.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,26 @@ jobs:
3737
echo "Source directory does not exist!"
3838
exit 1
3939
fi
40+
# Remove old API docs but preserve other folders
4041
rm -rf docs-repo/docs/API/
4142
mkdir -p docs-repo/docs/API
43+
44+
# Copy all markdown files and maintain directory structure
4245
cp -r source-repo/api/docs/public/. docs-repo/docs/API/
46+
47+
# Clean and copy images directory specifically
48+
rm -rf docs-repo/docs/API/images/
49+
mkdir -p docs-repo/docs/API/images
50+
51+
# Copy images from public/images if they exist
52+
if [ -d "source-repo/api/docs/public/images" ]; then
53+
cp -r source-repo/api/docs/public/images/. docs-repo/docs/API/images/
54+
fi
55+
56+
# Also copy any images from the parent docs/images directory
57+
if [ -d "source-repo/api/docs/images" ]; then
58+
cp -r source-repo/api/docs/images/. docs-repo/docs/API/images/
59+
fi
4360
- name: Create Pull Request
4461
uses: peter-evans/create-pull-request@v7
4562
with:

api/src/unraid-api/graph/resolvers/sso/oidc-auth.service.test.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,20 +1512,30 @@ describe('OidcAuthService', () => {
15121512
describe('getRedirectUri (private method)', () => {
15131513
it('should generate correct redirect URI with localhost (development)', () => {
15141514
const getRedirectUri = (service as any).getRedirectUri.bind(service);
1515-
const redirectUri = getRedirectUri('localhost:3001');
1515+
const redirectUri = getRedirectUri('http://localhost:3000');
15161516

15171517
expect(redirectUri).toBe('http://localhost:3000/graphql/api/auth/oidc/callback');
15181518
});
15191519

15201520
it('should generate correct redirect URI with non-localhost host', () => {
15211521
const getRedirectUri = (service as any).getRedirectUri.bind(service);
1522+
const redirectUri = getRedirectUri('https://example.com');
15221523

1523-
// Mock the ConfigService to return a production base URL
1524-
configService.get.mockReturnValue('https://example.com');
1524+
expect(redirectUri).toBe('https://example.com/graphql/api/auth/oidc/callback');
1525+
});
15251526

1526-
const redirectUri = getRedirectUri('example.com:443');
1527+
it('should handle HTTP protocol for non-localhost hosts', () => {
1528+
const getRedirectUri = (service as any).getRedirectUri.bind(service);
1529+
const redirectUri = getRedirectUri('http://tower.local');
15271530

1528-
expect(redirectUri).toBe('https://example.com/graphql/api/auth/oidc/callback');
1531+
expect(redirectUri).toBe('http://tower.local/graphql/api/auth/oidc/callback');
1532+
});
1533+
1534+
it('should handle non-standard ports correctly', () => {
1535+
const getRedirectUri = (service as any).getRedirectUri.bind(service);
1536+
const redirectUri = getRedirectUri('http://example.com:8080');
1537+
1538+
expect(redirectUri).toBe('http://example.com:8080/graphql/api/auth/oidc/callback');
15291539
});
15301540

15311541
it('should use default redirect URI when no request host provided', () => {

api/src/unraid-api/graph/resolvers/sso/oidc-auth.service.ts

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,17 @@ export class OidcAuthService {
3636
private readonly validationService: OidcValidationService
3737
) {}
3838

39-
async getAuthorizationUrl(providerId: string, state: string, requestHost?: string): Promise<string> {
39+
async getAuthorizationUrl(
40+
providerId: string,
41+
state: string,
42+
requestOrigin?: string
43+
): Promise<string> {
4044
const provider = await this.oidcConfig.getProvider(providerId);
4145
if (!provider) {
4246
throw new UnauthorizedException(`Provider ${providerId} not found`);
4347
}
4448

45-
const redirectUri = this.getRedirectUri(requestHost);
49+
const redirectUri = this.getRedirectUri(requestOrigin);
4650

4751
// Generate secure state with cryptographic signature
4852
const secureState = this.stateService.generateSecureState(providerId, state);
@@ -110,7 +114,7 @@ export class OidcAuthService {
110114
providerId: string,
111115
code: string,
112116
state: string,
113-
requestHost?: string,
117+
requestOrigin?: string,
114118
fullCallbackUrl?: string
115119
): Promise<string> {
116120
const provider = await this.oidcConfig.getProvider(providerId);
@@ -119,7 +123,7 @@ export class OidcAuthService {
119123
}
120124

121125
try {
122-
const redirectUri = this.getRedirectUri(requestHost);
126+
const redirectUri = this.getRedirectUri(requestOrigin);
123127

124128
// Always use openid-client for consistency
125129
const config = await this.getOrCreateConfig(provider);
@@ -634,30 +638,35 @@ export class OidcAuthService {
634638
return this.validationService.validateProvider(provider);
635639
}
636640

637-
private getRedirectUri(requestHost?: string): string {
638-
// Always use the proxied path through /graphql to match production
639-
if (requestHost && requestHost.includes('localhost')) {
640-
// In development, use the Nuxt proxy at port 3000
641-
return `http://localhost:3000/graphql/api/auth/oidc/callback`;
642-
}
641+
private getRedirectUri(requestOrigin?: string): string {
642+
// If we have the full origin (protocol://host), use it directly
643+
if (requestOrigin) {
644+
// Parse the origin to extract protocol and host
645+
try {
646+
const url = new URL(requestOrigin);
647+
const { protocol, hostname, port } = url;
648+
649+
// Reconstruct the URL, removing default ports
650+
let cleanOrigin = `${protocol}//${hostname}`;
651+
652+
// Add port if it's not the default for the protocol
653+
if (
654+
port &&
655+
!(protocol === 'https:' && port === '443') &&
656+
!(protocol === 'http:' && port === '80')
657+
) {
658+
cleanOrigin += `:${port}`;
659+
}
643660

644-
// In production, use the actual request host or configured base URL
645-
if (requestHost) {
646-
// Parse the host to handle port numbers properly
647-
const isLocalhost = requestHost.includes('localhost');
648-
const protocol = isLocalhost ? 'http' : 'https';
649-
650-
// Remove standard ports (:443 for HTTPS, :80 for HTTP)
651-
let cleanHost = requestHost;
652-
if (!isLocalhost) {
653-
if (requestHost.endsWith(':443')) {
654-
cleanHost = requestHost.slice(0, -4); // Remove :443
655-
} else if (requestHost.endsWith(':80')) {
656-
cleanHost = requestHost.slice(0, -3); // Remove :80
661+
// Special handling for localhost development with Nuxt proxy
662+
if (hostname === 'localhost' && port === '3000') {
663+
return `${cleanOrigin}/graphql/api/auth/oidc/callback`;
657664
}
658-
}
659665

660-
return `${protocol}://${cleanHost}/graphql/api/auth/oidc/callback`;
666+
return `${cleanOrigin}/graphql/api/auth/oidc/callback`;
667+
} catch (e) {
668+
this.logger.warn(`Failed to parse request origin: ${requestOrigin}, error: ${e}`);
669+
}
661670
}
662671

663672
// Fall back to configured BASE_URL or default

api/src/unraid-api/rest/rest.controller.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,16 @@ export class RestController {
7575
return res.status(400).send('State parameter is required');
7676
}
7777

78-
// Get the host from the request headers
79-
const host = req.headers.host || undefined;
78+
// Get the host and protocol from the request headers
79+
const protocol = (req.headers['x-forwarded-proto'] as string) || req.protocol || 'http';
80+
const host = (req.headers['x-forwarded-host'] as string) || req.headers.host || undefined;
81+
const requestInfo = host ? `${protocol}://${host}` : undefined;
8082

81-
const authUrl = await this.oidcAuthService.getAuthorizationUrl(providerId, state, host);
83+
const authUrl = await this.oidcAuthService.getAuthorizationUrl(
84+
providerId,
85+
state,
86+
requestInfo
87+
);
8288
this.logger.log(`Redirecting to OIDC provider: ${authUrl}`);
8389

8490
// Manually set redirect headers for better proxy compatibility
@@ -125,14 +131,15 @@ export class RestController {
125131
const host =
126132
(req.headers['x-forwarded-host'] as string) || req.headers.host || 'localhost:3000';
127133
const fullUrl = `${protocol}://${host}${req.url}`;
134+
const requestInfo = `${protocol}://${host}`;
128135

129136
this.logger.debug(`Full callback URL from request: ${fullUrl}`);
130137

131138
const paddedToken = await this.oidcAuthService.handleCallback(
132139
providerId,
133140
code,
134141
state,
135-
host,
142+
requestInfo,
136143
fullUrl
137144
);
138145

api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/.login.php.modified.snapshot.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ function verifyUsernamePasswordAndSSO(string $username, string $password): bool
1212
return true;
1313
}
1414
// We may have an SSO token, attempt validation
15-
if (strlen($password) > 800) {
15+
if (strlen($password) > 500) {
1616
if (!preg_match('/^[A-Za-z0-9-_]+.[A-Za-z0-9-_]+.[A-Za-z0-9-_]+$/', $password)) {
1717
my_logger("SSO Login Attempt Failed: Invalid token format");
1818
return false;

api/src/unraid-api/unraid-file-modifier/modifications/patches/sso.patch

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Index: /usr/local/emhttp/plugins/dynamix/include/.login.php
1717
+ return true;
1818
+ }
1919
+ // We may have an SSO token, attempt validation
20-
+ if (strlen($password) > 800) {
20+
+ if (strlen($password) > 500) {
2121
+ if (!preg_match('/^[A-Za-z0-9-_]+.[A-Za-z0-9-_]+.[A-Za-z0-9-_]+$/', $password)) {
2222
+ my_logger("SSO Login Attempt Failed: Invalid token format");
2323
+ return false;

api/src/unraid-api/unraid-file-modifier/modifications/sso.modification.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ function verifyUsernamePasswordAndSSO(string $username, string $password): bool
2424
return true;
2525
}
2626
// We may have an SSO token, attempt validation
27-
if (strlen($password) > 800) {
27+
if (strlen($password) > 500) {
2828
if (!preg_match('/^[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+\.[A-Za-z0-9-_]+$/', $password)) {
2929
my_logger("SSO Login Attempt Failed: Invalid token format");
3030
return false;

0 commit comments

Comments
 (0)