From 7f0ed8efe27a7c7b84f74d96aad9a284382b2600 Mon Sep 17 00:00:00 2001 From: SebastianW Date: Tue, 22 Oct 2024 12:10:16 +0200 Subject: [PATCH] login callback separate logic, session log in exception filter, manual session key setup for oidc, extended logging, session cookie domain set for production environments --- .../controllers/login-callback.controller.ts | 2 +- .../oidc/src/filters/http-exception.filter.ts | 4 +- libs/oidc/src/services/oidc.service.ts | 82 ++++++++++--------- libs/oidc/src/strategies/oidc.strategy.ts | 3 +- libs/oidc/src/utils/session/base-session.ts | 4 + 5 files changed, 52 insertions(+), 43 deletions(-) diff --git a/libs/oidc/src/controllers/login-callback.controller.ts b/libs/oidc/src/controllers/login-callback.controller.ts index 046a0e51..37fe73d7 100644 --- a/libs/oidc/src/controllers/login-callback.controller.ts +++ b/libs/oidc/src/controllers/login-callback.controller.ts @@ -10,6 +10,6 @@ export class LoginCallbackController { @Public() @Get() loginCallback(@Req() req: Request, @Res() res: Response, @Next() next: Function, @Param() params) { - this.oidcService.login(req, res, next, params); + this.oidcService.loginCallback(req, res, next, params); } } diff --git a/libs/oidc/src/filters/http-exception.filter.ts b/libs/oidc/src/filters/http-exception.filter.ts index da286bea..fae195e6 100644 --- a/libs/oidc/src/filters/http-exception.filter.ts +++ b/libs/oidc/src/filters/http-exception.filter.ts @@ -15,9 +15,9 @@ export class HttpExceptionFilter implements ExceptionFilter { const request = ctx.getRequest(); const status = exception instanceof HttpException ? exception.getStatus() : HttpStatus.INTERNAL_SERVER_ERROR; - const { body, headers, method, params, query, url, user } = request; + const { body, headers, method, params, query, url, user, session } = request; - this.logger.error({ request: { body, headers, method, params, query, url, user }, exception }); + this.logger.error({ request: { body, headers, method, params, query, url, user, session }, exception }); switch (status) { case MisdirectedStatus.MISDIRECTED: diff --git a/libs/oidc/src/services/oidc.service.ts b/libs/oidc/src/services/oidc.service.ts index c2d7d626..e7fac6b4 100644 --- a/libs/oidc/src/services/oidc.service.ts +++ b/libs/oidc/src/services/oidc.service.ts @@ -16,15 +16,18 @@ import passport = require('passport'); // const url = require('url'); const _loginCallbackPath = 'login/callback'; +// const _westEU = 'westeurope'; +// const _northEU = 'northeurope'; @Injectable() export class OidcService implements OnModuleInit { readonly logger = new Logger(OidcService.name); + #instanceID: string; + #oidc_redirect_uri: string; #region: string; - #redirect_uri: string; + #sessionKey: string; isMultitenant: boolean = false; strategy: any; - #instanceID: string; idpInfos: { [tokenName: string]: { trustIssuer: Issuer; @@ -42,7 +45,8 @@ export class OidcService implements OnModuleInit { this.isMultitenant = !!this.options.issuerOrigin; this.#instanceID = configService.get('SERVER_INSTANCE_ID'); this.#region = configService.get('REGION_NAME'); - this.#redirect_uri = configService.get('OIDC_REDIRECT_URI') ? `${configService.get('OIDC_REDIRECT_URI')}/${_loginCallbackPath}` : `${this.options.origin}/${_loginCallbackPath}`; + this.#sessionKey = `oidc:${configService.get('APIM_BASE_URL').replace('https://', '')}`; + this.#oidc_redirect_uri = configService.get('OIDC_REDIRECT_URI') ? `${configService.get('OIDC_REDIRECT_URI')}` : `${this.options.origin}`; } async onModuleInit() { @@ -85,10 +89,13 @@ export class OidcService implements OnModuleInit { tokenStore, strategy, }; - this.options.authParams.redirect_uri = this.#redirect_uri; + this.options.authParams.redirect_uri = `${this.#oidc_redirect_uri}/${_loginCallbackPath}`; this.options.authParams.nonce = this.options.authParams.nonce === 'true' ? uuid() : this.options.authParams.nonce; - strategy = new OidcStrategy(this, key, channelType); + // const hostname = url.parse(trustIssuer.issuer).hostname; + // this.#sessionKey = this.#buildSessionKey({ key: this.#sessionKey, hostname }); + + strategy = new OidcStrategy(this, key, this.#sessionKey, channelType); this.idpInfos[key].strategy = strategy; return strategy; @@ -126,16 +133,16 @@ export class OidcService implements OnModuleInit { } async login(@Req() req: Request, @Res() res: Response, @Next() next: Function, @Param() params) { - this.#sessionLog({ name: 'LOGIN', req, instanceID: this.#instanceID, region: this.#region }); + this.logger.log(this.#sessionLog({ name: 'LOGIN', req, instanceID: this.#instanceID, region: this.#region })); try { const tenantId = params.tenantId || req.session['tenant']; const channel = this.options.channelType || params.channelType || req.session['channel']; - const strategy = - this.strategy || - (this.idpInfos[this.getIdpInfosKey(tenantId, channel)] && - this.idpInfos[this.getIdpInfosKey(tenantId, channel)].strategy) || - (await this.createStrategy(tenantId, channel)); + // const strategy = + // this.strategy || + // (this.idpInfos[this.getIdpInfosKey(tenantId, channel)] && + // this.idpInfos[this.getIdpInfosKey(tenantId, channel)].strategy) || + // (await this.createStrategy(tenantId, channel)); const prefix = channel && tenantId ? (this.options.channelType ? `/${tenantId}` : `/${tenantId}/${channel}`) : ''; @@ -166,7 +173,7 @@ export class OidcService implements OnModuleInit { JSON.stringify({ redirect_url: `${prefix}${redirect_url}`, loginpopup: loginpopup }), 'utf-8', ).toString('base64'); - this.#sessionLog({ name: 'PRE_AUTHENTICATE', req, instanceID: this.#instanceID, region: this.#region }); + this.logger.log(this.#sessionLog({ name: 'PRE_AUTHENTICATE', req, instanceID: this.#instanceID, region: this.#region })); passport.authenticate( Object.create(strategy), @@ -178,6 +185,7 @@ export class OidcService implements OnModuleInit { (err, user, info) => { if (err || !user) { this.logger.error(this.#sessionLog({ name: 'AUTHENTICATE ERROR', req, instanceID: this.#instanceID, region: this.#region })); + this.logger.log(`AUTHENTICATE ERROR INFO`, JSON.stringify(info)) return next(err || info); } this.logger.log(this.#sessionLog({ name: 'PRE_LOGIN_REQ', req, instanceID: this.#instanceID, region: this.#region })); @@ -190,8 +198,6 @@ export class OidcService implements OnModuleInit { let state = req.query['state'] as string; const buff = Buffer.from(state, 'base64').toString('utf-8'); state = JSON.parse(buff); - let url: string = state['redirect_url']; - url = !url.startsWith('/') ? `/${url}` : url; const loginpopup = state['loginpopup']; this.logger.log(this.#sessionLog({ name: 'PRE_REDIRECT', req, instanceID: this.#instanceID, region: this.#region })); if (loginpopup) { @@ -201,7 +207,12 @@ export class OidcService implements OnModuleInit { `); } else { - this.logger.log(this.#sessionLog({ name: 'REDIRECT', req, instanceID: this.#instanceID, region: this.#region })); + this.logger.log(this.#sessionLog({ name: 'REDIRECT AFTER LOGIN CALLBACK', req, instanceID: this.#instanceID, region: this.#region })); + let url: string = state['redirect_url']; + // TODO: redirecting from region url to app prefix url with callback query params works, but also could lead to infinite loop so solve initial 500 first, then deal with redirect + // let redirect_url = this.#oidc_redirect_uri.replace(`-${this.#region}`, '').replace(_loginCallbackPath, ''); + // url = `${redirect_url}/${url}`; + url = !url.startsWith('/') ? `/${url}` : url; return res.redirect(url); } }); @@ -219,26 +230,20 @@ export class OidcService implements OnModuleInit { * @param res * @param next * @param params - * @returns redirect to other region when session details not found, login method when session params are present + * @returns redirect to other region when session details not found */ - // async loginCallback(@Req() req: Request, @Res() res: Response, @Next() next: Function, @Param() params) { - // this.logger.log(this.#sessionLog({ name: 'LOGIN CALLBACK', req, instanceID: this.#instanceID, region: this.#region })); - - // const tenantId = params.tenantId || req.session['tenant']; - // const channel = this.options.channelType || params.channelType || req.session['channel']; - // const key = this.getIdpInfosKey(tenantId, channel); - // const issuer = this.idpInfos[key].trustIssuer.issuer; - // const sessionKey = `oidc:${url.parse(issuer).hostname}`; - // const session = req.session[sessionKey]; - - // if (Object.keys(session || {}).length === 0) { - // const url = this.#getRegionToRedirect(this.#region, this.#redirect_uri); - // this.logger.log(this.#sessionLog({ name: 'REGION REDIRECT', req, instanceID: this.#instanceID, region: this.#region })); - // return res.redirect(url); - // } else { - // return this.login(req, res, next, params); - // } - // } + async loginCallback(@Req() req: Request, @Res() res: Response, @Next() next: Function, @Param() params) { + this.logger.log(this.#sessionLog({ name: 'LOGIN CALLBACK', req, instanceID: this.#instanceID, region: this.#region })); + const session = req.session[this.#sessionKey]; + if (Object.keys(session || {}).length === 0) { + // const url = this.#getRegionToRedirect(this.#region, this.#oidc_redirect_uri); + this.logger.log(this.#sessionLog({ name: 'SESSION EMPTY', req, instanceID: this.#instanceID, region: this.#region })); + // return res.redirect(url); + } + // } else { + return this.login(req, res, next, params); + // } + } async logout(@Req() req: Request, @Res() res: Response, @Param() params) { const id_token = req.user ? req.user['id_token'] : undefined; @@ -382,15 +387,14 @@ export class OidcService implements OnModuleInit { return [tenantPrefix, channelPrefix].filter(Boolean).join('/'); } - // #getRegionToRedirect(region = this.#region, redirectUri = this.#redirect_uri): string { - // const westEU: string = 'westeurope'; - // const northEU: string = 'northeurope'; - // const redirectRegion = region === westEU ? northEU : westEU; + // #getRegionToRedirect(region = this.#region, redirectUri = this.#oidc_redirect_uri): string { + // const redirectRegion = region === _westEU ? _northEU : _westEU; // return redirectUri.replace(this.#region, redirectRegion); // } #sessionLog({ name, req, instanceID = this.#instanceID, region = this.#region }: LogParams = {} as LogParams): string { - return `${name.toLocaleUpperCase()} url: ${req.url}, session: ${JSON.stringify(req.session)}, ip: ${req.ip}, method: ${req.method}, instanceID: ${instanceID}, region: ${region}`; + const session = req.session[this.#sessionKey] ?? {}; + return `${name.toLocaleUpperCase()} url: ${req.url}, oidc_session: ${JSON.stringify(session)}, method: ${req.method}, instanceID: ${instanceID}, region: ${region}`; } } diff --git a/libs/oidc/src/strategies/oidc.strategy.ts b/libs/oidc/src/strategies/oidc.strategy.ts index 64d395ef..6a0914a1 100644 --- a/libs/oidc/src/strategies/oidc.strategy.ts +++ b/libs/oidc/src/strategies/oidc.strategy.ts @@ -10,12 +10,13 @@ export class OidcStrategy extends PassportStrategy(Strategy, 'oidc') { userInfoCallback: any; - constructor(private oidcService: OidcService, private idpKey: string, private channelType?: ChannelType) { + constructor(private oidcService: OidcService, private idpKey: string, sessionKey: string, private channelType?: ChannelType) { super({ client: oidcService.idpInfos[idpKey].client, params: oidcService.options.authParams, passReqToCallback: false, usePKCE: oidcService.options.usePKCE, + sessionKey }); this.userInfoCallback = oidcService.options.userInfoCallback; } diff --git a/libs/oidc/src/utils/session/base-session.ts b/libs/oidc/src/utils/session/base-session.ts index 211e0459..0a473de1 100644 --- a/libs/oidc/src/utils/session/base-session.ts +++ b/libs/oidc/src/utils/session/base-session.ts @@ -18,4 +18,8 @@ let session = { // session.cookie.secure = true; // https only // } +if (process.env.NODE_ENV !== 'developement') { + session.cookie['domain'] = process.env.DNS_DOMAIN; +} + export const baseSession = session as SessionOptions;