Merge pull request backstage#26885 from jslott2sigma/skip-user-profile

Fixes backstage#26503 Add skipUserProfile flag to Microsoft authenticator
teknokeras · Oct 2, 2024 · 84a88a4 · 84a88a4
2 parents 5bff622 + 6b43d47
commit 84a88a4
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 24 deletions.
diff --git a/.changeset/heavy-ties-tell.md b/.changeset/heavy-ties-tell.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-auth-backend-module-microsoft-provider': patch
+---
+
+Add `skipUserProfile` config flag to Microsoft authenticator
diff --git a/docs/auth/microsoft/provider.md b/docs/auth/microsoft/provider.md
@@ -86,6 +86,7 @@ The Microsoft provider is a structure with three mandatory configuration keys:
   When specified, this reduces login friction for users with accounts in multiple tenants by automatically filtering away accounts from other tenants.
   For more details, see [Home Realm Discovery](https://learn.microsoft.com/en-us/azure/active-directory/manage-apps/home-realm-discovery-policy)
 - `additionalScopes` (optional): List of scopes for the App Registration, to be requested in addition to the required ones.
+- `skipUserProfile` (optional): If true, skips loading the user profile even if the `User.Read` scope is present. This is a performance optimization during login and can be used with resolvers that only needs the email address in `spec.profile.email` obtained when the `email` OAuth2 scope is present.
 
 ### Resolvers
 

diff --git a/plugins/auth-backend-module-microsoft-provider/config.d.ts b/plugins/auth-backend-module-microsoft-provider/config.d.ts
@@ -29,6 +29,7 @@ export interface Config {
           domainHint?: string;
           callbackUrl?: string;
           additionalScopes?: string | string[];
+          skipUserProfile?: boolean;
           signIn?: {
             resolvers: Array<
               | { resolver: 'emailMatchingUserEntityAnnotation' }

diff --git a/plugins/auth-backend-module-microsoft-provider/src/authenticator.test.ts b/plugins/auth-backend-module-microsoft-provider/src/authenticator.test.ts
@@ -262,7 +262,7 @@ describe('microsoftAuthenticator', () => {
       expect(profile.photos).toStrictEqual([{ value: photo }]);
     });
 
-    it('returns access token for  non-microsoft graph scope', async () => {
+    it('returns access token for non-microsoft graph scope', async () => {
       const foreignScope = 'aks-audience/user.read';
       const refreshResponse = await microsoftAuthenticator.refresh(
         createRefreshRequest(foreignScope),
@@ -274,5 +274,29 @@ describe('microsoftAuthenticator', () => {
         microsoftApi.generateAccessToken(foreignScope),
       );
     });
+
+    it('returns access token when skipping user profile load', async () => {
+      // Replace implementation to set skipUserProfile config
+      implementation = microsoftAuthenticator.initialize({
+        callbackUrl: 'https://backstage.test/callback',
+        config: new ConfigReader({
+          tenantId: 'tenantId',
+          clientId: 'clientId',
+          clientSecret: 'clientSecret',
+          additionalScopes: ['User.Read.All'],
+          skipUserProfile: true,
+        }),
+      });
+
+      const refreshResponse = await microsoftAuthenticator.refresh(
+        createRefreshRequest(scope),
+        implementation,
+      );
+
+      expect(refreshResponse.fullProfile).toBeUndefined();
+      expect(refreshResponse.session.accessToken).toBe(
+        microsoftApi.generateAccessToken(scope),
+      );
+    });
   });
 });
diff --git a/plugins/auth-backend-module-microsoft-provider/src/authenticator.ts b/plugins/auth-backend-module-microsoft-provider/src/authenticator.ts
@@ -45,31 +45,30 @@ export const microsoftAuthenticator = createOAuthAuthenticator({
     const clientSecret = config.getString('clientSecret');
     const tenantId = config.getString('tenantId');
     const domainHint = config.getOptionalString('domainHint');
+    const skipUserProfile =
+      config.getOptionalBoolean('skipUserProfile') ?? false;
 
-    const helper = PassportOAuthAuthenticatorHelper.from(
-      new ExtendedMicrosoftStrategy(
-        {
-          clientID: clientId,
-          clientSecret: clientSecret,
-          callbackURL: callbackUrl,
-          tenant: tenantId,
-        },
-        (
-          accessToken: string,
-          refreshToken: string,
-          params: any,
-          fullProfile: PassportProfile,
-          done: PassportOAuthDoneCallback,
-        ) => {
-          done(
-            undefined,
-            { fullProfile, params, accessToken },
-            { refreshToken },
-          );
-        },
-      ),
+    const strategy = new ExtendedMicrosoftStrategy(
+      {
+        clientID: clientId,
+        clientSecret: clientSecret,
+        callbackURL: callbackUrl,
+        tenant: tenantId,
+      },
+      (
+        accessToken: string,
+        refreshToken: string,
+        params: any,
+        fullProfile: PassportProfile,
+        done: PassportOAuthDoneCallback,
+      ) => {
+        done(undefined, { fullProfile, params, accessToken }, { refreshToken });
+      },
     );
 
+    strategy.setSkipUserProfile(skipUserProfile);
+    const helper = PassportOAuthAuthenticatorHelper.from(strategy);
+
     return {
       helper,
       domainHint,

diff --git a/plugins/auth-backend-module-microsoft-provider/src/strategy.ts b/plugins/auth-backend-module-microsoft-provider/src/strategy.ts
@@ -20,6 +20,12 @@ import fetch from 'node-fetch';
 import { Strategy as MicrosoftStrategy } from 'passport-microsoft';
 
 export class ExtendedMicrosoftStrategy extends MicrosoftStrategy {
+  private shouldSkipUserProfile = false;
+
+  public setSkipUserProfile(shouldSkipUserProfile: boolean): void {
+    this.shouldSkipUserProfile = shouldSkipUserProfile;
+  }
+
   userProfile(
     accessToken: string,
     done: (err?: unknown, profile?: PassportProfile) => void,
@@ -66,7 +72,7 @@ export class ExtendedMicrosoftStrategy extends MicrosoftStrategy {
 
   private skipUserProfile(accessToken: string): boolean {
     try {
-      return !this.hasGraphReadScope(accessToken);
+      return this.shouldSkipUserProfile || !this.hasGraphReadScope(accessToken);
     } catch {
       // If there is any error with checking the scope
       // we fall back to not skipping the user profile