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

AuthenticationRequestFilter must define a jarkarta priority #4580

Closed
scandinave opened this issue Oct 24, 2024 · 6 comments · Fixed by #4582
Closed

AuthenticationRequestFilter must define a jarkarta priority #4580

scandinave opened this issue Oct 24, 2024 · 6 comments · Fixed by #4582
Labels
triage all new issues awaiting classification

Comments

@scandinave
Copy link
Contributor

Bug Report

Describe the Bug

Currently, the AuthenticationRequestFilter does not defined priority. This makes adding another filter in the chain difficult.

Expected Behavior

The filter should define the @Priority(Priorities.AUTHENTICATION) to be in the correct place into the Jersey filter chain.

Observed Behavior

When I add another filter via a new Extension, the filter with @Priority(Priorities.AUTHORISATION) is trigger before the AuthenticationRequestFilter.

Steps to Reproduce

Steps to reproduce the behavior:

  1. Create a new ServiceExtension like this :
@Extension(ManagementApiAuthzConfigurationExtension.NAME)
public class ManagementApiAuthzConfigurationExtension implements ServiceExtension {

    public static final String NAME = "Authorization Extension for Management Api";

    @Inject
    private WebService webService;

    @Override
    public String name() {
        return NAME;
    }

    @Override
    public void initialize(ServiceExtensionContext context) {
        var monitor = context.getMonitor();
        var authorizationFilter = new AuthorizationRequestFilter(monitor);
        webService.registerResource(ApiContext.MANAGEMENT, authorizationFilter);
    }
}
  1. Create a AuthorizationRequestFilter :
@Priority(Priorities.AUTHORIZATION)
public class AuthorizationRequestFilter implements ContainerRequestFilter {


    private final Monitor monitor;

    public AuthorizationRequestFilter(Monitor monitor) {
        this.monitor = monitor;
    }

    @Override
    public void filter(ContainerRequestContext requestContext) throws IOException {
        // OPTIONS requests don't have credentials - do not authenticate
        if (!OPTIONS.equalsIgnoreCase(requestContext.getMethod())) {
            // todo handle oauth token verification.
            monitor.info("Checking authorization... ");
        }
    }
}
  1. When accessing the management API, AuthorizationFilter is called first.

Context Information

Add any other context about the problem here.

  • Used version EDC 0.9.1

Detailed Description

If applicable, add screenshots and logs to help explain your problem.

Possible Implementation

Adding @Priority(Priorities.AUTHENTICATION) should fix the problem.

@github-actions github-actions bot added the triage all new issues awaiting classification label Oct 24, 2024
Copy link

Thanks for your contribution 🔥 We will take a look asap 🚀

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Oct 24, 2024

note that this will pull in the jakarta.annotation:jakarta.annotation-api dependency (for just one annotation!).

By default, all filters and interceptors have the USER priority, so it should be easy to simply assign a higher priority to your interceptor?

@scandinave
Copy link
Contributor Author

Yes, I can do that and I will if its not possible to add this annotation, but it's strange to mark a Authorization filter that have a specific category into another one.

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Oct 24, 2024

actually, if you assign the AUTHORIZATION priority to your authz filter you should be good. The documentation states:

... the lower the number the higher the priority

so if the EDC AuthenticationRequestFilter has prio USER (=5000), and your filter has prio AUTHORIZATION (=2000) all is well.

@scandinave
Copy link
Contributor Author

Yes, That the problem. I don't want my authorizationFilter to be trigger before the Authentication one

@paullatzelsperger
Copy link
Member

Yes, That the problem. I don't want my authorizationFilter to be trigger before the Authentication one

ah, i thought it was the other way round :) ok that makes more sense then. In that case, feel free to create a PR that sets the priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage all new issues awaiting classification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants