From 499c784c970868d1e27d8464032190e258155b6f Mon Sep 17 00:00:00 2001
From: Raghunandan <soapraj@gmail.com>
Date: Thu, 4 Jun 2020 11:54:12 +0200
Subject: [PATCH] minor fixes

---
 .../implementations/auth/google/GoogleAuth.ts |  1 -
 .../src/layout/Sidebar/LoggedUserBadge.tsx    | 47 +++++++++----------
 .../core/src/layout/Sidebar/UserBadge.tsx     | 10 ++--
 packages/storybook/.storybook/apis.js         | 18 +++++++
 .../src/providers/OAuthProvider.ts            |  2 -
 .../providers/PassportStrategyHelper.test.ts  |  8 ++++
 .../src/providers/PassportStrategyHelper.ts   |  6 +--
 .../src/providers/google/provider.ts          |  8 ++--
 8 files changed, 62 insertions(+), 38 deletions(-)

diff --git a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts
index f40e164c82..d43741a9e8 100644
--- a/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts
+++ b/packages/core-api/src/apis/implementations/auth/google/GoogleAuth.ts
@@ -24,7 +24,6 @@ import {
   AccessTokenOptions,
   ProfileInfoApi,
   ProfileInfoOptions,
-  ProfileInfo,
 } from '../../../definitions/auth';
 import { OAuthRequestApi, AuthProvider } from '../../../definitions';
 import { SessionManager } from '../../../../lib/AuthSessionManager/types';
diff --git a/packages/core/src/layout/Sidebar/LoggedUserBadge.tsx b/packages/core/src/layout/Sidebar/LoggedUserBadge.tsx
index 1053d3a36e..d0c3dc62c2 100644
--- a/packages/core/src/layout/Sidebar/LoggedUserBadge.tsx
+++ b/packages/core/src/layout/Sidebar/LoggedUserBadge.tsx
@@ -28,11 +28,12 @@ import {
   ListItemSecondaryAction,
   IconButton,
   Tooltip,
+  Typography,
 } from '@material-ui/core';
 import { blueGrey } from '@material-ui/core/colors';
 import { useSetState } from 'react-use';
 import { Skeleton } from '@material-ui/lab';
-import { useApi, googleAuthApiRef, ProfileInfo } from '@backstage/core';
+import { useApi, googleAuthApiRef, ProfileInfo } from '@backstage/core-api';
 import LogoutIcon from '@material-ui/icons/PowerSettingsNew';
 import ControlPointIcon from '@material-ui/icons/ControlPoint';
 import AccountCircleIcon from '@material-ui/icons/AccountCircle';
@@ -90,7 +91,6 @@ const SessionListItem: FC<{
           <Skeleton variant="circle" width={40} height={40} />
         </ListItemIcon>
         <ListItemText
-          className={classes.listItemText}
           primary={<Skeleton component="span" width={120} />}
           secondary={<Skeleton component="span" width={60} />}
         />
@@ -103,16 +103,12 @@ const SessionListItem: FC<{
     );
   }
 
