fixed: snip the token out of tracing, and instead put public claims

PaloAltoNetworks · Oct 19, 2017 · ac2ac55 · ac2ac55
1 parent 5e0ee1a
commit ac2ac55
Show file tree

Hide file tree

Showing 2 changed files with 224 additions and 4 deletions.
diff --git a/request.go b/request.go
@@ -2,6 +2,7 @@ package elemental
 
 import (
 	"crypto/tls"
+	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -17,6 +18,8 @@ import (
 	uuid "github.com/satori/go.uuid"
 )
 
+var snipSlice = []string{"[snip]"}
+
 // A Request represents an abstract request on an elemental model.
 type Request struct {
 	RequestID            string                     `json:"rid"`
@@ -228,10 +231,33 @@ func (r *Request) StartTracing() {
 
 	r.span = opentracing.StartSpan(r.tracingName(), ext.RPCServerOption(r.wireContext))
 
+	// Remove senstive information from parameters.
+	safeParameters := url.Values{}
+	for k, v := range r.Parameters {
+		lk := strings.ToLower(k)
+		if lk == "token" || lk == "password" {
+			safeParameters[k] = snipSlice
+			continue
+		}
+		safeParameters[k] = v
+	}
+
+	// Remove senstive information from headers.
+	safeHeaders := http.Header{}
+	for k, v := range r.Headers {
+		lk := strings.ToLower(k)
+		if lk == "authorization" {
+			safeHeaders[k] = snipSlice
+			continue
+		}
+		safeHeaders[k] = v
+	}
+
 	r.span.SetTag("elemental.request.api_version", r.Version)
 	r.span.SetTag("elemental.request.external_tracking_id", r.ExternalTrackingID)
 	r.span.SetTag("elemental.request.external_tracking_type", r.ExternalTrackingType)
-	r.span.SetTag("elemental.request.headers", r.Headers)
+	r.span.SetTag("elemental.request.headers", safeHeaders)
+	r.span.SetTag("elemental.request.claims", r.extractClaims())
 	r.span.SetTag("elemental.request.id", r.RequestID)
 	r.span.SetTag("elemental.request.identity", r.Identity.Name)
 	r.span.SetTag("elemental.request.namespace", r.Namespace)
@@ -241,7 +267,7 @@ func (r *Request) StartTracing() {
 	r.span.SetTag("elemental.request.override_protection", r.OverrideProtection)
 	r.span.SetTag("elemental.request.page.number", r.Page)
 	r.span.SetTag("elemental.request.page.size", r.PageSize)
-	r.span.SetTag("elemental.request.parameters", r.Parameters)
+	r.span.SetTag("elemental.request.parameters", safeParameters)
 	r.span.SetTag("elemental.request.parent.id", r.ParentID)
 	r.span.SetTag("elemental.request.parent.identity", r.ParentIdentity.Name)
 	r.span.SetTag("elemental.request.recursive", r.Recursive)
@@ -384,5 +410,20 @@ func (r *Request) tracingName() string {
 		return fmt.Sprintf("elemental.request.patch.%s", r.Identity.Category)
 	}
 
-	return fmt.Sprintf("Unknown operation: %s", r)
+	return fmt.Sprintf("Unknown operation: %s", r.Operation)
+}
+
+func (r *Request) extractClaims() string {
+
+	tokenParts := strings.SplitN(r.Password, ".", 3)
+	if len(tokenParts) != 3 {
+		return "{}"
+	}
+
+	identity, err := base64.RawStdEncoding.DecodeString(tokenParts[1])
+	if err != nil {
+		return "{}"
+	}
+
+	return string(identity)
 }
diff --git a/request_test.go b/request_test.go
@@ -11,6 +11,7 @@ import (
 	"net/url"
 	"testing"
 
+	opentracing "github.com/opentracing/opentracing-go"
 	. "github.com/smartystreets/goconvey/convey"
 )
 
@@ -94,7 +95,7 @@ func TestRequest_FromHttp(t *testing.T) {
 
 	Convey("Given I have a get http request on /lists", t, func() {
 
-		req, _ := http.NewRequest(http.MethodGet, "http://server/v/10/lists?p=v", nil)
+		req, _ := http.NewRequest(http.MethodGet, "http://server/v/10/lists?p=v&page=1&pagesize=2&recursive=true&override=true", nil)
 		req.Header.Set("X-Namespace", "ns")
 		req.RemoteAddr = "42.42.42.42"
 
@@ -162,6 +163,22 @@ func TestRequest_FromHttp(t *testing.T) {
 				So(r.ClientIP, ShouldEqual, "42.42.42.42")
 			})
 
+			Convey("Then the Page should be 1", func() {
+				So(r.Page, ShouldEqual, 1)
+			})
+
+			Convey("Then the PageSize should be 2", func() {
+				So(r.PageSize, ShouldEqual, 2)
+			})
+
+			Convey("Then the Recursive should be true", func() {
+				So(r.Recursive, ShouldBeTrue)
+			})
+
+			Convey("Then the OverrideProtection should be true", func() {
+				So(r.OverrideProtection, ShouldBeTrue)
+			})
+
 		})
 	})
 
@@ -738,6 +755,8 @@ func TestRequest_Duplicate(t *testing.T) {
 		req.Version = 12
 		req.Order = []string{"key1", "key2"}
 		req.ClientIP = "1.2.3.4"
+		req.TrackingData = opentracing.TextMapCarrier{"a": "b"}
+		req.Metadata = map[string]interface{}{"a": 1}
 
 		Convey("When I use Duplicate()", func() {
 
@@ -762,7 +781,167 @@ func TestRequest_Duplicate(t *testing.T) {
 				So(req2.Version, ShouldEqual, req.Version)
 				So(req2.Order, ShouldResemble, req.Order)
 				So(req2.ClientIP, ShouldResemble, req.ClientIP)
+				So(req2.TrackingData, ShouldResemble, req.TrackingData)
+				So(req2.Metadata, ShouldResemble, req.Metadata)
+			})
+		})
+	})
+}
+
+func TestRequest_extractClaims(t *testing.T) {
+
+	token := "x.eyJyZWFsbSI6IkNlcnRpZmljYXRlIiwiZGF0YSI6eyJjb21tb25OYW1lIjoiYWRtaW4iLCJvcmdhbml6YXRpb24iOiJzeXN0ZW0iLCJvdTpyb290IjoidHJ1ZSIsInJlYWxtIjoiY2VydGlmaWNhdGUiLCJzZXJpYWxOdW1iZXIiOiIxODY3OTg0MjcyNDEzNDMwODM2NzY2MDU2NTk0NDg1NjUxNTk4MTcifSwiYXVkIjoiYXBvcmV0by5jb20iLCJleHAiOjE1MDg1MTYxMzEsImlhdCI6MTUwODQyOTczMSwiaXNzIjoibWlkZ2FyZC5hcG9tdXguY29tIiwic3ViIjoiMTg2Nzk4NDI3MjQxMzQzMDgzNjc2NjA1NjU5NDQ4NTY1MTU5ODE3In0.y"
+	tokenInavalid := "eyJyZWFsbSI6IkNlcnRpZmljYXRlIiwiZGF0YSI6eyJjb21tb25OYW1lIjoiYWRtaW4iLCJvcmdhbml6YXRpb24iOiJzeXN0ZW0iLCJvdTpyb290IjoidHJ1ZSIsInJlYWxtIjoiY2VydGlmaWNhdGUiLCJzZXJpYWxOdW1iZXIiOiIxODY3OTg0MjcyNDEzNDMwODM2NzY2MDU2NTk0NDg1NjUxNTk4MTcifSwiYXVkIjoiYXBvcmV0by5jb20iLCJleHAiOjE1MDg1MTYxMzEsImlhdCI6MTUwODQyOTczMSwiaXNzIjoibWlkZ2FyZC5hcG9tdXguY29tIiwic3ViIjoiMTg2Nzk4NDI3MjQxMzQzMDgzNjc2NjA1NjU5NDQ4NTY1MTU5ODE3In0.y"
+
+	Convey("Given I have a Request with Password", t, func() {
+
+		req := NewRequest()
+		req.Password = token
+
+		Convey("When I extract the claims", func() {
+
+			claims := req.extractClaims()
+
+			Convey("Then claims should be correct", func() {
+				So(claims, ShouldEqual, `{"realm":"Certificate","data":{"commonName":"admin","organization":"system","ou:root":"true","realm":"certificate","serialNumber":"186798427241343083676605659448565159817"},"aud":"aporeto.com","exp":1508516131,"iat":1508429731,"iss":"midgard.apomux.com","sub":"186798427241343083676605659448565159817"}`)
+			})
+		})
+	})
+
+	Convey("Given create a request from an http request", t, func() {
+
+		req, _ := http.NewRequest(http.MethodGet, "http://server/lists/xx/tasks?p=v", nil)
+		req.Header.Add("X-Namespace", "ns")
+		req.Header.Add("Authorization", "Bearer "+token)
+		r, _ := NewRequestFromHTTPRequest(req)
+
+		Convey("When I extract the claims", func() {
+
+			claims := r.extractClaims()
+
+			Convey("Then claims should be correct", func() {
+				So(claims, ShouldEqual, `{"realm":"Certificate","data":{"commonName":"admin","organization":"system","ou:root":"true","realm":"certificate","serialNumber":"186798427241343083676605659448565159817"},"aud":"aporeto.com","exp":1508516131,"iat":1508429731,"iss":"midgard.apomux.com","sub":"186798427241343083676605659448565159817"}`)
+			})
+		})
+	})
+
+	Convey("Given I have a Request with invalid token in Password", t, func() {
+
+		req := NewRequest()
+		req.Password = tokenInavalid
+
+		Convey("When I extract the claims", func() {
+
+			claims := req.extractClaims()
+
+			Convey("Then claims should be correct", func() {
+				So(claims, ShouldEqual, `{}`)
+			})
+		})
+	})
+
+	Convey("Given I have a Request with almost invalid token in Password", t, func() {
+
+		req := NewRequest()
+		req.Password = "a.b.c"
+
+		Convey("When I extract the claims", func() {
+
+			claims := req.extractClaims()
+
+			Convey("Then claims should be correct", func() {
+				So(claims, ShouldEqual, `{}`)
 			})
 		})
 	})
 }
