瀏覽代碼

lib/api: Allow `Bearer` authentication style with API key (#9002)

Currently, historically, we look for the `X-API-Key` header to
authenticate with an API key. There's nothing wrong with this, but in
some scenarios it's easier to produce an `Authorization` header with a
`Bearer $token` content, which is nowadays more common. This change adds
support for both, so that we will accept an API key either in our custom
header or as a bearer token.
Jakob Borg 2 年之前
父節點
當前提交
855c6dc67b
共有 3 個文件被更改,包括 199 次插入81 次删除
  1. 1 1
      lib/api/api_auth.go
  2. 12 1
      lib/api/api_csrf.go
  3. 186 79
      lib/api/api_test.go

+ 1 - 1
lib/api/api_auth.go

@@ -39,7 +39,7 @@ func emitLoginAttempt(success bool, username, address string, evLogger events.Lo
 
 func basicAuthAndSessionMiddleware(cookieName string, guiCfg config.GUIConfiguration, ldapCfg config.LDAPConfiguration, next http.Handler, evLogger events.Logger) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if guiCfg.IsValidAPIKey(r.Header.Get("X-API-Key")) {
+		if hasValidAPIKeyHeader(r, guiCfg) {
 			next.ServeHTTP(w, r)
 			return
 		}

+ 12 - 1
lib/api/api_csrf.go

@@ -59,7 +59,7 @@ func newCsrfManager(unique string, prefix string, apiKeyValidator apiKeyValidato
 
 func (m *csrfManager) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// Allow requests carrying a valid API key
-	if m.apiKeyValidator.IsValidAPIKey(r.Header.Get("X-API-Key")) {
+	if hasValidAPIKeyHeader(r, m.apiKeyValidator) {
 		// Set the access-control-allow-origin header for CORS requests
 		// since a valid API key has been provided
 		w.Header().Add("Access-Control-Allow-Origin", "*")
@@ -178,3 +178,14 @@ func (m *csrfManager) load() {
 		m.tokens = append(m.tokens, s.Text())
 	}
 }
+
+func hasValidAPIKeyHeader(r *http.Request, validator apiKeyValidator) bool {
+	if key := r.Header.Get("X-API-Key"); validator.IsValidAPIKey(key) {
+		return true
+	}
+	if auth := r.Header.Get("Authorization"); strings.HasPrefix(strings.ToLower(auth), "bearer ") {
+		bearerToken := auth[len("bearer "):]
+		return validator.IsValidAPIKey(bearerToken)
+	}
+	return false
+}

+ 186 - 79
lib/api/api_test.go

@@ -559,67 +559,129 @@ func TestHTTPLogin(t *testing.T) {
 	cfg.GUIReturns(config.GUIConfiguration{
 		User:     "üser",
 		Password: "$2a$10$IdIZTxTg/dCNuNEGlmLynOjqg4B1FvDKuIV5e0BB3pnWVHNb8.GSq", // bcrypt of "räksmörgås" in UTF-8
+		APIKey:   testAPIKey,
 	})
 	baseURL, cancel, err := startHTTP(cfg)
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer cancel()
-
-	// Verify rejection when not using authorization
+	t.Cleanup(cancel)
 
-	req, _ := http.NewRequest("GET", baseURL, nil)
-	resp, err := http.DefaultClient.Do(req)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if resp.StatusCode != http.StatusUnauthorized {
-		t.Errorf("Unexpected non-401 return code %d for unauthed request", resp.StatusCode)
-	}
+	t.Run("no auth is rejected", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusUnauthorized {
+			t.Errorf("Unexpected non-401 return code %d for unauthed request", resp.StatusCode)
+		}
+	})
 
-	// Verify that incorrect password is rejected
+	t.Run("incorrect password is rejected", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.SetBasicAuth("üser", "rksmrgs")
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusUnauthorized {
+			t.Errorf("Unexpected non-401 return code %d for incorrect password", resp.StatusCode)
+		}
+	})
 
-	req.SetBasicAuth("üser", "rksmrgs")
-	resp, err = http.DefaultClient.Do(req)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if resp.StatusCode != http.StatusUnauthorized {
-		t.Errorf("Unexpected non-401 return code %d for incorrect password", resp.StatusCode)
-	}
+	t.Run("incorrect username is rejected", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.SetBasicAuth("user", "räksmörgås") // string literals in Go source code are in UTF-8
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusUnauthorized {
+			t.Errorf("Unexpected non-401 return code %d for incorrect username", resp.StatusCode)
+		}
+	})
 
-	// Verify that incorrect username is rejected
+	t.Run("UTF-8 auth works", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.SetBasicAuth("üser", "räksmörgås") // string literals in Go source code are in UTF-8
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Unexpected non-200 return code %d for authed request (UTF-8)", resp.StatusCode)
+		}
+	})
 
-	req.SetBasicAuth("user", "räksmörgås") // string literals in Go source code are in UTF-8
-	resp, err = http.DefaultClient.Do(req)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if resp.StatusCode != http.StatusUnauthorized {
-		t.Errorf("Unexpected non-401 return code %d for incorrect username", resp.StatusCode)
-	}
+	t.Run("ISO-8859-1 auth work", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.SetBasicAuth("\xfcser", "r\xe4ksm\xf6rg\xe5s") // escaped ISO-8859-1
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Unexpected non-200 return code %d for authed request (ISO-8859-1)", resp.StatusCode)
+		}
+	})
 
