Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[inversify-express-utils]: Auth provider cannot rely on server-level middleware #374

Open
geersch opened this issue Apr 9, 2019 · 3 comments

Comments

@geersch
Copy link

geersch commented Apr 9, 2019

Using the inversify-express-utils you can create an express application and configure:

  • an auth(orization) provider
  • server level middleware

When building the server application the http context is created before the server level middleware is registered.

Here's an excerpt of the build method.

this._app.all("*", (
  req: express.Request,
  res: express.Response,
  next: express.NextFunction
) => {
  (async () => {
      const httpContext = await _self._createHttpContext(req, res, next);
      ...
      next();
  })();
});

// register server-level middleware before anything else
if (this._configFn) {
  this._configFn.apply(undefined, [this._app]);
}

For each request the http context will be created, which in turn creates the principal. After that the server level middleware configured via the setConfig is executed.

 private async _createHttpContext(
   req: express.Request,
   res: express.Response,
   next: express.NextFunction
) {
  const principal = await this._getCurrentUser(req, res, next);
  ...
}

Let's assume a use case where you have to read data from the body in the authorization provider. If you configure the body-parser via the setConfig it's too late. This is called after the principal is created. When trying to compose a principal (getCurrentUser) you cannot ready the body as it is undefined at the time. The same probably applies for cookies as this requires the cookie-parser middleware to be setup.

@Goodluckhf
Copy link

yep, I've faced with the same issue

@chadgarlandscg
Copy link

I just ran into this issue trying to grab the access token from the headers of a CORS request in my
AuthProvider when I have CORS middleware set in the config.

To clarify, I have the following in my AuthProvider:

@injectable()
export default class PlayerAuthProvider implements interfaces.AuthProvider {
    async getUser(req: Request, res: Response, next: NextFunction): Promise<interfaces.Principal> {
        let accessTokenFromClient = req.headers["x-auth-token"];

which is missing the header, therefore returns 401 for a CORS request, given the following setup that uses setConfig:

const server = new InversifyExpressServer(container, null, null, null, PlayerAuthProvider);
server.setConfig((app) => {
    app.use(cors({origin: true}));
    app.options('*', cors());
    app.use(bodyParser.json());
});

However, it goes on to successfully validate the token and populate the Principal, using the following setup that configures an express app and passes it as an argument:

const app = express();
app.use(cors({origin: true}));
app.options('*', cors());
app.use(bodyParser.json());
const server = new InversifyExpressServer(container, null, null, app, PlayerAuthProvider);

I have found that re-arranging the code in the build (as mentioned by @geersch) to setup the config first solves the issue, as shown below:

// register server-level middleware before anything else
if (this._configFn) {
  this._configFn.apply(undefined, [this._app]);
}

this._app.all("*", (
  req: express.Request,
  res: express.Response,
  next: express.NextFunction
) => {
  (async () => {
      const httpContext = await _self._createHttpContext(req, res, next);
      ...
      next();
  })();
});

I was thinking about opening a PR with this fix, but wanted to search it in the issues first, at which point, I ended up here.

I'd still be happy to submit a PR, but I wanted to at least post on here first and see if there's any reply. I'm especially curious whether this is a valid solution, or rather if there is a legitimate reason that the http context is created before the server middleware is registered. Don't wanna break anything; this library rocks! 😄❤️

@PodaruDragos PodaruDragos transferred this issue from inversify/InversifyJS Jan 28, 2022
@DharmPatel4300
Copy link

Error not handled by .setErrorConfig((app) => app.use(errorHandler)) which is generated by interfaces.AuthProvider implimenting class in inversify-express-utils
My app is crashing down here is the code of errorHandler and CustomAuthProvider
//errorHandler
const errorHandler = (err: any, req: Request, res: Response, next: NextFunction) => {
console.log("Error handler called");
let error = err as ApiError;

if (!(error instanceof ApiError)) {
    error = new ApiError(
        500,
        err.message || "Something went wrong",
        err,
        err.stack);
}

// Now we are sure that the `error` variable will be an instance of ApiError class
const response = new ApiResponse(
    error.statusCode,
    false,
    error.message,
    //...(process.env.NODE_ENV === "development" ? { stack: error.stack } : {}), // Error stack traces should be visible in development for debugging
    error.errors,
);

logger.error(`${error.errors} 
${error.stack}`);

//removeUnusedMulterImageFilesOnError(req);
// Send error response
return res.status(response.statusCode).json(response);

};

//authprovider
@Injectable()
export class CustomAuthProvider implements interfaces.AuthProvider {

@inject(TYPES.IAuthService)
private readonly _authService!: IAuthService;


public async getUser(
    req: Request,
    res: Response,
    next: NextFunction
): Promise<interfaces.Principal> {
    try {
        const token = req.headers["x-token"]
        if (!token) {
            throw new ApiError(401, "Unauthorized");
        }
        const user = await this._authService.getUserByToken(token as string);
        console.log("CustomAuthProvider", token);
        const principal = new Principal(user);
        return principal;
    } catch (error) {
        console.log("CustomAuthProvider", error);

        throw new ApiError(401, "Unauthorized");
    }
}

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants