authZ: more fixes

- fix naming and formatting
- provide more context when erroring auth
- do not capitalize errors
- fix wrong documentation
- remove ugly remoteError{}

Signed-off-by: Antonio Murdaca <runcom@redhat.com>

api/server/middleware.go

@@ -56,6 +56,7 @@ func debugRequestMiddleware(handler httputils.APIFunc) httputils.APIFunc {
 // authorizationMiddleware perform authorization on the request.
 func (s *Server) authorizationMiddleware(handler httputils.APIFunc) httputils.APIFunc {
 	return func(ctx context.Context, w http.ResponseWriter, r *http.Request, vars map[string]string) error {
+		// FIXME: fill when authN gets in
 		// User and UserAuthNMethod are taken from AuthN plugins
 		// Currently tracked in https://github.com/docker/docker/pull/13994
 		user := ""

docs/extend/authorization.md

@@ -104,9 +104,6 @@ Docker's authorization subsystem supports multiple `--authz-plugin` parameters.
 
 ### Calling authorized command (allow)
 
-Your plugin must support calling the `allow` command to authorize a command. 
-This call does not impact Docker's command line.
-
 ```bash
 $ docker pull centos
 ...
@@ -116,22 +113,20 @@ f1b10cd84249: Pull complete
 
 ### Calling unauthorized command (deny)
 
-Your plugin must support calling the `deny` command to report on the outcome of 
-a plugin interaction. This call returns messages to Docker's command line informing 
-the user of the outcome of each call.  
+```bash
+$ docker pull centos
+...
+docker: Error response from daemon: authorization denied by plugin PLUGIN_NAME: volumes are not allowed.
+```
+
+### Error from plugins
 
 ```bash
 $ docker pull centos
-…
-Authorization failed. Pull command for user 'john_doe' is 
-denied by authorization plugin 'ACME' with message 
-‘[ACME] User 'john_doe' is not allowed to perform the pull 
-command’
+...
+docker: Error response from daemon: plugin PLUGIN_NAME failed with error: AuthZPlugin.AuthZReq: Cannot connect to the Docker daemon. Is the docker daemon running on this host?.
 ```
 
-Where multiple authorization plugins are installed, multiple messages are expected.
-
-
 ## API schema and implementation
 
 In addition to Docker's standard plugin registration method, each plugin 

integration-cli/docker_cli_authz_unix_test.go

@@ -43,7 +43,6 @@ type authorizationController struct {
 	psRequestCnt  int                    // psRequestCnt counts the number of calls to list container request api
 	psResponseCnt int                    // psResponseCnt counts the number of calls to list containers response API
 	requestsURIs  []string               // requestsURIs stores all request URIs that are sent to the authorization controller
-
 }
 
 func (s *DockerAuthzSuite) SetUpTest(c *check.C) {
@@ -165,7 +164,6 @@ func (s *DockerAuthzSuite) TearDownSuite(c *check.C) {
 }
 
 func (s *DockerAuthzSuite) TestAuthZPluginAllowRequest(c *check.C) {
-
 	err := s.d.Start("--authz-plugin=" + testAuthZPlugin)
 	c.Assert(err, check.IsNil)
 	s.ctrl.reqRes.Allow = true
@@ -189,7 +187,6 @@ func (s *DockerAuthzSuite) TestAuthZPluginAllowRequest(c *check.C) {
 }
 
 func (s *DockerAuthzSuite) TestAuthZPluginDenyRequest(c *check.C) {
-
 	err := s.d.Start("--authz-plugin=" + testAuthZPlugin)
 	c.Assert(err, check.IsNil)
 	s.ctrl.reqRes.Allow = false
@@ -202,11 +199,10 @@ func (s *DockerAuthzSuite) TestAuthZPluginDenyRequest(c *check.C) {
 	c.Assert(s.ctrl.psResponseCnt, check.Equals, 0)
 
 	// Ensure unauthorized message appears in response
-	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: %s\n", unauthorizedMessage))
+	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: authorization denied by plugin %s: %s\n", testAuthZPlugin, unauthorizedMessage))
 }
 
 func (s *DockerAuthzSuite) TestAuthZPluginDenyResponse(c *check.C) {
-
 	err := s.d.Start("--authz-plugin=" + testAuthZPlugin)
 	c.Assert(err, check.IsNil)
 	s.ctrl.reqRes.Allow = true
@@ -220,7 +216,7 @@ func (s *DockerAuthzSuite) TestAuthZPluginDenyResponse(c *check.C) {
 	c.Assert(s.ctrl.psResponseCnt, check.Equals, 1)
 
 	// Ensure unauthorized message appears in response
-	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: %s\n", unauthorizedMessage))
+	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: authorization denied by plugin %s: %s\n", testAuthZPlugin, unauthorizedMessage))
 }
 
 func (s *DockerAuthzSuite) TestAuthZPluginErrorResponse(c *check.C) {
@@ -233,7 +229,7 @@ func (s *DockerAuthzSuite) TestAuthZPluginErrorResponse(c *check.C) {
 	res, err := s.d.Cmd("ps")
 	c.Assert(err, check.NotNil)
 
-	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: Plugin Error: %s, %s\n", errorMessage, authorization.AuthZApiResponse))
+	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: plugin %s failed with error: %s: %s\n", testAuthZPlugin, authorization.AuthZApiResponse, errorMessage))
 }
 
 func (s *DockerAuthzSuite) TestAuthZPluginErrorRequest(c *check.C) {
@@ -245,7 +241,7 @@ func (s *DockerAuthzSuite) TestAuthZPluginErrorRequest(c *check.C) {
 	res, err := s.d.Cmd("ps")
 	c.Assert(err, check.NotNil)
 
-	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: Plugin Error: %s, %s\n", errorMessage, authorization.AuthZApiRequest))
+	c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: plugin %s failed with error: %s: %s\n", testAuthZPlugin, authorization.AuthZApiRequest, errorMessage))
 }
 
 // assertURIRecorded verifies that the given URI was sent and recorded in the authz plugin

pkg/authorization/authz.go

@@ -7,6 +7,8 @@ import (
 	"io/ioutil"
 	"net/http"
 	"strings"
+
+	"github.com/Sirupsen/logrus"
 )
 
 // NewCtx creates new authZ context, it is used to store authorization information related to a specific docker
@@ -47,9 +49,9 @@ type Ctx struct {
 }
 
 // AuthZRequest authorized the request to the docker daemon using authZ plugins
-func (a *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
+func (ctx *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
 	var body []byte
-	if sendBody(a.requestURI, r.Header) {
+	if sendBody(ctx.requestURI, r.Header) {
 		var (
 			err         error
 			drainedBody io.ReadCloser
@@ -70,26 +72,25 @@ func (a *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
 		return err
 	}
 
-	a.authReq = &Request{
-		User:            a.user,
-		UserAuthNMethod: a.userAuthNMethod,
-		RequestMethod:   a.requestMethod,
-		RequestURI:      a.requestURI,
+	ctx.authReq = &Request{
+		User:            ctx.user,
+		UserAuthNMethod: ctx.userAuthNMethod,
+		RequestMethod:   ctx.requestMethod,
+		RequestURI:      ctx.requestURI,
 		RequestBody:     body,
-		RequestHeaders:  headers(r.Header)}
+		RequestHeaders:  headers(r.Header),
+	}
 
-	for _, plugin := range a.plugins {
-		authRes, err := plugin.AuthZRequest(a.authReq)
+	for _, plugin := range ctx.plugins {
+		logrus.Debugf("AuthZ request using plugin %s", plugin.Name())
+
+		authRes, err := plugin.AuthZRequest(ctx.authReq)
 		if err != nil {
-			return err
-		}
-
-		if authRes.Err != "" {
-			return fmt.Errorf(authRes.Err)
+			return fmt.Errorf("plugin %s failed with error: %s", plugin.Name(), err)
 		}
 
 		if !authRes.Allow {
-			return fmt.Errorf(authRes.Msg)
+			return fmt.Errorf("authorization denied by plugin %s: %s", plugin.Name(), authRes.Msg)
 		}
 	}
 
@@ -97,26 +98,24 @@ func (a *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
 }
 
 // AuthZResponse authorized and manipulates the response from docker daemon using authZ plugins
-func (a *Ctx) AuthZResponse(rm ResponseModifier, r *http.Request) error {
-	a.authReq.ResponseStatusCode = rm.StatusCode()
-	a.authReq.ResponseHeaders = headers(rm.Header())
+func (ctx *Ctx) AuthZResponse(rm ResponseModifier, r *http.Request) error {
+	ctx.authReq.ResponseStatusCode = rm.StatusCode()
+	ctx.authReq.ResponseHeaders = headers(rm.Header())
 
-	if sendBody(a.requestURI, rm.Header()) {
-		a.authReq.ResponseBody = rm.RawBody()
+	if sendBody(ctx.requestURI, rm.Header()) {
+		ctx.authReq.ResponseBody = rm.RawBody()
 	}
 
-	for _, plugin := range a.plugins {
-		authRes, err := plugin.AuthZResponse(a.authReq)
-		if err != nil {
-			return err
-		}
+	for _, plugin := range ctx.plugins {
+		logrus.Debugf("AuthZ response using plugin %s", plugin.Name())
 
-		if authRes.Err != "" {
-			return fmt.Errorf(authRes.Err)
+		authRes, err := plugin.AuthZResponse(ctx.authReq)
+		if err != nil {
+			return fmt.Errorf("plugin %s failed with error: %s", plugin.Name(), err)
 		}
 
 		if !authRes.Allow {
-			return fmt.Errorf(authRes.Msg)
+			return fmt.Errorf("authorization denied by plugin %s: %s", plugin.Name(), authRes.Msg)
 		}
 	}
 

pkg/authorization/plugin.go

@@ -1,13 +1,13 @@
 package authorization
 
-import (
-	"github.com/Sirupsen/logrus"
-	"github.com/docker/docker/pkg/plugins"
-)
+import "github.com/docker/docker/pkg/plugins"
 
 // Plugin allows third party plugins to authorize requests and responses
 // in the context of docker API
 type Plugin interface {
+	// Name returns the registered plugin name
+	Name() string
+
 	// AuthZRequest authorize the request from the client to the daemon
 	AuthZRequest(*Request) (*Response, error)
 
@@ -34,9 +34,11 @@ func newAuthorizationPlugin(name string) Plugin {
 	return &authorizationPlugin{name: name}
 }
 
-func (a *authorizationPlugin) AuthZRequest(authReq *Request) (*Response, error) {
-	logrus.Debugf("AuthZ requset using plugins %s", a.name)
+func (a *authorizationPlugin) Name() string {
+	return a.name
+}
 
+func (a *authorizationPlugin) AuthZRequest(authReq *Request) (*Response, error) {
 	if err := a.initPlugin(); err != nil {
 		return nil, err
 	}
@@ -50,8 +52,6 @@ func (a *authorizationPlugin) AuthZRequest(authReq *Request) (*Response, error)
 }
 
 func (a *authorizationPlugin) AuthZResponse(authReq *Request) (*Response, error) {
-	logrus.Debugf("AuthZ response using plugins %s", a.name)
-
 	if err := a.initPlugin(); err != nil {
 		return nil, err
 	}

pkg/plugins/client.go

@@ -20,15 +20,6 @@ const (
 	defaultTimeOut  = 30
 )
 
-type remoteError struct {
-	method string
-	err    string
-}
-
-func (e *remoteError) Error() string {
-	return fmt.Sprintf("Plugin Error: %s, %s", e.err, e.method)
-}
-
 // NewClient creates a new plugin client (http).
 func NewClient(addr string, tlsConfig tlsconfig.Options) (*Client, error) {
 	tr := &http.Transport{}
@@ -133,7 +124,7 @@ func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool)
 		if resp.StatusCode != http.StatusOK {
 			b, err := ioutil.ReadAll(resp.Body)
 			if err != nil {
-				return nil, &remoteError{method: serviceMethod, err: err.Error()}
+				return nil, fmt.Errorf("%s: %s", serviceMethod, err)
 			}
 
 			// Plugins' Response(s) should have an Err field indicating what went
@@ -144,13 +135,13 @@ func (c *Client) callWithRetry(serviceMethod string, data io.Reader, retry bool)
 			}
 			remoteErr := responseErr{}
 			if err := json.Unmarshal(b, &remoteErr); err != nil {
-				return nil, &remoteError{method: serviceMethod, err: err.Error()}
+				return nil, fmt.Errorf("%s: %s", serviceMethod, err)
 			}
 			if remoteErr.Err != "" {
-				return nil, &remoteError{method: serviceMethod, err: remoteErr.Err}
+				return nil, fmt.Errorf("%s: %s", serviceMethod, remoteErr.Err)
 			}
 			// old way...
-			return nil, &remoteError{method: serviceMethod, err: string(b)}
+			return nil, fmt.Errorf("%s: %s", serviceMethod, string(b))
 		}
 		return resp.Body, nil
 	}