+
+func TestRequest_tracingName(t *testing.T) {
+
+	Convey("Given I have a create request on some identity", t, func() {
+
+		req := NewRequest()
+		req.Identity = ListIdentity
+
+		Convey("When I call tracingName for operation create", func() {
+
+			req.Operation = OperationCreate
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "elemental.request.create.lists")
+			})
+		})
+
+		Convey("When I call tracingName for operation update", func() {
+
+			req.Operation = OperationUpdate
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "elemental.request.update.lists")
+			})
+		})
+
+		Convey("When I call tracingName for operation delete", func() {
+
+			req.Operation = OperationDelete
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "elemental.request.delete.lists")
+			})
+		})
+
+		Convey("When I call tracingName for operation info", func() {
+
+			req.Operation = OperationInfo
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "elemental.request.info.lists")
+			})
+		})
+
+		Convey("When I call tracingName for operation retrieve", func() {
+
+			req.Operation = OperationRetrieve
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "elemental.request.retrieve.lists")
+			})
+		})
+
+		Convey("When I call tracingName for operation retrieve many", func() {
+
+			req.Operation = OperationRetrieveMany
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "elemental.request.retrieve_many.lists")
+			})
+		})
+
+		Convey("When I call tracingName for operation patch", func() {
+
+			req.Operation = OperationPatch
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "elemental.request.patch.lists")
+			})
+		})
+
+		Convey("When I call tracingName for operation unknown", func() {
+
+			req.Operation = Operation("nope")
+			name := req.tracingName()
+
+			Convey("Then name should correct", func() {
+				So(name, ShouldEqual, "Unknown operation: nope")
+			})
+		})
+	})
+
+}