Consistently use extractRequestUser

Some locations used req.user.user, which was a holdover from a
different auth system that just happened to still work with NullAuth.

Author: Jacob Peddicord

package-lock.json

@@ -20,7 +20,8 @@ import { config } from '../../config';
 import { AccessError } from '../../errors';
 
 export async function canValidate(req) {
-  const groups = await auth.getGroups(req.user.user);
+  const user = auth.extractRequestUser(req);
+  const groups = await auth.getGroups(user);
 
   if (isUserInAnyGroup(groups, config.admin.verifiers)) {
     return true;
@@ -30,7 +31,7 @@ export async function canValidate(req) {
     return true;
   }
 
-  winston.warn('User %s cannot validate package metadata', req.user.user);
+  winston.warn('User %s cannot validate package metadata', user);
   return false;
 }
 

server/api/packages/auth.ts

@@ -15,6 +15,7 @@
 import * as Immutable from 'immutable';
 import * as winston from 'winston';
 
+import auth from '../../auth';
 import * as db from '../../db/packages';
 import { assertCanValidate } from './auth';
 import { WebPackage } from './interfaces';
@@ -111,7 +112,7 @@ export async function storePackage(req: any, packageId: number, info: Pick<WebPa
   // create a new revision if anything changed (or it didn't exist)
   let newId: number;
   if (shouldInsert) {
-    const createdBy = req.user.user;
+    const createdBy = auth.extractRequestUser(req);
     newId = await db.createPackageRevision(info.name, info.version, info.website,
       info.license, info.copyright, info.licenseText, createdBy);
     winston.info('Created a new package revision with ID %s (previous revision at %s) by %s',
@@ -126,12 +127,13 @@ export async function storePackage(req: any, packageId: number, info: Pick<WebPa
 
 export async function verifyPackage(req: any, packageId: number, verified: boolean,
                                     comments: string): Promise<Partial<WebPackage>> {
+  const user = auth.extractRequestUser(req);
   assertCanValidate(req);
   await Promise.all([
-    db.addVerification(packageId, req.user.user, comments),
+    db.addVerification(packageId, user, comments),
     db.verifyPackage(packageId, verified),
   ]);
-  winston.info('Package %s verified (%s) by %s', packageId, verified, req.user.user);
+  winston.info('Package %s verified (%s) by %s', packageId, verified, user);
   return {packageId};
 }
 

server/api/packages/index.ts

@@ -50,6 +50,7 @@ describe('projects auth', function () {
     mock = {
       auth: {
         getGroups: jasmine.createSpy('getGroups').and.returnValue(Promise.resolve(['a-nobody'])),
+        extractRequestUser: (req) => req.user.user,
       },
       config: {
         admin: {

server/api/projects/auth.spec.ts

@@ -22,9 +22,10 @@ import { AccessLevel, AccessLevelStrength } from './interfaces';
  * Check if the request's user is the project's contact list.
  */
 export function isInContacts(req: any, project: Pick<DbProject, 'contacts'>) {
+  const user = auth.extractRequestUser(req);
   for (const type of Object.keys(project.contacts)) {
     const contactList = project.contacts[type];
-    if (contactList.includes(req.user.user)) {
+    if (contactList.includes(user)) {
       return true;
     }
   }
@@ -50,7 +51,8 @@ export async function assertProjectAccess(req: any, project: ProjectAccess, leve
 }
 
 export async function effectivePermission(req: any, project: ProjectAccess): Promise<AccessLevel> {
-  const reqGroups = await auth.getGroups(req.user.user);
+  const user = auth.extractRequestUser(req);
+  const reqGroups = await auth.getGroups(user);
 
   // start by checking the global list
   // TODO: make global list ACL-like too

server/api/projects/auth.ts

@@ -23,7 +23,9 @@ describe('projects', function () {
     mockery.enable({useCleanCache: true, warnOnUnregistered: false});
     mock = {
       db: {},
-      auth: {},
+      auth: {
+        extractRequestUser: (req) => req.user.user,
+      },
       packagedb: {},
       assertProjectAccess: jasmine.createSpy('assertProjectAccess'),
       effectivePermission: jasmine.createSpy('effectivePermission'),

server/api/projects/index.spec.ts

@@ -59,7 +59,8 @@ export async function getProject(req: Request, projectId: string): Promise<WebPr
 }
 
 export async function searchProjects(req: Request): Promise<Array<Partial<WebProject>>> {
-  const groups = await auth.getGroups(req.user.user);
+  const user = auth.extractRequestUser(req);
+  const groups = await auth.getGroups(user);
 
   // all projects
   if (req.query.all) {
@@ -85,6 +86,7 @@ function mapProjectShortInfo(dbData): Partial<WebProject> {
 }
 
 export async function createProject(req: Request, body: WebProject): ProjectIdPromise {
+  const user = auth.extractRequestUser(req);
   const projectId = await db.createProject({
     title: body.title,
     version: body.version,
@@ -93,13 +95,14 @@ export async function createProject(req: Request, body: WebProject): ProjectIdPr
     contacts: body.contacts,
     acl: body.acl,
     metadata: body.metadata,
-  }, req.user.user);
+  }, user);
 
-  winston.info('Project %s created by %s', projectId, req.user.user);
+  winston.info('Project %s created by %s', projectId, user);
   return {projectId};
 }
 
 export async function patchProject(req: Request, projectId, changes): ProjectIdPromise {
+  const user = auth.extractRequestUser(req);
   const project = await db.getProject(projectId);
   await assertProjectAccess(req, project, 'editor');
 
@@ -122,15 +125,16 @@ export async function patchProject(req: Request, projectId, changes): ProjectIdP
     mappedChanges[internalMap[k]] = changes[k];
   }
 
-  await db.patchProject(projectId, mappedChanges, req.user.user);
+  await db.patchProject(projectId, mappedChanges, user);
 
-  winston.info('Project %s modified by %s', projectId, req.user.user);
+  winston.info('Project %s modified by %s', projectId, user);
   return {projectId};
 }
 
 export async function attachPackage(req: Request, projectId, info) {
   const { packageId, name, version, website, copyright, usage } = info;
   const { license, licenseText } = info;
+  const user = auth.extractRequestUser(req);
 
   // access check
   const project = await db.getProject(projectId);
@@ -151,7 +155,7 @@ export async function attachPackage(req: Request, projectId, info) {
   await db.updatePackagesUsed(projectId, [
     ...project.packages_used,
     usageInfo,
-  ], req.user.user);
+  ], user);
 
   // finally, return the updated/inserted package ID
   const addedPackageId = usageInfo.package_id;
@@ -160,19 +164,21 @@ export async function attachPackage(req: Request, projectId, info) {
 }
 
 export async function detachPackage(req: Request, projectId, packageId): ProjectIdPromise {
+  const user = auth.extractRequestUser(req);
   const project = await db.getProject(projectId);
   await assertProjectAccess(req, project, 'editor');
 
   const newUsage = project.packages_used.filter((item) => {
     return item.package_id !== packageId;
   });
 
-  await db.updatePackagesUsed(projectId, newUsage, req.user.user);
+  await db.updatePackagesUsed(projectId, newUsage, user);
   winston.info('Detached package %s from project %s', packageId, projectId);
   return {projectId};
 }
 
 export async function replacePackage(req: Request, projectId: string, oldId: number, newId: number): ProjectIdPromise {
+  const user = auth.extractRequestUser(req);
   const project = await db.getProject(projectId);
   await assertProjectAccess(req, project, 'editor');
 
@@ -183,12 +189,13 @@ export async function replacePackage(req: Request, projectId: string, oldId: num
     }
   }
 
-  await db.updatePackagesUsed(projectId, usage, req.user.user);
+  await db.updatePackagesUsed(projectId, usage, user);
   winston.info('Replaced package %s -> %s on project %s', oldId, newId, projectId);
   return {projectId};
 }
 
 export async function generateAttributionDocument(req: Request, projectId: string, store: boolean = false) {
+  const user = auth.extractRequestUser(req);
   const project = await db.getProject(projectId);
   await assertProjectAccess(req, project, 'viewer');
 
@@ -216,9 +223,8 @@ export async function generateAttributionDocument(req: Request, projectId: strin
 
   // save a copy if requested
   if (store) {
-    const createdBy = req.user.user;
-    documentdb.storeAttributionDocument(projectId, project.version, text, createdBy);
-    winston.info(`Document for project ${projectId} was stored by ${createdBy}`);
+    documentdb.storeAttributionDocument(projectId, project.version, text, user);
+    winston.info(`Document for project ${projectId} was stored by ${user}`);
     return {text};
   }