fix(oidc): normalize OIDC_SCOPE to handle whitespace issues (#1060)
Adds automatic scope normalization to prevent URL encoding issues when OIDC_SCOPE contains extra whitespace, tabs, or newlines. This addresses issue #1056 where spaces in the scope value could cause authentication failures in some environments. Changes: - Added normalizeScope() function to trim and collapse whitespace - Automatically adds 'openid' scope if missing with warning - Updated both single and multi-provider configurations - Added comprehensive tests for scope normalization edge cases - Added service tests to verify authorization URL construction - Updated documentation with scope formatting guidance Fixes #1056
This commit is contained in:
Chris
GitHub
parent
cfe943b475
commit
93bcbc0485
4 changed files with 293 additions and 5 deletions
|
|
@ -6,6 +6,21 @@ function parseCommaSeparated(value) {
|
|||
.filter(Boolean);
|
||||
}
|
||||
|
||||
function normalizeScope(scope) {
|
||||
if (!scope) return 'openid profile email';
|
||||
|
||||
const normalized = scope.trim().split(/\s+/).filter(Boolean).join(' ');
|
||||
|
||||
if (!normalized.includes('openid')) {
|
||||
console.warn(
|
||||
`OIDC scope does not include 'openid'. Adding it automatically. Original scope: "${scope}"`
|
||||
);
|
||||
return `openid ${normalized}`;
|
||||
}
|
||||
|
||||
return normalized;
|
||||
}
|
||||
|
||||
function loadProvidersFromEnv() {
|
||||
if (process.env.OIDC_ENABLED !== 'true') {
|
||||
console.log(
|
||||
|
|
@ -24,9 +39,7 @@ function loadProvidersFromEnv() {
|
|||
issuer: process.env[`OIDC_PROVIDER_${i}_ISSUER`],
|
||||
clientId: process.env[`OIDC_PROVIDER_${i}_CLIENT_ID`],
|
||||
clientSecret: process.env[`OIDC_PROVIDER_${i}_CLIENT_SECRET`],
|
||||
scope:
|
||||
process.env[`OIDC_PROVIDER_${i}_SCOPE`] ||
|
||||
'openid profile email',
|
||||
scope: normalizeScope(process.env[`OIDC_PROVIDER_${i}_SCOPE`]),
|
||||
autoProvision:
|
||||
process.env[`OIDC_PROVIDER_${i}_AUTO_PROVISION`] !== 'false',
|
||||
adminEmailDomains: parseCommaSeparated(
|
||||
|
|
@ -63,7 +76,7 @@ function loadProvidersFromEnv() {
|
|||
issuer: process.env.OIDC_ISSUER_URL,
|
||||
clientId: process.env.OIDC_CLIENT_ID,
|
||||
clientSecret: process.env.OIDC_CLIENT_SECRET,
|
||||
scope: process.env.OIDC_SCOPE || 'openid profile email',
|
||||
scope: normalizeScope(process.env.OIDC_SCOPE),
|
||||
autoProvision: process.env.OIDC_AUTO_PROVISION !== 'false',
|
||||
adminEmailDomains: parseCommaSeparated(
|
||||
process.env.OIDC_ADMIN_EMAIL_DOMAINS
|
||||
|
|
|
|||
|
|
@ -86,6 +86,60 @@ describe('OIDC Provider Configuration', () => {
|
|||
expect(provider.scope).toBe('openid profile email groups');
|
||||
});
|
||||
|
||||
it('should normalize scope with extra whitespace', () => {
|
||||
process.env.OIDC_ENABLED = 'true';
|
||||
process.env.OIDC_PROVIDER_NAME = 'Test';
|
||||
process.env.OIDC_PROVIDER_SLUG = 'test';
|
||||
process.env.OIDC_ISSUER_URL = 'https://auth.example.com';
|
||||
process.env.OIDC_CLIENT_ID = 'test-id';
|
||||
process.env.OIDC_CLIENT_SECRET = 'test-secret';
|
||||
process.env.OIDC_SCOPE = ' openid profile email ';
|
||||
|
||||
providerConfig.reloadProviders();
|
||||
const provider = providerConfig.getProvider('test');
|
||||
|
||||
expect(provider.scope).toBe('openid profile email');
|
||||
});
|
||||
|
||||
it('should add openid scope if missing', () => {
|
||||
const consoleWarnSpy = jest
|
||||
.spyOn(console, 'warn')
|
||||
.mockImplementation();
|
||||
|
||||
process.env.OIDC_ENABLED = 'true';
|
||||
process.env.OIDC_PROVIDER_NAME = 'Test';
|
||||
process.env.OIDC_PROVIDER_SLUG = 'test';
|
||||
process.env.OIDC_ISSUER_URL = 'https://auth.example.com';
|
||||
process.env.OIDC_CLIENT_ID = 'test-id';
|
||||
process.env.OIDC_CLIENT_SECRET = 'test-secret';
|
||||
process.env.OIDC_SCOPE = 'profile email';
|
||||
|
||||
providerConfig.reloadProviders();
|
||||
const provider = providerConfig.getProvider('test');
|
||||
|
||||
expect(provider.scope).toBe('openid profile email');
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("does not include 'openid'")
|
||||
);
|
||||
|
||||
consoleWarnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('should handle scope with tabs and newlines', () => {
|
||||
process.env.OIDC_ENABLED = 'true';
|
||||
process.env.OIDC_PROVIDER_NAME = 'Test';
|
||||
process.env.OIDC_PROVIDER_SLUG = 'test';
|
||||
process.env.OIDC_ISSUER_URL = 'https://auth.example.com';
|
||||
process.env.OIDC_CLIENT_ID = 'test-id';
|
||||
process.env.OIDC_CLIENT_SECRET = 'test-secret';
|
||||
process.env.OIDC_SCOPE = 'openid\tprofile\nemail';
|
||||
|
||||
providerConfig.reloadProviders();
|
||||
const provider = providerConfig.getProvider('test');
|
||||
|
||||
expect(provider.scope).toBe('openid profile email');
|
||||
});
|
||||
|
||||
it('should parse admin email domains', () => {
|
||||
process.env.OIDC_ENABLED = 'true';
|
||||
process.env.OIDC_PROVIDER_NAME = 'Google';
|
||||
|
|
@ -227,6 +281,32 @@ describe('OIDC Provider Configuration', () => {
|
|||
expect(corp.autoProvision).toBe(false);
|
||||
expect(corp.adminEmailDomains).toEqual(['corp.com']);
|
||||
});
|
||||
|
||||
it('should normalize scopes in multi-provider configuration', () => {
|
||||
process.env.OIDC_ENABLED = 'true';
|
||||
|
||||
process.env.OIDC_PROVIDER_1_NAME = 'Google';
|
||||
process.env.OIDC_PROVIDER_1_SLUG = 'google';
|
||||
process.env.OIDC_PROVIDER_1_ISSUER = 'https://accounts.google.com';
|
||||
process.env.OIDC_PROVIDER_1_CLIENT_ID = 'google-id';
|
||||
process.env.OIDC_PROVIDER_1_CLIENT_SECRET = 'google-secret';
|
||||
process.env.OIDC_PROVIDER_1_SCOPE = ' openid profile email ';
|
||||
|
||||
process.env.OIDC_PROVIDER_2_NAME = 'Okta';
|
||||
process.env.OIDC_PROVIDER_2_SLUG = 'okta';
|
||||
process.env.OIDC_PROVIDER_2_ISSUER = 'https://company.okta.com';
|
||||
process.env.OIDC_PROVIDER_2_CLIENT_ID = 'okta-id';
|
||||
process.env.OIDC_PROVIDER_2_CLIENT_SECRET = 'okta-secret';
|
||||
process.env.OIDC_PROVIDER_2_SCOPE = 'openid profile email groups';
|
||||
|
||||
providerConfig.reloadProviders();
|
||||
|
||||
const google = providerConfig.getProvider('google');
|
||||
const okta = providerConfig.getProvider('okta');
|
||||
|
||||
expect(google.scope).toBe('openid profile email');
|
||||
expect(okta.scope).toBe('openid profile email groups');
|
||||
});
|
||||
});
|
||||
|
||||
describe('getProvider', () => {
|
||||
|
|
|
|||
178
backend/tests/unit/modules/oidc/service.test.js
Normal file
178
backend/tests/unit/modules/oidc/service.test.js
Normal file
|
|
@ -0,0 +1,178 @@
|
|||
const { Issuer } = require('openid-client');
|
||||
const oidcService = require('../../../../modules/oidc/service');
|
||||
const providerConfig = require('../../../../modules/oidc/providerConfig');
|
||||
const stateManager = require('../../../../modules/oidc/stateManager');
|
||||
|
||||
jest.mock('../../../../modules/oidc/providerConfig');
|
||||
jest.mock('../../../../modules/oidc/stateManager');
|
||||
|
||||
describe('OIDC Service - Authorization URL Construction', () => {
|
||||
let originalEnv;
|
||||
let mockIssuer;
|
||||
let mockClient;
|
||||
|
||||
beforeEach(() => {
|
||||
originalEnv = { ...process.env };
|
||||
process.env.BASE_URL = 'https://todo.example.com';
|
||||
|
||||
mockClient = {
|
||||
authorizationUrl: jest.fn(),
|
||||
callback: jest.fn(),
|
||||
};
|
||||
|
||||
mockIssuer = {
|
||||
Client: jest.fn(() => mockClient),
|
||||
metadata: {
|
||||
issuer: 'https://auth.example.com',
|
||||
authorization_endpoint: 'https://auth.example.com/authorize',
|
||||
token_endpoint: 'https://auth.example.com/token',
|
||||
},
|
||||
};
|
||||
|
||||
jest.spyOn(Issuer, 'discover').mockResolvedValue(mockIssuer);
|
||||
|
||||
oidcService.clearIssuerCache();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env = originalEnv;
|
||||
jest.restoreAllMocks();
|
||||
oidcService.clearIssuerCache();
|
||||
});
|
||||
|
||||
describe('initiateAuthFlow with scope containing spaces', () => {
|
||||
it('should properly encode scope with spaces in authorization URL', async () => {
|
||||
const mockProvider = {
|
||||
slug: 'test-provider',
|
||||
name: 'Test Provider',
|
||||
issuer: 'https://auth.example.com',
|
||||
clientId: 'test-client-id',
|
||||
clientSecret: 'test-client-secret',
|
||||
scope: 'openid profile email',
|
||||
};
|
||||
|
||||
providerConfig.getProvider.mockReturnValue(mockProvider);
|
||||
|
||||
stateManager.createState.mockResolvedValue({
|
||||
state: 'test-state-123',
|
||||
nonce: 'test-nonce-456',
|
||||
});
|
||||
|
||||
const mockAuthUrl =
|
||||
'https://auth.example.com/authorize?client_id=test-client-id&scope=openid%20profile%20email&response_type=code&redirect_uri=https%3A%2F%2Ftodo.example.com%2Fapi%2Foidc%2Fcallback%2Ftest-provider&state=test-state-123&nonce=test-nonce-456';
|
||||
|
||||
mockClient.authorizationUrl.mockReturnValue(mockAuthUrl);
|
||||
|
||||
const result = await oidcService.initiateAuthFlow('test-provider');
|
||||
|
||||
expect(mockClient.authorizationUrl).toHaveBeenCalledWith({
|
||||
scope: 'openid profile email',
|
||||
state: 'test-state-123',
|
||||
nonce: 'test-nonce-456',
|
||||
});
|
||||
|
||||
expect(result.authUrl).toContain('scope=openid%20profile%20email');
|
||||
|
||||
expect(result.authUrl).not.toContain('scope=openid profile email');
|
||||
});
|
||||
|
||||
it('should handle scope with plus signs correctly', async () => {
|
||||
const mockProvider = {
|
||||
slug: 'test-provider',
|
||||
name: 'Test Provider',
|
||||
issuer: 'https://auth.example.com',
|
||||
clientId: 'test-client-id',
|
||||
clientSecret: 'test-client-secret',
|
||||
scope: 'openid+profile+email',
|
||||
};
|
||||
|
||||
providerConfig.getProvider.mockReturnValue(mockProvider);
|
||||
|
||||
stateManager.createState.mockResolvedValue({
|
||||
state: 'test-state-123',
|
||||
nonce: 'test-nonce-456',
|
||||
});
|
||||
|
||||
const mockAuthUrl =
|
||||
'https://auth.example.com/authorize?client_id=test-client-id&scope=openid%2Bprofile%2Bemail&response_type=code&redirect_uri=https%3A%2F%2Ftodo.example.com%2Fapi%2Foidc%2Fcallback%2Ftest-provider&state=test-state-123&nonce=test-nonce-456';
|
||||
|
||||
mockClient.authorizationUrl.mockReturnValue(mockAuthUrl);
|
||||
|
||||
const result = await oidcService.initiateAuthFlow('test-provider');
|
||||
|
||||
expect(mockClient.authorizationUrl).toHaveBeenCalledWith({
|
||||
scope: 'openid+profile+email',
|
||||
state: 'test-state-123',
|
||||
nonce: 'test-nonce-456',
|
||||
});
|
||||
|
||||
expect(result.authUrl).toBeDefined();
|
||||
});
|
||||
|
||||
it('should handle custom scopes with spaces', async () => {
|
||||
const mockProvider = {
|
||||
slug: 'test-provider',
|
||||
name: 'Test Provider',
|
||||
issuer: 'https://auth.example.com',
|
||||
clientId: 'test-client-id',
|
||||
clientSecret: 'test-client-secret',
|
||||
scope: 'openid profile email groups offline_access',
|
||||
};
|
||||
|
||||
providerConfig.getProvider.mockReturnValue(mockProvider);
|
||||
|
||||
stateManager.createState.mockResolvedValue({
|
||||
state: 'test-state-123',
|
||||
nonce: 'test-nonce-456',
|
||||
});
|
||||
|
||||
const mockAuthUrl =
|
||||
'https://auth.example.com/authorize?client_id=test-client-id&scope=openid%20profile%20email%20groups%20offline_access&response_type=code&redirect_uri=https%3A%2F%2Ftodo.example.com%2Fapi%2Foidc%2Fcallback%2Ftest-provider&state=test-state-123&nonce=test-nonce-456';
|
||||
|
||||
mockClient.authorizationUrl.mockReturnValue(mockAuthUrl);
|
||||
|
||||
const result = await oidcService.initiateAuthFlow('test-provider');
|
||||
|
||||
expect(mockClient.authorizationUrl).toHaveBeenCalledWith({
|
||||
scope: 'openid profile email groups offline_access',
|
||||
state: 'test-state-123',
|
||||
nonce: 'test-nonce-456',
|
||||
});
|
||||
|
||||
expect(result.authUrl).toContain(
|
||||
'scope=openid%20profile%20email%20groups%20offline_access'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('edge cases', () => {
|
||||
it('should handle scope with leading/trailing spaces', async () => {
|
||||
const mockProvider = {
|
||||
slug: 'test-provider',
|
||||
name: 'Test Provider',
|
||||
issuer: 'https://auth.example.com',
|
||||
clientId: 'test-client-id',
|
||||
clientSecret: 'test-client-secret',
|
||||
scope: ' openid profile email ',
|
||||
};
|
||||
|
||||
providerConfig.getProvider.mockReturnValue(mockProvider);
|
||||
|
||||
stateManager.createState.mockResolvedValue({
|
||||
state: 'test-state-123',
|
||||
nonce: 'test-nonce-456',
|
||||
});
|
||||
|
||||
mockClient.authorizationUrl.mockReturnValue(
|
||||
'https://auth.example.com/authorize?scope=openid%20profile%20email'
|
||||
);
|
||||
|
||||
const result = await oidcService.initiateAuthFlow('test-provider');
|
||||
|
||||
const scopeArgument =
|
||||
mockClient.authorizationUrl.mock.calls[0][0].scope;
|
||||
|
||||
expect(scopeArgument.trim()).toBe('openid profile email');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -169,11 +169,28 @@ OIDC_PROVIDER_3_AUTO_PROVISION=true
|
|||
| `OIDC_ISSUER_URL` | Yes | - | OIDC discovery endpoint |
|
||||
| `OIDC_CLIENT_ID` | Yes | - | OAuth client ID |
|
||||
| `OIDC_CLIENT_SECRET` | Yes | - | OAuth client secret |
|
||||
| `OIDC_SCOPE` | No | `openid profile email` | OAuth scopes |
|
||||
| `OIDC_SCOPE` | No | `openid profile email` | OAuth scopes (space-separated) |
|
||||
| `OIDC_AUTO_PROVISION` | No | `true` | Auto-create users on first login |
|
||||
| `OIDC_ADMIN_EMAIL_DOMAINS` | No | - | Comma-separated domains for auto-admin |
|
||||
| `BASE_URL` | Yes | - | Tududi base URL (for OAuth callbacks) |
|
||||
|
||||
**Scope Formatting:**
|
||||
|
||||
The `OIDC_SCOPE` parameter accepts space-separated OAuth scopes. Tududi automatically normalizes the scope value by:
|
||||
- Trimming leading/trailing whitespace
|
||||
- Collapsing multiple spaces into single spaces
|
||||
- Ensuring `openid` is always included (adding it if missing)
|
||||
- Properly URL-encoding the scope in authorization requests
|
||||
|
||||
Examples of valid scope formats:
|
||||
```bash
|
||||
OIDC_SCOPE=openid profile email
|
||||
OIDC_SCOPE="openid profile email groups"
|
||||
OIDC_SCOPE=openid profile email # Extra spaces are automatically normalized
|
||||
```
|
||||
|
||||
**Note:** Do not manually URL-encode the scope value (e.g., using `%20` or `+`). Use regular spaces - Tududi handles the encoding automatically.
|
||||
|
||||
**Important:** The `BASE_URL` variable must be set for OAuth redirects to work:
|
||||
```bash
|
||||
BASE_URL=http://localhost:3002 # Development
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue