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

Evaluate @Security in order of definition #1608

Open
2 of 4 tasks
kbublitz opened this issue Apr 11, 2024 · 3 comments
Open
2 of 4 tasks

Evaluate @Security in order of definition #1608

kbublitz opened this issue Apr 11, 2024 · 3 comments

Comments

@kbublitz
Copy link

Sorting

  • I'm submitting a ...

    • bug report
    • feature request
    • support request
  • I confirm that I

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

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:

  • Change my expressAuthentication method to have a single securityName that does the different authentication methods in the order I need. This is currently my preferred solution,
  • Use separate routes for authenticated and public access. This means I would have to move the @Security annotations to the routes instead of the controller (there are several, I just simplified for the example) and duplicate all of them, which doesn't feel right. This means I would also have to adjust the frontend code to call different routes based on my authentication token.
  • Change the generated code to evaluate my security methods in the expected order. I didn't create this issue as a feature request because I'm not sure how much of a use case it is for others. My guess is that usually you only have one of the auth methods resolve and the others reject.
  • I could use a custom middleware template, but there is a big danger warning and the documentation doesn't really tell how I can write my own template. This also feels like an excessive measure just to get the behaviour of one controller right.

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

  • Confirm you were using yarn not npm: [ ]
    I'm using npm but that shouldn't make any difference for my question
Copy link

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.👀

Copy link

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

@github-actions github-actions bot added the Stale label May 12, 2024
@kbublitz
Copy link
Author

kbublitz commented May 13, 2024

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.

@github-actions github-actions bot removed the Stale label May 14, 2024
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

1 participant