common/errors: Refactor common error type into pkg/common

Signed-off-by: Yannis Zarkadas <[email protected]>

Dockerfile

@@ -12,6 +12,7 @@ COPY go.sum .
 RUN go mod download
 # Copy in the code and compile
 COPY *.go ./
+COPY pkg pkg
 RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags '-extldflags "-static"' -o /go/bin/oidc-authservice
 
 

main.go

@@ -38,6 +38,7 @@ func main() {
 		log.Fatalf("Failed to parse configuration: %+v", err)
 	}
 	log.Infof("Config: %+v", c)
+	log.Infof("Secret value: %s", c.ClientSecret.Reveal())
 
 	// Start readiness probe immediately
 	log.Infof("Starting readiness probe at %v", c.ReadinessProbePort)

oidc.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"net/http"
 
+	"github.com/arrikto/oidc-authservice/pkg/common"
 	"github.com/coreos/go-oidc"
 	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
@@ -70,11 +71,8 @@ func GetUserInfo(ctx context.Context, provider *oidc.Provider, tokenSource oauth
 		return nil, err
 	}
 	if resp.StatusCode != http.StatusOK {
-		return nil, &requestError{
-			Response: resp,
-			Body:     body,
-			Err:      errors.Errorf("oidc: Calling UserInfo endpoint failed. body: %s", body),
-		}
+		return nil, errors.Wrap(common.NewRequestError(resp, body),
+			"oidc: Calling UserInfo endpoint failed.")
 	}
 
 	var userInfo UserInfo

oidc_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/arrikto/oidc-authservice/pkg/common"
 	"github.com/coreos/go-oidc"
 	"github.com/gorilla/mux"
 	"github.com/pkg/errors"
@@ -117,7 +118,7 @@ func TestGetUserInfo_ContextCancelled(t *testing.T) {
 		oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "test"}))
 
 	// Check that we find a wrapped requestError
-	var reqErr *requestError
+	var reqErr *common.RequestError
 	if !errors.As(err, &reqErr) {
 		log.Fatalf("Returned error is not a requestError. Got: %+v", reqErr)
 	}

pkg/common/errors.go

@@ -0,0 +1,32 @@
+package common
+
+import (
+	"fmt"
+	"net/http"
+)
+
+// RequestError is an error returned when an HTTP request goes wrong.
+// For example, an error status code is returned.
+type RequestError struct {
+	// Response is the HTTP Response struct
+	Response *http.Response
+	// Body is the bytes parsed from the Response struct. This must be done
+	// by the party making the HTTP request.
+	Body []byte
+}
+
+var _ error = &RequestError{}
+
+func NewRequestError(resp *http.Response, body []byte) error {
+	return &RequestError{
+		Body:     body,
+		Response: resp,
+	}
+}
+
+func (e *RequestError) Error() string {
+	// We don't log the body by default, because it can potentially contain
+	// security-sensitive information.
+	return fmt.Sprintf("An HTTP request went wrong. Status Code: %d",
+		e.Response.StatusCode)
+}

revoke.go

@@ -8,6 +8,7 @@ import (
 	"net/url"
 	"strings"
 
+	"github.com/arrikto/oidc-authservice/pkg/common"
 	"github.com/coreos/go-oidc"
 	"github.com/pkg/errors"
 	"golang.org/x/oauth2"
@@ -77,17 +78,11 @@ func revokeToken(ctx context.Context, revocationEndpoint string, token, tokenTyp
 		defer resp.Body.Close()
 		body, err := ioutil.ReadAll(resp.Body)
 		if err != nil {
-			return &requestError{
-				Response: resp,
-				Body:     body,
-				Err:      errors.New(fmt.Sprintf("Revocation endpoint returned code %v, failed to read body: %v", code, err)),
-			}
-		}
-		return &requestError{
-			Response: resp,
-			Body:     body,
-			Err:      errors.New(fmt.Sprintf("Revocation endpoint returned code %v, server returned: %v", code, body)),
+			return errors.Wrapf(common.NewRequestError(resp, nil),
+				"oidc: Token revocation failed and response body couldn't be read: %v", err)
 		}
+		return errors.Wrap(common.NewRequestError(resp, body),
+			"oidc: Token revocation failed")
 	}
 	return nil
 }

server.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/arrikto/oidc-authservice/pkg/common"
 	"github.com/coreos/go-oidc"
 	"github.com/gorilla/sessions"
 	"github.com/pkg/errors"
@@ -88,7 +89,7 @@ func (s *server) authenticate(w http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			// Check if the OAuth token has expired and if it has, delete the
 			// user's session
-			var reqErr *requestError
+			var reqErr *common.RequestError
 			if errors.As(err, &reqErr) && reqErr.Response.StatusCode == http.StatusUnauthorized {
 				logger.Info("UserInfo token has expired")
 				session.Options.MaxAge = -1
@@ -279,7 +280,7 @@ func (s *server) logout(w http.ResponseWriter, r *http.Request) {
 			logger.Errorf("Error revoking tokens: %v", err)
 			statusCode := http.StatusInternalServerError
 			// If the server returned 503, return it as well as the client might want to retry
-			if reqErr, ok := errors.Cause(err).(*requestError); ok {
+			if reqErr, ok := errors.Cause(err).(*common.RequestError); ok {
 				if reqErr.Response.StatusCode == http.StatusServiceUnavailable {
 					statusCode = reqErr.Response.StatusCode
 				}