-	// Verify that UTF-8 auth works
+	t.Run("bad X-API-Key is rejected", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.Header.Set("X-API-Key", testAPIKey+"X")
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusUnauthorized {
+			t.Errorf("Unexpected non-401 return code %d for bad API key", resp.StatusCode)
+		}
+	})
 
-	req.SetBasicAuth("üser", "räksmörgås") // string literals in Go source code are in UTF-8
-	resp, err = http.DefaultClient.Do(req)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if resp.StatusCode != http.StatusOK {
-		t.Errorf("Unexpected non-200 return code %d for authed request (UTF-8)", resp.StatusCode)
-	}
+	t.Run("good X-API-Key is accepted", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.Header.Set("X-API-Key", testAPIKey)
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Unexpected non-200 return code %d for API key", resp.StatusCode)
+		}
+	})
 
-	// Verify that ISO-8859-1 auth
+	t.Run("bad Bearer is rejected", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.Header.Set("Authorization", "Bearer "+testAPIKey+"X")
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusUnauthorized {
+			t.Errorf("Unexpected non-401 return code %d for bad API key", resp.StatusCode)
+		}
+	})
 
-	req.SetBasicAuth("\xfcser", "r\xe4ksm\xf6rg\xe5s") // escaped ISO-8859-1
-	resp, err = http.DefaultClient.Do(req)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if resp.StatusCode != http.StatusOK {
-		t.Errorf("Unexpected non-200 return code %d for authed request (ISO-8859-1)", resp.StatusCode)
-	}
+	t.Run("good Bearer is accepted", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL, nil)
+		req.Header.Set("Authorization", "Bearer "+testAPIKey)
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Unexpected non-200 return code %d for API key", resp.StatusCode)
+		}
+	})
 }
 
 func startHTTP(cfg config.Wrapper) (string, context.CancelFunc, error) {
@@ -681,7 +743,7 @@ func TestCSRFRequired(t *testing.T) {
 	if err != nil {
 		t.Fatal("Unexpected error from getting base URL:", err)
 	}
-	defer cancel()
+	t.Cleanup(cancel)
 
 	cli := &http.Client{
 		Timeout: time.Minute,
@@ -709,42 +771,87 @@ func TestCSRFRequired(t *testing.T) {
 		}
 	}
 
-	// Calling on /rest without a token should fail
+	t.Run("/rest without a token should fail", func(t *testing.T) {
+		t.Parallel()
+		resp, err = cli.Get(baseURL + "/rest/system/config")
+		if err != nil {
+			t.Fatal("Unexpected error from getting /rest/system/config:", err)
+		}
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusForbidden {
+			t.Fatal("Getting /rest/system/config without CSRF token should fail, not", resp.Status)
+		}
+	})
 
-	resp, err = cli.Get(baseURL + "/rest/system/config")
-	if err != nil {
-		t.Fatal("Unexpected error from getting /rest/system/config:", err)
-	}
-	resp.Body.Close()
-	if resp.StatusCode != http.StatusForbidden {
-		t.Fatal("Getting /rest/system/config without CSRF token should fail, not", resp.Status)
-	}
+	t.Run("/rest with a token should succeed", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL+"/rest/system/config", nil)
+		req.Header.Set("X-"+csrfTokenName, csrfTokenValue)
+		resp, err = cli.Do(req)
+		if err != nil {
+			t.Fatal("Unexpected error from getting /rest/system/config:", err)
+		}
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			t.Fatal("Getting /rest/system/config with CSRF token should succeed, not", resp.Status)
+		}
+	})
 
