Merge pull request #170 from icefoganalytics/issue-166/departmental-a…

…dministration-create-a-travel-auth-for-another-employee-within-my-department

Part 1: Departmental Administration: Create a Travel Auth for Another Employee Within My Department

api/src/controllers/travel-authorizations-controller.ts

@@ -170,10 +170,7 @@ export class TravelAuthorizationsController extends BaseController {
 
   private buildTravelAuthorization() {
     const attributes = this.request.body
-    const travelAuthorization = TravelAuthorization.build({
-      ...attributes,
-      userId: this.currentUser.id,
-    })
+    const travelAuthorization = TravelAuthorization.build(attributes)
     return travelAuthorization
   }
 

api/src/controllers/users-controller.ts

@@ -5,8 +5,28 @@ import BaseController from "./base-controller"
 import { User } from "@/models"
 import { UsersPolicy } from "@/policies"
 import { UsersSerializer } from "@/serializers"
+import { CreateService } from "@/services/users"
 
 export class UsersController extends BaseController {
+  async create() {
+    const user = await this.buildUser()
+    const policy = this.buildPolicy(user)
+    if (!policy.create()) {
+      return this.response
+        .status(403)
+        .json({ message: "You are not authorized to create this user." })
+    }
+
+    const permittedAttributes = policy.permitAttributesForCreate(this.request.body)
+    return CreateService.perform(permittedAttributes, this.currentUser)
+      .then((user) => {
+        return this.response.status(201).json({ user })
+      })
+      .catch((error) => {
+        return this.response.status(422).json({ message: `User creation failed: ${error}` })
+      })
+  }
+
   async show() {
     const user = await this.loadUser()
     if (isNil(user)) return this.response.status(404).json({ message: "User not found." })
@@ -22,6 +42,12 @@ export class UsersController extends BaseController {
     return this.response.status(200).json({ user: serializedUser })
   }
 
+  private async buildUser() {
+    const attributes = this.request.body
+    const user = User.build(attributes)
+    return user
+  }
+
   private loadUser(): Promise<User | null> {
     return User.findByPk(this.params.userId)
   }

api/src/db/migrations/20240122154624_make-user-role-names-more-readable.ts

@@ -0,0 +1,72 @@
+import { Knex } from "knex";
+
+
+export async function up(knex: Knex): Promise<void> {
+  await knex.raw(`
+    WITH unnested_roles AS (
+      SELECT
+        id
+        , CASE WHEN "role" = 'dept_admin' THEN 'department_admin'
+               ELSE "role"
+          END AS transformed_role
+      FROM
+        "users"
+        , unnest(roles) AS "role"
+      WHERE
+        roles IS NOT NULL
+    )
+    , aggregated_updated_roles AS (
+      SELECT
+        id
+        , array_agg(transformed_role) AS roles
+      FROM
+        unnested_roles
+      GROUP BY
+        id
+    )
+    UPDATE
+      "users"
+    SET
+      roles = aggregated_updated_roles.roles
+    FROM
+      aggregated_updated_roles
+    WHERE
+      "users".id = aggregated_updated_roles.id
+      AND "users".roles IS NOT NULL;
+  `)
+}
+
+
+export async function down(knex: Knex): Promise<void> {
+  await knex.raw(`
+    WITH unnested_roles AS (
+      SELECT
+        id
+        , CASE WHEN "role" = 'department_admin' THEN 'dept_admin'
+               ELSE "role"
+          END AS transformed_role
+      FROM
+        "users"
+        , unnest(roles) AS "role"
+      WHERE
+        roles IS NOT NULL
+    )
+    , aggregated_updated_roles AS (
+      SELECT
+        id
+        , array_agg(transformed_role) AS roles
+      FROM
+        unnested_roles
+      GROUP BY
+        id)
+    UPDATE
+      "users"
+    SET
+      roles = aggregated_updated_roles.roles
+    FROM
+      aggregated_updated_roles
+    WHERE
+      "users".id = aggregated_updated_roles.id
+      AND "users".roles IS NOT NULL;
+  `)
+}

api/src/models/user.ts

@@ -27,7 +27,7 @@ export enum Roles {
   ADMIN = "admin",
   USER = "user",
   PAT_ADMIN = "pat_admin",
-  DEPT_ADMIN = "dept_admin",
+  DEPARTMENT_ADMIN = "department_admin",
   TD_USER = "td_user",
 }
 

api/src/policies/travel-authorizations-policy.ts

@@ -6,6 +6,7 @@ import { User, TravelAuthorization } from "@/models"
 
 export class TravelAuthorizationsPolicy extends BasePolicy<TravelAuthorization> {
   create(): boolean {
+    if (this.user.roles.includes(User.Roles.ADMIN)) return true
     if (this.record.userId === this.user.id) return true
 
     return false
@@ -67,12 +68,12 @@ export class TravelAuthorizationsPolicy extends BasePolicy<TravelAuthorization>
     return modelClass.scope({ where })
   }
 
+  // NOTE: userId is always restricted after creation.
   permittedAttributes(): string[] {
     return [
-      "userId", // TODO: Permit but don't let non-admins create travel authorizations for anyone but themselves via policy
       "preappId",
       "purposeId",
-      "firstName", // all this user information should probably be restricted?
+      "firstName",
       "lastName",
       "department",
       "division",
@@ -105,7 +106,13 @@ export class TravelAuthorizationsPolicy extends BasePolicy<TravelAuthorization>
   }
 
   permittedAttributesForCreate() {
-    return ["slug", ...this.permittedAttributes()]
+    const permittedAttributes: string[] = [...this.permittedAttributes(), "slug", "stopsAttributes"]
+
+    if (this.user.roles.includes(User.Roles.ADMIN)) {
+      permittedAttributes.push("userId", "userAttributes")
+    }
+
+    return permittedAttributes
   }
 }
 

api/src/routes/index.ts

@@ -87,6 +87,7 @@ router.route("/api/locations/:locationId").get(LocationsController.show)
 router.route("/api/pre-approved-travelers").get(PreApprovedTravelersController.index)
 router.route("/api/pre-approved-travel-requests").get(PreApprovedTravelRequestsController.index)
 
+router.route("/api/users").post(UsersController.create)
 router.route("/api/users/:userId").get(UsersController.show)
 router
   .route("/api/users/:userId/yg-government-directory-sync")

api/src/routes/traveldesk-router.ts

@@ -92,7 +92,7 @@ travelDeskRouter.get(
     const adminScoping: WhereOptions<TravelAuthorization> = {}
     if (req?.user?.roles?.includes(User.Roles.ADMIN)) {
       // No additional conditions for Admin, selects all records
-    } else if (req?.user?.roles?.includes(User.Roles.DEPT_ADMIN)) {
+    } else if (req?.user?.roles?.includes(User.Roles.DEPARTMENT_ADMIN)) {
       adminScoping.department = req.user.department
     } else {
       adminScoping.userId = req.user.id

api/src/services/travel-authorizations/create-service.ts

@@ -5,8 +5,9 @@ import { v4 as uuid } from "uuid"
 import db from "@/db/db-client"
 
 import BaseService from "@/services/base-service"
-import { Stops, StopsService, ExpensesService } from "@/services"
+import { Stops, StopsService, ExpensesService, Users } from "@/services"
 import { AuditService } from "@/services/audit-service"
+import { UserCreationAttributes } from "@/services/users/create-service"
 import { Expense, Stop, TravelAuthorization, TravelAuthorizationActionLog, User } from "@/models"
 
 type StopsCreationAttributes = CreationAttributes<Stop>[]
@@ -18,6 +19,7 @@ type TravelAuthorizationCreationAttributes = Omit<
 } & {
   stopsAttributes?: StopsCreationAttributes
   expensesAttributes?: CreationAttributes<Expense>[]
+  userAttributes?: UserCreationAttributes
 }
 
 // TODO: upgrade this to the enhanced service pattern.
@@ -27,10 +29,12 @@ export class CreateService extends BaseService {
   private stopsAttributes: StopsCreationAttributes
   private expensesAttributes: CreationAttributes<Expense>[]
   private attributes: TravelAuthorizationCreationAttributes
+  private userAttributes?: UserCreationAttributes
   private currentUser: User
 
   constructor(
     {
+      userAttributes,
       stopsAttributes = [],
       expensesAttributes = [],
       ...attributes
@@ -39,21 +43,28 @@ export class CreateService extends BaseService {
   ) {
     super()
     this.attributes = attributes
+    this.userAttributes = userAttributes
     this.stopsAttributes = stopsAttributes
     this.expensesAttributes = expensesAttributes
     this.currentUser = currentUser
   }
 
   async perform(): Promise<TravelAuthorization> {
-    const secureAttributes = {
-      ...this.attributes,
-      status: TravelAuthorization.Statuses.DRAFT,
-      slug: this.attributes.slug || uuid(),
-      createdBy: this.currentUser.id,
-    }
-
     return db
       .transaction(async () => {
+        const secureAttributes = {
+          ...this.attributes,
+          status: TravelAuthorization.Statuses.DRAFT,
+          slug: this.attributes.slug || uuid(),
+          createdBy: this.currentUser.id,
+        }
+
+        secureAttributes.userId = await this.determineSecureUserId(
+          this.currentUser,
+          this.attributes.userId,
+          this.userAttributes
+        )
+
         const travelAuthorization = await TravelAuthorization.create(secureAttributes).catch(
           (error) => {
             throw new Error(`Could not create TravelAuthorization: ${error}`)
@@ -91,7 +102,33 @@ export class CreateService extends BaseService {
       })
   }
 
-  async createStops(
+  private async determineSecureUserId(
+    currentUser: User,
+    userId: number | undefined,
+    userAttributes: UserCreationAttributes | undefined
+  ): Promise<number> {
+    // This pattern is a bit off, but I can't think of a better place to put this logic.
+    // If the user is not an admin, the userId must come from the current user.
+    // TODO: consider putting this code in the policy?
+    if (!this.currentUser.roles.includes(User.Roles.ADMIN)) {
+      return this.currentUser.id
+    }
+
+    if (userId !== undefined && userAttributes !== undefined) {
+      throw new Error("Cannot specify both userId and userAttributes.")
+    } else if (userId === undefined && userAttributes === undefined) {
+      throw new Error("Must specify either userId or userAttributes.")
+    } else if (userId !== undefined) {
+      return userId
+    } else if (userAttributes !== undefined) {
+      const user = await Users.EnsureService.perform(userAttributes, this.currentUser)
+      return user.id
+    } else {
+      throw new Error("This should never be reached, but it makes TypeScript happy.")
+    }
+  }
+
+  private async createStops(
     travelAuthorization: TravelAuthorization,
     stopsAttributes: StopsCreationAttributes
   ) {
@@ -104,7 +141,7 @@ export class CreateService extends BaseService {
   }
 
   // TODO: might want to make this a validator against updates as well?
-  ensureMinimalDefaultStopsAttributes(
+  private ensureMinimalDefaultStopsAttributes(
     travelAuthorization: TravelAuthorization,
     stopsAttributes: StopsCreationAttributes
   ): StopsCreationAttributes {
@@ -123,7 +160,7 @@ export class CreateService extends BaseService {
     }
   }
 
-  ensureMinimalDefaultMultiDestinationStopsAttributes(
+  private ensureMinimalDefaultMultiDestinationStopsAttributes(
     travelAuthorizationId: number,
     stopsAttributes: StopsCreationAttributes
   ): StopsCreationAttributes {
@@ -155,7 +192,7 @@ export class CreateService extends BaseService {
     ]
   }
 
-  ensureMinimalDefaultOneWayStopsAttributes(
+  private ensureMinimalDefaultOneWayStopsAttributes(
     travelAuthorizationId: number,
     stopsAttributes: StopsCreationAttributes
   ): StopsCreationAttributes {
@@ -175,7 +212,7 @@ export class CreateService extends BaseService {
     ]
   }
 
-  ensureMinimalDefaultRoundTripStopsAttributes(
+  private ensureMinimalDefaultRoundTripStopsAttributes(
     travelAuthorizationId: number,
     stopsAttributes: StopsCreationAttributes
   ): StopsCreationAttributes {

api/src/services/users/create-service.ts

@@ -1,13 +1,13 @@
 import { CreationAttributes } from "sequelize"
-import { isEmpty } from "lodash"
+import { isEmpty, isUndefined } from "lodash"
 
 import db from "@/db/db-client"
 
 import BaseService from "@/services/base-service"
 import { User } from "@/models"
 import { YkGovernmentDirectorySyncService } from "@/services"
 
-type AttributesWithDefaults = "sub" | "roles" | "status"
+type AttributesWithDefaults = "sub" | "email" | "roles" | "status"
 export type UserCreationAttributes = Omit<CreationAttributes<User>, AttributesWithDefaults> &
   Partial<Pick<User, AttributesWithDefaults>>
 
@@ -27,7 +27,7 @@ export class CreateService extends BaseService {
     let fallbackEmail: string = ""
     let fallbackFirstName: string = ""
     let fallbackLastName: string = ""
-    if (!isEmpty(email)) {
+    if (!isUndefined(email) && !isEmpty(email)) {
       const names = email.split("@")[0].split(".")
       fallbackFirstName = names[0] || ""
       fallbackLastName = names[1] || ""

api/src/services/users/ensure-service.ts

@@ -1,5 +1,4 @@
 import { isNil } from "lodash"
-import { CreationAttributes } from "sequelize"
 
 import BaseService from "@/services/base-service"
 import { CreateService, type UserCreationAttributes } from "@/services/users/create-service"
@@ -18,9 +17,12 @@ export class EnsureService extends BaseService {
   async perform(): Promise<User> {
     const { email } = this.attributes
 
-    const user = await User.findOne({
-      where: { email },
-    })
+    let user: User | null = null
+    if (email !== undefined) {
+      user = await User.findOne({
+        where: { email },
+      })
+    }
 
     if (!isNil(user)) {
       return user