Merge pull request #9294 from backstage/rugvip/auth-refresh

auth-backend,core-app-api: GitHub auth refactor and fixes
2022-02-02 12:07:21 +01:00
parent 6acc8f7db7 40775bd263
commit 9888ccb678
12 changed files with 337 additions and 210 deletions
@@ -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.
@@ -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.
@@ -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.
@@ -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<string>;
-  // (undocumented)
-  getBackstageIdentity(
-    options?: AuthRequestOptions,
-  ): Promise<BackstageIdentityResponse | undefined>;
-  // (undocumented)
-  getProfile(options?: AuthRequestOptions): Promise<ProfileInfo | undefined>;
+export class GithubAuth {
  // (undocumented)
+  static create(options: OAuthApiCreateOptions): typeof githubAuthApiRef.T;
+  // @deprecated (undocumented)
  static normalizeScope(scope?: string): Set<string>;
-  // (undocumented)
-  sessionState$(): Observable<SessionState>;
-  // (undocumented)
-  signIn(): Promise<void>;
-  // (undocumented)
-  signOut(): Promise<void>;
 }

 // @public @deprecated
@@ -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']),
+    });
  });
 });
@@ -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<GithubSession>({
-      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<GithubSession>,
-  ) {}
-
-  async signIn() {
-    await this.getAccessToken();
-  }
-
-  async signOut() {
-    await this.sessionManager.removeSession();
-  }
-
-  sessionState$(): Observable<SessionState> {
-    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<BackstageIdentityResponse | undefined> {
-    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<string> {
    if (!scope) {
      return new Set();
@@ -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
 ```
@@ -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,
@@ -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<void> {
    // 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,
    });
  };
 }
@@ -87,6 +87,7 @@ export type OAuthState = {
  nonce: string;
  env: string;
  origin?: string;
+  scope?: string;
 };

 export type OAuthStartRequest = express.Request<{}> & {
@@ -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<PassportProfile>>;
+
+      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();
    });
  });
 });
@@ -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,
+    };
  }
 }