From 7a665c3c2b4b18bb1c4f5ed747058a98a5fd8533 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Fri, 29 May 2020 15:16:10 +0200 Subject: [PATCH 1/5] Add github as an auth provider to the backend --- plugins/auth-backend/package.json | 14 ++-- plugins/auth-backend/src/providers/config.ts | 8 ++ .../auth-backend/src/providers/factories.ts | 2 + .../src/providers/github/index.ts | 17 ++++ .../src/providers/github/provider.ts | 84 +++++++++++++++++++ yarn.lock | 33 ++++---- 6 files changed, 137 insertions(+), 21 deletions(-) create mode 100644 plugins/auth-backend/src/providers/github/index.ts create mode 100644 plugins/auth-backend/src/providers/github/provider.ts diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 9cb8017f15..505240853d 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -16,21 +16,23 @@ }, "dependencies": { "@backstage/backend-common": "^0.1.1-alpha.6", + "@types/cookie-parser": "^1.4.2", + "@types/passport": "^1.0.3", + "@types/passport-github2": "^1.2.4", + "@types/passport-google-oauth20": "^2.0.3", "compression": "^1.7.4", + "cookie-parser": "^1.4.5", "cors": "^2.8.5", "express": "^4.17.1", "express-promise-router": "^3.0.3", "fs-extra": "^9.0.0", "helmet": "^3.22.0", "morgan": "^1.10.0", - "winston": "^3.2.1", - "yn": "^4.0.0", "passport": "^0.4.1", + "passport-github2": "^0.1.12", "passport-google-oauth20": "^2.0.0", - "cookie-parser": "^1.4.5", - "@types/passport": "^1.0.3", - "@types/passport-google-oauth20": "^2.0.3", - "@types/cookie-parser": "^1.4.2" + "winston": "^3.2.1", + "yn": "^4.0.0" }, "devDependencies": { "@backstage/cli": "^0.1.1-alpha.6", diff --git a/plugins/auth-backend/src/providers/config.ts b/plugins/auth-backend/src/providers/config.ts index 45098afb89..dad87e0448 100644 --- a/plugins/auth-backend/src/providers/config.ts +++ b/plugins/auth-backend/src/providers/config.ts @@ -23,4 +23,12 @@ export const providers = [ callbackURL: 'http://localhost:7000/auth/google/handler/frame', }, }, + { + provider: 'github', + options: { + clientID: process.env.AUTH_GITHUB_CLIENT_ID!, + clientSecret: process.env.AUTH_GITHUB_CLIENT_SECRET!, + callbackURL: 'http://localhost:7000/auth/github/handler/frame', + }, + }, ]; diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts index 0a1e639082..10e4153714 100644 --- a/plugins/auth-backend/src/providers/factories.ts +++ b/plugins/auth-backend/src/providers/factories.ts @@ -16,10 +16,12 @@ import { AuthProviderFactories, AuthProviderFactory } from './types'; import { GoogleAuthProvider } from './google'; +import { GithubAuthProvider } from './github'; export class ProviderFactories { private static readonly providerFactories: AuthProviderFactories = { google: GoogleAuthProvider, + github: GithubAuthProvider, }; public static getProviderFactory(providerId: string): AuthProviderFactory { diff --git a/plugins/auth-backend/src/providers/github/index.ts b/plugins/auth-backend/src/providers/github/index.ts new file mode 100644 index 0000000000..c3a48d35e0 --- /dev/null +++ b/plugins/auth-backend/src/providers/github/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { GithubAuthProvider } from './provider'; diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts new file mode 100644 index 0000000000..1e8022e1cc --- /dev/null +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -0,0 +1,84 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import express from 'express'; +import { Strategy as GithubStrategy } from 'passport-github2'; +import { + executeFrameHandlerStrategy, + executeRedirectStrategy, + executeRefreshTokenStrategy, +} from '../PassportStrategyHelper'; +import { + OAuthProviderHandlers, + AuthProviderConfig, + RedirectInfo, + AuthInfoBase, + AuthInfoPrivate, +} from '../types'; + +export class GithubAuthProvider implements OAuthProviderHandlers { + private readonly providerConfig: AuthProviderConfig; + private readonly _strategy: GithubStrategy; + + constructor(providerConfig: AuthProviderConfig) { + this.providerConfig = providerConfig; + this._strategy = new GithubStrategy( + { ...this.providerConfig.options }, + ( + accessToken: any, + refreshToken: any, + params: any, + profile: any, + done: any, + ) => { + done( + undefined, + { + profile, + accessToken, + scope: 'user', // params.scope is an empty string here for some reason, so hardcoding for now + expiresInSeconds: params.expires_in, + }, + { refreshToken }, + ); + }, + ); + } + + async start(req: express.Request, options: any): Promise { + return await executeRedirectStrategy(req, this._strategy, options); + } + + async handler( + req: express.Request, + ): Promise<{ user: AuthInfoBase; info: AuthInfoPrivate }> { + return await executeFrameHandlerStrategy(req, this._strategy); + } + + async refresh(refreshToken: string, scope: string): Promise { + const { accessToken, params } = await executeRefreshTokenStrategy( + this._strategy, + refreshToken, + scope, + ); + + return { + accessToken, + expiresInSeconds: params.expires_in, + scope: params.scope, + }; + } +} diff --git a/yarn.lock b/yarn.lock index 8439f5b0b9..30537a6cac 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4281,6 +4281,15 @@ resolved "https://registry.npmjs.org/@types/parse-json/-/parse-json-4.0.0.tgz#2f8bb441434d163b35fb8ffdccd7138927ffb8c0" integrity sha512-//oorEZjL6sbPcKUaCdIGlIUeH26mgzimjBB77G6XRgnDl/L5wOnpyBGRe/Mmf5CVW3PwEBE1NjiMZ/ssFh4wA== +"@types/passport-github2@^1.2.4": + version "1.2.4" + resolved "https://registry.npmjs.org/@types/passport-github2/-/passport-github2-1.2.4.tgz#f56c386d1fe6435e359430e57adc1747a627bd86" + integrity sha512-dtGtA0Uyzk6ne3SrgQi/I1ClClLE3i7JmSiMaJgkGH8v1nbE9JdBpG7QWJ1XPlLdcf7EvoPdHmkWN2+Kln9y8g== + dependencies: + "@types/express" "*" + "@types/passport" "*" + "@types/passport-oauth2" "*" + "@types/passport-google-oauth20@^2.0.3": version "2.0.3" resolved "https://registry.npmjs.org/@types/passport-google-oauth20/-/passport-google-oauth20-2.0.3.tgz#f554ff6d39f395acff3f1d762e54462194dac8da" @@ -4290,15 +4299,7 @@ "@types/passport" "*" "@types/passport-oauth2" "*" -"@types/passport-oauth2-refresh@^1.1.1": - version "1.1.1" - resolved "https://registry.npmjs.org/@types/passport-oauth2-refresh/-/passport-oauth2-refresh-1.1.1.tgz#cbe466d4fcac36182fd75bf55279c0b1e953c382" - integrity sha512-Tw0JvfDPv9asgFPACd9oOGCaD/0/Uyi+QF7fmrJC74cJKC6I8N8wwhJJHyfd1N2E/qaLgTh431lhOa9jicpNdg== - dependencies: - "@types/oauth" "*" - "@types/passport-oauth2" "*" - -"@types/passport-oauth2@*", "@types/passport-oauth2@^1.4.9": +"@types/passport-oauth2@*": version "1.4.9" resolved "https://registry.npmjs.org/@types/passport-oauth2/-/passport-oauth2-1.4.9.tgz#134007c4b505a82548c9cb19094c5baeb2205c92" integrity sha512-QP0q+NVQOaIu2r0e10QWkiUA0Ya5mOBHRJN0UrI+LolMLOP1/VN4EVIpJ3xVwFo+xqNFRoFvFwJhBvKnk7kpUA== @@ -16103,6 +16104,13 @@ pascalcase@^0.1.1: resolved "https://registry.npmjs.org/pascalcase/-/pascalcase-0.1.1.tgz#b363e55e8006ca6fe21784d2db22bd15d7917f14" integrity sha1-s2PlXoAGym/iF4TS2yK9FdeRfxQ= +passport-github2@^0.1.12: + version "0.1.12" + resolved "https://registry.npmjs.org/passport-github2/-/passport-github2-0.1.12.tgz#a72ebff4fa52a35bc2c71122dcf470d1116f772c" + integrity sha512-3nPUCc7ttF/3HSP/k9sAXjz3SkGv5Nki84I05kSQPo01Jqq1NzJACgMblCK0fGcv9pKCG/KXU3AJRDGLqHLoIw== + dependencies: + passport-oauth2 "1.x.x" + passport-google-oauth20@^2.0.0: version "2.0.0" resolved "https://registry.npmjs.org/passport-google-oauth20/-/passport-google-oauth20-2.0.0.tgz#0d241b2d21ebd3dc7f2b60669ec4d587e3a674ef" @@ -16110,12 +16118,7 @@ passport-google-oauth20@^2.0.0: dependencies: passport-oauth2 "1.x.x" -passport-oauth2-refresh@^2.0.0: - version "2.0.0" - resolved "https://registry.npmjs.org/passport-oauth2-refresh/-/passport-oauth2-refresh-2.0.0.tgz#7b19c77ff3cc000819c69f6ad9e318450f57b85e" - integrity sha512-yXvCB6nem/O+WThhiyI3TlPXpzSGY+9+hy9OTx9QF8e9GInplyRHxHaaOhFylKvnof9UmWHAufQFZk8cO1Fb2g== - -passport-oauth2@1.x.x, passport-oauth2@^1.5.0: +passport-oauth2@1.x.x: version "1.5.0" resolved "https://registry.npmjs.org/passport-oauth2/-/passport-oauth2-1.5.0.tgz#64babbb54ac46a4dcab35e7f266ed5294e3c4108" integrity sha512-kqBt6vR/5VlCK8iCx1/KpY42kQ+NEHZwsSyt4Y6STiNjU+wWICG1i8ucc1FapXDGO15C5O5VZz7+7vRzrDPXXQ== From d3055d432162a17d06bc97a37d6fcab3053e6e67 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Fri, 29 May 2020 15:24:38 +0200 Subject: [PATCH 2/5] Add github to auth api --- packages/app/src/apis.ts | 11 ++ .../auth/github/GithubAuth.test.ts | 31 +++++ .../implementations/auth/github/GithubAuth.ts | 118 ++++++++++++++++++ .../apis/implementations/auth/github/index.ts | 18 +++ .../apis/implementations/auth/github/types.ts | 21 ++++ .../src/apis/implementations/auth/index.ts | 1 + 6 files changed, 200 insertions(+) create mode 100644 packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts create mode 100644 packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts create mode 100644 packages/core-api/src/apis/implementations/auth/github/index.ts create mode 100644 packages/core-api/src/apis/implementations/auth/github/types.ts diff --git a/packages/app/src/apis.ts b/packages/app/src/apis.ts index 8d76c75d91..d6658c891c 100644 --- a/packages/app/src/apis.ts +++ b/packages/app/src/apis.ts @@ -25,9 +25,11 @@ import { featureFlagsApiRef, FeatureFlags, GoogleAuth, + GithubAuth, oauthRequestApiRef, OAuthRequestManager, googleAuthApiRef, + githubAuthApiRef, } from '@backstage/core'; import { @@ -63,6 +65,15 @@ builder.add( }), ); +builder.add( + githubAuthApiRef, + GithubAuth.create({ + apiOrigin: 'http://localhost:7000', + basePath: '/auth/', + oauthRequestApi, + }), +); + builder.add( techRadarApiRef, new TechRadar({ diff --git a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts new file mode 100644 index 0000000000..3d3da266fc --- /dev/null +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts @@ -0,0 +1,31 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import GithubAuth from './GithubAuth'; + +const theFuture = new Date(Date.now() + 3600000); + +describe('GithubAuth', () => { + it('should get refreshed access token', async () => { + const getSession = jest + .fn() + .mockResolvedValue({ accessToken: 'access-token', expiresAt: theFuture }); + const githubAuth = new GithubAuth({ getSession } as any); + + expect(await githubAuth.getAccessToken()).toBe('access-token'); + expect(getSession).toBeCalledTimes(1); + }); +}); diff --git a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts new file mode 100644 index 0000000000..d20eaa54cb --- /dev/null +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -0,0 +1,118 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import GithubIcon from '@material-ui/icons/AcUnit'; +import { DefaultAuthConnector } from '../../../../lib/AuthConnector'; +import { GithubSession } from './types'; +import { OAuthApi, AccessTokenOptions } from '../../../definitions/auth'; +import { OAuthRequestApi, AuthProvider } from '../../../definitions'; +import { SessionManager } from '../../../../lib/AuthSessionManager/types'; +import { RefreshingAuthSessionManager } from '../../../../lib/AuthSessionManager'; + +type CreateOptions = { + // TODO(Rugvip): These two should be grabbed from global config when available, they're not unique to GithubAuth + apiOrigin: string; + basePath: string; + + oauthRequestApi: OAuthRequestApi; + + environment?: string; + provider?: AuthProvider & { id: string }; +}; + +export type GithubAuthResponse = { + accessToken: string; + idToken: string; + scope: string; + expiresInSeconds: number; +}; + +const DEFAULT_PROVIDER = { + id: 'github', + title: 'Github', + icon: GithubIcon, +}; + +class GithubAuth implements OAuthApi { + static create({ + apiOrigin, + basePath, + environment = 'dev', + provider = DEFAULT_PROVIDER, + oauthRequestApi, + }: CreateOptions) { + const connector = new DefaultAuthConnector({ + apiOrigin, + basePath, + environment, + provider, + oauthRequestApi: oauthRequestApi, + sessionTransform(res: GithubAuthResponse): GithubSession { + return { + accessToken: res.accessToken, + scopes: GithubAuth.normalizeScopes(res.scope), + expiresAt: new Date(Date.now() + res.expiresInSeconds * 1000), + }; + }, + }); + + const sessionManager = new RefreshingAuthSessionManager({ + connector, + defaultScopes: new Set(['user']), + sessionScopes: session => session.scopes, + sessionShouldRefresh: session => { + const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; + return expiresInSec < 60 * 5; + }, + }); + + return new GithubAuth(sessionManager); + } + + constructor(private readonly sessionManager: SessionManager) {} + + async getAccessToken( + scope?: string | string[], + options?: AccessTokenOptions, + ) { + const normalizedScopes = GithubAuth.normalizeScopes(scope); + const session = await this.sessionManager.getSession({ + ...options, + scopes: normalizedScopes, + }); + if (session) { + return session.accessToken; + } + return ''; + } + + async logout() { + await this.sessionManager.removeSession(); + } + + static normalizeScopes(scopes?: string | string[]): Set { + if (!scopes) { + return new Set(); + } + + const scopeList = Array.isArray(scopes) + ? scopes + : scopes.split(/[\s]/).filter(Boolean); + + return new Set(scopeList); + } +} +export default GithubAuth; diff --git a/packages/core-api/src/apis/implementations/auth/github/index.ts b/packages/core-api/src/apis/implementations/auth/github/index.ts new file mode 100644 index 0000000000..9e1722f4a4 --- /dev/null +++ b/packages/core-api/src/apis/implementations/auth/github/index.ts @@ -0,0 +1,18 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export * from './types'; +export { default as GithubAuth } from './GithubAuth'; diff --git a/packages/core-api/src/apis/implementations/auth/github/types.ts b/packages/core-api/src/apis/implementations/auth/github/types.ts new file mode 100644 index 0000000000..282017b80d --- /dev/null +++ b/packages/core-api/src/apis/implementations/auth/github/types.ts @@ -0,0 +1,21 @@ +/* + * Copyright 2020 Spotify AB + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export type GithubSession = { + accessToken: string; + scopes: Set; + expiresAt: Date; +}; diff --git a/packages/core-api/src/apis/implementations/auth/index.ts b/packages/core-api/src/apis/implementations/auth/index.ts index 5fa6644b2a..f13368b5c4 100644 --- a/packages/core-api/src/apis/implementations/auth/index.ts +++ b/packages/core-api/src/apis/implementations/auth/index.ts @@ -15,3 +15,4 @@ */ export * from './google'; +export * from './github'; From 438bdba2cf09dacd9a95bc0a82597b5e0841a7ac Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Mon, 1 Jun 2020 12:07:58 +0200 Subject: [PATCH 3/5] Use StaticAuthSessionManager for Github --- .../implementations/auth/github/GithubAuth.ts | 27 +++++-------- .../src/lib/AuthSessionManager/index.ts | 1 + .../src/providers/github/provider.ts | 39 ++++--------------- 3 files changed, 18 insertions(+), 49 deletions(-) diff --git a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts index d20eaa54cb..d75bceef97 100644 --- a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -20,7 +20,7 @@ import { GithubSession } from './types'; import { OAuthApi, AccessTokenOptions } from '../../../definitions/auth'; import { OAuthRequestApi, AuthProvider } from '../../../definitions'; import { SessionManager } from '../../../../lib/AuthSessionManager/types'; -import { RefreshingAuthSessionManager } from '../../../../lib/AuthSessionManager'; +import { StaticAuthSessionManager } from '../../../../lib/AuthSessionManager'; type CreateOptions = { // TODO(Rugvip): These two should be grabbed from global config when available, they're not unique to GithubAuth @@ -63,20 +63,16 @@ class GithubAuth implements OAuthApi { sessionTransform(res: GithubAuthResponse): GithubSession { return { accessToken: res.accessToken, - scopes: GithubAuth.normalizeScopes(res.scope), + scopes: GithubAuth.normalizeScope(res.scope), expiresAt: new Date(Date.now() + res.expiresInSeconds * 1000), }; }, }); - const sessionManager = new RefreshingAuthSessionManager({ + const sessionManager = new StaticAuthSessionManager({ connector, defaultScopes: new Set(['user']), sessionScopes: session => session.scopes, - sessionShouldRefresh: session => { - const expiresInSec = (session.expiresAt.getTime() - Date.now()) / 1000; - return expiresInSec < 60 * 5; - }, }); return new GithubAuth(sessionManager); @@ -84,11 +80,8 @@ class GithubAuth implements OAuthApi { constructor(private readonly sessionManager: SessionManager) {} - async getAccessToken( - scope?: string | string[], - options?: AccessTokenOptions, - ) { - const normalizedScopes = GithubAuth.normalizeScopes(scope); + async getAccessToken(scope?: string, options?: AccessTokenOptions) { + const normalizedScopes = GithubAuth.normalizeScope(scope); const session = await this.sessionManager.getSession({ ...options, scopes: normalizedScopes, @@ -103,14 +96,14 @@ class GithubAuth implements OAuthApi { await this.sessionManager.removeSession(); } - static normalizeScopes(scopes?: string | string[]): Set { - if (!scopes) { + static normalizeScope(scope?: string): Set { + if (!scope) { return new Set(); } - const scopeList = Array.isArray(scopes) - ? scopes - : scopes.split(/[\s]/).filter(Boolean); + const scopeList = Array.isArray(scope) + ? scope + : scope.split(/[\s|,]/).filter(Boolean); return new Set(scopeList); } diff --git a/packages/core-api/src/lib/AuthSessionManager/index.ts b/packages/core-api/src/lib/AuthSessionManager/index.ts index 426c514646..16a8d3c378 100644 --- a/packages/core-api/src/lib/AuthSessionManager/index.ts +++ b/packages/core-api/src/lib/AuthSessionManager/index.ts @@ -15,4 +15,5 @@ */ export { RefreshingAuthSessionManager } from './RefreshingAuthSessionManager'; +export { StaticAuthSessionManager } from './StaticAuthSessionManager'; export * from './types'; diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 1e8022e1cc..4445e98451 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -19,7 +19,6 @@ import { Strategy as GithubStrategy } from 'passport-github2'; import { executeFrameHandlerStrategy, executeRedirectStrategy, - executeRefreshTokenStrategy, } from '../PassportStrategyHelper'; import { OAuthProviderHandlers, @@ -37,23 +36,13 @@ export class GithubAuthProvider implements OAuthProviderHandlers { this.providerConfig = providerConfig; this._strategy = new GithubStrategy( { ...this.providerConfig.options }, - ( - accessToken: any, - refreshToken: any, - params: any, - profile: any, - done: any, - ) => { - done( - undefined, - { - profile, - accessToken, - scope: 'user', // params.scope is an empty string here for some reason, so hardcoding for now - expiresInSeconds: params.expires_in, - }, - { refreshToken }, - ); + (accessToken: any, _: any, params: any, profile: any, done: any) => { + done(undefined, { + profile, + accessToken, + scope: params.scope, + expiresInSeconds: params.expires_in, + }); }, ); } @@ -67,18 +56,4 @@ export class GithubAuthProvider implements OAuthProviderHandlers { ): Promise<{ user: AuthInfoBase; info: AuthInfoPrivate }> { return await executeFrameHandlerStrategy(req, this._strategy); } - - async refresh(refreshToken: string, scope: string): Promise { - const { accessToken, params } = await executeRefreshTokenStrategy( - this._strategy, - refreshToken, - scope, - ); - - return { - accessToken, - expiresInSeconds: params.expires_in, - scope: params.scope, - }; - } } From 86930c9dd2061381567c5b14564ae4e75ba6bb80 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Mon, 1 Jun 2020 12:09:05 +0200 Subject: [PATCH 4/5] Support providers without refresh tokens --- .../src/providers/OAuthProvider.ts | 36 +++++++++++++------ plugins/auth-backend/src/providers/config.ts | 1 + .../auth-backend/src/providers/factories.ts | 20 +++++++++-- plugins/auth-backend/src/providers/index.ts | 6 +--- plugins/auth-backend/src/providers/types.ts | 3 +- 5 files changed, 47 insertions(+), 19 deletions(-) diff --git a/plugins/auth-backend/src/providers/OAuthProvider.ts b/plugins/auth-backend/src/providers/OAuthProvider.ts index 81f5ed25a6..30fd60f51f 100644 --- a/plugins/auth-backend/src/providers/OAuthProvider.ts +++ b/plugins/auth-backend/src/providers/OAuthProvider.ts @@ -89,9 +89,15 @@ export const removeRefreshTokenCookie = ( export class OAuthProvider implements AuthProviderRouteHandlers { private readonly provider: string; private readonly providerHandlers: OAuthProviderHandlers; - constructor(providerHandlers: OAuthProviderHandlers, provider: string) { + private readonly disableRefresh: boolean; + constructor( + providerHandlers: OAuthProviderHandlers, + provider: string, + disableRefresh?: boolean, + ) { this.provider = provider; this.providerHandlers = providerHandlers; + this.disableRefresh = disableRefresh ?? false; } async start(req: express.Request, res: express.Response): Promise { @@ -129,14 +135,16 @@ export class OAuthProvider implements AuthProviderRouteHandlers { const { user, info } = await this.providerHandlers.handler(req); - // throw error if missing refresh token - const { refreshToken } = info; - if (!refreshToken) { - throw new Error('Missing refresh token'); - } + if (!this.disableRefresh) { + // throw error if missing refresh token + const { refreshToken } = info; + if (!refreshToken) { + throw new Error('Missing refresh token'); + } - // set new refresh token - setRefreshTokenCookie(res, this.provider, refreshToken); + // set new refresh token + setRefreshTokenCookie(res, this.provider, refreshToken); + } // post message back to popup if successful return postMessageResponse(res, { @@ -160,8 +168,10 @@ export class OAuthProvider implements AuthProviderRouteHandlers { return res.status(401).send('Invalid X-Requested-With header'); } - // remove refresh token cookie before logout - removeRefreshTokenCookie(res, this.provider); + if (!this.disableRefresh) { + // remove refresh token cookie before logout + removeRefreshTokenCookie(res, this.provider); + } return res.send('logout!'); } @@ -170,6 +180,12 @@ export class OAuthProvider implements AuthProviderRouteHandlers { return res.status(401).send('Invalid X-Requested-With header'); } + if (!this.providerHandlers.refresh || this.disableRefresh) { + return res.send( + `Refresh token not supported for provider: ${this.provider}`, + ); + } + try { const refreshToken = req.cookies[`${this.provider}-refresh-token`]; diff --git a/plugins/auth-backend/src/providers/config.ts b/plugins/auth-backend/src/providers/config.ts index dad87e0448..8d04dc5a1f 100644 --- a/plugins/auth-backend/src/providers/config.ts +++ b/plugins/auth-backend/src/providers/config.ts @@ -30,5 +30,6 @@ export const providers = [ clientSecret: process.env.AUTH_GITHUB_CLIENT_SECRET!, callbackURL: 'http://localhost:7000/auth/github/handler/frame', }, + disableRefresh: true, }, ]; diff --git a/plugins/auth-backend/src/providers/factories.ts b/plugins/auth-backend/src/providers/factories.ts index 10e4153714..0c9f99e878 100644 --- a/plugins/auth-backend/src/providers/factories.ts +++ b/plugins/auth-backend/src/providers/factories.ts @@ -14,9 +14,14 @@ * limitations under the License. */ -import { AuthProviderFactories, AuthProviderFactory } from './types'; +import { + AuthProviderFactories, + AuthProviderRouteHandlers, + AuthProviderConfig, +} from './types'; import { GoogleAuthProvider } from './google'; import { GithubAuthProvider } from './github'; +import { OAuthProvider } from './OAuthProvider'; export class ProviderFactories { private static readonly providerFactories: AuthProviderFactories = { @@ -24,13 +29,22 @@ export class ProviderFactories { github: GithubAuthProvider, }; - public static getProviderFactory(providerId: string): AuthProviderFactory { + public static getProviderFactory( + config: AuthProviderConfig, + ): AuthProviderRouteHandlers { + const providerId = config.provider; const ProviderImpl = ProviderFactories.providerFactories[providerId]; if (!ProviderImpl) { throw Error( `Provider Implementation missing for : ${providerId} auth provider`, ); } - return ProviderImpl; + const providerInstance = new ProviderImpl(config); + const oauthProvider = new OAuthProvider( + providerInstance, + providerId, + config.disableRefresh, + ); + return oauthProvider; } } diff --git a/plugins/auth-backend/src/providers/index.ts b/plugins/auth-backend/src/providers/index.ts index 59dd116fa6..cfbabbbad1 100644 --- a/plugins/auth-backend/src/providers/index.ts +++ b/plugins/auth-backend/src/providers/index.ts @@ -17,7 +17,6 @@ import Router from 'express-promise-router'; import { AuthProviderRouteHandlers, AuthProviderConfig } from './types'; import { ProviderFactories } from './factories'; -import { OAuthProvider } from './OAuthProvider'; export const defaultRouter = (provider: AuthProviderRouteHandlers) => { const router = Router(); @@ -32,10 +31,7 @@ export const defaultRouter = (provider: AuthProviderRouteHandlers) => { export const makeProvider = (config: AuthProviderConfig) => { const providerId = config.provider; - const ProviderImpl = ProviderFactories.getProviderFactory(providerId); - const providerInstance = new ProviderImpl(config); - - const oauthProvider = new OAuthProvider(providerInstance, providerId); + const oauthProvider = ProviderFactories.getProviderFactory(config); const providerRouter = defaultRouter(oauthProvider); return { providerId, providerRouter }; }; diff --git a/plugins/auth-backend/src/providers/types.ts b/plugins/auth-backend/src/providers/types.ts index dcbe6ad1de..dc83c19dd0 100644 --- a/plugins/auth-backend/src/providers/types.ts +++ b/plugins/auth-backend/src/providers/types.ts @@ -20,12 +20,13 @@ import passport from 'passport'; export type AuthProviderConfig = { provider: string; options: any; + disableRefresh?: boolean; }; export interface OAuthProviderHandlers { start(req: express.Request, options: any): Promise; handler(req: express.Request): Promise; - refresh(refreshToken: string, scope: string): Promise; + refresh?(refreshToken: string, scope: string): Promise; logout?(): Promise; } From 7d4c044d1a58923916e0d296e6ed7656d00da689 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Tue, 2 Jun 2020 10:04:20 +0200 Subject: [PATCH 5/5] Rename test to clarify what it is doing --- .../src/apis/implementations/auth/github/GithubAuth.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts index 3d3da266fc..7c8e6ce9f0 100644 --- a/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts +++ b/packages/core-api/src/apis/implementations/auth/github/GithubAuth.test.ts @@ -16,13 +16,11 @@ import GithubAuth from './GithubAuth'; -const theFuture = new Date(Date.now() + 3600000); - describe('GithubAuth', () => { - it('should get refreshed access token', async () => { + it('should get access token', async () => { const getSession = jest .fn() - .mockResolvedValue({ accessToken: 'access-token', expiresAt: theFuture }); + .mockResolvedValue({ accessToken: 'access-token' }); const githubAuth = new GithubAuth({ getSession } as any); expect(await githubAuth.getAccessToken()).toBe('access-token');