refactor(security): add NewContext and FromContext to security pkg (#10617)

1. Add `NewContext` and `FromContext` funcs in security pkg.
2. Add `Name` func in `security.Context` interface to make the checking
for the `/api/internal/configurations` API clear.
3. Get the security from the context to prepare change the security
filter to middleware.
4. Remove `GetSecurityContext` in filter pkg.

Signed-off-by: He Weiwei <hweiwei@vmware.com>

src/common/security/context.go

@@ -15,12 +15,16 @@
 package security
 
 import (
+	"context"
+
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/rbac"
 )
 
 // Context abstracts the operations related with authN and authZ
 type Context interface {
+	// Name returns the name of the security context
+	Name() string
 	// IsAuthenticated returns whether the context has been authenticated or not
 	IsAuthenticated() bool
 	// GetUsername returns the username of user related to the context
@@ -36,3 +40,16 @@ type Context interface {
 	// Can returns whether the user can do action on resource
 	Can(action rbac.Action, resource rbac.Resource) bool
 }
+
+type securityKey struct{}
+
+// NewContext returns context with security context
+func NewContext(ctx context.Context, security Context) context.Context {
+	return context.WithValue(ctx, securityKey{}, security)
+}
+
+// FromContext returns security context from the context
+func FromContext(ctx context.Context) (Context, bool) {
+	c, ok := ctx.Value(securityKey{}).(Context)
+	return c, ok
+}

src/common/security/local/context.go

@@ -38,6 +38,11 @@ func NewSecurityContext(user *models.User, pm promgr.ProjectManager) *SecurityCo
 	}
 }
 
+// Name returns the name of the security context
+func (s *SecurityContext) Name() string {
+	return "local"
+}
+
 // IsAuthenticated returns true if the user has been authenticated
 func (s *SecurityContext) IsAuthenticated() bool {
 	return s.user != nil

src/common/security/robot/context.go

@@ -37,6 +37,11 @@ func NewSecurityContext(robot *model.Robot, pm promgr.ProjectManager, policy []*
 	}
 }
 
+// Name returns the name of the security context
+func (s *SecurityContext) Name() string {
+	return "robot"
+}
+
 // IsAuthenticated returns true if the user has been authenticated
 func (s *SecurityContext) IsAuthenticated() bool {
 	return s.robot != nil

src/common/security/secret/context.go

@@ -38,6 +38,11 @@ func NewSecurityContext(secret string, store *secret.Store) *SecurityContext {
 	}
 }
 
