21217: Strip leading/trailing space chars from incoming tokens.

Depending on OS/browser, trailing spaces and newlines are easy to
include inadvertently when copying, and impossible to detect when
pasting into a password input that displays placeholder symbols
instead of the pasted text.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/sdk/go/auth/auth.go b/sdk/go/auth/auth.go
index f1c2e243b53a8f5d7ae604d1b67df55968430fcd..da9b4ea5b8f193fd83072e72ae3ece3cfa6602bc 100644 (file)
--- a/sdk/go/auth/auth.go
+++ b/sdk/go/auth/auth.go
@@ -54,13 +54,13 @@ func (a *Credentials) LoadTokensFromHTTPRequest(r *http.Request) {
        // Load plain token from "Authorization: OAuth2 ..." header
        // (typically used by smart API clients)
        if toks := strings.SplitN(r.Header.Get("Authorization"), " ", 2); len(toks) == 2 && (toks[0] == "OAuth2" || toks[0] == "Bearer") {
-               a.Tokens = append(a.Tokens, toks[1])
+               a.Tokens = append(a.Tokens, strings.TrimSpace(toks[1]))
        }
 
        // Load base64-encoded token from "Authorization: Basic ..."
        // header (typically used by git via credential helper)
        if _, password, ok := r.BasicAuth(); ok {
-               a.Tokens = append(a.Tokens, password)
+               a.Tokens = append(a.Tokens, strings.TrimSpace(password))
        }
 
        // Load tokens from query string. It's generally not a good
@@ -76,7 +76,9 @@ func (a *Credentials) LoadTokensFromHTTPRequest(r *http.Request) {
        // find/report decoding errors in a suitable way.
        qvalues, _ := url.ParseQuery(r.URL.RawQuery)
        if val, ok := qvalues["api_token"]; ok {
-               a.Tokens = append(a.Tokens, val...)
+               for _, token := range val {
+                       a.Tokens = append(a.Tokens, strings.TrimSpace(token))
+               }
        }
 
        a.loadTokenFromCookie(r)
@@ -94,7 +96,7 @@ func (a *Credentials) loadTokenFromCookie(r *http.Request) {
        if err != nil {
                return
        }
-       a.Tokens = append(a.Tokens, string(token))
+       a.Tokens = append(a.Tokens, strings.TrimSpace(string(token)))
 }
 
 // LoadTokensFromHTTPRequestBody loads credentials from the request
@@ -111,7 +113,7 @@ func (a *Credentials) LoadTokensFromHTTPRequestBody(r *http.Request) error {
                return err
        }
        if t := r.PostFormValue("api_token"); t != "" {
-               a.Tokens = append(a.Tokens, t)
+               a.Tokens = append(a.Tokens, strings.TrimSpace(t))
        }
        return nil
 }

diff --git a/sdk/go/auth/handlers_test.go b/sdk/go/auth/handlers_test.go
index 362aeb7f04feacf35dd47da718fc852dae4110f5..85ea8893a50f61bf9531e6e3a5acd01ff09b008a 100644 (file)
--- a/sdk/go/auth/handlers_test.go
+++ b/sdk/go/auth/handlers_test.go
@@ -7,6 +7,8 @@ package auth
 import (
        "net/http"
        "net/http/httptest"
+       "net/url"
+       "strings"
        "testing"
 
        check "gopkg.in/check.v1"
@@ -32,9 +34,36 @@ func (s *HandlersSuite) SetUpTest(c *check.C) {
 func (s *HandlersSuite) TestLoadToken(c *check.C) {
        handler := LoadToken(s)
        handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo/bar?api_token=xyzzy", nil))
-       c.Assert(s.gotCredentials, check.NotNil)
-       c.Assert(s.gotCredentials.Tokens, check.HasLen, 1)
-       c.Check(s.gotCredentials.Tokens[0], check.Equals, "xyzzy")
+       c.Check(s.gotCredentials.Tokens, check.DeepEquals, []string{"xyzzy"})
+}
+
+// Ignore leading and trailing spaces, newlines, etc. in case a user
+// has added them inadvertently during copy/paste.
+func (s *HandlersSuite) TestTrimSpaceInQuery(c *check.C) {
+       handler := LoadToken(s)
+       handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo/bar?api_token=%20xyzzy%0a", nil))
+       c.Check(s.gotCredentials.Tokens, check.DeepEquals, []string{"xyzzy"})
+}
+func (s *HandlersSuite) TestTrimSpaceInPostForm(c *check.C) {
+       handler := LoadToken(s)
+       req := httptest.NewRequest("POST", "/foo/bar", strings.NewReader(url.Values{"api_token": []string{"\nxyzzy\n"}}.Encode()))
+       req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+       handler.ServeHTTP(httptest.NewRecorder(), req)
+       c.Check(s.gotCredentials.Tokens, check.DeepEquals, []string{"xyzzy"})
+}
+func (s *HandlersSuite) TestTrimSpaceInCookie(c *check.C) {
+       handler := LoadToken(s)
+       req := httptest.NewRequest("GET", "/foo/bar", nil)
+       req.AddCookie(&http.Cookie{Name: "arvados_api_token", Value: EncodeTokenCookie([]byte("\vxyzzy\n"))})
+       handler.ServeHTTP(httptest.NewRecorder(), req)
+       c.Check(s.gotCredentials.Tokens, check.DeepEquals, []string{"xyzzy"})
+}
+func (s *HandlersSuite) TestTrimSpaceInBasicAuth(c *check.C) {
+       handler := LoadToken(s)
+       req := httptest.NewRequest("GET", "/foo/bar", nil)
+       req.SetBasicAuth("username", "\txyzzy\n")
+       handler.ServeHTTP(httptest.NewRecorder(), req)
+       c.Check(s.gotCredentials.Tokens, check.DeepEquals, []string{"xyzzy"})
 }
 
 func (s *HandlersSuite) TestRequireLiteralTokenEmpty(c *check.C) {
@@ -76,4 +105,5 @@ func (s *HandlersSuite) TestRequireLiteralToken(c *check.C) {
 func (s *HandlersSuite) ServeHTTP(w http.ResponseWriter, r *http.Request) {
        s.served++
        s.gotCredentials = CredentialsFromRequest(r)
+       s.gotCredentials.LoadTokensFromHTTPRequestBody(r)
 }

diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index eba682149a8d2d36b25c32a41a0d0101706a8642..c14789889d6d6631b6a7734bccea719e0020e6a0 100644 (file)
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -351,6 +351,27 @@ func authzViaCookieValue(r *http.Request, tok string) int {
        return http.StatusUnauthorized
 }
 
+func (s *IntegrationSuite) TestVhostViaHTTPBasicAuth(c *check.C) {
+       s.doVhostRequests(c, authzViaHTTPBasicAuth)
+}
+func authzViaHTTPBasicAuth(r *http.Request, tok string) int {
+       r.AddCookie(&http.Cookie{
+               Name:  "arvados_api_token",
+               Value: auth.EncodeTokenCookie([]byte(tok)),
+       })
+       return http.StatusUnauthorized
+}
+
+func (s *IntegrationSuite) TestVhostViaHTTPBasicAuthWithExtraSpaceChars(c *check.C) {
+       s.doVhostRequests(c, func(r *http.Request, tok string) int {
+               r.AddCookie(&http.Cookie{
+                       Name:  "arvados_api_token",
+                       Value: auth.EncodeTokenCookie([]byte(" " + tok + "\n")),
+               })
+               return http.StatusUnauthorized
+       })
+}
+
 func (s *IntegrationSuite) TestVhostViaPath(c *check.C) {
        s.doVhostRequests(c, authzViaPath)
 }