Archives/gristlabs_grist-core
1
0
Fork 0
mirror of https://github.com/gristlabs/grist-core.git synced 2024-10-27 20:44:07 +00:00
Code Issues Projects Releases Wiki Activity

Issue 740 OIDC login redirect (#742)

Browse Source 
* Fix OIDC redirects from team site to personal page after login #740

Also:
 - compare state in session and state passed through parameters
 (otherwise the state won't have any effect regarding the security).
 - delete the session even after an authentication failure

* More logs for OIDC #740

---------

Co-authored-by: Florent FAYOLLE <florent.fayolle@beta.gouv.fr>
This commit is contained in:
Florent 2023-11-15 15:23:32 +01:00 committed by GitHub
parent ffec5265f2
commit e8789e6531
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/server/lib/BrowserSession.ts
View File

@ -72,6 +72,8 @@ export interface SessionObj {
oidc?: { oidc?: {
// codeVerifier is used during OIDC authentication, to protect against attacks like CSRF. // codeVerifier is used during OIDC authentication, to protect against attacks like CSRF.
codeVerifier?: string; codeVerifier?: string;
state?: string;
targetUrl?: string;
} }
} }

32
app/server/lib/OIDCConfig.ts
View File

@ -78,6 +78,7 @@ export class OIDCConfig {
redirect_uris: [ this._redirectUrl ], redirect_uris: [ this._redirectUrl ],
response_types: [ 'code' ], response_types: [ 'code' ],
}); });
log.info(`OIDCConfig: initialized with issuer ${issuerUrl}`);
} }
public addEndpoints(app: express.Application, sessions: Sessions): void { public addEndpoints(app: express.Application, sessions: Sessions): void {
@ -85,15 +86,18 @@ export class OIDCConfig {
} }
public async handleCallback(sessions: Sessions, req: express.Request, res: express.Response): Promise<void> { public async handleCallback(sessions: Sessions, req: express.Request, res: express.Response): Promise<void> {
const mreq = req as RequestWithLogin;
try { try {
const params = this._client.callbackParams(req); const params = this._client.callbackParams(req);
const { state } = params; const { state, targetUrl } = mreq.session?.oidc ?? {};
if (!state) { if (!state) {
throw new Error('Login or logout failed to complete'); throw new Error('Login or logout failed to complete');
} }
const codeVerifier = await this._retrieveCodeVerifierFromSession(req); const codeVerifier = await this._retrieveCodeVerifierFromSession(req);
// The callback function will compare the state present in the params and the one we retrieved from the session.
// If they don't match, it will throw an error.
const tokenSet = await this._client.callback( const tokenSet = await this._client.callback(
this._redirectUrl, this._redirectUrl,
params, params,
@ -102,23 +106,29 @@ export class OIDCConfig {
const userInfo = await this._client.userinfo(tokenSet); const userInfo = await this._client.userinfo(tokenSet);
const profile = this._makeUserProfileFromUserInfo(userInfo); const profile = this._makeUserProfileFromUserInfo(userInfo);
log.info(`OIDCConfig: got OIDC response for ${profile.email} (${profile.name}) redirecting to ${targetUrl}`);
const scopedSession = sessions.getOrCreateSessionFromRequest(req); const scopedSession = sessions.getOrCreateSessionFromRequest(req);
await scopedSession.operateOnScopedSession(req, async (user) => Object.assign(user, { await scopedSession.operateOnScopedSession(req, async (user) => Object.assign(user, {
profile, profile,
})); }));
res.redirect('/'); delete mreq.session.oidc;
res.redirect(targetUrl ?? '/');
} catch (err) { } catch (err) {
log.error(`OIDC callback failed: ${err.message}`); log.error(`OIDC callback failed: ${err.stack}`);
res.status(500).send(`OIDC callback failed: ${err.message}`); // Delete the session data even if the login failed.
// This way, we prevent several login attempts.
//
// Also session deletion must be done before sending the response.
delete mreq.session.oidc;
res.status(500).send(`OIDC callback failed.`);
} }
} }
public async getLoginRedirectUrl(req: express.Request): Promise<string> { public async getLoginRedirectUrl(req: express.Request, targetUrl: URL): Promise<string> {
const codeVerifier = await this._generateAndStoreCodeVerifier(req); const { codeVerifier, state } = await this._generateAndStoreConnectionInfo(req, targetUrl.href);
const codeChallenge = generators.codeChallenge(codeVerifier); const codeChallenge = generators.codeChallenge(codeVerifier);
const state = generators.state();
const authUrl = this._client.authorizationUrl({ const authUrl = this._client.authorizationUrl({
scope: process.env.GRIST_OIDC_IDP_SCOPES || 'openid email profile', scope: process.env.GRIST_OIDC_IDP_SCOPES || 'openid email profile',
@ -135,15 +145,18 @@ export class OIDCConfig {
}); });
} }
private async _generateAndStoreCodeVerifier(req: express.Request) { private async _generateAndStoreConnectionInfo(req: express.Request, targetUrl: string) {
const mreq = req as RequestWithLogin; const mreq = req as RequestWithLogin;
if (!mreq.session) { throw new Error('no session available'); } if (!mreq.session) { throw new Error('no session available'); }
const codeVerifier = generators.codeVerifier(); const codeVerifier = generators.codeVerifier();
const state = generators.state();
mreq.session.oidc = { mreq.session.oidc = {
codeVerifier, codeVerifier,
state,
targetUrl
}; };
return codeVerifier; return { codeVerifier, state };
} }
private async _retrieveCodeVerifierFromSession(req: express.Request) { private async _retrieveCodeVerifierFromSession(req: express.Request) {
@ -151,7 +164,6 @@ export class OIDCConfig {
if (!mreq.session) { throw new Error('no session available'); } if (!mreq.session) { throw new Error('no session available'); }
const codeVerifier = mreq.session.oidc?.codeVerifier; const codeVerifier = mreq.session.oidc?.codeVerifier;
if (!codeVerifier) { throw new Error('Login is stale'); } if (!codeVerifier) { throw new Error('Login is stale'); }
delete mreq.session.oidc?.codeVerifier;
return codeVerifier; return codeVerifier;
} }