-	// Calling on /rest with a token should succeed
+	t.Run("/rest with an incorrect API key should fail, X-API-Key version", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL+"/rest/system/config", nil)
+		req.Header.Set("X-API-Key", testAPIKey+"X")
+		resp, err = cli.Do(req)
+		if err != nil {
+			t.Fatal("Unexpected error from getting /rest/system/config:", err)
+		}
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusForbidden {
+			t.Fatal("Getting /rest/system/config with incorrect API token should fail, not", resp.Status)
+		}
+	})
 
-	req, _ := http.NewRequest("GET", baseURL+"/rest/system/config", nil)
-	req.Header.Set("X-"+csrfTokenName, csrfTokenValue)
-	resp, err = cli.Do(req)
-	if err != nil {
-		t.Fatal("Unexpected error from getting /rest/system/config:", err)
-	}
-	resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		t.Fatal("Getting /rest/system/config with CSRF token should succeed, not", resp.Status)
-	}
+	t.Run("/rest with an incorrect API key should fail, Bearer auth version", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL+"/rest/system/config", nil)
+		req.Header.Set("Authorization", "Bearer "+testAPIKey+"X")
+		resp, err = cli.Do(req)
+		if err != nil {
+			t.Fatal("Unexpected error from getting /rest/system/config:", err)
+		}
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusForbidden {
+			t.Fatal("Getting /rest/system/config with incorrect API token should fail, not", resp.Status)
+		}
+	})
 
-	// Calling on /rest with the API key should succeed
+	t.Run("/rest with the API key should succeed", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL+"/rest/system/config", nil)
+		req.Header.Set("X-API-Key", testAPIKey)
+		resp, err = cli.Do(req)
+		if err != nil {
+			t.Fatal("Unexpected error from getting /rest/system/config:", err)
+		}
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			t.Fatal("Getting /rest/system/config with API key should succeed, not", resp.Status)
+		}
+	})
 
-	req, _ = http.NewRequest("GET", baseURL+"/rest/system/config", nil)
-	req.Header.Set("X-API-Key", testAPIKey)
-	resp, err = cli.Do(req)
-	if err != nil {
-		t.Fatal("Unexpected error from getting /rest/system/config:", err)
-	}
-	resp.Body.Close()
-	if resp.StatusCode != http.StatusOK {
-		t.Fatal("Getting /rest/system/config with API key should succeed, not", resp.Status)
-	}
+	t.Run("/rest with the API key as a bearer token should succeed", func(t *testing.T) {
+		t.Parallel()
+		req, _ := http.NewRequest("GET", baseURL+"/rest/system/config", nil)
+		req.Header.Set("Authorization", "Bearer "+testAPIKey)
+		resp, err = cli.Do(req)
+		if err != nil {
+			t.Fatal("Unexpected error from getting /rest/system/config:", err)
+		}
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			t.Fatal("Getting /rest/system/config with API key should succeed, not", resp.Status)
+		}
+	})
 }
 
 func TestRandomString(t *testing.T) {