fix(auth): read a repeated trusted header as one value, refuse conflicts
A proxy that sets the username header twice, as when Authentik's nginx snippet and the documented lines are both present, made Node join the values into "brad, brad" and create an account of that name instead of signing in brad. Read each occurrence separately: identical repeats count once, and conflicting ones are refused. The docs now note Authentik's snippet already sets the headers.
This commit is contained in:
@@ -86,6 +86,17 @@ describe('trusted header login', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('signs in once when a misconfigured proxy repeats the header', async () => {
|
||||
// superagent sends an array as repeated header lines; its types only allow that for Cookie
|
||||
const twice = (value: string) => [value, value] as unknown as string;
|
||||
const res = await request(app)
|
||||
.post('/api/auth/trusted-login')
|
||||
.set('X-authentik-username', twice('brad'))
|
||||
.set('X-authentik-email', twice('brad@example.com'));
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.user).toMatchObject({ username: 'brad', email: 'brad@example.com' });
|
||||
});
|
||||
|
||||
it('signs an existing user back in, matching the username case-insensitively', async () => {
|
||||
const again = await trustedLogin(app, 'BRAD', 'someone-else@example.com');
|
||||
expect(again.status).toBe(200);
|
||||
|
||||
@@ -12,8 +12,11 @@ const config = vi.hoisted(() => ({
|
||||
}));
|
||||
vi.mock('../config', () => ({ config }));
|
||||
|
||||
function request(remoteAddress: string, headers: Record<string, string> = {}): Request {
|
||||
return { socket: { remoteAddress }, headers } as unknown as Request;
|
||||
function request(remoteAddress: string, headers: Record<string, string | string[]> = {}): Request {
|
||||
const headersDistinct = Object.fromEntries(
|
||||
Object.entries(headers).map(([name, value]) => [name, Array.isArray(value) ? value : [value]])
|
||||
);
|
||||
return { socket: { remoteAddress }, headersDistinct } as unknown as Request;
|
||||
}
|
||||
|
||||
describe('trusted header identity', () => {
|
||||
@@ -51,6 +54,22 @@ describe('trusted header identity', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('accepts a header repeated with the same value, and refuses conflicting repeats', async () => {
|
||||
const { getTrustedIdentity } = await load();
|
||||
|
||||
expect(
|
||||
getTrustedIdentity(
|
||||
request('192.0.2.10', {
|
||||
'x-authentik-username': ['brad', 'brad'],
|
||||
'x-authentik-email': ['brad@example.com', 'brad@example.com'],
|
||||
})
|
||||
)
|
||||
).toEqual({ username: 'brad', email: 'brad@example.com' });
|
||||
expect(
|
||||
getTrustedIdentity(request('192.0.2.10', { 'x-authentik-username': ['brad', 'mallory'] }))
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('is off without a header name', async () => {
|
||||
config.trustedHeaderUsername = null;
|
||||
const { getTrustedIdentity, isTrustedHeaderEnabled } = await load();
|
||||
|
||||
@@ -43,8 +43,16 @@ function fromTrustedProxy(req: Request): boolean {
|
||||
|
||||
function header(req: Request, name: string | null): string | null {
|
||||
if (!name) return null;
|
||||
const value = req.headers[name.toLowerCase()];
|
||||
return typeof value === 'string' && value.trim() ? value.trim() : null;
|
||||
// Read each occurrence: Node joins repeats into one value with ", ", which a
|
||||
// proxy setting the header twice would turn into a different username
|
||||
const values = (req.headersDistinct[name.toLowerCase()] ?? [])
|
||||
.map((value) => value.trim())
|
||||
.filter(Boolean);
|
||||
if (values.some((value) => value !== values[0])) {
|
||||
logger.warn(`[TrustedHeader] Ignoring ${name}: the proxy sent conflicting values`);
|
||||
return null;
|
||||
}
|
||||
return values[0] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -154,8 +154,9 @@ LAN.
|
||||
| `TRUSTED_HEADER_EMAIL` | - | Optional header holding the email used for new accounts |
|
||||
| `TRUSTED_PROXIES` | - | Comma-separated proxy addresses or CIDR ranges allowed to set the header |
|
||||
|
||||
With nginx and Authentik's forward auth, pass the header from the auth
|
||||
subrequest in each protected `location`:
|
||||
With nginx and Authentik's forward auth, each protected `location` needs these
|
||||
lines once. Authentik's generated nginx snippet already includes them; adding
|
||||
them again sends each header twice:
|
||||
|
||||
```nginx
|
||||
auth_request_set $authentik_username $upstream_http_x_authentik_username;
|
||||
|
||||
Reference in New Issue
Block a user