Skip to content

Handle MissingPersonalAccessTokenError on Go middleware

Hugo Ortiz requested to merge 292732-handle-missing-personal-access-token-error-on-go-middleware into master

What does this MR do and why?

As explained in #292732 (closed) :

  • The Go middleware may authenticate the user.
  • During this authentication, a Gitlab::Auth::MissingPersonalAccessTokenError can be thrown.
  • This error is not handled by the middleware, which ultimately causes a 500 Internal Server Error response.

We don't want a 500 error response for an error condition that is actually accounted for in the authentication logic. Moreover, we don't want free 500s that can skew SLOs 😅.

This MR changes the Go middleware to rescue from the MissingPersonalAccessTokenError and respond with 401 Unauthorized instead of letting the error bubble up and end in a 500 Internal Server Error response.

Additional Information

There are a couple code paths that may throw the MissingPersonalAccessTokenError, but the one used here for the purpose of reproduction and verification is reached by sending the user/password credentials (basic auth) for a two-factor authentication enabled user with the user's GitLab account password instead of a personal access token as the password.

Screenshots or screen recordings

📵

How to set up and validate locally

We'll reproduce the issue using curl to make a GET request with the following conditions:

  1. Use the path format expected by the Go middleware:

    Go subpackages may be in the form of namespace/project/path1/path2/../pathN.

  2. Add the go-get=1 query string paramater in the URL (also as expected by the middleware).

  3. Use basic authentication:

    1. For a user with two-factor authentication enabled.
    2. Sending the GitLab account password, instead of a personal access token, as the password.

Setup

