aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Borg <jakob@kastelo.net>2023-07-26 13:13:06 +0200
committerGitHub <noreply@github.com>2023-07-26 13:13:06 +0200
commit855c6dc67b96abdd1afcc66bad262bb9849d9189 (patch)
tree32b2305befae52460536440e0a1083016e55798c
parentdc5e10fa2c59ae21376aab94a6019e807718b781 (diff)
downloadsyncthing-855c6dc67b96abdd1afcc66bad262bb9849d9189.tar.gz
syncthing-855c6dc67b96abdd1afcc66bad262bb9849d9189.zip
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.
-rw-r--r--lib/api/api_auth.go2
-rw-r--r--lib/api/api_csrf.go13
-rw-r--r--lib/api/api_test.go265
3 files changed, 199 insertions, 81 deletions
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()
-
- // 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) {