+// Name returns the name of the security context
+func (s *SecurityContext) Name() string {
+	return "secret"
+}
+
 // IsAuthenticated returns true if the secret is valid
 func (s *SecurityContext) IsAuthenticated() bool {
 	if s.store == nil {

src/core/api/base.go

@@ -18,11 +18,11 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/goharbor/harbor/src/common/models"
 	"net/http"
 
 	"github.com/ghodss/yaml"
 	"github.com/goharbor/harbor/src/common/api"
+	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/rbac"
 	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/utils"
@@ -68,9 +68,9 @@ type BaseController struct {
 // Prepare inits security context and project manager from request
 // context
 func (b *BaseController) Prepare() {
-	ctx, err := filter.GetSecurityContext(b.Ctx.Request)
-	if err != nil {
-		log.Errorf("failed to get security context: %v", err)
+	ctx, ok := security.FromContext(b.Ctx.Request.Context())
+	if !ok {
+		log.Errorf("failed to get security context")
 		b.SendInternalServerError(errors.New(""))
 		return
 	}

src/core/api/config.go

@@ -23,11 +23,10 @@ import (
 	"github.com/goharbor/harbor/src/common/config"
 	"github.com/goharbor/harbor/src/common/config/metadata"
 	"github.com/goharbor/harbor/src/common/dao"
-	"github.com/goharbor/harbor/src/common/security/secret"
+	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/core/api/models"
 	corecfg "github.com/goharbor/harbor/src/core/config"
-	"github.com/goharbor/harbor/src/core/filter"
 )
 
 // ConfigAPI ...
@@ -47,7 +46,7 @@ func (c *ConfigAPI) Prepare() {
 
 	// Only internal container can access /api/internal/configurations
 	if strings.EqualFold(c.Ctx.Request.RequestURI, "/api/internal/configurations") {
-		if _, ok := c.Ctx.Request.Context().Value(filter.SecurCtxKey).(*secret.SecurityContext); !ok {
+		if s, ok := security.FromContext(c.Ctx.Request.Context()); !ok || s.Name() != "secret" {
 			c.SendUnAuthorizedError(errors.New("UnAuthorized"))
 			return
 		}

src/core/filter/security.go

@@ -51,9 +51,6 @@ type pathMethod struct {
 }
 
 const (
-	// SecurCtxKey is context value key for security context
-	SecurCtxKey ContextValueKey = "harbor_security_context"
-
 	// PmKey is context value key for the project manager
 	PmKey ContextValueKey = "harbor_project_manager"
 	// AuthModeKey is context key for auth mode
@@ -426,7 +423,7 @@ func (u *unauthorizedReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
 }
 
 func setSecurCtxAndPM(req *http.Request, ctx security.Context, pm promgr.ProjectManager) {
-	addToReqContext(req, SecurCtxKey, ctx)
+	*req = *(req.WithContext(security.NewContext(req.Context(), ctx)))
 	addToReqContext(req, PmKey, pm)
 }
 
@@ -434,25 +431,6 @@ func addToReqContext(req *http.Request, key, value interface{}) {
 	*req = *(req.WithContext(context.WithValue(req.Context(), key, value)))
 }
 
-// GetSecurityContext tries to get security context from request and returns it
-func GetSecurityContext(req *http.Request) (security.Context, error) {
-	if req == nil {
-		return nil, fmt.Errorf("request is nil")
-	}
-
-	ctx := req.Context().Value(SecurCtxKey)
-	if ctx == nil {
-		return nil, fmt.Errorf("the security context got from request is nil")
-	}
-
-	c, ok := ctx.(security.Context)
-	if !ok {
-		return nil, fmt.Errorf("the variable got from request is not security context type")
-	}
-
-	return c, nil
-}
-
 // GetProjectManager tries to get project manager from request and returns it
 func GetProjectManager(req *http.Request) (promgr.ProjectManager, error) {
 	if req == nil {

src/core/filter/security_test.go

@@ -432,9 +432,13 @@ func addSessionIDToCookie(req *http.Request, sessionID string) {
 }
 
 func securityContext(ctx *beegoctx.Context) interface{} {
-	c, err := GetSecurityContext(ctx.Request)
-	if err != nil {
-		log.Printf("failed to get security context: %v \n", err)
+	if ctx.Request == nil {
+		return nil
+	}
+
+	c, ok := security.FromContext(ctx.Request.Context())
+	if !ok {
+		log.Printf("failed to get security context")
 		return nil
 	}
 	return c
@@ -447,36 +451,6 @@ func projectManager(ctx *beegoctx.Context) interface{} {
 	return ctx.Request.Context().Value(PmKey)
 }
 
-func TestGetSecurityContext(t *testing.T) {
-	// nil request
-	ctx, err := GetSecurityContext(nil)
-	assert.NotNil(t, err)
-
-	// the request contains no security context
-	req, err := http.NewRequest("", "", nil)
-	assert.Nil(t, err)
-	ctx, err = GetSecurityContext(req)
-	assert.NotNil(t, err)
-
-	// the request contains a variable which is not the correct type
-	req, err = http.NewRequest("", "", nil)
-	assert.Nil(t, err)
-	req = req.WithContext(context.WithValue(req.Context(),
-		SecurCtxKey, "test"))
-	ctx, err = GetSecurityContext(req)
-	assert.NotNil(t, err)
-
-	// the request contains a correct variable
-	req, err = http.NewRequest("", "", nil)
-	assert.Nil(t, err)
-	req = req.WithContext(context.WithValue(req.Context(),
-		SecurCtxKey, local.NewSecurityContext(nil, nil)))
-	ctx, err = GetSecurityContext(req)
-	assert.Nil(t, err)
-	_, ok := ctx.(security.Context)
-	assert.True(t, ok)
-}
-
 func TestGetProjectManager(t *testing.T) {
 	// nil request
 	pm, err := GetProjectManager(nil)

src/core/middlewares/inlet.go

@@ -16,11 +16,12 @@ package middlewares
 
 import (
 	"errors"
+	"net/http"
+
+	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/utils/log"
-	"github.com/goharbor/harbor/src/core/filter"
 	"github.com/goharbor/harbor/src/core/middlewares/registryproxy"
 	"github.com/goharbor/harbor/src/core/middlewares/util"
-	"net/http"
 )
 
 var head http.Handler
@@ -37,9 +38,9 @@ func Init() error {
 
 // Handle handles the request.
 func Handle(rw http.ResponseWriter, req *http.Request) {
-	securityCtx, err := filter.GetSecurityContext(req)
-	if err != nil {
-		log.Errorf("failed to get security context in middlerware: %v", err)
+	securityCtx, ok := security.FromContext(req.Context())
+	if !ok {
+		log.Errorf("failed to get security context in middlerware")
 		// error to get security context, use the default chain.
 		head = New(Middlewares).Create().Then(proxy)
 	} else {

src/core/service/token/creator.go

@@ -203,8 +203,8 @@ func (g generalCreator) Create(r *http.Request) (*models.Token, error) {
 	scopes := parseScopes(r.URL)
 	log.Debugf("scopes: %v", scopes)
 
-	ctx, err := filter.GetSecurityContext(r)
-	if err != nil {
+	ctx, ok := security.FromContext(r.Context())
+	if !ok {
 		return nil, fmt.Errorf("failed to  get security context from request")
 	}
 

src/core/service/token/token_test.go

@@ -226,6 +226,10 @@ type fakeSecurityContext struct {
 	isAdmin bool
 }
 
+func (f *fakeSecurityContext) Name() string {
+	return "fake"
+}
+
 func (f *fakeSecurityContext) IsAuthenticated() bool {
 	return true
 }

src/server/middleware/v2auth/auth.go

@@ -15,18 +15,19 @@
 package v2auth
 
 import (
+	"context"
 	"fmt"
+	"net/http"
+	"sync"
+
 	"github.com/goharbor/harbor/src/common/rbac"
+	"github.com/goharbor/harbor/src/common/security"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/core/config"
-	"github.com/goharbor/harbor/src/core/filter"
 	"github.com/goharbor/harbor/src/core/promgr"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/server/middleware"
 	reg_err "github.com/goharbor/harbor/src/server/registry/error"
-	"golang.org/x/net/context"
-	"net/http"
-	"sync"
 )
 
 type reqChecker struct {
@@ -38,9 +39,9 @@ func (rc *reqChecker) check(req *http.Request) error {
 		// TODO: May consider implement a local authorizer for registry, more details see #10602
 		return nil
 	}
-	securityCtx, err := filter.GetSecurityContext(req)
-	if err != nil {
-		return err
+	securityCtx, ok := security.FromContext(req.Context())
+	if !ok {
+		return fmt.Errorf("the security context got from request is nil")
 	}
 	if a, ok := middleware.ArtifactInfoFromContext(req.Context()); ok {
 		action := getAction(req)

src/server/middleware/v2auth/auth_test.go

@@ -15,19 +15,20 @@
 package v2auth
 
 import (
-	"github.com/goharbor/harbor/src/common/models"
-	"github.com/goharbor/harbor/src/common/rbac"
-	"github.com/goharbor/harbor/src/core/filter"
-	"github.com/goharbor/harbor/src/core/promgr/metamgr"
-	"github.com/goharbor/harbor/src/server/middleware"
-	"github.com/stretchr/testify/assert"
-	"golang.org/x/net/context"
+	"context"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"strconv"
 	"strings"
 	"testing"
+
+	"github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/common/rbac"
+	"github.com/goharbor/harbor/src/common/security"
+	"github.com/goharbor/harbor/src/core/promgr/metamgr"
+	"github.com/goharbor/harbor/src/server/middleware"
+	"github.com/stretchr/testify/assert"
 )
 
 type mockPM struct{}
@@ -78,6 +79,10 @@ func (mockPM) GetMetadataManager() metamgr.ProjectMetadataManager {
 
 type mockSC struct{}
 
+func (mockSC) Name() string {
+	return "mock"
+}
+
 func (mockSC) IsAuthenticated() bool {
 	return true
 }
@@ -136,7 +141,7 @@ func TestMiddleware(t *testing.T) {
 		w.WriteHeader(200)
 	})
 
-	baseCtx := context.WithValue(context.Background(), filter.SecurCtxKey, mockSC{})
+	baseCtx := security.NewContext(context.Background(), mockSC{})
 	ar1 := &middleware.ArtifactInfo{
 		Repository:  "project_1/hello-world",
 		Reference:   "v1",