Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-13701

JWTAuthPlugin calls authenticationFailure (which calls HttpServletResponsesendError) before updating metrics - breaks tests

    XMLWordPrintableJSON

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: master (9.0), 8.3
    • Component/s: None
    • Labels:
      None

      Description

      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.

        Attachments

        1. SOLR-13701.patch
          6 kB
          Chris M. Hostetter

          Activity

            People

            • Assignee:
              hossman Chris M. Hostetter
              Reporter:
              hossman Chris M. Hostetter
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: