fix(auth-backend): cleanup

* remove unused method "extractGitlabUserId"
* consistent profile id
* move avatarUrl check to makeProfileInfo (extend passport type)

Signed-off-by: Tom Opdebeeck <tom.opdebeeck@studiohyperdrive.be>

plugins/auth-backend/src/lib/passport/PassportStrategyHelper.ts

@@ -17,9 +17,11 @@
 import express from 'express';
 import passport from 'passport';
 import jwtDecoder from 'jwt-decode';
-import { ProfileInfo, RedirectInfo } from '../../providers/types';
 import { InternalOAuthError } from 'passport-oauth2';
 
+import { PassportProfile } from './types';
+import { ProfileInfo, RedirectInfo } from '../../providers/types';
+
 export type PassportDoneCallback<Res, Private = never> = (
   err?: Error,
   response?: Res,
@@ -27,7 +29,7 @@ export type PassportDoneCallback<Res, Private = never> = (
 ) => void;
 
 export const makeProfileInfo = (
-  profile: passport.Profile,
+  profile: PassportProfile,
   idToken?: string,
 ): ProfileInfo => {
   let { displayName } = profile;
@@ -39,7 +41,9 @@ export const makeProfileInfo = (
   }
 
   let picture: string | undefined = undefined;
-  if (profile.photos && profile.photos.length > 0) {
+  if (profile.avatarUrl) {
+    picture = profile.avatarUrl;
+  } else if (profile.photos && profile.photos.length > 0) {
     const [firstPhoto] = profile.photos;
     picture = firstPhoto.value;
   }
@@ -193,12 +197,12 @@ type ProviderStrategy = {
 export const executeFetchUserProfileStrategy = async (
   providerStrategy: passport.Strategy,
   accessToken: string,
-): Promise<passport.Profile> => {
+): Promise<PassportProfile> => {
   return new Promise((resolve, reject) => {
     const anyStrategy = providerStrategy as unknown as ProviderStrategy;
     anyStrategy.userProfile(
       accessToken,
-      (error: Error, rawProfile: passport.Profile) => {
+      (error: Error, rawProfile: PassportProfile) => {
         if (error) {
           reject(error);
         } else {

plugins/auth-backend/src/lib/passport/types.ts

@@ -0,0 +1,20 @@
+/*
+ * Copyright 2020 The Backstage Authors
+ *
+ * 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 passport from 'passport';
+
+export type PassportProfile = passport.Profile & {
+  avatarUrl?: string;
+};

plugins/auth-backend/src/providers/gitlab/provider.ts

@@ -31,7 +31,6 @@ import {
   AuthProviderFactory,
   SignInResolver,
   AuthHandler,
-  ProfileInfo,
 } from '../types';
 import {
   OAuthAdapter,
@@ -47,10 +46,6 @@ import {
 import { TokenIssuer } from '../../identity';
 import { CatalogIdentityClient } from '../../lib/catalog';
 
-type FullProfile = OAuthResult['fullProfile'] & {
-  avatarUrl?: string;
-};
-
 type PrivateInfo = {
   refreshToken: string;
 };
@@ -64,17 +59,17 @@ export type GitlabAuthProviderOptions = OAuthProviderOptions & {
   logger: Logger;
 };
 
-export const extractGitLabUserId = (profile: ProfileInfo): string => {
-  return profile.email?.split('@')[0] as string;
-};
-
 export const gitlabDefaultSignInResolver: SignInResolver<OAuthResult> = async (
   info,
   ctx,
 ) => {
-  const { result } = info;
+  const { profile, result } = info;
 
-  const id = result.fullProfile.username || result.fullProfile.id;
+  let id = result.fullProfile.id;
+
+  if (profile.email) {
+    id = profile.email.split('@')[0];
+  }
 
   const token = await ctx.tokenIssuer.issueToken({
     claims: { sub: id, ent: [`user:default/${id}`] },
@@ -86,18 +81,9 @@ export const gitlabDefaultSignInResolver: SignInResolver<OAuthResult> = async (
 export const gitlabDefaultAuthHandler: AuthHandler<OAuthResult> = async ({
   fullProfile,
   params,
-}) => {
-  fullProfile.photos = [
-    ...(fullProfile.photos ?? []),
-    ...((fullProfile as FullProfile).avatarUrl
-      ? [{ value: (fullProfile as FullProfile).avatarUrl as string }]
-      : []),
-  ];
-
-  return {
-    profile: makeProfileInfo(fullProfile, params.id_token),
-  };
-};
+}) => ({
+  profile: makeProfileInfo(fullProfile, params.id_token),
+});
 
 export class GitlabAuthProvider implements OAuthHandlers {
   private readonly _strategy: GitlabStrategy;