fix(auth): separate Pireon clients for authorization code and password
Pireon OAuth clients allow a single grant type. With one PIREON_CLIENT_ID of type password, email + password worked but "Sign in with Pireon" failed with `unauthorized_client`. - PIREON_CLIENT_ID/SECRET: authorization_code client (Sign in with Pireon, Swagger UI). New PIREON_PASSWORD_CLIENT_ID/SECRET: password client, falling back to the former so existing setups keep working. - Refresh tries both clients: a refresh token only works with its issuer. - PIREON_PASSWORD_GRANT=false now also rejects password grants in the API (before it only hid the form). - /api/auth/config returns clientId: null when there is no authorization_code client, and the web hides "Sign in with Pireon". - docker-compose passes the new variables; docs 05/07, .env.example, openapi.json; unit tests for the client selection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -31,6 +31,10 @@ import { RedisSessionCache } from './redis/redis-session.cache';
|
||||
requestTimeoutMs: Number(config.get('PIREON_TIMEOUT_MS', 5000)),
|
||||
clientId: config.get<string>('PIREON_CLIENT_ID') || undefined,
|
||||
clientSecret: config.get<string>('PIREON_CLIENT_SECRET') || undefined,
|
||||
passwordClientId:
|
||||
config.get<string>('PIREON_PASSWORD_CLIENT_ID') || undefined,
|
||||
passwordClientSecret:
|
||||
config.get<string>('PIREON_PASSWORD_CLIENT_SECRET') || undefined,
|
||||
scopes: String(config.get('PIREON_SCOPES', 'users:read'))
|
||||
.split(/\s+/)
|
||||
.filter(Boolean),
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
import {
|
||||
ForbiddenException,
|
||||
ServiceUnavailableException,
|
||||
} from '@nestjs/common';
|
||||
import { InvalidCredentialsError } from '@task-stream/api/identity/application';
|
||||
import { UserFactory } from '@task-stream/api/identity/domain';
|
||||
import { PireonConfig } from './pireon.config';
|
||||
import { PireonIdentityProvider } from './pireon-identity.provider';
|
||||
|
||||
const TOKEN = {
|
||||
access_token: 'access',
|
||||
refresh_token: 'refresh',
|
||||
expires_in: 3600,
|
||||
token_type: 'Bearer',
|
||||
};
|
||||
|
||||
/** Pireon clients allow a single grant type: one client for code, one for password. */
|
||||
function setup(overrides: Partial<PireonConfig> = {}) {
|
||||
const config: PireonConfig = {
|
||||
baseUrl: 'https://oauth.pireon.dev',
|
||||
sessionTtlSeconds: 60,
|
||||
requestTimeoutMs: 1000,
|
||||
clientId: 'code-client',
|
||||
clientSecret: 'code-secret',
|
||||
passwordClientId: 'password-client',
|
||||
passwordClientSecret: 'password-secret',
|
||||
scopes: ['users:read'],
|
||||
passwordGrant: true,
|
||||
...overrides,
|
||||
};
|
||||
const calls: URLSearchParams[] = [];
|
||||
/** Answers 401 to the clients in `rejects`, a token otherwise. */
|
||||
const respond = (rejects: string[] = []) =>
|
||||
jest.spyOn(globalThis, 'fetch').mockImplementation(async (_url, init) => {
|
||||
const body = init?.body as URLSearchParams;
|
||||
calls.push(body);
|
||||
return rejects.includes(body.get('client_id') ?? '')
|
||||
? new Response(
|
||||
JSON.stringify({
|
||||
error: 'invalid_request',
|
||||
error_description: 'The refresh token is invalid.',
|
||||
}),
|
||||
{ status: 401 },
|
||||
)
|
||||
: new Response(JSON.stringify(TOKEN), { status: 200 });
|
||||
});
|
||||
return {
|
||||
provider: new PireonIdentityProvider(config, new UserFactory()),
|
||||
calls,
|
||||
respond,
|
||||
};
|
||||
}
|
||||
|
||||
const password = {
|
||||
type: 'password',
|
||||
username: 'ana@pireon.dev',
|
||||
password: 'x',
|
||||
} as const;
|
||||
const code = {
|
||||
type: 'authorization_code',
|
||||
code: 'c',
|
||||
codeVerifier: 'v',
|
||||
redirectUri: 'https://taskstream.pireon.dev/auth/callback',
|
||||
} as const;
|
||||
const refresh = { type: 'refresh_token', refreshToken: 'r' } as const;
|
||||
|
||||
describe('PireonIdentityProvider tokens', () => {
|
||||
afterEach(() => jest.restoreAllMocks());
|
||||
|
||||
it('uses the password client for email + password', async () => {
|
||||
const { provider, calls, respond } = setup();
|
||||
respond();
|
||||
await provider.issueToken(password);
|
||||
expect(calls[0].get('client_id')).toBe('password-client');
|
||||
expect(calls[0].get('client_secret')).toBe('password-secret');
|
||||
expect(calls[0].get('grant_type')).toBe('password');
|
||||
});
|
||||
|
||||
it('uses the authorization_code client for "Sign in with Pireon"', async () => {
|
||||
const { provider, calls, respond } = setup();
|
||||
respond();
|
||||
await provider.issueToken(code);
|
||||
expect(calls[0].get('client_id')).toBe('code-client');
|
||||
expect(calls[0].get('code_verifier')).toBe('v');
|
||||
});
|
||||
|
||||
it('falls back to the main client when there is no password client', async () => {
|
||||
const { provider, calls, respond } = setup({
|
||||
passwordClientId: undefined,
|
||||
passwordClientSecret: undefined,
|
||||
});
|
||||
respond();
|
||||
await provider.issueToken(password);
|
||||
expect(calls[0].get('client_id')).toBe('code-client');
|
||||
});
|
||||
|
||||
it('refreshes with whichever client issued the token', async () => {
|
||||
const { provider, calls, respond } = setup();
|
||||
respond(['password-client']);
|
||||
await expect(provider.issueToken(refresh)).resolves.toMatchObject({
|
||||
accessToken: 'access',
|
||||
});
|
||||
expect(calls.map((c) => c.get('client_id'))).toEqual([
|
||||
'password-client',
|
||||
'code-client',
|
||||
]);
|
||||
});
|
||||
|
||||
it('rejects the refresh when no client accepts it', async () => {
|
||||
const { provider, respond } = setup();
|
||||
respond(['password-client', 'code-client']);
|
||||
await expect(provider.issueToken(refresh)).rejects.toBeInstanceOf(
|
||||
InvalidCredentialsError,
|
||||
);
|
||||
});
|
||||
|
||||
it('refuses email + password when PIREON_PASSWORD_GRANT=false', async () => {
|
||||
const { provider, calls, respond } = setup({ passwordGrant: false });
|
||||
respond();
|
||||
await expect(provider.issueToken(password)).rejects.toBeInstanceOf(
|
||||
ForbiddenException,
|
||||
);
|
||||
expect(calls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('says what is missing when a flow is not configured', async () => {
|
||||
const { provider } = setup({
|
||||
clientId: undefined,
|
||||
clientSecret: undefined,
|
||||
});
|
||||
await expect(provider.issueToken(code)).rejects.toBeInstanceOf(
|
||||
ServiceUnavailableException,
|
||||
);
|
||||
});
|
||||
|
||||
it('tells the browser which flows exist', () => {
|
||||
expect(setup().provider.authorizationConfig()).toMatchObject({
|
||||
clientId: 'code-client',
|
||||
passwordGrant: true,
|
||||
});
|
||||
expect(
|
||||
setup({
|
||||
clientId: undefined,
|
||||
clientSecret: undefined,
|
||||
}).provider.authorizationConfig(),
|
||||
).toMatchObject({ clientId: null, passwordGrant: true });
|
||||
expect(
|
||||
setup({
|
||||
clientId: undefined,
|
||||
passwordClientId: undefined,
|
||||
}).provider.authorizationConfig(),
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
import {
|
||||
ForbiddenException,
|
||||
Inject,
|
||||
Injectable,
|
||||
Logger,
|
||||
@@ -28,6 +29,11 @@ interface PireonUser {
|
||||
app_roles?: string[];
|
||||
}
|
||||
|
||||
interface OAuthClient {
|
||||
id: string;
|
||||
secret: string | undefined;
|
||||
}
|
||||
|
||||
/** Driven adapter for the Pireon OAuth2 identity server (Laravel Passport). */
|
||||
@Injectable()
|
||||
export class PireonIdentityProvider extends IdentityProvider {
|
||||
@@ -69,14 +75,78 @@ export class PireonIdentityProvider extends IdentityProvider {
|
||||
}
|
||||
|
||||
async issueToken(grant: TokenGrant): Promise<TokenSet> {
|
||||
const { clientId, clientSecret, scopes } = this.config;
|
||||
if (!clientId)
|
||||
throw new ServiceUnavailableException(
|
||||
'Sign-in is not configured (PIREON_CLIENT_ID)',
|
||||
);
|
||||
const clients = this.clientsFor(grant);
|
||||
// A refresh token only works with the client that issued it, and we don't
|
||||
// know which one that was: try each until one accepts it.
|
||||
let rejected: InvalidCredentialsError | undefined;
|
||||
for (const client of clients) {
|
||||
try {
|
||||
return await this.requestToken(client, grant);
|
||||
} catch (error) {
|
||||
if (
|
||||
grant.type !== 'refresh_token' ||
|
||||
!(error instanceof InvalidCredentialsError)
|
||||
)
|
||||
throw error;
|
||||
rejected = error;
|
||||
}
|
||||
}
|
||||
throw rejected ?? new InvalidCredentialsError();
|
||||
}
|
||||
|
||||
const body = new URLSearchParams({ client_id: clientId });
|
||||
if (clientSecret) body.set('client_secret', clientSecret);
|
||||
/**
|
||||
* Pireon clients allow a single grant type: authorization_code uses
|
||||
* PIREON_CLIENT_ID, password PIREON_PASSWORD_CLIENT_ID (or the former).
|
||||
*/
|
||||
private clientsFor(grant: TokenGrant): OAuthClient[] {
|
||||
const { clientId, clientSecret, passwordClientId, passwordClientSecret } =
|
||||
this.config;
|
||||
const code: OAuthClient | undefined = clientId
|
||||
? { id: clientId, secret: clientSecret }
|
||||
: undefined;
|
||||
const password: OAuthClient | undefined = passwordClientId
|
||||
? { id: passwordClientId, secret: passwordClientSecret }
|
||||
: code;
|
||||
|
||||
switch (grant.type) {
|
||||
case 'authorization_code':
|
||||
if (!code)
|
||||
throw new ServiceUnavailableException(
|
||||
'Sign-in with Pireon is not configured (PIREON_CLIENT_ID)',
|
||||
);
|
||||
return [code];
|
||||
case 'password':
|
||||
if (!this.config.passwordGrant)
|
||||
throw new ForbiddenException(
|
||||
'Email and password sign-in is disabled',
|
||||
);
|
||||
if (!password)
|
||||
throw new ServiceUnavailableException(
|
||||
'Email and password sign-in is not configured (PIREON_PASSWORD_CLIENT_ID)',
|
||||
);
|
||||
return [password];
|
||||
case 'refresh_token': {
|
||||
const candidates = [password, code].filter(
|
||||
(client): client is OAuthClient => !!client,
|
||||
);
|
||||
if (!candidates.length)
|
||||
throw new ServiceUnavailableException(
|
||||
'Sign-in is not configured (PIREON_CLIENT_ID)',
|
||||
);
|
||||
return candidates.filter(
|
||||
(client, i) => candidates.findIndex((c) => c.id === client.id) === i,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private async requestToken(
|
||||
client: OAuthClient,
|
||||
grant: TokenGrant,
|
||||
): Promise<TokenSet> {
|
||||
const { scopes } = this.config;
|
||||
const body = new URLSearchParams({ client_id: client.id });
|
||||
if (client.secret) body.set('client_secret', client.secret);
|
||||
switch (grant.type) {
|
||||
case 'authorization_code':
|
||||
body.set('grant_type', 'authorization_code');
|
||||
@@ -138,12 +208,14 @@ export class PireonIdentityProvider extends IdentityProvider {
|
||||
}
|
||||
|
||||
authorizationConfig(): AuthorizationConfig | null {
|
||||
if (!this.config.clientId) return null;
|
||||
const { clientId, passwordClientId, passwordGrant } = this.config;
|
||||
const password = passwordGrant && !!(passwordClientId ?? clientId);
|
||||
if (!clientId && !password) return null;
|
||||
return {
|
||||
authorizeUrl: new URL('/oauth/authorize', this.config.baseUrl).toString(),
|
||||
clientId: this.config.clientId,
|
||||
clientId: clientId ?? null,
|
||||
scopes: this.config.scopes,
|
||||
passwordGrant: this.config.passwordGrant,
|
||||
passwordGrant: password,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -4,10 +4,17 @@ export interface PireonConfig {
|
||||
/** Seconds a resolved token is trusted before asking Pireon again. */
|
||||
sessionTtlSeconds: number;
|
||||
requestTimeoutMs: number;
|
||||
/** OAuth client of task-stream in Pireon (Authorization Code + PKCE / password grant). */
|
||||
/**
|
||||
* Pireon OAuth client with grant type `authorization_code` ("Sign in with
|
||||
* Pireon", Swagger UI). Pireon clients allow a single grant type, so email +
|
||||
* password uses its own client below.
|
||||
*/
|
||||
clientId?: string;
|
||||
/** Kept server-side: browsers sign in through /api/auth/token. */
|
||||
clientSecret?: string;
|
||||
/** Pireon OAuth client with grant type `password`; defaults to the one above. */
|
||||
passwordClientId?: string;
|
||||
passwordClientSecret?: string;
|
||||
scopes: string[];
|
||||
passwordGrant: boolean;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user