Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SSO fixes and improvements #10301

Merged
merged 6 commits into from Dec 6, 2021
Merged

SSO fixes and improvements #10301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c1e6696
Update refresh_token on refresh, allow custom authorize params, handl…
aidenfoxx Dec 4, 2021
bf51fe5
Added default to generateAuthUrl
aidenfoxx Dec 4, 2021
e24a0b1
Apparently issuer can be undefined now
aidenfoxx Dec 4, 2021
d38d71a
Update api/src/auth/drivers/openid.ts
aidenfoxx Dec 4, 2021
330c783
Update api/src/auth/drivers/oauth2.ts
aidenfoxx Dec 4, 2021
bd64dc8
Make linter happy
aidenfoxx Dec 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 41 additions & 16 deletions api/src/auth/drivers/oauth2.ts
View file
Open in desktop
Expand Up @@ -7,7 +7,12 @@ import { getAuthProvider } from '../../auth';
import env from '../../env';
import { AuthenticationService, UsersService } from '../../services';
import { AuthDriverOptions, User, AuthData, SessionData } from '../../types';
import { InvalidCredentialsException, ServiceUnavailableException, InvalidConfigException } from '../../exceptions';
import {
InvalidCredentialsException,
ServiceUnavailableException,
InvalidConfigException,
InvalidTokenException,
} from '../../exceptions';
import { respond } from '../../middleware/respond';
import asyncHandler from '../../utils/async-handler';
import { Url } from '../../utils/url';
Expand Down Expand Up @@ -38,7 +43,8 @@ export class OAuth2AuthDriver extends LocalAuthDriver {
authorization_endpoint: authorizeUrl,
token_endpoint: accessUrl,
userinfo_endpoint: profileUrl,
issuer: additionalConfig.provider,
// Required for openid providers (openid flow should be preferred!)
issuer: additionalConfig.issuerUrl,
});

