Skip to content

Commit 0eb7127

Browse files
elibosleyelibosley
committed
refactor: update OIDC redirect URI handling in service and controller
- Renamed `requestHost` to `requestOrigin` in OidcAuthService methods for clarity. - Enhanced `getRedirectUri` method to handle full origin URLs, including protocol and non-standard ports. - Updated tests to reflect changes in redirect URI generation for various scenarios, including localhost and non-localhost hosts. - Modified RestController to construct request information using protocol and host headers for improved compatibility.
1 parent 7503061 commit 0eb7127

3 files changed

Lines changed: 60 additions & 34 deletions

File tree

‎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

0 commit comments

Comments
 (0)