Merge info from multiple @Security methods into `req.user`

[daxadal]

Sorting

• I'm submitting a ...

  • [ ] bug report
  • [ ] feature request
  • [x] support request

• I confirm that I

  • [x] used the search to make sure that a similar issue hasn't already been submit

Expected Behavior

I have a game-like API in which users have characters and can perform certain actions "in-character". I implemented auth methods both for the user (Bearer Auth) and for the character (Token in Header). To perform some actions, I need to know which user and character they're using. Here's an example code:

@Route("game")
@Tags("game")
@Security({ bearerAuth: [], characterHeader: [] })
export class GameController extends Controller {
  /* ... */

  @Post("/attack")
  async attack(
    @Request() req: any,
    @Body() body: AttackBody
  ): Promise<ExpandedGameType> {
    const character: CharacterDocument = req.user.character;
    const game = await this.useCase.attack(character, body);
    return this.format(game);
  }
}

By defining the security like this, both methods should succeed and return its respective relevant info and save the conjoined info into request.user.

Current Behavior

The code snippet it generates for auth handling is this:

// routes.ts (Auto-generated)

if (Object.keys(secMethod).length > 1) {
  const secMethodAndPromises: Promise<any>[] = [];

  for (const name in secMethod) {
    secMethodAndPromises.push(
      expressAuthentication(request, name, secMethod[name]).catch(
        pushAndRethrow
      )
    );
  }

  secMethodOrPromises.push(
    Promise.all(secMethodAndPromises).then((users) => {
      return users[0]; // <-- Only saves info from first middleware into `request.user`
    })
  );
}

Possible Solution

To have both the user and character info available, instead of just picking users[0] I'd expect it to be merged someway like:

// routes.ts (Auto-generated)

secMethodOrPromises.push(
  Promise.all(secMethodAndPromises).then((users) => {
    return users.reduceRight((prev, current) => ({ ...prev, ...current }));
  })
);

Steps to Reproduce

Here's some example of authentication code to reproduce:

// authentication.ts

export async function expressAuthentication(
  request: Request,
  securityName: string,
  scopes?: string[]
): Promise<{
  token?: string;
  user?: UserDocument;
  character?: CharacterDocument;
}> {
  if (securityName === "bearerAuth") {
    const { token, user } = await getTokenAndUser(request);

    return { token, user };
  } else if (securityName === "characterHeader") {
    const { token, character } = await getHeaderAndCharacter(request);

    return { token, character };
  } else {
    throw new InternalServerError();
  }
}

export async function getTokenAndUser(
  req: Request
): Promise<{ token?: string; user?: UserDocument }> {
  // Mock implementation
  return {
    token: "user-token",
    user: { name: "username", email: "username@exapmple.com" },
  };
}

export async function getHeaderAndCharacter(
  req: Request
): Promise<{ token?: string; character?: CharacterDocument }> {
  // Mock implementation
  return { token: "character-token", character: { name: "character-name" } };
}

Context (Environment)

Version of the library: 4.1.3 Version of NodeJS: v16.17.0

• Confirm you were using yarn not npm: I'm using npm actually, but it should affect the outcome.

Detailed Description

When auto-generating the routes.ts file, if many mandatory authentication methods are provided, the request.user is populated with the joined info returned by all auth middlewares.

Breaking change?

It probably won't be a breaking change. When one auth method is provided, or just one is necessary, the response should be the same. When multiple auth methods must pass, the request.user may change, producing a breaking change. Nevertheless, if the auth responses ar reduced from the right, users[0] fields will dominate over the rest, reducing the likeliness of behavioural changes.

We may also want to take into account cases when the returned info is not an object, and the reduction strategy provided may not work.

[github-actions[bot]]

Hello there daxadal 👋

Thank you for opening your very first issue in this project.

We will try to get back to you as soon as we can.👀

[midoelhawy]

HI @daxadal,

I think you are using auth wrongly.

In the case of Character, you need to implement a middleware like this:

import { NextFunction, Response } from 'express';

export async function HeaderAndCharacterMiddleware(request: any, response: Response, next: NextFunction) {
  if (!request.header.character) {
    return response.status(500).json({
      success: false,
      message: 'No Character Found',
    });
  }
  // You can inject any prop to request to use it in your method controller
  request.myCharacterExample = request.header.character; // What you want
  return next();
}

Next, you need to use it in your endpoints:

@Route("game")
@Tags("game")
@Security({ bearerAuth: [] })
@Middlewares(HeaderAndCharacterMiddleware)
export class GameController extends Controller {

  @Post("/attack")
  async attack(
    @Request() req: any,
    @Body() body: AttackBody
  ): Promise<ExpandedGameType> {
    const character: CharacterDocument = req.myCharacterExample;
    const game = await this.useCase.attack(character, body);
    return this.format(game);
  }
}

NOTE: You can create a custom request TypeScript interface and use it; for example: @Request() req: Request & { myCharacterExample: CharacterDocument },

[daxadal]

Thank you for the response, @midoelhawy . Yes, that's a viable response, using a custom middleware instead of an auth middleware. Also, I though about creating an auth method that checks both the Bearer JWT for the user and the Header JWT for the character.

Yeah, I know my approach is unconventional. Just like the user token is required to access certain methods and performing certain actions as the user, the character token is also required to access certain methods and performing certain actions as the character. And that token would be required and consistent for many methods. That's why it feels very "auth-like" to me, and why I was adding "signin and signout-like" methods for the character. Also, I like how it then presents in the documentation when I define it as a security method. That way, I only set it up once.

That also arises a question for me. When I enforce multiple security methods on an API, and all of them must pass, but the information about the user is only retrieved on the first one, what would be the rest for? Just checking API tokens, origin and stuff, and giving them a thumbs up? What if you "misorder" the security methods and check the API Token first? The authentication would break, right? What's the use case then? Seeing the generated code rises some questions for me.

Nevertheless, thank you for your input., I do appreciate it. It might be the way to go, but I kind of prefer to exhaust other options and try to haev it "my way" first. And also, try to understand the current implementation and the use cases and needs it covers.

[github-actions[bot]]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days