Merge pull request #865 from stevvooe/next-generation

Integrate auth.AccessController into registry app

api/v2/descriptors.go

@@ -39,6 +39,14 @@ var ErrorDescriptors = []ErrorDescriptor{
 		Description: `Generic error returned when the error does not have an
 		API classification.`,
 	},
+	{
+		Code:    ErrorCodeUnauthorized,
+		Value:   "UNAUTHORIZED",
+		Message: "access to the requested resource is not authorized",
+		Description: `The access controller denied access for the operation on
+		a resource. Often this will be accompanied by a 401 Unauthorized
+		response status.`,
+	},
 	{
 		Code:    ErrorCodeDigestInvalid,
 		Value:   "DIGEST_INVALID",

api/v2/errors.go

@@ -13,6 +13,9 @@ const (
 	// ErrorCodeUnknown is a catch-all for errors not defined below.
 	ErrorCodeUnknown ErrorCode = iota
 
+	// ErrorCodeUnauthorized is returned if a request is not authorized.
+	ErrorCodeUnauthorized
+
 	// ErrorCodeDigestInvalid is returned when uploading a blob if the
 	// provided digest does not match the blob contents.
 	ErrorCodeDigestInvalid

app.go

@@ -5,11 +5,11 @@ import (
 	"net/http"
 
 	"github.com/docker/docker-registry/api/v2"
-	"github.com/docker/docker-registry/storagedriver"
+	"github.com/docker/docker-registry/auth"
-	"github.com/docker/docker-registry/storagedriver/factory"
-
 	"github.com/docker/docker-registry/configuration"
 	"github.com/docker/docker-registry/storage"
+	"github.com/docker/docker-registry/storagedriver"
+	"github.com/docker/docker-registry/storagedriver/factory"
 
 	log "github.com/Sirupsen/logrus"
 	"github.com/gorilla/mux"
@@ -28,6 +28,8 @@ type App struct {
 
 	// services contains the main services instance for the application.
 	services *storage.Services
+
+	accessController auth.AccessController
 }
 
 // NewApp takes a configuration and returns a configured app, ready to serve
@@ -61,6 +63,16 @@ func NewApp(configuration configuration.Configuration) *App {
 	app.driver = driver
 	app.services = storage.NewServices(app.driver)
 
+	authType := configuration.Auth.Type()
+
+	if authType != "" {
+		accessController, err := auth.GetAccessController(configuration.Auth.Type(), configuration.Auth.Parameters())
+		if err != nil {
+			panic(fmt.Sprintf("unable to configure authorization (%s): %v", authType, err))
+		}
+		app.accessController = accessController
+	}
+
 	return app
 }
 
@@ -111,15 +123,11 @@ func (ssrw *singleStatusResponseWriter) WriteHeader(status int) {
 // handler, using the dispatch factory function.
 func (app *App) dispatcher(dispatch dispatchFunc) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		vars := mux.Vars(r)
+		context := app.context(r)
-		context := &Context{
-			App:        app,
-			Name:       vars["name"],
-			urlBuilder: v2.NewURLBuilderFromRequest(r),
-		}
 
-		// Store vars for underlying handlers.
+		if err := app.authorized(w, r, context); err != nil {
-		context.vars = vars
+			return
+		}
 
 		context.log = log.WithField("name", context.Name)
 		handler := dispatch(context, r)
@@ -140,6 +148,86 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler {
 	})
 }
 
+// context constructs the context object for the application. This only be
+// called once per request.
+func (app *App) context(r *http.Request) *Context {
+	vars := mux.Vars(r)
+	context := &Context{
+		App:        app,
+		Name:       vars["name"],
+		urlBuilder: v2.NewURLBuilderFromRequest(r),
+	}
+
+	// Store vars for underlying handlers.
+	context.vars = vars
+
+	return context
+}
+
+// authorized checks if the request can proceed with with request access-
+// level. If it cannot, the method will return an error.
+func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Context) error {
+	if app.accessController == nil {
+		return nil // access controller is not enabled.
+	}
+
+	var accessRecords []auth.Access
+	resource := auth.Resource{
+		Type: "repository",
+		Name: context.Name,
+	}
+
+	switch r.Method {
+	case "GET", "HEAD":
+		accessRecords = append(accessRecords,
+			auth.Access{
+				Resource: resource,
+				Action:   "pull",
+			})
+	case "POST", "PUT", "PATCH":
+		accessRecords = append(accessRecords,
+			auth.Access{
+				Resource: resource,
+				Action:   "pull",
+			},
+			auth.Access{
+				Resource: resource,
+				Action:   "push",
+			})
+	case "DELETE":
+		// DELETE access requires full admin rights, which is represented
+		// as "*". This may not be ideal.
+		accessRecords = append(accessRecords,
+			auth.Access{
+				Resource: resource,
+				Action:   "*",
+			})
+	}
+
+	if err := app.accessController.Authorized(r, accessRecords...); err != nil {
+		switch err := err.(type) {
+		case auth.Challenge:
+			w.Header().Set("Content-Type", "application/json")
+			err.ServeHTTP(w, r)
+
+			var errs v2.Errors
+			errs.Push(v2.ErrorCodeUnauthorized, accessRecords)
+			serveJSON(w, errs)
+		default:
+			// This condition is a potential security problem either in
+			// the configuration or whatever is backing the access
+			// controller. Just return a bad request with no information
+			// to avoid exposure. The request should not proceed.
+			context.log.Errorf("error checking authorization: %v", err)
+			w.WriteHeader(http.StatusBadRequest)
+		}
+
+		return err
+	}
+
+	return nil
+}
+
 // apiBase implements a simple yes-man for doing overall checks against the
 // api. This can support auth roundtrips to support docker login.
 func apiBase(w http.ResponseWriter, r *http.Request) {

app_test.go

@@ -1,12 +1,14 @@
 package registry
 
 import (
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"testing"
 
 	"github.com/docker/docker-registry/api/v2"
+	_ "github.com/docker/docker-registry/auth/silly"
 	"github.com/docker/docker-registry/configuration"
 )
 
@@ -124,3 +126,64 @@ func TestAppDispatcher(t *testing.T) {
 		}
 	}
 }