this.client = new issuer.Client({
Expand All @@ -53,16 +59,20 @@ export class OAuth2AuthDriver extends LocalAuthDriver {
return generators.codeVerifier();
}

generateAuthUrl(codeVerifier: string): string {
generateAuthUrl(codeVerifier: string, prompt = false): string {
aidenfoxx marked this conversation as resolved.
Show resolved Hide resolved
try {
const codeChallenge = generators.codeChallenge(codeVerifier);
const paramsConfig = typeof this.config.params === 'object' ? this.config.params : {};

return this.client.authorizationUrl({
scope: this.config.scope ?? 'email',
access_type: 'offline',
prompt: prompt ? 'consent' : undefined,
...paramsConfig,
code_challenge: codeChallenge,
code_challenge_method: 'S256',
// Some providers require state even with PKCE
state: codeChallenge,
access_type: 'offline',
});
} catch (e) {
throw handleError(e);
Expand Down Expand Up @@ -160,24 +170,29 @@ export class OAuth2AuthDriver extends LocalAuthDriver {
}
}

if (!authData?.refreshToken) {
return sessionData;
if (authData?.refreshToken) {
try {
const tokenSet = await this.client.refresh(authData.refreshToken);
// Update user refreshToken if provided
if (tokenSet.refresh_token) {
await this.usersService.updateOne(user.id, {
auth_data: JSON.stringify({ refreshToken: tokenSet.refresh_token }),
});
}
} catch (e) {
throw handleError(e);
}
}

try {
const tokenSet = await this.client.refresh(authData.refreshToken);
return { accessToken: tokenSet.access_token };
} catch (e) {
throw handleError(e);
}
return sessionData;
}
}

const handleError = (e: any) => {
if (e instanceof errors.OPError) {
if (e.error === 'invalid_grant') {
// Invalid token
return new InvalidCredentialsException();
return new InvalidTokenException();
}
// Server response error
return new ServiceUnavailableException('Service returned unexpected response', {
Expand All @@ -199,7 +214,8 @@ export function createOAuth2AuthRouter(providerName: string): Router {
(req, res) => {
const provider = getAuthProvider(providerName) as OAuth2AuthDriver;
const codeVerifier = provider.generateCodeVerifier();
const token = jwt.sign({ verifier: codeVerifier, redirect: req.query.redirect }, env.SECRET as string, {
const prompt = !!req.query.prompt;
const token = jwt.sign({ verifier: codeVerifier, redirect: req.query.redirect, prompt }, env.SECRET as string, {
expiresIn: '5m',
issuer: 'directus',
});
Expand All @@ -209,7 +225,7 @@ export function createOAuth2AuthRouter(providerName: string): Router {
sameSite: 'lax',
});

return res.redirect(provider.generateAuthUrl(codeVerifier));
return res.redirect(provider.generateAuthUrl(codeVerifier, prompt));
},
respond
);
Expand All @@ -223,12 +239,14 @@ export function createOAuth2AuthRouter(providerName: string): Router {
tokenData = jwt.verify(req.cookies[`oauth2.${providerName}`], env.SECRET as string, { issuer: 'directus' }) as {
verifier: string;
redirect?: string;
prompt: boolean;
};
} catch (e) {
logger.warn(`Couldn't verify OAuth2 cookie`);
throw new InvalidCredentialsException();
}

const { verifier, redirect } = tokenData;
const { verifier, redirect, prompt } = tokenData;

const authenticationService = new AuthenticationService({
accountability: {
Expand All @@ -254,6 +272,11 @@ export function createOAuth2AuthRouter(providerName: string): Router {
state: req.query.state,
});
} catch (error: any) {
// Prompt user for a new refresh_token if invalidated
if (error instanceof InvalidTokenException && !prompt) {
return res.redirect(`./?${redirect ? `redirect=${redirect}&` : ''}prompt=true`);
}

logger.warn(error);

if (redirect) {
Expand All @@ -263,6 +286,8 @@ export function createOAuth2AuthRouter(providerName: string): Router {
reason = 'SERVICE_UNAVAILABLE';
} else if (error instanceof InvalidCredentialsException) {
reason = 'INVALID_USER';
} else if (error instanceof InvalidTokenException) {
reason = 'INVALID_TOKEN';
}

return res.redirect(`${redirect.split('?')[0]}?reason=${reason}`);
Expand Down
56 changes: 40 additions & 16 deletions api/src/auth/drivers/openid.ts
View file
Open in desktop
Expand Up @@ -7,7 +7,12 @@ import { getAuthProvider } from '../../auth';
import env from '../../env';
import { AuthenticationService, UsersService } from '../../services';
import { AuthDriverOptions, User, AuthData, SessionData } from '../../types';
import { InvalidCredentialsException, ServiceUnavailableException, InvalidConfigException } from '../../exceptions';
import {
InvalidCredentialsException,
ServiceUnavailableException,
InvalidConfigException,
InvalidTokenException,
} from '../../exceptions';
import { respond } from '../../middleware/respond';
import asyncHandler from '../../utils/async-handler';
import { Url } from '../../utils/url';
Expand Down Expand Up @@ -62,17 +67,21 @@ export class OpenIDAuthDriver extends LocalAuthDriver {
return generators.codeVerifier();
}

async generateAuthUrl(codeVerifier: string): Promise<string> {
async generateAuthUrl(codeVerifier: string, prompt = false): Promise<string> {
aidenfoxx marked this conversation as resolved.
Show resolved Hide resolved
try {
const client = await this.client;
const codeChallenge = generators.codeChallenge(codeVerifier);
const paramsConfig = typeof this.config.params === 'object' ? this.config.params : {};

return client.authorizationUrl({
scope: this.config.scope ?? 'openid profile email',
access_type: 'offline',
prompt: prompt ? 'consent' : undefined,
...paramsConfig,
code_challenge: codeChallenge,
code_challenge_method: 'S256',
// Some providers require state even with PKCE
state: codeChallenge,
access_type: 'offline',
});
} catch (e) {
throw handleError(e);
Expand Down Expand Up @@ -173,25 +182,30 @@ export class OpenIDAuthDriver extends LocalAuthDriver {
}
}

if (!authData?.refreshToken) {
return sessionData;
if (authData?.refreshToken) {
try {
const client = await this.client;
const tokenSet = await client.refresh(authData.refreshToken);
// Update user refreshToken if provided
if (tokenSet.refresh_token) {
await this.usersService.updateOne(user.id, {
auth_data: JSON.stringify({ refreshToken: tokenSet.refresh_token }),
});
}
} catch (e) {
throw handleError(e);
}
}

try {
const client = await this.client;
const tokenSet = await client.refresh(authData.refreshToken);
return { accessToken: tokenSet.access_token };
} catch (e) {
throw handleError(e);
}
return sessionData;
}
}

const handleError = (e: any) => {
if (e instanceof errors.OPError) {
if (e.error === 'invalid_grant') {
// Invalid token
return new InvalidCredentialsException();
return new InvalidTokenException();
}
// Server response error
return new ServiceUnavailableException('Service returned unexpected response', {
Expand All @@ -213,7 +227,8 @@ export function createOpenIDAuthRouter(providerName: string): Router {
asyncHandler(async (req, res) => {
const provider = getAuthProvider(providerName) as OpenIDAuthDriver;
const codeVerifier = provider.generateCodeVerifier();
const token = jwt.sign({ verifier: codeVerifier, redirect: req.query.redirect }, env.SECRET as string, {
const prompt = !!req.query.prompt;
const token = jwt.sign({ verifier: codeVerifier, redirect: req.query.redirect, prompt }, env.SECRET as string, {
expiresIn: '5m',
issuer: 'directus',
});
Expand All @@ -223,7 +238,7 @@ export function createOpenIDAuthRouter(providerName: string): Router {
sameSite: 'lax',
});

return res.redirect(await provider.generateAuthUrl(codeVerifier));
return res.redirect(await provider.generateAuthUrl(codeVerifier, prompt));
}),
respond
);
Expand All @@ -237,12 +252,14 @@ export function createOpenIDAuthRouter(providerName: string): Router {
tokenData = jwt.verify(req.cookies[`openid.${providerName}`], env.SECRET as string, { issuer: 'directus' }) as {
verifier: string;
redirect?: string;
prompt: boolean;
};
} catch (e) {
logger.warn(`Couldn't verify OpenID cookie`);
throw new InvalidCredentialsException();
}

const { verifier, redirect } = tokenData;
const { verifier, redirect, prompt } = tokenData;

const authenticationService = new AuthenticationService({
accountability: {
Expand All @@ -268,6 +285,11 @@ export function createOpenIDAuthRouter(providerName: string): Router {
state: req.query.state,
});
} catch (error: any) {
// Prompt user for a new refresh_token if invalidated
if (error instanceof InvalidTokenException && !prompt) {
return res.redirect(`./?${redirect ? `redirect=${redirect}&` : ''}prompt=true`);
}

logger.warn(error);

if (redirect) {
Expand All @@ -277,6 +299,8 @@ export function createOpenIDAuthRouter(providerName: string): Router {
reason = 'SERVICE_UNAVAILABLE';
} else if (error instanceof InvalidCredentialsException) {
reason = 'INVALID_USER';
} else if (error instanceof InvalidTokenException) {
reason = 'INVALID_TOKEN';
}

return res.redirect(`${redirect.split('?')[0]}?reason=${reason}`);
Expand Down
1 change: 1 addition & 0 deletions api/src/exceptions/index.ts
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ export * from './invalid-ip';
export * from './invalid-otp';
export * from './invalid-payload';
export * from './invalid-query';
export * from './invalid-token';
export * from './method-not-allowed';
export * from './range-not-satisfiable';
export * from './route-not-found';
Expand Down
7 changes: 7 additions & 0 deletions api/src/exceptions/invalid-token.ts
View file
Open in desktop
@@ -0,0 +1,7 @@
import { BaseException } from '@directus/shared/exceptions';

export class InvalidTokenException extends BaseException {
constructor(message = 'Invalid token') {
super(message, 403, 'INVALID_TOKEN');
}
}