This must be done for both bug reproduction and fix verification:

  1. Set up a user with 2FA enabled.
  2. Disable Better Errors and the native development-friendly Rails error page (not sure what's called) by commenting out lines 13 and 61 in development.rb. This way we'll receive the same error responses as on Production.

Bug Reproduction

On master branch:

  1. Make a curl request that will be handled by the Go middleware:

    curl -i -u user:password http://gdk.test:3000/a/b?go-get=1

    ⚠️ Please note that the namespace/project part of the path may not be actually validated. So, for the purpose of reproduction we can send a/b above even if that's not a valid path.

  2. Confirm how the Go middleware returns a 500 Internal Server Error HTTP status code and the generic "Whoops, something went wrong on our end." HTML page as the response body ❌.

    Show expected output. 
    HTTP/1.1 500 Internal Server Error
Content-Length: 2926
Content-Type: text/html; charset=utf-8
Server: thin
X-Request-Id: 01FJE6CCYH86DRG57S9NS70RPX
X-Runtime: 0.208038
Date: Wed, 20 Oct 2021 06:31:56 GMT

<!DOCTYPE html>
<html>
<head>
  <meta content="width=device-width, initial-scale=1, maximum-scale=1" name="viewport">
  <title>Something went wrong (500)</title>
  <style>
    body {
      color: #666;
      text-align: center;
      font-family: "Helvetica Neue", Helvetica, Arial, sans-serif;
      margin: auto;
      font-size: 14px;
    }

    h1 {
      font-size: 56px;
      line-height: 100px;
      font-weight: 400;
      color: #456;
    }

    h2 {
      font-size: 24px;
      color: #666;
      line-height: 1.5em;
    }

    h3 {
      color: #456;
      font-size: 20px;
      font-weight: 400;
      line-height: 28px;
    }

    hr {
      max-width: 800px;
      margin: 18px auto;
      border: 0;
      border-top: 1px solid #EEE;
      border-bottom: 1px solid white;
    }

    img {
      max-width: 40vw;
      display: block;
      margin: 40px auto;
    }

    a {
      line-height: 100px;
      font-weight: 400;
      color: #4A8BEE;
      font-size: 18px;
      text-decoration: none;
    }

    .container {
      margin: auto 20px;
    }

    .go-back {
      display: none;
    }

  </style>
</head>

<body>
  <a href="/">
    <img src="data:image/svg+xml;base64,   PHN2ZyB3aWR0aD0iMjEwIiBoZWlnaHQ9IjIxMCIgdmlld0JveD0iMCAwIDIxMCAyMTAiIHhtbG5zPSJodHRwOi8vd3 3LnczL   m9yZy8yMDAwL3N2ZyI   +CiAgPHBhdGggZD0iTTEwNS4wNjE0IDIwMy42NTVsMzguNjQtMTE4LjkyMWgtNzcuMjhsMzguNjQgMTE4LjkyMXoiI ZpbGw9   IiNlMjQzMjkiLz4KICA8cGF0aCBkPSJNMTA1LjA2MTQgMjAzLjY1NDhsLTM4LjY0LTExOC45MjFoLTU0LjE1M2w5Mi 3OTMgM   TE4LjkyMXoiIGZpbGw9IiNmYzZkMjYiLz4KICA8cGF0aCBkPSJNMTIuMjY4NSA4NC43MzQxbC0xMS43NDIgMzYuMTM Yy0xLj   A3MSAzLjI5Ni4xMDIgNi45MDcgMi45MDYgOC45NDRsMTAxLjYyOSA3My44MzgtOTIuNzkzLTExOC45MjF6IiBmaWxs SIjZmN   hMzI2Ii8   +CiAgPHBhdGggZD0iTTEyLjI2ODUgODQuNzM0Mmg1NC4xNTNsLTIzLjI3My03MS42MjVjLTEuMTk3LTMuNjg2LTYuN ExLTMu   Njg1LTcuNjA4IDBsLTIzLjI3MiA3MS42MjV6IiBmaWxsPSIjZTI0MzI5Ii8   +CiAgPHBhdGggZD0iTTEwNS4wNjE0IDIwMy42NTQ4bDM4LjY0LTExOC45MjFoNTQuMTUzbC05Mi43OTMgMTE4LjkyM oiIGZp   bGw9IiNmYzZkMjYiLz4KICA8cGF0aCBkPSJNMTk3Ljg1NDQgODQuNzM0MWwxMS43NDIgMzYuMTM5YzEuMDcxIDMuMj 2LS4xM   DIgNi45MDctMi45MDYgOC45NDRsLTEwMS42MjkgNzMuODM4IDkyLjc5My0xMTguOTIxeiIgZmlsbD0iI2ZjYTMyNiI PgogID   xwYXRoIGQ9Ik0xOTcuODU0NCA4NC43MzQyaC01NC4xNTNsMjMuMjczLTcxLjYyNWMxLjE5Ny0zLjY4NiA2LjQxMS0z jY4NSA   3LjYwOCAwbDIzLjI3MiA3MS42MjV6IiBmaWxsPSIjZTI0MzI5Ii8+Cjwvc3ZnPgo="
        alt="GitLab Logo" />
  </a>
  <h1>
    500
  </h1>
  <div class="container">
    <h3>Whoops, something went wrong on our end.</h3>
    <hr />
    <p>Try refreshing the page, or going back and attempting the action again.</p>
    <p>Please contact your GitLab administrator if this problem persists.</p>
    <a href="javascript:history.back()" class="js-go-back go-back">Go back</a>
  </div>
  <script>
    (function () {
      var goBack = document.querySelector('.js-go-back');

      if (history.length > 1) {
        goBack.style.display = 'inline';
      }
    })();
  </script>
</body>
</html>

Bug Fix Verification

On 292732-handle-missing-personal-access-token-error-on-go-middleware (this) branch:

  1. Make a curl request that will be handled by the Go middleware:

    curl -i -u user:password http://gdk.test:3000/a/b?go-get=1

  2. Confirm that the Go middleware now returns a 401 Unauthorized HTTP status code and an empty body ✅.

    HTTP/1.1 401 Unauthorized
Cache-Control: no-cache
Server: thin
X-Request-Id: 01FJE5T839Y9B3A8533YXNAAW6
X-Runtime: 0.235985
Date: Wed, 20 Oct 2021 06:22:01 GMT
Content-Length: 0

Tasks

  • Handle Gitlab::Auth::MissingPersonalAccessTokenError in Gitlab::Middleware::Go#call, responding with 401 Unauthorized.
  • Add specs in spec/lib/gitlab/middleware/go_spec.rb.
  • Add a changelog trailer, as this is a client-facing change to the REST API.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Hugo Ortiz

Merge request reports