-  //TODO: Not functional yet to sign in from the sidebar
+  // TODO: Not functional yet to sign in from the sidebar
   if (!user) {
     return (
       <ListItem {...props}>
         <ListItemIcon style={{ marginRight: 0 }}>{icon}</ListItemIcon>
-        <ListItemText
-          className={classes.listItemText}
-          primary="Sign In"
-          secondary={title}
-        />
+        <ListItemText primary="Sign In" secondary={title} />
         <ListItemSecondaryAction>
           <Tooltip
             title={`Sign in with ${title}`}
@@ -139,7 +135,11 @@ const SessionListItem: FC<{
       </ListItemAvatar>
       <ListItemText
         className={classes.listItemText}
-        primary={id}
+        primary={
+          <Typography className={classes.listItemText} variant="body2">
+            {id}
+          </Typography>
+        }
         secondary={title}
       />
       <ListItemSecondaryAction style={{ marginLeft: '30px' }}>
@@ -163,25 +163,21 @@ const useGoogleLoginState = (open: boolean) => {
   const [profile, setProfile] = useState<ProfileInfo>();
 
   useEffect(() => {
-    if (!open) {
-      return;
-    }
-
     let didCancel = false;
 
-    googleAuth.getProfile().then(profile => {
-      if (didCancel) {
-        return;
-      }
-
-      setProfile(profile);
-      setLoading(false);
-    });
+    if (open) {
+      googleAuth.getProfile().then(_profile => {
+        if (!didCancel) {
+          setProfile(_profile);
+          setLoading(false);
+        }
+      });
+    }
 
     return () => {
       didCancel = true;
     };
-  }, [open]);
+  }, [open, googleAuth]);
 
   if (loading) {
     return { loading: true };
@@ -261,8 +257,11 @@ export const LoggedUserBadge: FC<Props> = ({
           </ListItemAvatar>
           {!collapsedMode && (
             <ListItemText
-              primary={displayName}
-              className={classes.listItemText}
+              primary={
+                <Typography className={classes.listItemText} variant="body2">
+                  {displayName}
+                </Typography>
+              }
             />
           )}
         </ListItem>
diff --git a/packages/core/src/layout/Sidebar/UserBadge.tsx b/packages/core/src/layout/Sidebar/UserBadge.tsx
index 0a1424ee6e..d3e02d26cc 100644
--- a/packages/core/src/layout/Sidebar/UserBadge.tsx
+++ b/packages/core/src/layout/Sidebar/UserBadge.tsx
@@ -16,14 +16,14 @@
 
 import React, { FC, useContext, useEffect, useState } from 'react';
 import { makeStyles } from '@material-ui/core';
-import People from '@material-ui/icons/People';
+import AccountCircleIcon from '@material-ui/icons/AccountCircle';
 import { SidebarContext } from './config';
 import { SidebarItem } from './Items';
 import { LoggedUserBadge } from './LoggedUserBadge';
 import DoubleArrowIcon from '@material-ui/icons/DoubleArrow';
 import { BackstageTheme } from '@backstage/theme';
 import { SidebarPinStateContext } from './Page';
-import { useApi, googleAuthApiRef, ProfileInfo } from '@backstage/core';
+import { useApi, googleAuthApiRef, ProfileInfo } from '@backstage/core-api';
 
 const ARROW_BUTTON_SIZE = 20;
 const useStyles = makeStyles<BackstageTheme, { isPinned: boolean }>(theme => {
@@ -63,8 +63,8 @@ export const SidebarUserBadge: FC<{}> = () => {
   const [profile, setProfile] = useState<ProfileInfo>();
 
   useEffect(() => {
-    //TODO(soapraj): How to observe if the user is logged in
-    //TODO(soapraj): Enumerate all the providers supported by the app and let user log in from here
+    // TODO(soapraj): How to observe if the user is logged in
+    // TODO(soapraj): List all the providers supported by the app and let user log in from here
     googleAuth.getProfile({ optional: true }).then(googleProfile => {
       setProfile(googleProfile);
     });
@@ -82,7 +82,7 @@ export const SidebarUserBadge: FC<{}> = () => {
           />
         </>
       ) : (
-        <SidebarItem icon={People} text="" disableSelected />
+        <SidebarItem icon={AccountCircleIcon} text="" disableSelected />
       )}
       {isOpen && (
         <button
diff --git a/packages/storybook/.storybook/apis.js b/packages/storybook/.storybook/apis.js
index 5743dac50b..cacc5af354 100644
--- a/packages/storybook/.storybook/apis.js
+++ b/packages/storybook/.storybook/apis.js
@@ -2,9 +2,13 @@ import {
   ApiRegistry,
   alertApiRef,
   errorApiRef,
+  oauthRequestApiRef,
+  OAuthRequestManager,
+  googleAuthApiRef,
   AlertApiForwarder,
   ErrorApiForwarder,
   ErrorAlerter,
+  GoogleAuth,
 } from '@backstage/core';
 
 const builder = ApiRegistry.builder();
@@ -13,4 +17,18 @@ const alertApi = builder.add(alertApiRef, new AlertApiForwarder());
 
 builder.add(errorApiRef, new ErrorAlerter(alertApi, new ErrorApiForwarder()));
 
+const oauthRequestApi = builder.add(
+  oauthRequestApiRef,
+  new OAuthRequestManager(),
+);
+
+builder.add(
+  googleAuthApiRef,
+  GoogleAuth.create({
+    apiOrigin: 'http://localhost:7000',
+    basePath: '/auth/',
+    oauthRequestApi,
+  }),
+);
+
 export const apis = builder.build();
diff --git a/plugins/auth-backend/src/providers/OAuthProvider.ts b/plugins/auth-backend/src/providers/OAuthProvider.ts
index 322afb26b9..a2be783155 100644
--- a/plugins/auth-backend/src/providers/OAuthProvider.ts
+++ b/plugins/auth-backend/src/providers/OAuthProvider.ts
@@ -199,7 +199,6 @@ export class OAuthProvider implements AuthProviderRouteHandlers {
   }
 
   async logout(req: express.Request, res: express.Response): Promise<any> {
-    //TODO(soapraj): Logout doesnt work yet
     if (!ensuresXRequestedWith(req)) {
       return res.status(401).send('Invalid X-Requested-With header');
     }
@@ -234,7 +233,6 @@ export class OAuthProvider implements AuthProviderRouteHandlers {
 
       // get new access_token
       const refreshInfo = await this.providerHandlers.refresh(
-        this.provider,
         refreshToken,
         scope,
       );
diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts b/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts
index e977ee877d..071c949a21 100644
--- a/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts
+++ b/plugins/auth-backend/src/providers/PassportStrategyHelper.test.ts
@@ -143,6 +143,14 @@ describe('PassportStrategyHelper', () => {
       class MyCustomRefreshTokenSuccess extends passport.Strategy {
         // @ts-ignore
         private _oauth2 = new MyCustomOAuth2Success();
+        userProfile(_accessToken: string, callback: Function) {
+          callback(null, {
+            provider: 'a',
+            email: 'b',
+            name: 'c',
+            picture: 'd',
+          });
+        }
       }
 
       const mockStrategy = new MyCustomRefreshTokenSuccess();
diff --git a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts b/plugins/auth-backend/src/providers/PassportStrategyHelper.ts
index bea349ce97..d329210b8e 100644
--- a/plugins/auth-backend/src/providers/PassportStrategyHelper.ts
+++ b/plugins/auth-backend/src/providers/PassportStrategyHelper.ts
@@ -137,9 +137,9 @@ export const executeRefreshTokenStrategy = async (
 
         anyStrategy.userProfile(
           accessToken,
-          (err: Error, passportProfile: passport.Profile) => {
-            if (err) {
-              reject(err);
+          (error: Error, passportProfile: passport.Profile) => {
+            if (error) {
+              reject(error);
             }
 
             const profile = makeProfileInfo(passportProfile, params);
diff --git a/plugins/auth-backend/src/providers/google/provider.ts b/plugins/auth-backend/src/providers/google/provider.ts
index b6690b0b82..761aae1ddf 100644
--- a/plugins/auth-backend/src/providers/google/provider.ts
+++ b/plugins/auth-backend/src/providers/google/provider.ts
@@ -20,6 +20,7 @@ import {
   executeFrameHandlerStrategy,
   executeRedirectStrategy,
   executeRefreshTokenStrategy,
+  makeProfileInfo,
 } from '../PassportStrategyHelper';
 import {
   OAuthProviderHandlers,
@@ -30,6 +31,7 @@ import {
   AuthInfoWithProfile,
 } from '../types';
 import { OAuthProvider } from '../OAuthProvider';
+import passport from 'passport';
 
 export class GoogleAuthProvider implements OAuthProviderHandlers {
   private readonly providerConfig: AuthProviderConfig;
@@ -44,14 +46,14 @@ export class GoogleAuthProvider implements OAuthProviderHandlers {
         accessToken: any,
         refreshToken: any,
         params: any,
-        profile: any,
+        profile: passport.Profile,
         done: any,
       ) => {
+        const profileInfo = makeProfileInfo(profile, params);
         done(
           undefined,
           {
-            //TODO(soapraj): extract ProfileInfo from passport.Profile and send only that
-            profile,
+            profile: profileInfo,
             idToken: params.id_token,
             accessToken,
             scope: params.scope,