scimmyjs / scimmy

SCIM m(ade eas)y - SCIM 2.0 library for NodeJS
https://scimmyjs.github.io
MIT License
41 stars 10 forks source link

Cannot Read Properties of Undefined (Reading 'Resources') in NestJS Integration #29

Closed omidraha closed 4 months ago

omidraha commented 4 months ago

Description:

I'm encountering an issue while integrating SCIMMY with a NestJS project. After following the installation and initialization steps, I receive the following error when starting the application:


npm run start

> scim@0.0.1 start
> nest start

[Nest] 216727  - 2024-06-10, 11:44:21 a.m.     LOG [NestFactory] Starting Nest application...
[Nest] 216727  - 2024-06-10, 11:44:21 a.m.   ERROR [ExceptionHandler] Cannot read properties of undefined (reading 'Resources')

TypeError: Cannot read properties of undefined (reading 'Resources')
    at new ScimService (git/scim/src/scim/scim.service.ts:7:34)
    at Injector.instantiateClass (git/scim/node_modules/@nestjs/core/injector/injector.js:365:19)
    at callback (git/scim/node_modules/@nestjs/core/injector/injector.js:65:45)
    at Injector.resolveConstructorParams (git/scim/node_modules/@nestjs/core/injector/injector.js:144:24)
    at Injector.loadInstance (git/scim/node_modules/@nestjs/core/injector/injector.js:70:13)
    at Injector.loadProvider (git/scim/node_modules/@nestjs/core/injector/injector.js:97:9)
    at git/scim/node_modules/@nestjs/core/injector/instance-loader.js:56:13
    at async Promise.all (index 3)
    at InstanceLoader.createInstancesOfProviders (git/scim/node_modules/@nestjs/core/injector/instance-loader.js:55:9)
    at git/scim/node_modules/@nestjs/core/injector/instance-loader.js:40:13

Steps to Reproduce:

  1. Install SCIMMY via npm install scimmy.
  2. Create a service in NestJS to initialize SCIMMY.
  3. Import and use SCIMMY.Resources.
  4. Start the application.

Code Example:

import { Injectable } from '@nestjs/common';
import SCIMMY from 'scimmy';

@Injectable()
export class ScimService {
  constructor() {
    const resources = new SCIMMY.Resources.User();

    SCIMMY.Resources.declare(resources)
      .ingress((resource, data) => this.handleUserCreation(resource, data))
      .egress((resource) => this.handleUserRetrieval(resource))
      .degress((resource) => this.handleUserDeletion(resource));
  }

  handleUserCreation(resource, data) {
    return { message: 'User created', resource, data };
  }

  handleUserRetrieval(resource) {
    return { message: 'User retrieved', resource };
  }

  handleUserDeletion(resource) {
    return { message: 'User deleted', resource };
  }
}

Environment:

Node.js version: v22.2.0 NestJS version: 10.0.0 SCIMMY version: 1.2.1 OS: Ubuntu

Additional Context:

The project compiles without issues, but fails at runtime with the mentioned error.

Expected Behavior: SCIMMY should initialize correctly and allow the application to run without errors.

Sample Source Code:

https://github.com/omidraha/nestjs-example/tree/main/src/scim

sleelin commented 4 months ago

Hi @omidraha, thanks for your thorough issue report, the format and level of detail are greatly appreciated 🙂
Using your linked sample I was able to reproduce the issue, determine the cause, and come up with a solution.

Cause: It seems that the default TypeScript tsconfig.json for NestJS is set up to transpile outputs into CommonJS format, presumably for compatibility reasons. This shouldn't be an issue - since SCIMMY itself is written as ESM-first, the build process also includes a transpilation to CommonJS step.

The underlying cause of the issue is that SCIMMY's transpiled CommonJS entrypoint is set using the module.exports method, instead of the exports.default method expected by TypeScript. Investigation of the build output of the scim.service.ts file confirmed this - all references to the imported SCIMMY class have been transpiled to point at the default value of the import, which is not set.

Solution: Fortunately, the solution to this issue has actually been built in to TypeScript since version 2.7, but has not been enabled in the default NestJS tsconfig.json for some reason. You can enable it by setting the esModuleInterop property of compilerOptions to true in your tsconfig.json file, as below:

{
  "compilerOptions": {
    "module": "commonjs",
    "esModuleInterop": true,  // <-- add this line
    "declaration": true,
    /* the rest of your existing tsconfig */
  }
}

