Skip to content

Commit 526abc0

Browse files
author
Liron Levin
committed
Fix authorization issue - when request is denied return forbbiden exist 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 <[email protected]>
1 parent b0a5762 commit 526abc0

File tree

4 files changed

+48
-5
lines changed

4 files changed

+48
-5
lines changed

daemon/network.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package daemon
33
import (
44
"fmt"
55
"net"
6-
"net/http"
76
"strings"
87

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

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

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

227226
if err := nw.Delete(); err != nil {

errors/errors.go

+6
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ func NewBadRequestError(err error) error {
2828
return NewErrorWithStatusCode(err, http.StatusBadRequest)
2929
}
3030

31+
// NewRequestForbiddenError creates a new API error
32+
// that has the 403 HTTP status code associated to it.
33+
func NewRequestForbiddenError(err error) error {
34+
return NewErrorWithStatusCode(err, http.StatusForbidden)
35+
}
36+
3137
// NewRequestNotFoundError creates a new API error
3238
// that has the 404 HTTP status code associated to it.
3339
func NewRequestNotFoundError(err error) error {

integration-cli/docker_cli_authz_unix_test.go

+24
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ import (
2121
"github.com/docker/docker/pkg/integration/checker"
2222
"github.com/docker/docker/pkg/plugins"
2323
"github.com/go-check/check"
24+
"net"
25+
"net/http/httputil"
26+
"net/url"
2427
)
2528

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

278+
// TestAuthZPluginApiDenyResponse validates that when authorization plugin deny the request, the status code is forbidden
279+
func (s *DockerAuthzSuite) TestAuthZPluginApiDenyResponse(c *check.C) {
280+
err := s.d.Start("--authorization-plugin=" + testAuthZPlugin)
281+
c.Assert(err, check.IsNil)
282+
s.ctrl.reqRes.Allow = false
283+
s.ctrl.resRes.Msg = unauthorizedMessage
284+
285+
daemonURL, err := url.Parse(s.d.sock())
286+
287+
conn, err := net.DialTimeout(daemonURL.Scheme, daemonURL.Path, time.Second*10)
288+
c.Assert(err, check.IsNil)
289+
client := httputil.NewClientConn(conn, nil)
290+
req, err := http.NewRequest("GET", "/version", nil)
291+
c.Assert(err, check.IsNil)
292+
resp, err := client.Do(req)
293+
294+
c.Assert(err, check.IsNil)
295+
c.Assert(resp.StatusCode, checker.Equals, http.StatusForbidden)
296+
c.Assert(err, checker.IsNil)
297+
}
298+
275299
func (s *DockerAuthzSuite) TestAuthZPluginDenyResponse(c *check.C) {
276300
err := s.d.Start("--authorization-plugin=" + testAuthZPlugin)
277301
c.Assert(err, check.IsNil)

pkg/authorization/authz.go

+16-2
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (ctx *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
8585
}
8686

8787
if !authRes.Allow {
88-
return fmt.Errorf("authorization denied by plugin %s: %s", plugin.Name(), authRes.Msg)
88+
return newAuthorizationError(plugin.Name(), authRes.Msg)
8989
}
9090
}
9191

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

112112
if !authRes.Allow {
113-
return fmt.Errorf("authorization denied by plugin %s: %s", plugin.Name(), authRes.Msg)
113+
return newAuthorizationError(plugin.Name(), authRes.Msg)
114114
}
115115
}
116116

@@ -163,3 +163,17 @@ func headers(header http.Header) map[string]string {
163163
}
164164
return v
165165
}
166+
167+
// authorizationError represents an authorization deny error
168+
type authorizationError struct {
169+
error
170+
}
171+
172+
// HTTPErrorStatusCode returns the authorization error status code (forbidden)
173+
func (e authorizationError) HTTPErrorStatusCode() int {
174+
return http.StatusForbidden
175+
}
176+
177+
func newAuthorizationError(plugin, msg string) authorizationError {
178+
return authorizationError{error: fmt.Errorf("authorization denied by plugin %s: %s", plugin, msg)}
179+
}

0 commit comments

Comments
 (0)