From f5db2e1ff750f682cfc6d12806feecccda45c12c Mon Sep 17 00:00:00 2001 From: Jehiah Czebotar Date: Sat, 6 Jun 2015 14:15:43 -0400 Subject: [PATCH] More complete HTTP error logging --- README.md | 2 +- api/api.go | 19 ++++--------- oauthproxy.go | 50 ++++++++++------------------------- providers/github.go | 28 ++++++++++++++------ providers/internal_util.go | 3 +-- providers/provider_default.go | 19 +++++++++++++ providers/providers.go | 1 + 7 files changed, 61 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index d75d7b4..2c2158f 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,7 @@ OAuth2 Proxy responds directly to the following endpoints. All other endpoints w ## Logging Format -OAuth2 Proxy Proxy logs requests to stdout in a format similar to Apache Combined Log. +OAuth2 Proxy logs requests to stdout in a format similar to Apache Combined Log. ``` - [19/Mar/2015:17:20:19 -0400] GET "/path/" HTTP/1.1 "" diff --git a/api/api.go b/api/api.go index 19e75e9..0dac604 100644 --- a/api/api.go +++ b/api/api.go @@ -1,9 +1,8 @@ package api import ( - "errors" + "fmt" "io/ioutil" - "log" "net/http" "github.com/bitly/go-simplejson" @@ -20,8 +19,7 @@ func Request(req *http.Request) (*simplejson.Json, error) { return nil, err } if resp.StatusCode != 200 { - log.Printf("got response code %d - %s", resp.StatusCode, body) - return nil, errors.New("api request returned non 200 status code") + return nil, fmt.Errorf("got %d %s", resp.StatusCode, body) } data, err := simplejson.NewJson(body) if err != nil { @@ -30,19 +28,12 @@ func Request(req *http.Request) (*simplejson.Json, error) { return data, nil } -func RequestUnparsedResponse(url string, header http.Header) ( - response *http.Response, err error) { +func RequestUnparsedResponse(url string, header http.Header) (resp *http.Response, err error) { req, err := http.NewRequest("GET", url, nil) if err != nil { - return nil, errors.New("failed building request for " + - url + ": " + err.Error()) + return nil, err } req.Header = header - httpclient := &http.Client{} - if response, err = httpclient.Do(req); err != nil { - return nil, errors.New("request failed for " + - url + ": " + err.Error()) - } - return + return http.DefaultClient.Do(req) } diff --git a/oauthproxy.go b/oauthproxy.go index ed9e9a2..fed5fb8 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -37,11 +37,6 @@ type OauthProxy struct { redirectUrl *url.URL // the url to receive requests at provider providers.Provider - oauthLoginUrl *url.URL // to redirect the user to - oauthValidateUrl *url.URL // to validate the access token - oauthScope string - clientID string - clientSecret string ProxyPrefix string SignInMessage string HtpasswdFile *HtpasswdFile @@ -147,25 +142,20 @@ func NewOauthProxy(opts *Options, validator func(string) bool) *OauthProxy { OauthStartPath: fmt.Sprintf("%s/start", opts.ProxyPrefix), OauthCallbackPath: fmt.Sprintf("%s/callback", opts.ProxyPrefix), - clientID: opts.ClientID, - clientSecret: opts.ClientSecret, - ProxyPrefix: opts.ProxyPrefix, - oauthScope: opts.provider.Data().Scope, - provider: opts.provider, - oauthLoginUrl: opts.provider.Data().LoginUrl, - oauthValidateUrl: opts.provider.Data().ValidateUrl, - serveMux: serveMux, - redirectUrl: redirectUrl, - skipAuthRegex: opts.SkipAuthRegex, - compiledRegex: opts.CompiledRegex, - PassBasicAuth: opts.PassBasicAuth, - PassAccessToken: opts.PassAccessToken, - AesCipher: aes_cipher, - templates: loadTemplates(opts.CustomTemplatesDir), + ProxyPrefix: opts.ProxyPrefix, + provider: opts.provider, + serveMux: serveMux, + redirectUrl: redirectUrl, + skipAuthRegex: opts.SkipAuthRegex, + compiledRegex: opts.CompiledRegex, + PassBasicAuth: opts.PassBasicAuth, + PassAccessToken: opts.PassAccessToken, + AesCipher: aes_cipher, + templates: loadTemplates(opts.CustomTemplatesDir), } } -func (p *OauthProxy) GetRedirectUrl(host string) string { +func (p *OauthProxy) GetRedirectURI(host string) string { // default to the request Host if not set if p.redirectUrl.Host != "" { return p.redirectUrl.String() @@ -183,19 +173,6 @@ func (p *OauthProxy) GetRedirectUrl(host string) string { return u.String() } -func (p *OauthProxy) GetLoginURL(host, redirect string) string { - params := url.Values{} - params.Add("redirect_uri", p.GetRedirectUrl(host)) - params.Add("approval_prompt", "force") - params.Add("scope", p.oauthScope) - params.Add("client_id", p.clientID) - params.Add("response_type", "code") - if strings.HasPrefix(redirect, "/") { - params.Add("state", redirect) - } - return fmt.Sprintf("%s?%s", p.oauthLoginUrl, params.Encode()) -} - func (p *OauthProxy) displayCustomLoginForm() bool { return p.HtpasswdFile != nil && p.DisplayHtpasswdForm } @@ -204,7 +181,7 @@ func (p *OauthProxy) redeemCode(host, code string) (string, string, error) { if code == "" { return "", "", errors.New("missing code") } - redirectUri := p.GetRedirectUrl(host) + redirectUri := p.GetRedirectURI(host) body, access_token, err := p.provider.Redeem(redirectUri, code) if err != nil { return "", "", err @@ -416,7 +393,8 @@ func (p *OauthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { p.ErrorPage(rw, 500, "Internal Error", err.Error()) return } - http.Redirect(rw, req, p.GetLoginURL(req.Host, redirect), 302) + redirectURI := p.GetRedirectURI(req.Host) + http.Redirect(rw, req, p.provider.GetLoginURL(redirectURI, redirect), 302) return } if req.URL.Path == p.OauthCallbackPath { diff --git a/providers/github.go b/providers/github.go index 3f8d342..b138af6 100644 --- a/providers/github.go +++ b/providers/github.go @@ -2,6 +2,7 @@ package providers import ( "encoding/json" + "fmt" "io/ioutil" "net/http" "net/url" @@ -58,10 +59,11 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) { params := url.Values{ "access_token": {accessToken}, - "limit": {"100"}, + "limit": {"100"}, } - req, _ := http.NewRequest("GET", "https://api.github.com/user/orgs?"+params.Encode(), nil) + endpoint := "https://api.github.com/user/orgs?" + params.Encode() + req, _ := http.NewRequest("GET", endpoint, nil) req.Header.Set("Accept", "application/vnd.github.moondragon+json") resp, err := http.DefaultClient.Do(req) if err != nil { @@ -73,6 +75,9 @@ func (p *GitHubProvider) hasOrg(accessToken string) (bool, error) { if err != nil { return false, err } + if resp.StatusCode != 200 { + return false, fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint, body) + } if err := json.Unmarshal(body, &orgs); err != nil { return false, err @@ -99,10 +104,11 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) { params := url.Values{ "access_token": {accessToken}, - "limit": {"100"}, + "limit": {"100"}, } - req, _ := http.NewRequest("GET", "https://api.github.com/user/teams?"+params.Encode(), nil) + endpoint := "https://api.github.com/user/teams?" + params.Encode() + req, _ := http.NewRequest("GET", endpoint, nil) req.Header.Set("Accept", "application/vnd.github.moondragon+json") resp, err := http.DefaultClient.Do(req) if err != nil { @@ -114,9 +120,12 @@ func (p *GitHubProvider) hasOrgAndTeam(accessToken string) (bool, error) { if err != nil { return false, err } + if resp.StatusCode != 200 { + return false, fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint, body) + } if err := json.Unmarshal(body, &teams); err != nil { - return false, err + return false, fmt.Errorf("%s unmarshaling %s", err, body) } for _, team := range teams { @@ -136,7 +145,6 @@ func (p *GitHubProvider) GetEmailAddress(body []byte, access_token string) (stri Primary bool `json:"primary"` } - // if we require an Org or Team, check that first if p.Org != "" { if p.Team != "" { @@ -153,7 +161,8 @@ func (p *GitHubProvider) GetEmailAddress(body []byte, access_token string) (stri params := url.Values{ "access_token": {access_token}, } - resp, err := http.DefaultClient.Get("https://api.github.com/user/emails?" + params.Encode()) + endpoint := "https://api.github.com/user/emails?" + params.Encode() + resp, err := http.DefaultClient.Get(endpoint) if err != nil { return "", err } @@ -162,9 +171,12 @@ func (p *GitHubProvider) GetEmailAddress(body []byte, access_token string) (stri if err != nil { return "", err } + if resp.StatusCode != 200 { + return "", fmt.Errorf("got %d from %q %s", resp.StatusCode, endpoint, body) + } if err := json.Unmarshal(body, &emails); err != nil { - return "", err + return "", fmt.Errorf("%s unmarshaling %s", err, body) } for _, email := range emails { diff --git a/providers/internal_util.go b/providers/internal_util.go index 50a8780..0e2f39a 100644 --- a/providers/internal_util.go +++ b/providers/internal_util.go @@ -6,8 +6,7 @@ import ( "net/http" ) -func validateToken(p Provider, access_token string, - header http.Header) bool { +func validateToken(p Provider, access_token string, header http.Header) bool { if access_token == "" || p.Data().ValidateUrl == nil { return false } diff --git a/providers/provider_default.go b/providers/provider_default.go index d962fd9..7a884a6 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -4,9 +4,11 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "io/ioutil" "net/http" "net/url" + "strings" ) func (p *ProviderData) Redeem(redirectUrl, code string) (body []byte, token string, err error) { @@ -37,6 +39,10 @@ func (p *ProviderData) Redeem(redirectUrl, code string) (body []byte, token stri return nil, "", err } + if resp.StatusCode != 200 { + return body, "", fmt.Errorf("got %d from %q %s", resp.StatusCode, p.RedeemUrl.String(), body) + } + // blindly try json and x-www-form-urlencoded var jsonResponse struct { AccessToken string `json:"access_token"` @@ -49,3 +55,16 @@ func (p *ProviderData) Redeem(redirectUrl, code string) (body []byte, token stri v, err := url.ParseQuery(string(body)) return body, v.Get("access_token"), err } + +func (p *ProviderData) GetLoginURL(redirectURI, finalRedirect string) string { + params := url.Values{} + params.Add("redirect_uri", redirectURI) + params.Add("approval_prompt", "force") + params.Add("scope", p.Scope) + params.Add("client_id", p.ClientID) + params.Add("response_type", "code") + if strings.HasPrefix(finalRedirect, "/") { + params.Add("state", finalRedirect) + } + return fmt.Sprintf("%s?%s", p.LoginUrl, params.Encode()) +} diff --git a/providers/providers.go b/providers/providers.go index 6c7f592..b7e84eb 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -5,6 +5,7 @@ type Provider interface { GetEmailAddress(body []byte, access_token string) (string, error) Redeem(string, string) ([]byte, string, error) ValidateToken(access_token string) bool + GetLoginURL(redirectURI, finalRedirect string) string } func New(provider string, p *ProviderData) Provider {