Skip to content

Commit

Permalink
Fix authorization issue - when request is denied return forbbiden exi…
Browse files Browse the repository at this point in the history
…st code (403).

- Return 403 (forbidden) when request is denied in authorization flows
(including integration test)
- Fix moby#22428
- Close moby#22431

Signed-off-by: Liron Levin <liron@twistlock.com>
  • Loading branch information
Liron Levin committed May 2, 2016
1 parent b0a5762 commit 526abc0
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 5 deletions.
5 changes: 2 additions & 3 deletions daemon/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package daemon
import (
"fmt"
"net"
"net/http"
"strings"

netsettings "github.com/docker/docker/daemon/network"
Expand Down Expand Up @@ -97,7 +96,7 @@ func (daemon *Daemon) getAllNetworks() []libnetwork.Network {
func (daemon *Daemon) CreateNetwork(create types.NetworkCreateRequest) (*types.NetworkCreateResponse, error) {
if runconfig.IsPreDefinedNetwork(create.Name) {
err := fmt.Errorf("%s is a pre-defined network and cannot be created", create.Name)
return nil, errors.NewErrorWithStatusCode(err, http.StatusForbidden)
return nil, errors.NewRequestForbiddenError(err)
}

var warning string
Expand Down Expand Up @@ -221,7 +220,7 @@ func (daemon *Daemon) DeleteNetwork(networkID string) error {

if runconfig.IsPreDefinedNetwork(nw.Name()) {
err := fmt.Errorf("%s is a pre-defined network and cannot be removed", nw.Name())
return errors.NewErrorWithStatusCode(err, http.StatusForbidden)
return errors.NewRequestForbiddenError(err)
}

if err := nw.Delete(); err != nil {
Expand Down
6 changes: 6 additions & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ func NewBadRequestError(err error) error {
return NewErrorWithStatusCode(err, http.StatusBadRequest)
}

// NewRequestForbiddenError creates a new API error
// that has the 403 HTTP status code associated to it.
func NewRequestForbiddenError(err error) error {
return NewErrorWithStatusCode(err, http.StatusForbidden)
}

// NewRequestNotFoundError creates a new API error
// that has the 404 HTTP status code associated to it.
func NewRequestNotFoundError(err error) error {
Expand Down
24 changes: 24 additions & 0 deletions integration-cli/docker_cli_authz_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import (
"github.com/docker/docker/pkg/integration/checker"
"github.com/docker/docker/pkg/plugins"
"github.com/go-check/check"
"net"
"net/http/httputil"
"net/url"
)

const (
Expand Down Expand Up @@ -272,6 +275,27 @@ func (s *DockerAuthzSuite) TestAuthZPluginDenyRequest(c *check.C) {
c.Assert(res, check.Equals, fmt.Sprintf("Error response from daemon: authorization denied by plugin %s: %s\n", testAuthZPlugin, unauthorizedMessage))
}

// TestAuthZPluginApiDenyResponse validates that when authorization plugin deny the request, the status code is forbidden
func (s *DockerAuthzSuite) TestAuthZPluginApiDenyResponse(c *check.C) {
err := s.d.Start("--authorization-plugin=" + testAuthZPlugin)
c.Assert(err, check.IsNil)
s.ctrl.reqRes.Allow = false
s.ctrl.resRes.Msg = unauthorizedMessage

daemonURL, err := url.Parse(s.d.sock())

conn, err := net.DialTimeout(daemonURL.Scheme, daemonURL.Path, time.Second*10)
c.Assert(err, check.IsNil)
client := httputil.NewClientConn(conn, nil)
req, err := http.NewRequest("GET", "/version", nil)
c.Assert(err, check.IsNil)
resp, err := client.Do(req)

c.Assert(err, check.IsNil)
c.Assert(resp.StatusCode, checker.Equals, http.StatusForbidden)
c.Assert(err, checker.IsNil)
}

func (s *DockerAuthzSuite) TestAuthZPluginDenyResponse(c *check.C) {
err := s.d.Start("--authorization-plugin=" + testAuthZPlugin)
c.Assert(err, check.IsNil)
Expand Down
18 changes: 16 additions & 2 deletions pkg/authorization/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (ctx *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
}

if !authRes.Allow {
return fmt.Errorf("authorization denied by plugin %s: %s", plugin.Name(), authRes.Msg)
return newAuthorizationError(plugin.Name(), authRes.Msg)
}
}

Expand All @@ -110,7 +110,7 @@ func (ctx *Ctx) AuthZResponse(rm ResponseModifier, r *http.Request) error {
}

if !authRes.Allow {
return fmt.Errorf("authorization denied by plugin %s: %s", plugin.Name(), authRes.Msg)
return newAuthorizationError(plugin.Name(), authRes.Msg)
}
}

Expand Down Expand Up @@ -163,3 +163,17 @@ func headers(header http.Header) map[string]string {
}
return v
}

// authorizationError represents an authorization deny error
type authorizationError struct {
error
}

// HTTPErrorStatusCode returns the authorization error status code (forbidden)
func (e authorizationError) HTTPErrorStatusCode() int {
return http.StatusForbidden
}

func newAuthorizationError(plugin, msg string) authorizationError {
return authorizationError{error: fmt.Errorf("authorization denied by plugin %s: %s", plugin, msg)}
}

0 comments on commit 526abc0

Please sign in to comment.