Commit fb5ddd90 authored by Alessio Caiazza's avatar Alessio Caiazza

Merge branch '261-10io-add-signed-params-to-file-uploads' into 'master'

Add a new JWT signed field for uploads

See merge request gitlab-org/gitlab-workhorse!490
parents 6378ac2a 40a18941
...@@ -7,7 +7,7 @@ import ( ...@@ -7,7 +7,7 @@ import (
"regexp" "regexp"
"testing" "testing"
jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api" "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
......
---
title: Add a signed field on upload requests containing all the workhorse parameters
merge_request: 490
author:
type: added
...@@ -97,7 +97,12 @@ func (a *artifactsUploadProcessor) ProcessFile(ctx context.Context, formName str ...@@ -97,7 +97,12 @@ func (a *artifactsUploadProcessor) ProcessFile(ctx context.Context, formName str
} }
if metadata != nil { if metadata != nil {
for k, v := range metadata.GitLabFinalizeFields("metadata") { fields, err := metadata.GitLabFinalizeFields("metadata")
if err != nil {
return fmt.Errorf("finalize metadata field error: %v", err)
}
for k, v := range fields {
writer.WriteField(k, v) writer.WriteField(k, v)
} }
......
...@@ -13,7 +13,7 @@ import ( ...@@ -13,7 +13,7 @@ import (
"os" "os"
"testing" "testing"
jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api" "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore" "gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore"
...@@ -157,14 +157,16 @@ func testUploadArtifacts(t *testing.T, contentType, url string, body io.Reader) ...@@ -157,14 +157,16 @@ func testUploadArtifacts(t *testing.T, contentType, url string, body io.Reader)
func TestUploadHandlerAddingMetadata(t *testing.T) { func TestUploadHandlerAddingMetadata(t *testing.T) {
s := setupWithTmpPath(t, "file", s := setupWithTmpPath(t, "file",
func(w http.ResponseWriter, r *http.Request) { func(w http.ResponseWriter, r *http.Request) {
jwtToken, err := jwt.Parse(r.Header.Get(upload.RewrittenFieldsHeader), testhelper.ParseJWT) token, err := jwt.ParseWithClaims(r.Header.Get(upload.RewrittenFieldsHeader), &upload.MultipartClaims{}, testhelper.ParseJWT)
require.NoError(t, err) require.NoError(t, err)
rewrittenFields := jwtToken.Claims.(jwt.MapClaims)["rewritten_fields"].(map[string]interface{}) rewrittenFields := token.Claims.(*upload.MultipartClaims).RewrittenFields
require.Equal(t, 2, len(rewrittenFields)) require.Equal(t, 2, len(rewrittenFields))
require.Contains(t, rewrittenFields, "file") require.Contains(t, rewrittenFields, "file")
require.Contains(t, rewrittenFields, "metadata") require.Contains(t, rewrittenFields, "metadata")
require.Contains(t, r.PostForm, "file.gitlab-workhorse-upload")
require.Contains(t, r.PostForm, "metadata.gitlab-workhorse-upload")
}, },
) )
defer s.cleanup() defer s.cleanup()
......
...@@ -9,7 +9,10 @@ import ( ...@@ -9,7 +9,10 @@ import (
"os" "os"
"strconv" "strconv"
"github.com/dgrijalva/jwt-go"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/objectstore" "gitlab.com/gitlab-org/gitlab-workhorse/internal/objectstore"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/secret"
) )
type SizeError error type SizeError error
...@@ -39,6 +42,11 @@ type FileHandler struct { ...@@ -39,6 +42,11 @@ type FileHandler struct {
hashes map[string]string hashes map[string]string
} }
type uploadClaims struct {
Upload map[string]string `json:"upload"`
jwt.StandardClaims
}
// SHA256 hash of the handled file // SHA256 hash of the handled file
func (fh *FileHandler) SHA256() string { func (fh *FileHandler) SHA256() string {
return fh.hashes["sha256"] return fh.hashes["sha256"]
...@@ -50,8 +58,10 @@ func (fh *FileHandler) MD5() string { ...@@ -50,8 +58,10 @@ func (fh *FileHandler) MD5() string {
} }
// GitLabFinalizeFields returns a map with all the fields GitLab Rails needs in order to finalize the upload. // GitLabFinalizeFields returns a map with all the fields GitLab Rails needs in order to finalize the upload.
func (fh *FileHandler) GitLabFinalizeFields(prefix string) map[string]string { func (fh *FileHandler) GitLabFinalizeFields(prefix string) (map[string]string, error) {
// TODO: remove `data` these once rails fully and exclusively support `signedData` (https://gitlab.com/gitlab-org/gitlab-workhorse/-/issues/263)
data := make(map[string]string) data := make(map[string]string)
signedData := make(map[string]string)
key := func(field string) string { key := func(field string) string {
if prefix == "" { if prefix == "" {
return field return field
...@@ -60,16 +70,30 @@ func (fh *FileHandler) GitLabFinalizeFields(prefix string) map[string]string { ...@@ -60,16 +70,30 @@ func (fh *FileHandler) GitLabFinalizeFields(prefix string) map[string]string {
return fmt.Sprintf("%s.%s", prefix, field) return fmt.Sprintf("%s.%s", prefix, field)
} }
data[key("name")] = fh.Name for k, v := range map[string]string{
data[key("path")] = fh.LocalPath "name": fh.Name,
data[key("remote_url")] = fh.RemoteURL "path": fh.LocalPath,
data[key("remote_id")] = fh.RemoteID "remote_url": fh.RemoteURL,
data[key("size")] = strconv.FormatInt(fh.Size, 10) "remote_id": fh.RemoteID,
"size": strconv.FormatInt(fh.Size, 10),
} {
data[key(k)] = v
signedData[k] = v
}
for hashName, hash := range fh.hashes { for hashName, hash := range fh.hashes {
data[key(hashName)] = hash data[key(hashName)] = hash
signedData[hashName] = hash
}
claims := uploadClaims{Upload: signedData, StandardClaims: secret.DefaultClaims}
jwtData, err := secret.JWTTokenString(claims)
if err != nil {
return nil, err
} }
data[key("gitlab-workhorse-upload")] = jwtData
return data return data, nil
} }
// SaveFileFromReader persists the provided reader content to all the location specified in opts. A cleanup will be performed once ctx is Done // SaveFileFromReader persists the provided reader content to all the location specified in opts. A cleanup will be performed once ctx is Done
......
...@@ -11,11 +11,13 @@ import ( ...@@ -11,11 +11,13 @@ import (
"testing" "testing"
"time" "time"
"github.com/dgrijalva/jwt-go"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore" "gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/objectstore/test" "gitlab.com/gitlab-org/gitlab-workhorse/internal/objectstore/test"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/testhelper"
) )
func testDeadline() time.Time { func testDeadline() time.Time {
...@@ -154,6 +156,8 @@ func TestSaveFileFromDiskToLocalPath(t *testing.T) { ...@@ -154,6 +156,8 @@ func TestSaveFileFromDiskToLocalPath(t *testing.T) {
} }
func TestSaveFile(t *testing.T) { func TestSaveFile(t *testing.T) {
testhelper.ConfigureSecret()
type remote int type remote int
const ( const (
notRemote remote = iota notRemote remote = iota
...@@ -227,8 +231,8 @@ func TestSaveFile(t *testing.T) { ...@@ -227,8 +231,8 @@ func TestSaveFile(t *testing.T) {
assert.NoError(err) assert.NoError(err)
require.NotNil(t, fh) require.NotNil(t, fh)
assert.Equal(opts.RemoteID, fh.RemoteID) require.Equal(t, opts.RemoteID, fh.RemoteID)
assert.Equal(opts.RemoteURL, fh.RemoteURL) require.Equal(t, opts.RemoteURL, fh.RemoteURL)
if spec.local { if spec.local {
assert.NotEmpty(fh.LocalPath, "File not persisted on disk") assert.NotEmpty(fh.LocalPath, "File not persisted on disk")
...@@ -236,7 +240,7 @@ func TestSaveFile(t *testing.T) { ...@@ -236,7 +240,7 @@ func TestSaveFile(t *testing.T) {
assert.NoError(err) assert.NoError(err)
dir := path.Dir(fh.LocalPath) dir := path.Dir(fh.LocalPath)
assert.Equal(opts.LocalTempPath, dir) require.Equal(t, opts.LocalTempPath, dir)
filename := path.Base(fh.LocalPath) filename := path.Base(fh.LocalPath)
beginsWithPrefix := strings.HasPrefix(filename, opts.TempFilePrefix) beginsWithPrefix := strings.HasPrefix(filename, opts.TempFilePrefix)
assert.True(beginsWithPrefix, fmt.Sprintf("LocalPath filename %q do not begin with TempFilePrefix %q", filename, opts.TempFilePrefix)) assert.True(beginsWithPrefix, fmt.Sprintf("LocalPath filename %q do not begin with TempFilePrefix %q", filename, opts.TempFilePrefix))
...@@ -244,34 +248,29 @@ func TestSaveFile(t *testing.T) { ...@@ -244,34 +248,29 @@ func TestSaveFile(t *testing.T) {
assert.Empty(fh.LocalPath, "LocalPath must be empty for non local uploads") assert.Empty(fh.LocalPath, "LocalPath must be empty for non local uploads")
} }
assert.Equal(test.ObjectSize, fh.Size) require.Equal(t, test.ObjectSize, fh.Size)
assert.Equal(test.ObjectMD5, fh.MD5()) require.Equal(t, test.ObjectMD5, fh.MD5())
assert.Equal(test.ObjectSHA256, fh.SHA256()) require.Equal(t, test.ObjectSHA256, fh.SHA256())
assert.Equal(expectedPuts, osStub.PutsCnt(), "ObjectStore PutObject count mismatch") require.Equal(t, expectedPuts, osStub.PutsCnt(), "ObjectStore PutObject count mismatch")
assert.Equal(0, osStub.DeletesCnt(), "File deleted too early") require.Equal(t, 0, osStub.DeletesCnt(), "File deleted too early")
cancel() // this will trigger an async cleanup cancel() // this will trigger an async cleanup
assertObjectStoreDeletedAsync(t, expectedDeletes, osStub) assertObjectStoreDeletedAsync(t, expectedDeletes, osStub)
assertFileGetsRemovedAsync(t, fh.LocalPath) assertFileGetsRemovedAsync(t, fh.LocalPath)
// checking generated fields // checking generated fields
fields := fh.GitLabFinalizeFields("file") fields, err := fh.GitLabFinalizeFields("file")
require.NoError(t, err)
assert.Equal(fh.Name, fields["file.name"])
assert.Equal(fh.LocalPath, fields["file.path"]) checkFileHandlerWithFields(t, fh, fields, "file", spec.remote == notRemote)
assert.Equal(fh.RemoteURL, fields["file.remote_url"])
assert.Equal(fh.RemoteID, fields["file.remote_id"]) token, jwtErr := jwt.ParseWithClaims(fields["file.gitlab-workhorse-upload"], &testhelper.UploadClaims{}, testhelper.ParseJWT)
assert.Equal(strconv.FormatInt(test.ObjectSize, 10), fields["file.size"]) require.NoError(t, jwtErr)
assert.Equal(test.ObjectMD5, fields["file.md5"])
assert.Equal(test.ObjectSHA1, fields["file.sha1"]) uploadFields := token.Claims.(*testhelper.UploadClaims).Upload
assert.Equal(test.ObjectSHA256, fields["file.sha256"])
assert.Equal(test.ObjectSHA512, fields["file.sha512"]) checkFileHandlerWithFields(t, fh, uploadFields, "", spec.remote == notRemote)
if spec.remote == notRemote {
assert.NotContains(fields, "file.etag")
} else {
assert.Contains(fields, "file.etag")
}
}) })
} }
} }
...@@ -305,3 +304,28 @@ func TestSaveMultipartInBodyFailure(t *testing.T) { ...@@ -305,3 +304,28 @@ func TestSaveMultipartInBodyFailure(t *testing.T) {
require.Error(t, err) require.Error(t, err)
assert.EqualError(err, test.MultipartUploadInternalError().Error()) assert.EqualError(err, test.MultipartUploadInternalError().Error())
} }
func checkFileHandlerWithFields(t *testing.T, fh *filestore.FileHandler, fields map[string]string, prefix string, remote bool) {
key := func(field string) string {
if prefix == "" {
return field
}
return fmt.Sprintf("%s.%s", prefix, field)
}
require.Equal(t, fh.Name, fields[key("name")])
require.Equal(t, fh.LocalPath, fields[key("path")])
require.Equal(t, fh.RemoteURL, fields[key("remote_url")])
require.Equal(t, fh.RemoteID, fields[key("remote_id")])
require.Equal(t, strconv.FormatInt(test.ObjectSize, 10), fields[key("size")])
require.Equal(t, test.ObjectMD5, fields[key("md5")])
require.Equal(t, test.ObjectSHA1, fields[key("sha1")])
require.Equal(t, test.ObjectSHA256, fields[key("sha256")])
require.Equal(t, test.ObjectSHA512, fields[key("sha512")])
if remote {
require.NotContains(t, fields, key("etag"))
} else {
require.Contains(t, fields, key("etag"))
}
}
...@@ -10,6 +10,7 @@ import ( ...@@ -10,6 +10,7 @@ import (
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api" "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore" "gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/upload"
) )
type object struct { type object struct {
...@@ -31,7 +32,7 @@ func (l *object) Verify(fh *filestore.FileHandler) error { ...@@ -31,7 +32,7 @@ func (l *object) Verify(fh *filestore.FileHandler) error {
type uploadPreparer struct{} type uploadPreparer struct{}
func (l *uploadPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, filestore.UploadVerifier, error) { func (l *uploadPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, upload.Verifier, error) {
opts := filestore.GetOpts(a) opts := filestore.GetOpts(a)
opts.TempFilePrefix = a.LfsOid opts.TempFilePrefix = a.LfsOid
...@@ -39,5 +40,5 @@ func (l *uploadPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, file ...@@ -39,5 +40,5 @@ func (l *uploadPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, file
} }
func PutStore(a *api.API, h http.Handler) http.Handler { func PutStore(a *api.API, h http.Handler) http.Handler {
return filestore.BodyUploader(a, h, &uploadPreparer{}) return upload.BodyUploader(a, h, &uploadPreparer{})
} }
...@@ -3,7 +3,7 @@ package secret ...@@ -3,7 +3,7 @@ package secret
import ( import (
"fmt" "fmt"
jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go"
) )
var ( var (
......
...@@ -15,7 +15,7 @@ import ( ...@@ -15,7 +15,7 @@ import (
"strings" "strings"
"testing" "testing"
jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go"
"gitlab.com/gitlab-org/labkit/log" "gitlab.com/gitlab-org/labkit/log"
...@@ -191,3 +191,9 @@ func ParseJWT(token *jwt.Token) (interface{}, error) { ...@@ -191,3 +191,9 @@ func ParseJWT(token *jwt.Token) (interface{}, error) {
return secretBytes, nil return secretBytes, nil
} }
// UploadClaims represents the JWT claim for upload parameters
type UploadClaims struct {
Upload map[string]string `json:"upload"`
jwt.StandardClaims
}
...@@ -3,10 +3,9 @@ package upload ...@@ -3,10 +3,9 @@ package upload
import ( import (
"net/http" "net/http"
jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api" "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore"
) )
const RewrittenFieldsHeader = "Gitlab-Workhorse-Multipart-Fields" const RewrittenFieldsHeader = "Gitlab-Workhorse-Multipart-Fields"
...@@ -16,7 +15,7 @@ type MultipartClaims struct { ...@@ -16,7 +15,7 @@ type MultipartClaims struct {
jwt.StandardClaims jwt.StandardClaims
} }
func Accelerate(rails filestore.PreAuthorizer, h http.Handler) http.Handler { func Accelerate(rails PreAuthorizer, h http.Handler) http.Handler {
return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) { return rails.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
s := &SavedFileTracker{Request: r} s := &SavedFileTracker{Request: r}
HandleFileUploads(w, r, h, a, s) HandleFileUploads(w, r, h, a, s)
......
package filestore package upload
import ( import (
"fmt" "fmt"
...@@ -8,6 +8,7 @@ import ( ...@@ -8,6 +8,7 @@ import (
"strings" "strings"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api" "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
) )
...@@ -15,29 +16,29 @@ type PreAuthorizer interface { ...@@ -15,29 +16,29 @@ type PreAuthorizer interface {
PreAuthorizeHandler(next api.HandleFunc, suffix string) http.Handler PreAuthorizeHandler(next api.HandleFunc, suffix string) http.Handler
} }
// UploadVerifier allows to check an upload before sending it to rails // Verifier allows to check an upload before sending it to rails
type UploadVerifier interface { type Verifier interface {
// Verify can abort the upload returning an error // Verify can abort the upload returning an error
Verify(handler *FileHandler) error Verify(handler *filestore.FileHandler) error
} }
// UploadPreparer allows to customize BodyUploader configuration // Preparer allows to customize BodyUploader configuration
type UploadPreparer interface { type Preparer interface {
// Prepare converts api.Response into a *SaveFileOpts, it can optionally return an UploadVerifier that will be // Prepare converts api.Response into a *SaveFileOpts, it can optionally return an Verifier that will be
// invoked after the real upload, before the finalization with rails // invoked after the real upload, before the finalization with rails
Prepare(a *api.Response) (*SaveFileOpts, UploadVerifier, error) Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error)
} }
type defaultPreparer struct{} type defaultPreparer struct{}
func (s *defaultPreparer) Prepare(a *api.Response) (*SaveFileOpts, UploadVerifier, error) { func (s *defaultPreparer) Prepare(a *api.Response) (*filestore.SaveFileOpts, Verifier, error) {
return GetOpts(a), nil, nil return filestore.GetOpts(a), nil, nil
} }
// BodyUploader is an http.Handler that perform a pre authorization call to rails before hijacking the request body and // BodyUploader is an http.Handler that perform a pre authorization call to rails before hijacking the request body and
// uploading it. // uploading it.
// Providing an UploadPreparer allows to customize the upload process // Providing an Preparer allows to customize the upload process
func BodyUploader(rails PreAuthorizer, h http.Handler, p UploadPreparer) http.Handler { func BodyUploader(rails PreAuthorizer, h http.Handler, p Preparer) http.Handler {
if p == nil { if p == nil {
p = &defaultPreparer{} p = &defaultPreparer{}
} }
...@@ -49,7 +50,7 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p UploadPreparer) http.Ha ...@@ -49,7 +50,7 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p UploadPreparer) http.Ha
return return
} }
fh, err := SaveFileFromReader(r.Context(), r.Body, r.ContentLength, opts) fh, err := filestore.SaveFileFromReader(r.Context(), r.Body, r.ContentLength, opts)
if err != nil { if err != nil {
helper.Fail500(w, r, fmt.Errorf("BodyUploader: upload failed: %v", err)) helper.Fail500(w, r, fmt.Errorf("BodyUploader: upload failed: %v", err))
return return
...@@ -63,7 +64,13 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p UploadPreparer) http.Ha ...@@ -63,7 +64,13 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p UploadPreparer) http.Ha
} }
data := url.Values{} data := url.Values{}
for k, v := range fh.GitLabFinalizeFields("file") { fields, err := fh.GitLabFinalizeFields("file")
if err != nil {
helper.Fail500(w, r, fmt.Errorf("BodyUploader: finalize fields failed: %v", err))
return
}
for k, v := range fields {
data.Set(k, v) data.Set(k, v)
} }
...@@ -73,6 +80,13 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p UploadPreparer) http.Ha ...@@ -73,6 +80,13 @@ func BodyUploader(rails PreAuthorizer, h http.Handler, p UploadPreparer) http.Ha
r.ContentLength = int64(len(body)) r.ContentLength = int64(len(body))
r.Header.Set("Content-Type", "application/x-www-form-urlencoded") r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
sft := SavedFileTracker{Request: r}
sft.Track("file", fh.LocalPath)
if err := sft.Finalize(r.Context()); err != nil {
helper.Fail500(w, r, fmt.Errorf("BodyUploader: finalize failed: %v", err))
return
}
// And proxy the request // And proxy the request
h.ServeHTTP(w, r) h.ServeHTTP(w, r)
}, "/authorize") }, "/authorize")
......
package filestore_test package upload
import ( import (
"fmt" "fmt"
...@@ -11,10 +11,12 @@ import ( ...@@ -11,10 +11,12 @@ import (
"strings" "strings"
"testing" "testing"
"github.com/dgrijalva/jwt-go"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/api" "gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore" "gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/testhelper"
) )
const ( const (
...@@ -23,6 +25,8 @@ const ( ...@@ -23,6 +25,8 @@ const (
) )
func TestBodyUploader(t *testing.T) { func TestBodyUploader(t *testing.T) {
testhelper.ConfigureSecret()
body := strings.NewReader(fileContent) body := strings.NewReader(fileContent)
resp := testUpload(&rails{}, nil, echoProxy(t, fileLen), body) resp := testUpload(&rails{}, nil, echoProxy(t, fileLen), body)
...@@ -78,7 +82,7 @@ func TestBodyUploaderErrors(t *testing.T) { ...@@ -78,7 +82,7 @@ func TestBodyUploaderErrors(t *testing.T) {
} }
} }
func testNoProxyInvocation(t *testing.T, expectedStatus int, auth filestore.PreAuthorizer, preparer filestore.UploadPreparer) { func testNoProxyInvocation(t *testing.T, expectedStatus int, auth PreAuthorizer, preparer Preparer) {
proxy := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { proxy := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Fail(t, "request proxied upstream") require.Fail(t, "request proxied upstream")
}) })
...@@ -87,11 +91,11 @@ func testNoProxyInvocation(t *testing.T, expectedStatus int, auth filestore.PreA ...@@ -87,11 +91,11 @@ func testNoProxyInvocation(t *testing.T, expectedStatus int, auth filestore.PreA
require.Equal(t, expectedStatus, resp.StatusCode) require.Equal(t, expectedStatus, resp.StatusCode)
} }
func testUpload(auth filestore.PreAuthorizer, preparer filestore.UploadPreparer, proxy http.Handler, body io.Reader) *http.Response { func testUpload(auth PreAuthorizer, preparer Preparer, proxy http.Handler, body io.Reader) *http.Response {
req := httptest.NewRequest("POST", "http://example.com/upload", body) req := httptest.NewRequest("POST", "http://example.com/upload", body)
w := httptest.NewRecorder() w := httptest.NewRecorder()
filestore.BodyUploader(auth, proxy, preparer).ServeHTTP(w, req) BodyUploader(auth, proxy, preparer).ServeHTTP(w, req)
return w.Result() return w.Result()
} }
...@@ -112,8 +116,31 @@ func echoProxy(t *testing.T, expectedBodyLength int) http.Handler { ...@@ -112,8 +116,31 @@ func echoProxy(t *testing.T, expectedBodyLength int) http.Handler {
require.Contains(r.PostForm, "file.path") require.Contains(r.PostForm, "file.path")
require.Contains(r.PostForm, "file.size") require.Contains(r.PostForm, "file.size")
require.Contains(r.PostForm, "file.gitlab-workhorse-upload")
require.Equal(strconv.Itoa(expectedBodyLength), r.PostFormValue("file.size")) require.Equal(strconv.Itoa(expectedBodyLength), r.PostFormValue("file.size"))
token, err := jwt.ParseWithClaims(r.Header.Get(RewrittenFieldsHeader), &MultipartClaims{}, testhelper.ParseJWT)
require.NoError(err, "Wrong JWT header")
rewrittenFields := token.Claims.(*MultipartClaims).RewrittenFields
if len(rewrittenFields) != 1 || len(rewrittenFields["file"]) == 0 {
t.Fatalf("Unexpected rewritten_fields value: %v", rewrittenFields)
}
token, jwtErr := jwt.ParseWithClaims(r.PostFormValue("file.gitlab-workhorse-upload"), &testhelper.UploadClaims{}, testhelper.ParseJWT)
require.NoError(jwtErr, "Wrong signed upload fields")
uploadFields := token.Claims.(*testhelper.UploadClaims).Upload
require.Contains(uploadFields, "name")
require.Contains(uploadFields, "path")
require.Contains(uploadFields, "remote_url")
require.Contains(uploadFields, "remote_id")
require.Contains(uploadFields, "size")
require.Contains(uploadFields, "md5")
require.Contains(uploadFields, "sha1")
require.Contains(uploadFields, "sha256")
require.Contains(uploadFields, "sha512")
path := r.PostFormValue("file.path") path := r.PostFormValue("file.path")
uploaded, err := os.Open(path) uploaded, err := os.Open(path)
require.NoError(err, "File not uploaded") require.NoError(err, "File not uploaded")
...@@ -140,11 +167,11 @@ func (r *rails) PreAuthorizeHandler(next api.HandleFunc, _ string) http.Handler ...@@ -140,11 +167,11 @@ func (r *rails) PreAuthorizeHandler(next api.HandleFunc, _ string) http.Handler
} }
type alwaysLocalPreparer struct { type alwaysLocalPreparer struct {
verifier filestore.UploadVerifier verifier Verifier
prepareError error prepareError error
} }
func (a *alwaysLocalPreparer) Prepare(_ *api.Response) (*filestore.SaveFileOpts, filestore.UploadVerifier, error) { func (a *alwaysLocalPreparer) Prepare(_ *api.Response) (*filestore.SaveFileOpts, Verifier, error) {
return filestore.GetOpts(&api.Response{TempPath: os.TempDir()}), a.verifier, a.prepareError return filestore.GetOpts(&api.Response{TempPath: os.TempDir()}), a.verifier, a.prepareError
} }
......
...@@ -150,7 +150,12 @@ func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipa ...@@ -150,7 +150,12 @@ func (rew *rewriter) handleFilePart(ctx context.Context, name string, p *multipa
} }
} }
for key, value := range fh.GitLabFinalizeFields(name) { fields, err := fh.GitLabFinalizeFields(name)
if err != nil {
return fmt.Errorf("failed to finalize fields: %v", err)
}
for key, value := range fields {
rew.writer.WriteField(key, value) rew.writer.WriteField(key, value)
rew.finalizedFields[key] = true rew.finalizedFields[key] = true
} }
......
...@@ -3,7 +3,7 @@ package upload ...@@ -3,7 +3,7 @@ package upload
import ( import (
"context" "context"
jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go"
"net/http" "net/http"
"testing" "testing"
...@@ -29,10 +29,10 @@ func TestSavedFileTracking(t *testing.T) { ...@@ -29,10 +29,10 @@ func TestSavedFileTracking(t *testing.T) {
require.Equal(t, 1, tracker.Count()) require.Equal(t, 1, tracker.Count())
tracker.Finalize(ctx) tracker.Finalize(ctx)
jwtToken, err := jwt.Parse(r.Header.Get(RewrittenFieldsHeader), testhelper.ParseJWT) token, err := jwt.ParseWithClaims(r.Header.Get(RewrittenFieldsHeader), &MultipartClaims{}, testhelper.ParseJWT)
require.NoError(t, err) require.NoError(t, err)
rewrittenFields := jwtToken.Claims.(jwt.MapClaims)["rewritten_fields"].(map[string]interface{}) rewrittenFields := token.Claims.(*MultipartClaims).RewrittenFields
require.Equal(t, 1, len(rewrittenFields)) require.Equal(t, 1, len(rewrittenFields))
require.Contains(t, rewrittenFields, "test") require.Contains(t, rewrittenFields, "test")
......
...@@ -159,8 +159,8 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) { ...@@ -159,8 +159,8 @@ func TestUploadHandlerRewritingMultiPartData(t *testing.T) {
} }
} }
if valueCnt := len(r.MultipartForm.Value); valueCnt != 10 { if valueCnt := len(r.MultipartForm.Value); valueCnt != 11 {
t.Fatal("Expected to receive exactly 10 values but got", valueCnt) t.Fatal("Expected to receive exactly 11 values but got", valueCnt)
} }
w.WriteHeader(202) w.WriteHeader(202)
......
...@@ -14,7 +14,6 @@ import ( ...@@ -14,7 +14,6 @@ import (
"gitlab.com/gitlab-org/gitlab-workhorse/internal/artifacts" "gitlab.com/gitlab-org/gitlab-workhorse/internal/artifacts"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/builds" "gitlab.com/gitlab-org/gitlab-workhorse/internal/builds"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/channel" "gitlab.com/gitlab-org/gitlab-workhorse/internal/channel"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/filestore"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/git" "gitlab.com/gitlab-org/gitlab-workhorse/internal/git"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
"gitlab.com/gitlab-org/gitlab-workhorse/internal/lfs" "gitlab.com/gitlab-org/gitlab-workhorse/internal/lfs"
...@@ -207,10 +206,10 @@ func (u *upstream) configureRoutes() { ...@@ -207,10 +206,10 @@ func (u *upstream) configureRoutes() {
route("", ciAPIPattern+`v1/builds/register.json\z`, ciAPILongPolling), route("", ciAPIPattern+`v1/builds/register.json\z`, ciAPILongPolling),
// Maven Artifact Repository // Maven Artifact Repository
route("PUT", apiPattern+`v4/projects/[0-9]+/packages/maven/`, filestore.BodyUploader(api, signingProxy, nil)), route("PUT", apiPattern+`v4/projects/[0-9]+/packages/maven/`, upload.BodyUploader(api, signingProxy, nil)),
// Conan Artifact Repository // Conan Artifact Repository
route("PUT", apiPattern+`v4/packages/conan/`, filestore.BodyUploader(api, signingProxy, nil)), route("PUT", apiPattern+`v4/packages/conan/`, upload.BodyUploader(api, signingProxy, nil)),
// NuGet Artifact Repository // NuGet Artifact Repository
route("PUT", apiPattern+`v4/projects/[0-9]+/packages/nuget/`, upload.Accelerate(api, signingProxy)), route("PUT", apiPattern+`v4/projects/[0-9]+/packages/nuget/`, upload.Accelerate(api, signingProxy)),
......
...@@ -14,7 +14,7 @@ import ( ...@@ -14,7 +14,7 @@ import (
"strings" "strings"
"testing" "testing"
jwt "github.com/dgrijalva/jwt-go" "github.com/dgrijalva/jwt-go"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
...@@ -83,7 +83,7 @@ func uploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest. ...@@ -83,7 +83,7 @@ func uploadTestServer(t *testing.T, extraTests func(r *http.Request)) *httptest.
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
nValues := 9 // file name, path, remote_url, remote_id, size, md5, sha1, sha256, sha512 for just the upload (no metadata because we are not POSTing a valid zip file) nValues := 10 // file name, path, remote_url, remote_id, size, md5, sha1, sha256, sha512, gitlab-workhorse-upload for just the upload (no metadata because we are not POSTing a valid zip file)
if len(r.MultipartForm.Value) != nValues { if len(r.MultipartForm.Value) != nValues {
t.Errorf("Expected to receive exactly %d values", nValues) t.Errorf("Expected to receive exactly %d values", nValues)
} }
...@@ -135,13 +135,27 @@ func TestAcceleratedUpload(t *testing.T) { ...@@ -135,13 +135,27 @@ func TestAcceleratedUpload(t *testing.T) {
expectSignedRequest(t, r) expectSignedRequest(t, r)
} }
jwtToken, err := jwt.Parse(r.Header.Get(upload.RewrittenFieldsHeader), testhelper.ParseJWT) token, err := jwt.ParseWithClaims(r.Header.Get(upload.RewrittenFieldsHeader), &upload.MultipartClaims{}, testhelper.ParseJWT)
require.NoError(t, err) require.NoError(t, err)
rewrittenFields := jwtToken.Claims.(jwt.MapClaims)["rewritten_fields"].(map[string]interface{}) rewrittenFields := token.Claims.(*upload.MultipartClaims).RewrittenFields
if len(rewrittenFields) != 1 || len(rewrittenFields["file"].(string)) == 0 { if len(rewrittenFields) != 1 || len(rewrittenFields["file"]) == 0 {
t.Fatalf("Unexpected rewritten_fields value: %v", rewrittenFields) t.Fatalf("Unexpected rewritten_fields value: %v", rewrittenFields)
} }
token, jwtErr := jwt.ParseWithClaims(r.PostFormValue("file.gitlab-workhorse-upload"), &testhelper.UploadClaims{}, testhelper.ParseJWT)
require.NoError(t, jwtErr)
uploadFields := token.Claims.(*testhelper.UploadClaims).Upload
require.Contains(t, uploadFields, "name")
require.Contains(t, uploadFields, "path")
require.Contains(t, uploadFields, "remote_url")
require.Contains(t, uploadFields, "remote_id")
require.Contains(t, uploadFields, "size")
require.Contains(t, uploadFields, "md5")
require.Contains(t, uploadFields, "sha1")
require.Contains(t, uploadFields, "sha256")
require.Contains(t, uploadFields, "sha512")
}) })
defer ts.Close() defer ts.Close()
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment