Merge pull request #4592 from reasonerjt/secret-in-header

Store secret in header instead of cookie

src/adminserver/auth/auth.go

@@ -15,6 +15,7 @@
 package auth
 
 import (
+	"github.com/vmware/harbor/src/common/secret"
 	"net/http"
 )
 
@@ -41,21 +42,10 @@ func (s *secretAuthenticator) Authenticate(req *http.Request) (bool, error) {
 	if len(s.secrets) == 0 {
 		return true, nil
 	}
-
-	secret, err := req.Cookie("secret")
-	if err != nil {
-		if err == http.ErrNoCookie {
-			return false, nil
-		}
-		return false, err
-	}
-
-	if secret == nil {
-		return false, nil
-	}
+	reqSecret := secret.FromRequest(req)
 
 	for _, v := range s.secrets {
-		if secret.Value == v {
+		if reqSecret == v {
 			return true, nil
 		}
 	}

src/adminserver/auth/auth_test.go

@@ -19,6 +19,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	commonsecret "github.com/vmware/harbor/src/common/secret"
 )
 
 func TestAuthenticate(t *testing.T) {
@@ -32,11 +33,7 @@ func TestAuthenticate(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to create request: %v", err)
 	}
-	req2.AddCookie(&http.Cookie{
-		Name:  "secret",
-		Value: secret,
-	})
-
+	_ = commonsecret.AddToRequest(req2, secret)
 	cases := []struct {
 		secrets map[string]string
 		req     *http.Request

src/common/http/modifier/auth/auth.go

@@ -19,10 +19,7 @@ import (
 	"net/http"
 
 	"github.com/vmware/harbor/src/common/http/modifier"
-)
-
-const (
-	secretCookieName = "secret"
+	"github.com/vmware/harbor/src/common/secret"
 )
 
 // Authorizer is a kind of Modifier used to authorize the requests
@@ -45,10 +42,6 @@ func (s *SecretAuthorizer) Modify(req *http.Request) error {
 	if req == nil {
 		return errors.New("the request is null")
 	}
-
-	req.AddCookie(&http.Cookie{
-		Name:  secretCookieName,
-		Value: s.secret,
-	})
+	secret.AddToRequest(req, s.secret)
 	return nil
 }

src/common/http/modifier/auth/auth_test.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	commonsecret "github.com/vmware/harbor/src/common/secret"
 )
 
 func TestAuthorizeOfSecretAuthorizer(t *testing.T) {
@@ -32,8 +33,5 @@ func TestAuthorizeOfSecretAuthorizer(t *testing.T) {
 	req, err := http.NewRequest("", "", nil)
 	require.Nil(t, err)
 	require.Nil(t, authorizer.Modify(req))
-	require.Equal(t, 1, len(req.Cookies()))
-	v, err := req.Cookie(secretCookieName)
-	require.Nil(t, err)
-	assert.Equal(t, secret, v.Value)
+	assert.Equal(t, secret, commonsecret.FromRequest(req))
 }

src/common/models/replication_job.go

@@ -28,8 +28,6 @@ const (
 	RepOpDelete string = "delete"
 	//RepOpSchedule represents the operation of a job to schedule the real replication process
 	RepOpSchedule string = "schedule"
-	//UISecretCookie is the cookie name to contain the UI secret
-	UISecretCookie string = "secret"
 	//RepTargetTable is the table name for replication targets
 	RepTargetTable = "replication_target"
 	//RepJobTable is the table name for replication jobs

src/common/secret/request.go

@@ -0,0 +1,47 @@
+// Copyright (c) 2017 VMware, Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package secret
+
+import (
+	"fmt"
+	"net/http"
+	"strings"
+)
+
+//HeaderPrefix is the prefix of the value of Authorization header.
+//It has the space.
+const HeaderPrefix = "Harbor-Secret "
+
+//FromRequest tries to get Harbor Secret from request header.
+//It will return empty string if the reqeust is nil.
+func FromRequest(req *http.Request) string {
+	if req == nil {
+		return ""
+	}
+	auth := req.Header.Get("Authorization")
+	if strings.HasPrefix(auth, HeaderPrefix) {
+		return strings.TrimPrefix(auth, HeaderPrefix)
+	}
+	return ""
+}
+
+//AddToRequest add the secret to request
+func AddToRequest(req *http.Request, secret string) error {
+	if req == nil {
+		return fmt.Errorf("input request is nil, unable to set secret")
+	}
+	req.Header.Set("Authorization", fmt.Sprintf("%s%s", HeaderPrefix, secret))
+	return nil
+}

src/common/secret/request_test.go

@@ -0,0 +1,49 @@
+// Copyright (c) 2017 VMware, Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package secret
+
+import (
+	"net/http"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestMain(m *testing.M) {
+	rc := m.Run()
+	if rc != 0 {
+		os.Exit(rc)
+	}
+}
+
+func TestFromRequest(t *testing.T) {
+	assert := assert.New(t)
+	secret := "mysecret"
+	req, _ := http.NewRequest("GET", "http://test.com", nil)
+	req.Header.Add("Authorization", "Harbor-Secret "+secret)
+	assert.Equal(secret, FromRequest(req))
+	assert.Equal("", FromRequest(nil))
+}
+
+func TestAddToRequest(t *testing.T) {
+	assert := assert.New(t)
+	secret := "mysecret"
+	req, _ := http.NewRequest("GET", "http://test.com", nil)
+	err := AddToRequest(req, secret)
+	assert.Nil(err)
+	assert.Equal(secret, FromRequest(req))
+	assert.NotNil(AddToRequest(nil, secret))
+}

src/common/utils/registry/auth/credential.go

@@ -46,25 +46,3 @@ func (b *basicAuthCredential) Modify(req *http.Request) error {
 	b.AddAuthorization(req)
 	return nil
 }
-
-type cookieCredential struct {
-	cookie *http.Cookie
-}
-
-// NewCookieCredential initialize a cookie based crendential handler, the cookie in parameter will be added to request to registry
-// if this crendential is attached to a registry client.
-func NewCookieCredential(c *http.Cookie) Credential {
-	return &cookieCredential{
-		cookie: c,
-	}
-}
-
-func (c *cookieCredential) AddAuthorization(req *http.Request) {
-	req.AddCookie(c.cookie)
-}
-
-// implement github.com/vmware/harbor/src/common/http/modifier.Modifier
-func (c *cookieCredential) Modify(req *http.Request) error {
-	c.AddAuthorization(req)
-	return nil
-}

src/common/utils/registry/auth/credential_test.go

@@ -41,26 +41,3 @@ func TestAddAuthorizationOfBasicAuthCredential(t *testing.T) {
 		t.Errorf("unexpected password: %s != pwd", pwd)
 	}
 }
-
-func TestAddAuthorizationOfCookieCredential(t *testing.T) {
-	cookie := &http.Cookie{
-		Name:  "name",
-		Value: "value",
-	}
-	cred := NewCookieCredential(cookie)
-	req, err := http.NewRequest("GET", "http://example.com", nil)
-	if err != nil {
-		t.Fatalf("failed to create request: %v", err)
-	}
-
-	cred.Modify(req)
-
-	ck, err := req.Cookie("name")
-	if err != nil {
-		t.Fatalf("failed to get cookie: %v", err)
-	}
-
-	if ck.Value != "value" {
-		t.Errorf("unexpected value: %s != value", ck.Value)
-	}
-}

src/jobservice/job/impl/replication/replicate.go

@@ -5,9 +5,8 @@ import (
 	"net/http"
 
 	common_http "github.com/vmware/harbor/src/common/http"
-	"github.com/vmware/harbor/src/common/models"
+	"github.com/vmware/harbor/src/common/http/modifier/auth"
 	reg "github.com/vmware/harbor/src/common/utils/registry"
-	"github.com/vmware/harbor/src/common/utils/registry/auth"
 	"github.com/vmware/harbor/src/jobservice/env"
 	"github.com/vmware/harbor/src/jobservice/logger"
 )
@@ -57,10 +56,7 @@ func (r *Replicator) init(ctx env.JobContext, params map[string]interface{}) err
 	r.policyID = (int64)(params["policy_id"].(float64))
 	r.url = params["url"].(string)
 	r.insecure = params["insecure"].(bool)
-	cred := auth.NewCookieCredential(&http.Cookie{
-		Name:  models.UISecretCookie,
-		Value: secret(),
-	})
+	cred := auth.NewSecretAuthorizer(secret())
 
 	r.client = common_http.NewClient(&http.Client{
 		Transport: reg.GetHTTPTransport(r.insecure),

src/jobservice/job/impl/replication/transfer.go

@@ -13,7 +13,7 @@ import (
 	"github.com/docker/distribution/manifest/schema2"
 	common_http "github.com/vmware/harbor/src/common/http"
 	"github.com/vmware/harbor/src/common/http/modifier"
-	"github.com/vmware/harbor/src/common/models"
+	httpauth "github.com/vmware/harbor/src/common/http/modifier/auth"
 	"github.com/vmware/harbor/src/common/utils"
 	reg "github.com/vmware/harbor/src/common/utils/registry"
 	"github.com/vmware/harbor/src/common/utils/registry/auth"
@@ -108,10 +108,7 @@ func (t *Transfer) init(ctx env.JobContext, params map[string]interface{}) error
 	// init source registry client
 	srcURL := params["src_registry_url"].(string)
 	srcInsecure := params["src_registry_insecure"].(bool)
-	srcCred := auth.NewCookieCredential(&http.Cookie{
-		Name:  models.UISecretCookie,
-		Value: secret(),
-	})
+	srcCred := httpauth.NewSecretAuthorizer(secret())
 	srcTokenServiceURL := ""
 	if stsu, ok := params["src_token_service_url"]; ok {
 		srcTokenServiceURL = stsu.(string)

src/jobservice/job/impl/utils/utils.go

@@ -5,7 +5,7 @@ import (
 	"net/http"
 
 	"github.com/docker/distribution/registry/auth/token"
-	"github.com/vmware/harbor/src/common/models"
+	httpauth "github.com/vmware/harbor/src/common/http/modifier/auth"
 	"github.com/vmware/harbor/src/common/utils/registry"
 	"github.com/vmware/harbor/src/common/utils/registry/auth"
 )
@@ -33,11 +33,7 @@ func NewRepositoryClient(endpoint string, insecure bool, credential auth.Credent
 // access the internal registry
 func NewRepositoryClientForJobservice(repository, internalRegistryURL, secret, internalTokenServiceURL string) (*registry.Repository, error) {
 	transport := registry.GetHTTPTransport()
-
-	credential := auth.NewCookieCredential(&http.Cookie{
-		Name:  models.UISecretCookie,
-		Value: secret,
-	})
+	credential := httpauth.NewSecretAuthorizer(secret)
 
 	authorizer := auth.NewStandardTokenAuthorizer(&http.Client{
 		Transport: transport,
@@ -70,9 +66,8 @@ func BuildBlobURL(endpoint, repository, digest string) string {
 
 //GetTokenForRepo is used for job handler to get a token for clair.
 func GetTokenForRepo(repository, secret, internalTokenServiceURL string) (string, error) {
-	c := &http.Cookie{Name: models.UISecretCookie, Value: secret}
-	credentail := auth.NewCookieCredential(c)
-	t, err := auth.GetToken(internalTokenServiceURL, true, credentail,
+	credential := httpauth.NewSecretAuthorizer(secret)
+	t, err := auth.GetToken(internalTokenServiceURL, true, credential,
 		[]*token.ResourceActions{&token.ResourceActions{
 			Type:    "repository",
 			Name:    repository,

src/ui/config/config.go

@@ -40,7 +40,6 @@ import (
 const (
 	defaultKeyPath       string = "/etc/ui/key"
 	defaultTokenFilePath string = "/etc/ui/token/tokens.properties"
-	secretCookieName     string = "secret"
 )
 
 var (

src/ui/filter/security.go

@@ -126,7 +126,7 @@ type secretReqCtxModifier struct {
 }
 
 func (s *secretReqCtxModifier) Modify(ctx *beegoctx.Context) bool {
-	scrt := ctx.GetCookie("secret")
+	scrt := secstore.FromRequest(ctx.Request)
 	if len(scrt) == 0 {
 		return false
 	}

src/ui/filter/security_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/astaxie/beego/session"
 	"github.com/stretchr/testify/assert"
 	"github.com/vmware/harbor/src/common/dao"
+	commonsecret "github.com/vmware/harbor/src/common/secret"
 	"github.com/vmware/harbor/src/common/security"
 	"github.com/vmware/harbor/src/common/security/local"
 	"github.com/vmware/harbor/src/common/security/secret"
@@ -106,11 +107,7 @@ func TestSecretReqCtxModifier(t *testing.T) {
 	if err != nil {
 		t.Fatalf("failed to create request: %v", req)
 	}
-	req.AddCookie(&http.Cookie{
-		Name:  "secret",
-		Value: "secret",
-	})
-
+	commonsecret.AddToRequest(req, "secret")
 	ctx, err := newContext(req)
 	if err != nil {
 		t.Fatalf("failed to crate context: %v", err)