git.iko.soy/typeable/oauth2-proxy
on master help sign in
Merge pull request #292 from nickmiller-wf/redact-access-token

Strip sensitive URL parameters from provider log output

commita0e4a368211f03db66c159190690e317068875de authorJehiah Czebotar <jehiah@gmail.com> dateTue 2 Aug 2016, 23:19 (10 years ago) parent78f2fe1c566648
3 files changed+55−5
compare with master
unifiedsplit
providers/github.go+7−4 providers/internal_util.go+37−1 providers/internal_util_test.go+11
providers/github.go+7−4
@@ -85,7 +85,8 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) {
8585 return false, err
8686 }
8787 if resp.StatusCode != 200 {
88− return false, fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint, body)
88+ return false, fmt.Errorf(
89+ "got %d from %q %s", resp.StatusCode, stripToken(endpoint.String()), body)
8990 }
9091
9192 if err := json.Unmarshal(body, &orgs); err != nil {
@@ -140,7 +141,8 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) {
140141 return false, err
141142 }
142143 if resp.StatusCode != 200 {
143− return false, fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint, body)
144+ return false, fmt.Errorf(
145+ "got %d from %q %s", resp.StatusCode, stripToken(endpoint.String()), body)
144146 }
145147
146148 if err := json.Unmarshal(body, &teams); err != nil {
@@ -217,9 +219,10 @@ func (p *GitHubProvider) GetEmailAddress(s *SessionState) (string, error) {
217219 }
218220
219221 if resp.StatusCode != 200 {
220− return "", fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint, body)
222+ return "", fmt.Errorf("got %d from %q %s",
223+ resp.StatusCode, stripToken(endpoint.String()), body)
221224 } else {
222− log.Printf("got %d from %q %s", resp.StatusCode, endpoint, body)
225+ log.Printf("got %d from %q %s", resp.StatusCode, stripToken(endpoint.String()), body)
223226 }
224227
225228 if err := json.Unmarshal(body, &emails); err != nil {
providers/internal_util.go+37−1
@@ -9,6 +9,42 @@ import (
99 "github.com/bitly/oauth2_proxy/api"
1010)
1111
12+// stripToken is a helper function to obfuscate "access_token"
13+// query parameters
14+func stripToken(endpoint string) string {
15+ return stripParam("access_token", endpoint)
16+}
17+
18+// stripParam generalizes the obfuscation of a particular
19+// query parameter - typically 'access_token' or 'client_secret'
20+// The parameter's second half is replaced by '...' and returned
21+// as part of the encoded query parameters.
22+// If the target parameter isn't found, the endpoint is returned
23+// unmodified.
24+func stripParam(param, endpoint string) string {
25+ u, err := url.Parse(endpoint)
26+ if err != nil {
27+ log.Printf("error attempting to strip %s: %s", param, err)
28+ return endpoint
29+ }
30+
31+ if u.RawQuery != "" {
32+ values, err := url.ParseQuery(u.RawQuery)
33+ if err != nil {
34+ log.Printf("error attempting to strip %s: %s", param, err)
35+ return u.String()
36+ }
37+
38+ if val := values.Get(param); val != "" {
39+ values.Set(param, val[:(len(val)/2)]+"...")
40+ u.RawQuery = values.Encode()
41+ return u.String()
42+ }
43+ }
44+
45+ return endpoint
46+}
47+
1248// validateToken returns true if token is valid
1349func validateToken(p Provider, access_token string, header http.Header) bool {
1450 if access_token == "" || p.Data().ValidateURL == nil {
@@ -28,7 +64,7 @@ func validateToken(p Provider, access_token string, header http.Header) bool {
2864
2965 body, _ := ioutil.ReadAll(resp.Body)
3066 resp.Body.Close()
31− log.Printf("%d GET %s %s", resp.StatusCode, endpoint, body)
67+ log.Printf("%d GET %s %s", resp.StatusCode, stripToken(endpoint), body)
3268
3369 if resp.StatusCode == 200 {
3470 return true
providers/internal_util_test.go+11
@@ -119,3 +119,14 @@ func TestValidateSessionStateExpiredToken(t *testing.T) {
119119 vt_test.response_code = 401
120120 assert.Equal(t, false, validateToken(vt_test.provider, "foobar", nil))
121121}
122+
123+func TestStripTokenNotPresent(t *testing.T) {
124+ test := "http://local.test/api/test?a=1&b=2"
125+ assert.Equal(t, test, stripToken(test))
126+}
127+
128+func TestStripToken(t *testing.T) {
129+ test := "http://local.test/api/test?access_token=deadbeef&b=1&c=2"
130+ expected := "http://local.test/api/test?access_token=dead...&b=1&c=2"
131+ assert.Equal(t, expected, stripToken(test))
132+}