+
+// TestNewApp covers the creation of an application via NewApp with a
+// configuration.
+func TestNewApp(t *testing.T) {
+	config := configuration.Configuration{
+		Storage: configuration.Storage{
+			"inmemory": nil,
+		},
+		Auth: configuration.Auth{
+			// For now, we simply test that new auth results in a viable
+			// application.
+			"silly": {
+				"realm":   "realm-test",
+				"service": "service-test",
+			},
+		},
+	}
+
+	// Mostly, with this test, given a sane configuration, we are simply
+	// ensuring that NewApp doesn't panic. We might want to tweak this
+	// behavior.
+	app := NewApp(config)
+
+	server := httptest.NewServer(app)
+	builder, err := v2.NewURLBuilderFromString(server.URL)
+	if err != nil {
+		t.Fatalf("error creating urlbuilder: %v", err)
+	}
+
+	baseURL, err := builder.BuildBaseURL()
+	if err != nil {
+		t.Fatalf("error creating baseURL: %v", err)
+	}
+
+	// TODO(stevvooe): The rest of this test might belong in the API tests.
+
+	// Just hit the app and make sure we get a 401 Unauthorized error.
+	req, err := http.Get(baseURL)
+	if err != nil {
+		t.Fatalf("unexpected error during GET: %v", err)
+	}
+	defer req.Body.Close()
+
+	if req.StatusCode != http.StatusUnauthorized {
+		t.Fatalf("unexpected status code during request: %v", err)
+	}
+
+	if req.Header.Get("Content-Type") != "application/json" {
+		t.Fatalf("unexpected content-type: %v != %v", req.Header.Get("Content-Type"), "application/json")
+	}
+
+	var errs v2.Errors
+	dec := json.NewDecoder(req.Body)
+	if err := dec.Decode(&errs); err != nil {
+		t.Fatalf("error decoding error response: %v", err)
+	}
+
+	if errs.Errors[0].Code != v2.ErrorCodeUnauthorized {
+		t.Fatalf("unexpected error code: %v != %v", errs.Errors[0].Code, v2.ErrorCodeUnauthorized)
+	}
+}

auth/silly/access.go

