[jira] [Updated] (SOLR-13701) JWTAuthPlugin calls authenticationFailure (which calls HttpServletResponsesendError) before updating metrics - breaks tests

["Hoss Man (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SOLR-13701?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hoss Man updated SOLR-13701:
----------------------------
    Attachment: SOLR-13701.patch
        Status: Open  (was: Open)


Attaching patch that addressess this.  

I also updates the existing code paths that propogate the request via {{filterChain.doFilter(...)}} to ensure that the associated metrics ( {{numPassThrough}} and/or {{numAuthenticated}} ) are updated *before* {{filterChain.doFilter(...)}} is called, so that they are correct even if a subsequent filter (or ultimately, the SolrCore/RequestHandler) encounters an error or otherwise rejects the request.

[~janhoy] - would appreciate if you could review.

> JWTAuthPlugin calls authenticationFailure (which calls HttpServletResponsesendError) before updating metrics - breaks tests
> ---------------------------------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-13701
>                 URL: https://issues.apache.org/jira/browse/SOLR-13701
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>            Reporter: Hoss Man
>            Assignee: Hoss Man
>            Priority: Major
>         Attachments: SOLR-13701.patch
>
>
> The way JWTAuthPlugin is currently implemented, any failures are sent to the remote client (via {{authenticationFailure(...)}} which calls {{HttpServletResponsesendError(...)}}) *before* {{JWTAuthPlugin.doAuthenticate(...)}} has a chance to update it's metrics (like {{numErrors}} and {{numWrongCredentials}})
> This causes a race condition in tests where test threads can:
>  * see an error response/Exception before the server thread has updated metrics (like {{numErrors}} and {{numWrongCredentials}})
>  * call white box methods like {{SolrCloudAuthTestCase.assertAuthMetricsMinimums(...)}} to assert expected metrics
> ...all before the server thread has ever gotten around to being able to update the metrics in question.
> {{SolrCloudAuthTestCase.assertAuthMetricsMinimums(...)}} currently has some {{"First metrics count assert failed, pausing 2s before re-attempt"}} evidently to try and work around this bug, but it's still no garuntee that the server thread will be scheduled before the retry happens.
> We can/should just fix JWTAuthPlugin to ensure the metrics are updated before {{authenticationFailure(...)}} is called, and then remove the "pausing 2s before re-attempt" logic from {{SolrCloudAuthTestCase}} - between this bug fix, and the existing work around for SOLR-13464, there should be absolutely no reason to "retry" reading hte metrics.
> (NOTE: BasicAuthPlugin has a similar {{authenticationFailure(...)}} method that also calls {{HttpServletResponse.sendError(...)}} - but it already (correctly) updates the error/failure metrics *before* calling that method.



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org