With esModuleInterop enabled, the transpiled code finds the correct entrypoint for SCIMMY, and the application can be started as expected.

Other Observations: It's also worth noting that the ScimService example code will still not work correctly. This is because the SCIMMY.Resources.declare method expects an overall resource class to be declared, as opposed to an instance of a resource class (see the documentation for more details on this process). With that in mind, declaration may be better suited inside the controller class constructor, which would look something like the following:

import {Controller, Post, Get, Delete, Body, Param, Query, Put} from "@nestjs/common";
import {ScimService} from "./scim.service";
import SCIMMY from "scimmy";

@Controller('scim')
export class ScimController {
  constructor(private readonly scimService: ScimService) {
    // Declare the entire User resource class, and wire up handlers
    SCIMMY.Resources.declare(SCIMMY.Resources.User)
        .ingress(({id}, data: any) => id ? this.scimService.handleUserUpdating(id, data) : this.scimService.handleUserCreation(data))
        .egress((resource: any) => this.scimService.handleUserRetrieval(resource))
        .degress(({id}) => this.scimService.handleUserDeletion(id));
  }

  @Get('Users')
  async getUsers(@Query() query: any) {
    try {
      // Delegate to SCIMMY for retrieval of many users
      return await new SCIMMY.Resources.User(query).read(null);
    } catch (ex) {
      // Generate SCIM Error messages for exceptions
      return new SCIMMY.Messages.Error(ex);
    }
  }

  @Post('Users')
  async createUser(@Body() body: any, @Query() query: any) {
    try {
      // Delegate to SCIMMY for user creation
      return await new SCIMMY.Resources.User(query).write(body, null);
    } catch (ex) {
      // Generate SCIM Error messages for exceptions
      return new SCIMMY.Messages.Error(ex);
    }
  }

  @Get('Users/:id')
  async getUser(@Param('id') id: string, @Query() query: any) {
    try {
      // Delegate to SCIMMY for retrieval of one user
      return await new SCIMMY.Resources.User(id, query).read(null);
    } catch (ex) {
      // Generate SCIM Error messages for exceptions
      return new SCIMMY.Messages.Error(ex);
    }
  }

  @Put('Users/:id')
  async updateUser(@Param('id') id: string, @Body() body: any, @Query() query: any) {
    try {
      // Delegate to SCIMMY for updating of one user
      return await new SCIMMY.Resources.User(id, query).write(body, null);
    } catch (ex) {
      // Generate SCIM Error messages for exceptions
      return new SCIMMY.Messages.Error(ex);
    }
  }

  @Delete('Users/:id')
  async deleteUser(@Param('id') id: string, @Query() query: any) {
    try {
      // Delegate to SCIMMY for deletion of one user
      return await new SCIMMY.Resources.User(id, query).dispose(null);
    } catch (ex) {
      // Generate SCIM Error messages for exceptions
      return new SCIMMY.Messages.Error(ex);
    }
  }
}

This would mean the associated service class only needs to handle database interactions, and could be re-used elsewhere if needed. It would look something like the following:

import {Injectable} from "@nestjs/common";

@Injectable()
export class ScimService {
  constructor() {}

  handleUserCreation(data: any) {
    // Some database call to create the new user
    return {/* an object with all User attributes */};
  }

  handleUserRetrieval(resource: any) {
    // Useful things for your database query
    const {id, filter, attributes, constraints} = resource;

    if (id) {
      // Some database call to find one existing user
      return {/* an object with all User attributes */};
    } else {
      // Some database call to find many existing users
      return [/* many objects, each with all User attributes */];
    }
  }

  handleUserUpdating(id: string, data: any) {
    // Some database call to update an existing user
    return {/* an object with all User attributes */};
  }

  handleUserDeletion(id: string) {
    // Some database call to delete an existing user
    return; /* nothing, delete requests do not send back content */
  }
}

The above deals only with the User resource, though the same principle applies to Group or custom resources.

[!WARNING] While the /User endpoint may be all you need, the SCIM protocol defines many supporting endpoints such as /Schemas and /ServiceProviderConfig, which are equally important, and used to inform consumers of how your SCIM service behaves.

Final Notes: It looks as though, under the hood, NestJS uses express by default. Provided you weren't intending on changing this to the Fastify adapter, you may be better off looking for a way to integrate the scimmy-routers package with your Nest project. The advantage here would be that scimmy-routers, which uses scimmy internally, already implements all SCIM endpoints as express middleware, which may save you the hassle of implementing them all again.

Thanks, Sam