@@ -0,0 +1,89 @@
+// Package silly provides a simple authentication scheme that checks for the
+// existence of an Authorization header and issues access if is present and
+// non-empty.
+//
+// This package is present as an example implementation of a minimal
+// auth.AccessController and for testing. This is not suitable for any kind of
+// production security.
+package silly
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+
+	"github.com/docker/docker-registry/auth"
+)
+
+// accessController provides a simple implementation of auth.AccessController
+// that simply checks for a non-empty Authorization header. It is useful for
+// demonstration and testing.
+type accessController struct {
+	realm   string
+	service string
+}
+
+var _ auth.AccessController = &accessController{}
+
+func newAccessController(options map[string]interface{}) (auth.AccessController, error) {
+	realm, present := options["realm"]
+	if _, ok := realm.(string); !present || !ok {
+		return nil, fmt.Errorf(`"realm" must be set for silly access controller`)
+	}
+
+	service, present := options["service"]
+	if _, ok := service.(string); !present || !ok {
+		return nil, fmt.Errorf(`"service" must be set for silly access controller`)
+	}
+
+	return &accessController{realm: realm.(string), service: service.(string)}, nil
+}
+
+// Authorized simply checks for the existence of the authorization header,
+// responding with a bearer challenge if it doesn't exist.
+func (ac *accessController) Authorized(req *http.Request, accessRecords ...auth.Access) error {
+	if req.Header.Get("Authorization") == "" {
+		challenge := challenge{
+			realm:   ac.realm,
+			service: ac.service,
+		}
+
+		if len(accessRecords) > 0 {
+			var scopes []string
+			for _, access := range accessRecords {
+				scopes = append(scopes, fmt.Sprintf("%s:%s:%s", access.Type, access.Resource, access.Action))
+			}
+			challenge.scope = strings.Join(scopes, " ")
+		}
+
+		return &challenge
+	}
+
+	return nil
+}
+
+type challenge struct {
+	realm   string
+	service string
+	scope   string
+}
+
+func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	header := fmt.Sprintf("Bearer realm=%q,service=%q", ch.realm, ch.service)
+
+	if ch.scope != "" {
+		header = fmt.Sprintf("%s,scope=%q", header, ch.scope)
+	}
+
+	w.Header().Set("Authorization", header)
+	w.WriteHeader(http.StatusUnauthorized)
+}
+
+func (ch *challenge) Error() string {
+	return fmt.Sprintf("silly authentication challenge: %#v", ch)
+}
+
+// init registers the silly auth backend.
+func init() {
+	auth.Register("silly", auth.InitFunc(newAccessController))
+}

auth/silly/access_test.go

@@ -0,0 +1,58 @@
+package silly
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/docker/docker-registry/auth"
+)
+
+func TestSillyAccessController(t *testing.T) {
+	ac := &accessController{
+		realm:   "test-realm",
+		service: "test-service",
+	}
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if err := ac.Authorized(r); err != nil {
+			switch err := err.(type) {
+			case auth.Challenge:
+				err.ServeHTTP(w, r)
+				return
+			default:
+				t.Fatalf("unexpected error authorizing request: %v", err)
+			}
+		}
+
+		w.WriteHeader(http.StatusNoContent)
+	}))
+
+	resp, err := http.Get(server.URL)
+	if err != nil {
+		t.Fatalf("unexpected error during GET: %v", err)
+	}
+	defer resp.Body.Close()
+
+	// Request should not be authorized
+	if resp.StatusCode != http.StatusUnauthorized {
+		t.Fatalf("unexpected response status: %v != %v", resp.StatusCode, http.StatusUnauthorized)
+	}
+
+	req, err := http.NewRequest("GET", server.URL, nil)
+	if err != nil {
+		t.Fatalf("unexpected error creating new request: %v", err)
+	}
+	req.Header.Set("Authorization", "seriously, anything")
+
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatalf("unexpected error during GET: %v", err)
+	}
+	defer resp.Body.Close()
+
+	// Request should not be authorized
+	if resp.StatusCode != http.StatusNoContent {
+		t.Fatalf("unexpected response status: %v != %v", resp.StatusCode, http.StatusNoContent)
+	}
+}

cmd/registry/main.go

