lukeautry / tsoa

Build OpenAPI-compliant REST APIs using TypeScript and Node
MIT License
3.32k stars 481 forks source link

Evaluate @Security in order of definition #1608

Closed kbublitz closed 1 day ago

kbublitz commented 2 months ago

Sorting

Expected Behavior

I have a controller and routes that can be authenticated in several ways. I would like that my @Security methods are evaluated in the order I define them. So when the @Security handler I define first resolves, that one should determine the "user" field.

My controller looks something like this (simplified as much as possible, but note that there are two @Security annotations. The goal is that when the user has a valid auth token, I get the user information from the token, especially the access permissions the user has. But the route should still be accessible for unauthorized users, if the accessed file is marked as public.

@Security("authToken", ["file.read"])
@Security("public")
@Route("api")
export class ApiController extends Controller {

    @Get("file/*")
    public async getFile(@Request() req: express.Request): Promise<File> {
        const documentPath = req.params[0];
        const user = req.user;

        // The fileStorage will check that the user has the necessary access permissions
        return await fileStorage.getFile(filePath, user);
    }

I have implemented the expressAuthentication method something like this:

export async function expressAuthentication(
    request: express.Request,
    securityName: string,
    scopes?: string[],
): Promise<any> {
    switch (securityName) {
        case "authToken":
            // This will use jsonwebtoken to verify a jwt and resolve with a user object like { userName: "John Doe", "permissions": ["readPrivateFiles", "readPublicFiles"] }
            return await evaluateToken(request, scopes);

        case "public":
            return { userName: "public", permissions: ["readPublicFiles"] };
    }

    return Promise.reject("invalid-authentication-method");
}

So if I access the route now with a valid access token, I want the token to be evaluated and the route have the access privileges as defined in the token. Only if no valid token is defined, the route should still be accessible but with limited permissions.

Current Behavior

If I have multiple @Security annotations for a controller, they are evaluated asynchronously and the first one that completes defines the result of the req.user field and the route handler continues execution. In case of the example code above, the "public" authentication allways wins because it does not requre async execution and is immediatelly resolved.

The generatd code in routes.ts looks like this:

            const secMethodOrPromises: Promise<any>[] = [];
            for (const secMethod of security) {
                if(...) {
                // ... case for "secMethodAndPromises" omitted
                } else {
                    for (const name in secMethod) {
                        secMethodOrPromises.push(
                            expressAuthentication(request, name, secMethod[name])
                                .catch(pushAndRethrow)
                        );
                    }
                }                
            }

            // WARNING: This file was auto-generated with tsoa. Please do not modify it. Re-run tsoa to re-generate this file: https://github.com/lukeautry/tsoa

            try {
                request['user'] = await Promise.any(secMethodOrPromises);
                next();
            }

Now the generated code populates the user field with whatever resolves first. Since my second auth method does not require any async calculations, it allways "wins the race" and whatever the authToken security returns is ignored.

Possible Solution

I'm open for any suggestions how I could solve my issue. Maybe I'm just doing it wrong and there is a better way.

Here are some solutions I came up with but I'm not sure which one is best:

Steps to Reproduce

See example code above, basically have two @Security annotations on a controller where both methods resolve with a user object.

Context (Environment)

Version of the library: 6.0.1 Version of NodeJS: 20.8.0

App uses the express framework

github-actions[bot] commented 2 months ago

Hello there kbublitz 👋

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

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

github-actions[bot] commented 1 month ago

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

kbublitz commented 1 month ago

Now before my issue gets closed without any response, let me give a short update. I now implemented it the way I suggested in my initial question. It looks like the following snippet and seems to work well for my use case.

export async function expressAuthentication(
    request: express.Request,
    securityName: string,
    scopes?: string[],
): Promise<any> {
    switch (securityName) {
        case "authToken":
            // This will use jsonwebtoken to verify a jwt and resolve with a user object like { userName: "John Doe", "permissions": ["readPrivateFiles", "readPublicFiles"] }
            return await evaluateToken(request, scopes);

        case "authTokenOrPublic":
            return await maybeEvaluateToken(request, scopes);
    }

    return Promise.reject("invalid-authentication-method");
}

async function maybeEvaluateToken(request, scopes) {
    try {
        return await evaluateToken(request, scopes);
    } catch (error) {
        return { userName: "public", permissions: ["readPublicFiles"] };
    }
}

async function evaluateToken(request, scopes) {
    // check jwt and so on ...
    return realUserObject;
}

The controller is then annotated only with @Security("authTokenOrPublic").

I'll leave the issue open until it closes automatically, still any feedback is welcome.

daxadal commented 1 month ago

I tried to implement the same behaviour, and I used the scopes for that. I included an optional scope, and i only throw and Unauthorized error when the optional scope is not present. Then, you'll only need the authToken security method. He's an expample code based on yours:

export async function expressAuthentication(
  request: express.Request,
  securityName: string,
  scopes?: string[]
): Promise<any> {
  switch (securityName) {
    case "authToken":
      // This will use jsonwebtoken to verify a jwt and resolve with a user object like { userName: "John Doe", "permissions": ["readPrivateFiles", "readPublicFiles"] }
      return await evaluateToken(request, scopes);
  }

  return Promise.reject("invalid-authentication-method");
}

async function evaluateToken(request, scopes) {
  const user = await chekJwtAndGetUser(request);
  if (!scopes?.includes("optional") && !user) {
    throw new UnauthorizedError();
  } else if (!user) {
    return { userName: "public", permissions: ["readPublicFiles"] }
  }
  return user;
}

You can flip the conditions by using a requiredscope instead, if it's easier to understand.

github-actions[bot] commented 1 week ago

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