adutra opened a new issue, #819:
URL: https://github.com/apache/polaris/issues/819

   ### Is your feature request related to a problem? Please describe.
   
   Currently Polaris has a number of filters, some of them being "regular" 
JAX-RS filters, some others being Vert.x route filters:
   
   | Filter | Type | Priority |
   | ------|-----| --------|
   | `org.apache.polaris.service.quarkus.logging.QuarkusLoggingMDCFilter` | 
Vert.x | `RouteFilter.DEFAULT_PRIORITY + 100` (110) |
   | `org.apache.polaris.service.quarkus.tracing.QuarkusTracingFilter` | Vert.x 
| `QuarkusLoggingMDCFilter.PRIORITY - 1` (109) |
   | `org.apache.polaris.service.auth.PolarisPrincipalAuthenticatorFilter` | 
JAX-RS | `Piorities.AUTHENTICATION ` (1000) |
   | `org.apache.polaris.service.auth.PolarisPrincipalRolesProviderFilter`  | 
JAX-RS | `Piorities.AUTHENTICATION  + 1` (1001) |
   | `org.apache.polaris.service.ratelimiter.RateLimiterFilter` | JAX-RS | 
`Piorities.USER ` (5000) |
   
   The above has a number of issues:
   
   First off, from my experience, Vert.x filters always execute first, 
regardless of priorities (also, Vert.x priorities are inverted compared to 
JAX-RS ones). This induces a wrong order of execution:
   
   1. `QuarkusLoggingMDCFilter`
   2. `QuarkusTracingFilter`
   3. `PolarisPrincipalAuthenticatorFilter`
   4. `PolarisPrincipalRolesProviderFilter`
   5. `RateLimiterFilter`
   
   Secondly, we ideally need a filter to verify realm IDs. For two reasons:
   
   1. The default realm ID resolver throws an exception if the realm is 
unknown. That's fine, but the error at that stage, since the resolver is a CDI 
bean, is translated into HTTP 500. If we want to return something more 
meaningful, like HTTP 403, we'd need to do that in a filter.  Related to this: 
#805.
   2. Also, `QuarkusLoggingMDCFilter` and `QuarkusTracingFilter` both need the 
realm ID. So they should execute after the realm ID filter.
   
   ### Describe the solution you'd like
   
   I think we need to reorder the filters as below:
   
   1. `RealmIdFilter`(to be created)
   2. `QuarkusLoggingMDCFilter`
   3. `QuarkusTracingFilter`
   4. `PolarisPrincipalAuthenticatorFilter`
   5. `PolarisPrincipalRolesProviderFilter`
   6. `RateLimiterFilter`
   
   Or maybe as below if we want the authenticator to kick in first:
   
   1. `PolarisPrincipalAuthenticatorFilter`
   2. `RealmIdFilter`(to be created)
   3. `PolarisPrincipalRolesProviderFilter`
   4. `QuarkusLoggingMDCFilter`
   5. `QuarkusTracingFilter`
   6. `RateLimiterFilter`
   
   
   ### Describe alternatives you've considered
   
   _No response_
   
   ### Additional context
   
   _No response_


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to