@@ -7,14 +7,14 @@ import (
 	_ "net/http/pprof"
 	"os"
 
-	"github.com/gorilla/handlers"
-
 	log "github.com/Sirupsen/logrus"
-
 	"github.com/bugsnag/bugsnag-go"
+	"github.com/gorilla/handlers"
 	"github.com/yvasiyarov/gorelic"
 
 	"github.com/docker/docker-registry"
+	_ "github.com/docker/docker-registry/auth/silly"
+	_ "github.com/docker/docker-registry/auth/token"
 	"github.com/docker/docker-registry/configuration"
 	_ "github.com/docker/docker-registry/storagedriver/filesystem"
 	_ "github.com/docker/docker-registry/storagedriver/inmemory"

configuration/configuration.go

@@ -20,6 +20,10 @@ type Configuration struct {
 	// Storage is the configuration for the registry's storage driver
 	Storage Storage `yaml:"storage"`
 
+	// Auth allows configuration of various authorization methods that may be
+	// used to gate requests.
+	Auth Auth `yaml:"auth"`
+
 	// Reporting is the configuration for error reporting
 	Reporting Reporting `yaml:"reporting"`
 
@@ -85,6 +89,9 @@ func (loglevel *Loglevel) UnmarshalYAML(unmarshal func(interface{}) error) error
 	return nil
 }
 
+// Parameters defines a key-value parameters mapping
+type Parameters map[string]interface{}
+
 // Storage defines the configuration for registry object storage
 type Storage map[string]Parameters
 
@@ -137,13 +144,71 @@ func (storage *Storage) UnmarshalYAML(unmarshal func(interface{}) error) error {
 // MarshalYAML implements the yaml.Marshaler interface
 func (storage Storage) MarshalYAML() (interface{}, error) {
 	if storage.Parameters() == nil {
-		return storage.Type, nil
+		return storage.Type(), nil
 	}
 	return map[string]Parameters(storage), nil
 }
 
-// Parameters defines a key-value parameters mapping
+// Auth defines the configuration for registry authorization.
-type Parameters map[string]interface{}
+type Auth map[string]Parameters
+
+// Type returns the storage driver type, such as filesystem or s3
+func (auth Auth) Type() string {
+	// Return only key in this map
+	for k := range auth {
+		return k
+	}
+	return ""
+}
+
+// Parameters returns the Parameters map for an Auth configuration
+func (auth Auth) Parameters() Parameters {
+	return auth[auth.Type()]
+}
+
+// setParameter changes the parameter at the provided key to the new value
+func (auth Auth) setParameter(key string, value interface{}) {
+	auth[auth.Type()][key] = value
+}
+
+// UnmarshalYAML implements the yaml.Unmarshaler interface
+// Unmarshals a single item map into a Storage or a string into a Storage type with no parameters
+func (auth *Auth) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var m map[string]Parameters
+	err := unmarshal(&m)
+	if err == nil {
+		if len(m) > 1 {
+			types := make([]string, 0, len(m))
+			for k := range m {
+				types = append(types, k)
+			}
+
+			// TODO(stevvooe): May want to change this slightly for
+			// authorization to allow multiple challenges.
+			return fmt.Errorf("must provide exactly one type. Provided: %v", types)
+
+		}
+		*auth = m
+		return nil
+	}
+
+	var authType string
+	err = unmarshal(&authType)
+	if err == nil {
+		*auth = Auth{authType: Parameters{}}
+		return nil
+	}
+
+	return err
+}
+
+// MarshalYAML implements the yaml.Marshaler interface
+func (auth Auth) MarshalYAML() (interface{}, error) {
+	if auth.Parameters() == nil {
+		return auth.Type(), nil
+	}
+	return map[string]Parameters(auth), nil
+}
 
 // Reporting defines error reporting methods.
 type Reporting struct {

configuration/configuration_test.go

@@ -29,6 +29,12 @@ var configStruct = Configuration{
 			"port":      42,
 		},
 	},
+	Auth: Auth{
+		"silly": Parameters{
+			"realm":   "silly",
+			"service": "silly",
+		},
+	},
 	Reporting: Reporting{
 		Bugsnag: BugsnagReporting{
 			APIKey: "BugsnagApiKey",
@@ -51,6 +57,10 @@ storage:
     secretkey: SUPERSECRET
     host: ~
     port: 42
+auth:
+  silly:
+    realm: silly
+    service: silly
 reporting:
   bugsnag:
     apikey: BugsnagApiKey
@@ -62,6 +72,10 @@ var inmemoryConfigYamlV0_1 = `
 version: 0.1
 loglevel: info
 storage: inmemory
+auth:
+  silly:
+    realm: silly
+    service: silly
 `
 
 type ConfigSuite struct {
@@ -113,10 +127,13 @@ func (suite *ConfigSuite) TestParseIncomplete(c *C) {
 	c.Assert(err, NotNil)
 
 	suite.expectedConfig.Storage = Storage{"filesystem": Parameters{"rootdirectory": "/tmp/testroot"}}
+	suite.expectedConfig.Auth = Auth{"silly": Parameters{"realm": "silly"}}
 	suite.expectedConfig.Reporting = Reporting{}
 
 	os.Setenv("REGISTRY_STORAGE", "filesystem")
 	os.Setenv("REGISTRY_STORAGE_FILESYSTEM_ROOTDIRECTORY", "/tmp/testroot")
+	os.Setenv("REGISTRY_AUTH", "silly")
+	os.Setenv("REGISTRY_AUTH_SILLY_REALM", "silly")
 
 	config, err := Parse(bytes.NewReader([]byte(incompleteConfigYaml)))
 	c.Assert(err, IsNil)
@@ -259,5 +276,10 @@ func copyConfig(config Configuration) *Configuration {
 		NewRelic: NewRelicReporting{config.Reporting.NewRelic.LicenseKey, config.Reporting.NewRelic.Name},
 	}
 
+	configCopy.Auth = Auth{config.Auth.Type(): Parameters{}}
+	for k, v := range config.Auth.Parameters() {
+		configCopy.Auth.setParameter(k, v)
+	}
+
 	return configCopy
 }