diff --git a/.changeset/chilled-papayas-wonder.md b/.changeset/chilled-papayas-wonder.md new file mode 100644 index 0000000000..4a3c2322e1 --- /dev/null +++ b/.changeset/chilled-papayas-wonder.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Fixed a bug where providers that tracked the granted scopes through a cookie would not take failed authentication attempts into account. diff --git a/.changeset/strong-taxis-refuse.md b/.changeset/strong-taxis-refuse.md new file mode 100644 index 0000000000..f198dc4bab --- /dev/null +++ b/.changeset/strong-taxis-refuse.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Added support for storing static GitHub access tokens in cookies and using them to refresh the Backstage session. diff --git a/.changeset/tasty-pandas-design.md b/.changeset/tasty-pandas-design.md new file mode 100644 index 0000000000..302631ee60 --- /dev/null +++ b/.changeset/tasty-pandas-design.md @@ -0,0 +1,7 @@ +--- +'@backstage/core-app-api': patch +--- + +Switched out the `GithubAuth` implementation to use the common `OAuth2` implementation. This relies on the simultaneous change in `@backstage/plugin-auth-backend` that enabled access token storage in cookies rather than the current solution that's based on `LocalStorage`. + +> **NOTE:** Make sure you upgrade the `auth-backend` deployment before or at the same time as you deploy this change. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index ba2ed8cb62..3b53813615 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -35,6 +35,7 @@ import { FeatureFlag } from '@backstage/core-plugin-api'; import { FeatureFlagsApi } from '@backstage/core-plugin-api'; import { FeatureFlagsSaveOptions } from '@backstage/core-plugin-api'; import { FetchApi } from '@backstage/core-plugin-api'; +import { githubAuthApiRef } from '@backstage/core-plugin-api'; import { gitlabAuthApiRef } from '@backstage/core-plugin-api'; import { googleAuthApiRef } from '@backstage/core-plugin-api'; import { IconComponent } from '@backstage/core-plugin-api'; @@ -379,25 +380,11 @@ export type FlatRoutesProps = { }; // @public -export class GithubAuth implements OAuthApi, SessionApi { - // (undocumented) - static create(options: OAuthApiCreateOptions): GithubAuth; - // (undocumented) - getAccessToken(scope?: string, options?: AuthRequestOptions): Promise; - // (undocumented) - getBackstageIdentity( - options?: AuthRequestOptions, - ): Promise; - // (undocumented) - getProfile(options?: AuthRequestOptions): Promise; +export class GithubAuth { // (undocumented) + static create(options: OAuthApiCreateOptions): typeof githubAuthApiRef.T; + // @deprecated (undocumented) static normalizeScope(scope?: string): Set; - // (undocumented) - sessionState$(): Observable; - // (undocumented) - signIn(): Promise; - // (undocumented) - signOut(): Promise; } // @public @deprecated diff --git a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts index 8bcd4cb7a5..ea7b4ac688 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.test.ts @@ -14,16 +14,33 @@ * limitations under the License. */ +import { UrlPatternDiscovery } from '../../DiscoveryApi'; +import MockOAuthApi from '../../OAuthRequestApi/MockOAuthApi'; import GithubAuth from './GithubAuth'; -describe('GithubAuth', () => { - it('should get access token', async () => { - const getSession = jest - .fn() - .mockResolvedValue({ providerInfo: { accessToken: 'access-token' } }); - const githubAuth = new (GithubAuth as any)({ getSession }) as GithubAuth; +const getSession = jest.fn(); - expect(await githubAuth.getAccessToken()).toBe('access-token'); - expect(getSession).toBeCalledTimes(1); +jest.mock('../../../../lib/AuthSessionManager', () => ({ + ...(jest.requireActual('../../../../lib/AuthSessionManager') as any), + RefreshingAuthSessionManager: class { + getSession = getSession; + }, +})); + +describe('GithubAuth', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + it('should forward access token request to session manager', async () => { + const githubAuth = GithubAuth.create({ + oauthRequestApi: new MockOAuthApi(), + discoveryApi: UrlPatternDiscovery.compile('http://example.com'), + }); + + githubAuth.getAccessToken('repo'); + expect(getSession).toHaveBeenCalledWith({ + scopes: new Set(['repo']), + }); }); }); diff --git a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts index dc2c15bc16..b0af4c7ade 100644 --- a/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts +++ b/packages/core-app-api/src/apis/implementations/auth/github/GithubAuth.ts @@ -14,35 +14,9 @@ * limitations under the License. */ -import { - AuthRequestOptions, - BackstageIdentityResponse, - OAuthApi, - ProfileInfo, - SessionApi, - SessionState, -} from '@backstage/core-plugin-api'; -import { Observable } from '@backstage/types'; -import { DefaultAuthConnector } from '../../../../lib/AuthConnector'; -import { - AuthSessionStore, - RefreshingAuthSessionManager, - StaticAuthSessionManager, -} from '../../../../lib/AuthSessionManager'; -import { OptionalRefreshSessionManagerMux } from '../../../../lib/AuthSessionManager/OptionalRefreshSessionManagerMux'; -import { SessionManager } from '../../../../lib/AuthSessionManager/types'; +import { githubAuthApiRef } from '@backstage/core-plugin-api'; +import { OAuth2 } from '../oauth2'; import { OAuthApiCreateOptions } from '../types'; -import { GithubSession, githubSessionSchema } from './types'; - -export type GithubAuthResponse = { - providerInfo: { - accessToken: string; - scope: string; - expiresInSeconds?: number; - }; - profile: ProfileInfo; - backstageIdentity: BackstageIdentityResponse; -}; const DEFAULT_PROVIDER = { id: 'github', @@ -55,8 +29,8 @@ const DEFAULT_PROVIDER = { * * @public */ -export default class GithubAuth implements OAuthApi, SessionApi { - static create(options: OAuthApiCreateOptions) { +export default class GithubAuth { + static create(options: OAuthApiCreateOptions): typeof githubAuthApiRef.T { const { discoveryApi, environment = 'development', @@ -65,96 +39,18 @@ export default class GithubAuth implements OAuthApi, SessionApi { defaultScopes = ['read:user'], } = options; - const connector = new DefaultAuthConnector({ + return OAuth2.create({ discoveryApi, - environment, + oauthRequestApi, provider, - oauthRequestApi: oauthRequestApi, - sessionTransform(res: GithubAuthResponse): GithubSession { - return { - ...res, - providerInfo: { - accessToken: res.providerInfo.accessToken, - scopes: GithubAuth.normalizeScope(res.providerInfo.scope), - expiresAt: res.providerInfo.expiresInSeconds - ? new Date(Date.now() + res.providerInfo.expiresInSeconds * 1000) - : undefined, - }, - }; - }, + environment, + defaultScopes, }); - - const refreshingSessionManager = new RefreshingAuthSessionManager({ - connector, - defaultScopes: new Set(defaultScopes), - sessionScopes: (session: GithubSession) => session.providerInfo.scopes, - sessionShouldRefresh: (session: GithubSession) => { - const { expiresAt } = session.providerInfo; - if (!expiresAt) { - return false; - } - const expiresInSec = (expiresAt.getTime() - Date.now()) / 1000; - return expiresInSec < 60 * 5; - }, - }); - - const staticSessionManager = new AuthSessionStore({ - manager: new StaticAuthSessionManager({ - connector, - defaultScopes: new Set(defaultScopes), - sessionScopes: (session: GithubSession) => session.providerInfo.scopes, - }), - storageKey: `${provider.id}Session`, - schema: githubSessionSchema, - sessionScopes: (session: GithubSession) => session.providerInfo.scopes, - }); - - const sessionManagerMux = new OptionalRefreshSessionManagerMux({ - refreshingSessionManager, - staticSessionManager, - sessionCanRefresh: session => - session.providerInfo.expiresAt !== undefined, - }); - - return new GithubAuth(sessionManagerMux); - } - - private constructor( - private readonly sessionManager: SessionManager, - ) {} - - async signIn() { - await this.getAccessToken(); - } - - async signOut() { - await this.sessionManager.removeSession(); - } - - sessionState$(): Observable { - return this.sessionManager.sessionState$(); - } - - async getAccessToken(scope?: string, options?: AuthRequestOptions) { - const session = await this.sessionManager.getSession({ - ...options, - scopes: GithubAuth.normalizeScope(scope), - }); - return session?.providerInfo.accessToken ?? ''; - } - - async getBackstageIdentity( - options: AuthRequestOptions = {}, - ): Promise { - const session = await this.sessionManager.getSession(options); - return session?.backstageIdentity; - } - - async getProfile(options: AuthRequestOptions = {}) { - const session = await this.sessionManager.getSession(options); - return session?.profile; } + /** + * @deprecated This method is deprecated and will be removed in a future release. + */ static normalizeScope(scope?: string): Set { if (!scope) { return new Set(); diff --git a/plugins/auth-backend/api-report.md b/plugins/auth-backend/api-report.md index 7cdc33f0e2..88b75d99b0 100644 --- a/plugins/auth-backend/api-report.md +++ b/plugins/auth-backend/api-report.md @@ -592,6 +592,7 @@ export type OAuthState = { nonce: string; env: string; origin?: string; + scope?: string; }; // @public @@ -734,6 +735,6 @@ export type WebMessageResponse = // // src/identity/types.d.ts:31:9 - (ae-forgotten-export) The symbol "AnyJWK" needs to be exported by the entry point index.d.ts // src/providers/aws-alb/provider.d.ts:77:5 - (ae-forgotten-export) The symbol "AwsAlbResult" needs to be exported by the entry point index.d.ts -// src/providers/github/provider.d.ts:81:5 - (ae-forgotten-export) The symbol "StateEncoder" needs to be exported by the entry point index.d.ts +// src/providers/github/provider.d.ts:97:5 - (ae-forgotten-export) The symbol "StateEncoder" needs to be exported by the entry point index.d.ts // src/providers/types.d.ts:98:5 - (ae-forgotten-export) The symbol "AuthProviderConfig" needs to be exported by the entry point index.d.ts ``` diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts index a85dd49fba..d1057b19a8 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.test.ts @@ -17,7 +17,7 @@ import express from 'express'; import { THOUSAND_DAYS_MS, TEN_MINUTES_MS, OAuthAdapter } from './OAuthAdapter'; import { encodeState } from './helpers'; -import { OAuthHandlers, OAuthResponse } from './types'; +import { OAuthHandlers, OAuthResponse, OAuthState } from './types'; const mockResponseData = { providerInfo: { @@ -148,6 +148,102 @@ describe('OAuthAdapter', () => { ); }); + it('persists scope through cookie if enabled', async () => { + const handlers = { + start: jest.fn(async (_req: { state: OAuthState }) => ({ + url: '/url', + status: 301, + })), + handler: jest.fn(async () => ({ response: mockResponseData })), + refresh: jest.fn(async () => ({ response: mockResponseData })), + }; + const oauthProvider = new OAuthAdapter(handlers, { + ...oAuthProviderOptions, + disableRefresh: false, + persistScopes: true, + }); + + // First we test the /start request, making sure state is set + const mockStartReq = { + query: { + scope: 'user', + env: 'development', + }, + } as unknown as express.Request; + const mockStartRes = { + cookie: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + statusCode: jest.fn().mockReturnThis(), + } as unknown as express.Response; + + await oauthProvider.start(mockStartReq, mockStartRes); + + expect(handlers.start).toHaveBeenCalledTimes(1); + expect(handlers.start).toHaveBeenCalledWith({ + query: { + scope: 'user', + env: 'development', + }, + scope: 'user', + state: { + nonce: expect.any(String), + env: 'development', + origin: undefined, + scope: 'user', + }, + }); + + // Then test the /handler, making sure the granted scope cookie is set + const providedState = handlers.start.mock.calls[0][0].state; + const mockHandleReq = { + cookies: { + 'test-provider-nonce': providedState.nonce, + }, + query: { + state: encodeState(providedState), + }, + } as unknown as express.Request; + const mockHandleRes = { + cookie: jest.fn().mockReturnThis(), + setHeader: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + } as unknown as express.Response; + + await oauthProvider.frameHandler(mockHandleReq, mockHandleRes); + expect(mockHandleRes.cookie).toHaveBeenCalledTimes(1); + expect(mockHandleRes.cookie).toHaveBeenCalledWith( + 'test-provider-granted-scope', + 'user', + expect.objectContaining({ + path: '/auth/test-provider', + maxAge: THOUSAND_DAYS_MS, + }), + ); + + // Them make sure scopes are forwarded correctly during refresh + const mockRefreshReq = { + query: { scope: 'ignore-me' }, + cookies: { + 'test-provider-granted-scope': 'user', + 'test-provider-refresh-token': 'refresh-token', + }, + header: jest.fn().mockReturnValue('XMLHttpRequest'), + } as unknown as express.Request; + const mockRefreshRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + } as unknown as express.Response; + await oauthProvider.refresh(mockRefreshReq, mockRefreshRes); + expect(handlers.refresh).toHaveBeenCalledTimes(1); + expect(handlers.refresh).toHaveBeenCalledWith( + expect.objectContaining({ + scope: 'user', + refreshToken: 'refresh-token', + }), + ); + }); + it('does not set the refresh cookie if refresh is disabled', async () => { const oauthProvider = new OAuthAdapter(providerInstance, { ...oAuthProviderOptions, diff --git a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts index 564d5c20de..2e2d26db61 100644 --- a/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts +++ b/plugins/auth-backend/src/lib/oauth/OAuthAdapter.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import express from 'express'; +import express, { CookieOptions } from 'express'; import crypto from 'crypto'; import { URL } from 'url'; import { @@ -90,10 +90,20 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { }); } + private readonly baseCookieOptions: CookieOptions; + constructor( private readonly handlers: OAuthHandlers, private readonly options: Options, - ) {} + ) { + this.baseCookieOptions = { + httpOnly: true, + sameSite: 'lax', + secure: this.options.secure, + path: this.options.cookiePath, + domain: this.options.cookieDomain, + }; + } async start(req: express.Request, res: express.Response): Promise { // retrieve scopes from request @@ -105,15 +115,17 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { throw new InputError('No env provided in request query parameters'); } - if (this.options.persistScopes) { - this.setScopesCookie(res, scope); - } - const nonce = crypto.randomBytes(16).toString('base64'); // set a nonce cookie before redirecting to oauth provider this.setNonceCookie(res, nonce); - const state = { nonce, env, origin }; + const state: OAuthState = { nonce, env, origin }; + + // If scopes are persisted then we pass them through the state so that we + // can set the cookie on successful auth + if (this.options.persistScopes) { + state.scope = scope; + } const forwardReq = Object.assign(req, { scope, state }); const { url, status } = await this.handlers.start( @@ -151,12 +163,11 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { const { response, refreshToken } = await this.handlers.handler(req); - if (this.options.persistScopes) { - const grantedScopes = this.getScopesFromCookie( - req, - this.options.providerId, - ); - response.providerInfo.scope = grantedScopes; + // Store the scope that we have been granted for this session. This is useful if + // the provider does not return granted scopes on refresh or if they are normalized. + if (this.options.persistScopes && state.scope) { + this.setGrantedScopeCookie(res, state.scope); + response.providerInfo.scope = state.scope; } if (refreshToken && !this.options.disableRefresh) { @@ -214,8 +225,10 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { throw new InputError('Missing session cookie'); } - const scope = req.query.scope?.toString() ?? ''; - + let scope = req.query.scope?.toString() ?? ''; + if (this.options.persistScopes) { + scope = this.getGrantedScopeFromCookie(req); + } const forwardReq = Object.assign(req, { scope, refreshToken }); // get new access_token @@ -267,27 +280,20 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { private setNonceCookie = (res: express.Response, nonce: string) => { res.cookie(`${this.options.providerId}-nonce`, nonce, { maxAge: TEN_MINUTES_MS, - secure: this.options.secure, - sameSite: 'lax', - domain: this.options.cookieDomain, + ...this.baseCookieOptions, path: `${this.options.cookiePath}/handler`, - httpOnly: true, }); }; - private setScopesCookie = (res: express.Response, scope: string) => { - res.cookie(`${this.options.providerId}-scope`, scope, { - maxAge: TEN_MINUTES_MS, - secure: this.options.secure, - sameSite: 'lax', - domain: this.options.cookieDomain, - path: `${this.options.cookiePath}/handler`, - httpOnly: true, + private setGrantedScopeCookie = (res: express.Response, scope: string) => { + res.cookie(`${this.options.providerId}-granted-scope`, scope, { + maxAge: THOUSAND_DAYS_MS, + ...this.baseCookieOptions, }); }; - private getScopesFromCookie = (req: express.Request, providerId: string) => { - return req.cookies[`${providerId}-scope`]; + private getGrantedScopeFromCookie = (req: express.Request) => { + return req.cookies[`${this.options.providerId}-granted-scope`]; }; private setRefreshTokenCookie = ( @@ -296,22 +302,14 @@ export class OAuthAdapter implements AuthProviderRouteHandlers { ) => { res.cookie(`${this.options.providerId}-refresh-token`, refreshToken, { maxAge: THOUSAND_DAYS_MS, - secure: this.options.secure, - sameSite: 'lax', - domain: this.options.cookieDomain, - path: this.options.cookiePath, - httpOnly: true, + ...this.baseCookieOptions, }); }; private removeRefreshTokenCookie = (res: express.Response) => { res.cookie(`${this.options.providerId}-refresh-token`, '', { maxAge: 0, - secure: this.options.secure, - sameSite: 'lax', - domain: this.options.cookieDomain, - path: this.options.cookiePath, - httpOnly: true, + ...this.baseCookieOptions, }); }; } diff --git a/plugins/auth-backend/src/lib/oauth/types.ts b/plugins/auth-backend/src/lib/oauth/types.ts index 9ddef007a7..6973e99569 100644 --- a/plugins/auth-backend/src/lib/oauth/types.ts +++ b/plugins/auth-backend/src/lib/oauth/types.ts @@ -87,6 +87,7 @@ export type OAuthState = { nonce: string; env: string; origin?: string; + scope?: string; }; export type OAuthStartRequest = express.Request<{}> & { diff --git a/plugins/auth-backend/src/providers/github/provider.test.ts b/plugins/auth-backend/src/providers/github/provider.test.ts index 15eeccc07c..206df52083 100644 --- a/plugins/auth-backend/src/providers/github/provider.test.ts +++ b/plugins/auth-backend/src/providers/github/provider.test.ts @@ -98,6 +98,7 @@ describe('GithubAuthProvider', () => { providerInfo: { accessToken: '19xasczxcm9n7gacn9jdgm19me', scope: 'read:scope', + expiresInSeconds: 3600, }, profile: { email: 'jimmymarkum@gmail.com', @@ -143,6 +144,7 @@ describe('GithubAuthProvider', () => { providerInfo: { accessToken: '19xasczxcm9n7gacn9jdgm19me', scope: 'read:scope', + expiresInSeconds: 3600, }, profile: { displayName: 'Jimmy Markum', @@ -186,6 +188,7 @@ describe('GithubAuthProvider', () => { providerInfo: { accessToken: '19xasczxcm9n7gacn9jdgm19me', scope: 'read:scope', + expiresInSeconds: 3600, }, profile: { displayName: 'jimmymarkum', @@ -230,6 +233,7 @@ describe('GithubAuthProvider', () => { accessToken: 'ajakljsdoiahoawxbrouawucmbawe.awkxjemaneasdxwe.sodijxqeqwexeqwxe', scope: 'read:user', + expiresInSeconds: 3600, }, profile: { displayName: 'Dave Boyle', @@ -316,7 +320,7 @@ describe('GithubAuthProvider', () => { ], }); - const result = await provider.refresh({} as any); + const result = await provider.refresh({ scope: 'actual-scope' } as any); expect(result).toEqual({ response: { @@ -332,11 +336,65 @@ describe('GithubAuthProvider', () => { providerInfo: { accessToken: 'a.b.c', expiresInSeconds: 123, - scope: 'read_user', + scope: 'actual-scope', }, }, refreshToken: 'dont-forget-to-send-refresh', }); + + mockRefreshToken.mockRestore(); + mockUserProfile.mockRestore(); + }); + + it('should use access token as refresh token', async () => { + const mockUserProfile = jest.spyOn( + helpers, + 'executeFetchUserProfileStrategy', + ) as unknown as jest.MockedFunction<() => Promise>; + + mockUserProfile.mockResolvedValueOnce({ + id: 'mockid', + username: 'mockuser', + provider: 'github', + displayName: 'Mocked User', + emails: [ + { + value: 'mockuser@gmail.com', + }, + ], + }); + + const result = await provider.refresh({ + refreshToken: 'access-token.le-token', + scope: 'the-scope', + } as any); + + expect(mockUserProfile).toHaveBeenCalledTimes(1); + expect(mockUserProfile).toHaveBeenCalledWith( + expect.anything(), + 'le-token', + ); + expect(result).toEqual({ + response: { + backstageIdentity: { + id: 'mockuser', + token: 'token-for-user:default/mockuser', + }, + profile: { + displayName: 'Mocked User', + email: 'mockuser@gmail.com', + picture: undefined, + }, + providerInfo: { + accessToken: 'le-token', + expiresInSeconds: 3600, + scope: 'the-scope', + }, + }, + refreshToken: 'access-token.le-token', + }); + + mockUserProfile.mockRestore(); }); }); }); diff --git a/plugins/auth-backend/src/providers/github/provider.ts b/plugins/auth-backend/src/providers/github/provider.ts index 9dc70cf60e..ec7f8cafa6 100644 --- a/plugins/auth-backend/src/providers/github/provider.ts +++ b/plugins/auth-backend/src/providers/github/provider.ts @@ -41,11 +41,15 @@ import { OAuthStartRequest, encodeState, OAuthRefreshRequest, - OAuthResponse, } from '../../lib/oauth'; import { CatalogIdentityClient } from '../../lib/catalog'; import { TokenIssuer } from '../../identity'; +const ACCESS_TOKEN_PREFIX = 'access-token.'; + +// TODO(Rugvip): Auth providers need a way to access this in a less hardcoded way +const BACKSTAGE_SESSION_EXPIRATION = 3600; + type PrivateInfo = { refreshToken?: string; }; @@ -123,31 +127,69 @@ export class GithubAuthProvider implements OAuthHandlers { PrivateInfo >(req, this._strategy); + let refreshToken = privateInfo.refreshToken; + + // If we do not have a real refresh token and we have a non-expiring + // access token, then we use that as our refresh token. + if (!refreshToken && !result.params.expires_in) { + refreshToken = ACCESS_TOKEN_PREFIX + result.accessToken; + } + return { response: await this.handleResult(result), - refreshToken: privateInfo.refreshToken, + refreshToken, }; } async refresh(req: OAuthRefreshRequest) { - const { accessToken, refreshToken, params } = - await executeRefreshTokenStrategy( - this._strategy, - req.refreshToken, - req.scope, - ); - const fullProfile = await executeFetchUserProfileStrategy( - this._strategy, - accessToken, - ); + // We've enable persisting scope in the OAuth provider, so scope here will + // be whatever was stored in the cookie + const { scope, refreshToken } = req; + // This is the OAuth App flow. A non-expiring access token is stored in the + // refresh token cookie. We use that token to fetch the user profile and + // refresh the Backstage session when needed. + if (refreshToken?.startsWith(ACCESS_TOKEN_PREFIX)) { + const accessToken = refreshToken.slice(ACCESS_TOKEN_PREFIX.length); + + const fullProfile = await executeFetchUserProfileStrategy( + this._strategy, + accessToken, + ).catch(error => { + if (error.oauthError?.statusCode === 401) { + throw new Error('Invalid access token'); + } + throw error; + }); + + return { + response: await this.handleResult({ + fullProfile, + params: { scope }, + accessToken, + }), + refreshToken, + }; + } + + // This is the App flow, which is close to a standard OAuth refresh flow. It has a + // pretty long session expiration, and it also ignores the requested scope, instead + // just allowing access to whatever is configured as part of the app installation. + const result = await executeRefreshTokenStrategy( + this._strategy, + refreshToken, + scope, + ); return { response: await this.handleResult({ - fullProfile, - params, - accessToken, + fullProfile: await executeFetchUserProfileStrategy( + this._strategy, + result.accessToken, + ), + params: { ...result.params, scope }, + accessToken: result.accessToken, }), - refreshToken, + refreshToken: result.refreshToken, }; } @@ -160,27 +202,41 @@ export class GithubAuthProvider implements OAuthHandlers { const { profile } = await this.authHandler(result, context); const expiresInStr = result.params.expires_in; - const response: OAuthResponse = { - providerInfo: { - accessToken: result.accessToken, - scope: result.params.scope, - expiresInSeconds: - expiresInStr === undefined ? undefined : Number(expiresInStr), - }, - profile, - }; + let expiresInSeconds = + expiresInStr === undefined ? undefined : Number(expiresInStr); + + let backstageIdentity = undefined; if (this.signInResolver) { - response.backstageIdentity = await this.signInResolver( + backstageIdentity = await this.signInResolver( { result, profile, }, context, ); + + // GitHub sessions last longer than Backstage sessions, so if we're using + // GitHub for sign-in, then we need to expire the sessions earlier + if (expiresInSeconds) { + expiresInSeconds = Math.min( + expiresInSeconds, + BACKSTAGE_SESSION_EXPIRATION, + ); + } else { + expiresInSeconds = BACKSTAGE_SESSION_EXPIRATION; + } } - return response; + return { + backstageIdentity, + providerInfo: { + accessToken: result.accessToken, + scope: result.params.scope, + expiresInSeconds, + }, + profile, + }; } }