From 1665dbbb46f2b08dce7da0cd4c3c61e13c3c1e1c Mon Sep 17 00:00:00 2001 From: Govindarajan Nagarajan Date: Wed, 26 Aug 2020 13:29:53 +0200 Subject: [PATCH 1/2] Update Refresh Token, if provided by the server in the refresh grant The OAuth2 specifications specify that during the refresh grant, the server may provide an alternate refresh Token along with the access Token when performing a token refresh. The oauth2 provider in the `auth-backend` ignores the newer refresh token during the refresh grant. This commit will update the refresh token cookie at the end of the call to the /auth/oauth2/refresh endpoint if the server provides a new refresh token grant a --- plugins/auth-backend/src/lib/OAuthProvider.ts | 7 +++++++ .../auth-backend/src/lib/PassportStrategyHelper.ts | 4 ++-- plugins/auth-backend/src/providers/oauth2/provider.ts | 11 +++++++---- plugins/auth-backend/src/providers/types.ts | 8 ++++++++ 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/plugins/auth-backend/src/lib/OAuthProvider.ts b/plugins/auth-backend/src/lib/OAuthProvider.ts index f2b8c8e09c..8e9cd510b3 100644 --- a/plugins/auth-backend/src/lib/OAuthProvider.ts +++ b/plugins/auth-backend/src/lib/OAuthProvider.ts @@ -258,6 +258,13 @@ export class OAuthProvider implements AuthProviderRouteHandlers { await this.populateIdentity(response.backstageIdentity); + if ( + response.providerInfo.refreshToken && + response.providerInfo.refreshToken !== refreshToken + ) { + this.setRefreshTokenCookie(res, response.providerInfo.refreshToken); + } + res.send(response); } catch (error) { res.status(401).send(`${error.message}`); diff --git a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts index 4cc0de4444..9a169f9696 100644 --- a/plugins/auth-backend/src/lib/PassportStrategyHelper.ts +++ b/plugins/auth-backend/src/lib/PassportStrategyHelper.ts @@ -45,7 +45,6 @@ export const makeProfileInfo = ( if ((!email || !picture) && idToken) { try { const decoded: Record = jwtDecoder(idToken); - if (!email && decoded.email) { email = decoded.email; } @@ -133,7 +132,7 @@ export const executeRefreshTokenStrategy = async ( ( err: Error | null, accessToken: string, - _refreshToken: string, + newRefreshToken: string, params: any, ) => { if (err) { @@ -149,6 +148,7 @@ export const executeRefreshTokenStrategy = async ( resolve({ accessToken, + refreshToken: newRefreshToken, params, }); }, diff --git a/plugins/auth-backend/src/providers/oauth2/provider.ts b/plugins/auth-backend/src/providers/oauth2/provider.ts index c03f7c4371..3a8982c680 100644 --- a/plugins/auth-backend/src/providers/oauth2/provider.ts +++ b/plugins/auth-backend/src/providers/oauth2/provider.ts @@ -55,6 +55,7 @@ export class OAuth2AuthProvider implements OAuthProviderHandlers { done: PassportDoneCallback, ) => { const profile = makeProfileInfo(rawProfile, params.id_token); + done( undefined, { @@ -101,21 +102,24 @@ export class OAuth2AuthProvider implements OAuthProviderHandlers { } async refresh(refreshToken: string, scope: string): Promise { - const { accessToken, params } = await executeRefreshTokenStrategy( + const refreshTokenResponse = await executeRefreshTokenStrategy( this._strategy, refreshToken, scope, ); + const { accessToken, params } = refreshTokenResponse; + const updatedRefreshToken = refreshTokenResponse.refreshToken; const profile = await executeFetchUserProfileStrategy( this._strategy, - accessToken, - params.id_token, + refreshTokenResponse.accessToken, + refreshTokenResponse.params.id_token, ); return this.populateIdentity({ providerInfo: { accessToken, + refreshToken: updatedRefreshToken, idToken: params.id_token, expiresInSeconds: params.expires_in, scope: params.scope, @@ -134,7 +138,6 @@ export class OAuth2AuthProvider implements OAuthProviderHandlers { if (!profile.email) { throw new Error('Profile does not contain a profile'); } - const id = profile.email.split('@')[0]; return { ...response, backstageIdentity: { id } }; diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index d372fd1b94..0d4b7c656a 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -259,6 +259,10 @@ export type OAuthProviderInfo = { * Scopes granted for the access token. */ scope: string; + /** + * A refresh token issued for the signed in user + */ + refreshToken?: string; }; export type OAuthPrivateInfo = { @@ -326,6 +330,10 @@ export type RefreshTokenResponse = { * An access token issued for the signed in user. */ accessToken: string; + /** + * Optionally, the server can issue a new Refresh Token for the user + */ + refreshToken?: string; params: any; }; From 9da9a1c94e4744d9887ffc4f54f0d234ff2871d4 Mon Sep 17 00:00:00 2001 From: Govindarajan Nagarajan Date: Thu, 27 Aug 2020 11:08:23 +0200 Subject: [PATCH 2/2] refactor: styling and variable naming --- plugins/auth-backend/src/providers/oauth2/provider.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/plugins/auth-backend/src/providers/oauth2/provider.ts b/plugins/auth-backend/src/providers/oauth2/provider.ts index 3a8982c680..d74e40bf64 100644 --- a/plugins/auth-backend/src/providers/oauth2/provider.ts +++ b/plugins/auth-backend/src/providers/oauth2/provider.ts @@ -107,13 +107,16 @@ export class OAuth2AuthProvider implements OAuthProviderHandlers { refreshToken, scope, ); - const { accessToken, params } = refreshTokenResponse; - const updatedRefreshToken = refreshTokenResponse.refreshToken; + const { + accessToken, + params, + refreshToken: updatedRefreshToken, + } = refreshTokenResponse; const profile = await executeFetchUserProfileStrategy( this._strategy, - refreshTokenResponse.accessToken, - refreshTokenResponse.params.id_token, + accessToken, + params.id_token, ); return this.populateIdentity({