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

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

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 8.3, 9.0
    • None
    • 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

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

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

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment