diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go index ad3d86638..58a97e558 100644 --- a/lib/api/api_auth.go +++ b/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 } diff --git a/lib/api/api_csrf.go b/lib/api/api_csrf.go index b597c2e49..25e613a42 100644 --- a/lib/api/api_csrf.go +++ b/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 +} diff --git a/lib/api/api_test.go b/lib/api/api_test.go index 3cb01d789..89077c24c 100644 --- a/lib/api/api_test.go +++ b/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() + t.Cleanup(cancel) - // Verify rejection when not using authorization + 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) + } + }) - 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("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) + } + }) - // Verify that incorrect password is rejected + 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) + } + }) - 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("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) + } + }) - // Verify that incorrect username is rejected + 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) + } + }) - 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("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) + } + }) - // Verify that UTF-8 auth works + 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) + } + }) - 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("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) + } + }) - // Verify that ISO-8859-1 auth - - 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) {