Skip to content

Commit

Permalink
Merge pull request #643 from neicnordic/refactor/s3inbox
Browse files Browse the repository at this point in the history 
[s3inbox] do authentication before we start processing the request
  • Loading branch information
@jbygdell
jbygdell authored Feb 21, 2024
2 parents 5e2d2d0 + 0bf4ccf commit 87b617f
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 126 deletions.
38 changes: 28 additions & 10 deletions sda/cmd/s3inbox/proxy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,14 @@ func NewProxy(s3conf storage.S3Conf, auth userauth.Authenticator, messenger *bro
}

func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
token, err := p.auth.Authenticate(r)
if err != nil {
log.Debugf("Request not authenticated (%v)", err)
p.notAuthorized(w, r)

return
}

switch t := p.detectRequestType(r); t {
case MakeBucket, RemoveBucket, Delete, Policy, Get:
// Not allowed
Expand All @@ -91,7 +99,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
case Put, List, Other, AbortMultipart:
// Allowed
log.Debug("allowed known")
p.allowedResponse(w, r)
p.allowedResponse(w, r, token)
default:
log.Debugf("Unexpected request (%v) not allowed", r)
p.notAllowedResponse(w, r)
Expand All @@ -113,22 +121,32 @@ func (p *Proxy) notAuthorized(w http.ResponseWriter, _ *http.Request) {
reportError(http.StatusUnauthorized, "not authorized", w)
}

func (p *Proxy) allowedResponse(w http.ResponseWriter, r *http.Request) {
claims, err := p.auth.Authenticate(r)
if err != nil {
log.Debugf("Request not authenticated (%v)", err)
p.notAuthorized(w, r)
func (p *Proxy) allowedResponse(w http.ResponseWriter, r *http.Request, token jwt.Token) {
log.Debug("prepend")
// Check whether token username and filepath match
str, err := url.ParseRequestURI(r.URL.Path)
if err != nil || str.Path == "" {
reportError(http.StatusBadRequest, err.Error(), w)
}

path := strings.Split(str.Path, "/")
if strings.Contains(token.Subject(), "@") {
if strings.ReplaceAll(token.Subject(), "@", "_") != path[1] {
reportError(http.StatusBadRequest, fmt.Sprintf("token supplied username: %s, but URL had: %s", token.Subject(), path[1]), w)

return
}
} else if token.Subject() != path[1] {
reportError(http.StatusBadRequest, fmt.Sprintf("token supplied username: %s, but URL had: %s", token.Subject(), path[1]), w)

return
}

log.Debug("prepend")
err = p.prependBucketToHostPath(r)
if err != nil {
reportError(http.StatusBadRequest, err.Error(), w)
}

username := claims.Subject()
username := token.Subject()
rawFilepath := strings.Replace(r.URL.Path, "/"+p.s3.Bucket+"/", "", 1)

filepath, err := formatUploadFilePath(rawFilepath)
Expand Down Expand Up @@ -161,7 +179,7 @@ func (p *Proxy) allowedResponse(w http.ResponseWriter, r *http.Request) {
// Send message to upstream and set file as uploaded in the database
if p.uploadFinishedSuccessfully(r, s3response) {
log.Debug("create message")
message, err := p.CreateMessageFromRequest(r, claims)
message, err := p.CreateMessageFromRequest(r, token)
if err != nil {
p.internalServerError(w, r, err.Error())

Expand Down
102 changes: 65 additions & 37 deletions sda/cmd/s3inbox/proxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,17 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"os"
"testing"

"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/lestrrat-go/jwx/v2/jws"
"github.com/lestrrat-go/jwx/v2/jwt"
"github.com/neicnordic/sensitive-data-archive/internal/broker"
"github.com/neicnordic/sensitive-data-archive/internal/database"
"github.com/neicnordic/sensitive-data-archive/internal/helper"
"github.com/neicnordic/sensitive-data-archive/internal/storage"
"github.com/neicnordic/sensitive-data-archive/internal/userauth"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
)
Expand All @@ -28,6 +32,8 @@ type ProxyTests struct {
MQConf broker.MQConf
messenger *broker.AMQPBroker
database *database.SDAdb
auth *userauth.ValidateFromToken
token jwt.Token
}

func TestProxyTestSuite(t *testing.T) {
Expand Down Expand Up @@ -75,6 +81,32 @@ func (suite *ProxyTests) SetupTest() {
}

suite.database = &database.SDAdb{}

// Create temp demo rsa key pair
demoKeysPath := "demo-rsa-keys"
defer os.RemoveAll(demoKeysPath)
prKeyPath, pubKeyPath, err := helper.MakeFolder(demoKeysPath)
assert.NoError(suite.T(), err)

err = helper.CreateRSAkeys(prKeyPath, pubKeyPath)
assert.NoError(suite.T(), err)

// Parse demo private key
prKeyParsed, err := helper.ParsePrivateRSAKey(prKeyPath, "/rsa")
assert.NoError(suite.T(), err)

// Create token and set up request defaults
defaultToken, err := helper.CreateRSAToken(prKeyParsed, "RS256", helper.DefaultTokenClaims)
assert.NoError(suite.T(), err)

jwtpubkeypath := demoKeysPath + "/public-key/"
suite.auth = userauth.NewValidateFromToken(jwk.NewSet())
_ = suite.auth.ReadJwtPubKeyPath(jwtpubkeypath)

suite.token, err = jwt.Parse([]byte(defaultToken), jwt.WithKeySet(suite.auth.Keyset, jws.WithInferAlgorithmFromKey(true)), jwt.WithValidate(true))
if err != nil {
suite.T().FailNow()
}
}

func (suite *ProxyTests) TearDownTest() {
Expand Down Expand Up @@ -141,8 +173,7 @@ func (m *MockMessenger) SendMessage(uuid string, body []byte) error {

// nolint:bodyclose
func (suite *ProxyTests) TestServeHTTP_disallowed() {
// Start mock messenger that denies everything
proxy := NewProxy(suite.S3conf, &helper.AlwaysDeny{}, suite.messenger, suite.database, new(tls.Config))
proxy := NewProxy(suite.S3conf, &helper.AlwaysAllow{}, suite.messenger, suite.database, new(tls.Config))

r, _ := http.NewRequest("", "", nil)
w := httptest.NewRecorder()
Expand Down Expand Up @@ -194,6 +225,7 @@ func (suite *ProxyTests) TestServeHTTP_disallowed() {
assert.Equal(suite.T(), false, suite.fakeServer.PingedAndRestore())

// Not authorized user get 401 response
proxy = NewProxy(suite.S3conf, &helper.AlwaysDeny{}, suite.messenger, suite.database, new(tls.Config))
w = httptest.NewRecorder()
r.Method = "GET"
r.URL, _ = url.Parse("/username/file")
Expand All @@ -217,8 +249,8 @@ func (suite *ProxyTests) TestServeHTTPS3Unresponsive() {

// Just try to list the files
r.Method = "GET"
r.URL, _ = url.Parse("/asdf/asdf")
proxy.ServeHTTP(w, r)
r.URL, _ = url.Parse("/dummy/asdf")
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 500, w.Result().StatusCode) // nolint:bodyclose
}

Expand All @@ -232,10 +264,10 @@ func (suite *ProxyTests) TestServeHTTP_MQConnectionClosed() {
// Test that the mq connection will be restored when needed
proxy.messenger.Connection.Close()
assert.True(suite.T(), proxy.messenger.Connection.IsClosed())
r, _ := http.NewRequest("PUT", "/username/connectionclosed-file", nil)
r, _ := http.NewRequest("PUT", "/dummy/connectionclosed-file", nil)
w := httptest.NewRecorder()
suite.fakeServer.resp = "<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\"><Name>test</Name><Prefix>/elixirid/db-test-file.txt</Prefix><KeyCount>1</KeyCount><MaxKeys>2</MaxKeys><Delimiter></Delimiter><IsTruncated>false</IsTruncated><Contents><Key>/elixirid/file.txt</Key><LastModified>2020-03-10T13:20:15.000Z</LastModified><ETag>&#34;0a44282bd39178db9680f24813c41aec-1&#34;</ETag><Size>5</Size><Owner><ID></ID><DisplayName></DisplayName></Owner><StorageClass>STANDARD</StorageClass></Contents></ListBucketResult>"
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode) // nolint:bodyclose
assert.False(suite.T(), proxy.messenger.Connection.IsClosed())
}
Expand All @@ -250,10 +282,10 @@ func (suite *ProxyTests) TestServeHTTP_MQChannelClosed() {
// Test that the mq channel will be restored when needed
proxy.messenger.Channel.Close()
assert.True(suite.T(), proxy.messenger.Channel.IsClosed())
r, _ := http.NewRequest("PUT", "/username/channelclosed-file", nil)
r, _ := http.NewRequest("PUT", "/dummy/channelclosed-file", nil)
w := httptest.NewRecorder()
suite.fakeServer.resp = "<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\"><Name>test</Name><Prefix>/elixirid/db-test-file.txt</Prefix><KeyCount>1</KeyCount><MaxKeys>2</MaxKeys><Delimiter></Delimiter><IsTruncated>false</IsTruncated><Contents><Key>/elixirid/file.txt</Key><LastModified>2020-03-10T13:20:15.000Z</LastModified><ETag>&#34;0a44282bd39178db9680f24813c41aec-1&#34;</ETag><Size>5</Size><Owner><ID></ID><DisplayName></DisplayName></Owner><StorageClass>STANDARD</StorageClass></Contents></ListBucketResult>"
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode) // nolint:bodyclose
assert.False(suite.T(), proxy.messenger.Channel.IsClosed())
}
Expand All @@ -269,27 +301,25 @@ func (suite *ProxyTests) TestServeHTTP_MQ_Unavailable() {
proxy.messenger.Conf.Port = 123456
proxy.messenger.Connection.Close()
assert.True(suite.T(), proxy.messenger.Connection.IsClosed())
r, _ := http.NewRequest("PUT", "/username/mqunavailable-file", nil)
r, _ := http.NewRequest("PUT", "/dummy/mqunavailable-file", nil)
w := httptest.NewRecorder()
suite.fakeServer.resp = "<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\"><Name>test</Name><Prefix>/elixirid/db-test-file.txt</Prefix><KeyCount>1</KeyCount><MaxKeys>2</MaxKeys><Delimiter></Delimiter><IsTruncated>false</IsTruncated><Contents><Key>/elixirid/file.txt</Key><LastModified>2020-03-10T13:20:15.000Z</LastModified><ETag>&#34;0a44282bd39178db9680f24813c41aec-1&#34;</ETag><Size>5</Size><Owner><ID></ID><DisplayName></DisplayName></Owner><StorageClass>STANDARD</StorageClass></Contents></ListBucketResult>"
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 500, w.Result().StatusCode) // nolint:bodyclose
}

// nolint:bodyclose
func (suite *ProxyTests) TestServeHTTP_allowed() {

// Start proxy that allows everything
messenger, err := broker.NewMQ(suite.MQConf)
assert.NoError(suite.T(), err)
database, _ := database.NewSDAdb(suite.DBConf)
proxy := NewProxy(suite.S3conf, helper.NewAlwaysAllow(), messenger, database, new(tls.Config))

// List files works
r, err := http.NewRequest("GET", "/username/file", nil)
r, err := http.NewRequest("GET", "/dummy/file", nil)
assert.NoError(suite.T(), err)
w := httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())
assert.Equal(suite.T(), false, suite.fakeServer.PingedAndRestore()) // Testing the pinged interface
Expand All @@ -298,86 +328,84 @@ func (suite *ProxyTests) TestServeHTTP_allowed() {
w = httptest.NewRecorder()
r.Method = "PUT"
suite.fakeServer.resp = "<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\"><Name>test</Name><Prefix>/elixirid/file.txt</Prefix><KeyCount>1</KeyCount><MaxKeys>2</MaxKeys><Delimiter></Delimiter><IsTruncated>false</IsTruncated><Contents><Key>/elixirid/file.txt</Key><LastModified>2020-03-10T13:20:15.000Z</LastModified><ETag>&#34;0a44282bd39178db9680f24813c41aec-1&#34;</ETag><Size>5</Size><Owner><ID></ID><DisplayName></DisplayName></Owner><StorageClass>STANDARD</StorageClass></Contents></ListBucketResult>"
assert.False(suite.T(), messenger.IsConnClosed())
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Put with partnumber sends no message
w = httptest.NewRecorder()
r.Method = "PUT"
r.URL, _ = url.Parse("/username/file?partNumber=5")
proxy.ServeHTTP(w, r)
r.URL, _ = url.Parse("/dummy/file?partNumber=5")
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Post with uploadId sends message
r.Method = "POST"
r.URL, _ = url.Parse("/username/file?uploadId=5")
r.URL, _ = url.Parse("/dummy/file?uploadId=5")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Post without uploadId sends no message
r.Method = "POST"
r.URL, _ = url.Parse("/username/file")
r.URL, _ = url.Parse("/dummy/file")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Abort multipart works
r.Method = "DELETE"
r.URL, _ = url.Parse("/asdf/asdf?uploadId=123")
r.URL, _ = url.Parse("/dummy/asdf?uploadId=123")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Going through the different extra stuff that can be in the get request
// that trigger different code paths in the code.
// Delimiter alone
r.Method = "GET"
r.URL, _ = url.Parse("/username/file?delimiter=puppe")
r.URL, _ = url.Parse("/dummy/file?delimiter=puppe")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Show multiparts uploads
r.Method = "GET"
r.URL, _ = url.Parse("/username/file?uploads")
r.URL, _ = url.Parse("/dummy/file?uploads")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Delimiter alone together with prefix
r.Method = "GET"
r.URL, _ = url.Parse("/username/file?delimiter=puppe&prefix=asdf")
r.URL, _ = url.Parse("/dummy/file?delimiter=puppe&prefix=asdf")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Location parameter
r.Method = "GET"
r.URL, _ = url.Parse("/username/file?location=fnuffe")
r.URL, _ = url.Parse("/dummy/file?location=fnuffe")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 200, w.Result().StatusCode)
assert.Equal(suite.T(), true, suite.fakeServer.PingedAndRestore())

// Filenames with platform incompatible characters are disallowed
// not checked in TestServeHTTP_allowed() because we look for a non 403 response
r.Method = "PUT"
r.URL, _ = url.Parse("/username/fi|le")
r.URL, _ = url.Parse("/dummy/fi|le")
w = httptest.NewRecorder()
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
assert.Equal(suite.T(), 406, w.Result().StatusCode)
assert.Equal(suite.T(), false, suite.fakeServer.PingedAndRestore())

}

func (suite *ProxyTests) TestMessageFormatting() {
Expand Down Expand Up @@ -426,11 +454,11 @@ func (suite *ProxyTests) TestDatabaseConnection() {
proxy := NewProxy(suite.S3conf, helper.NewAlwaysAllow(), messenger, database, new(tls.Config))

// PUT a file into the system
filename := "/username/db-test-file"
filename := "/dummy/db-test-file"
r, _ := http.NewRequest("PUT", filename, nil)
w := httptest.NewRecorder()
suite.fakeServer.resp = "<ListBucketResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\"><Name>test</Name><Prefix>/elixirid/db-test-file.txt</Prefix><KeyCount>1</KeyCount><MaxKeys>2</MaxKeys><Delimiter></Delimiter><IsTruncated>false</IsTruncated><Contents><Key>/elixirid/file.txt</Key><LastModified>2020-03-10T13:20:15.000Z</LastModified><ETag>&#34;0a44282bd39178db9680f24813c41aec-1&#34;</ETag><Size>5</Size><Owner><ID></ID><DisplayName></DisplayName></Owner><StorageClass>STANDARD</StorageClass></Contents></ListBucketResult>"
proxy.ServeHTTP(w, r)
proxy.allowedResponse(w, r, suite.token)
res := w.Result()
defer res.Body.Close()
assert.Equal(suite.T(), 200, res.StatusCode)
Expand Down
21 changes: 0 additions & 21 deletions sda/internal/userauth/userauth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,6 @@ func (u *ValidateFromToken) Authenticate(r *http.Request) (jwt.Token, error) {
return nil, fmt.Errorf("failed to get issuer from token (%v)", iss)
}

// Check whether token username and filepath match
str, err := url.ParseRequestURI(r.URL.Path)
if err != nil || str.Path == "" {
return nil, fmt.Errorf("failed to get path from query (%v)", r.URL.Path)
}

path := strings.Split(str.Path, "/")
if len(path) < 2 {
return nil, fmt.Errorf("length of path split was shorter than expected: %s", str.Path)
}
username := path[1]

// Case for Elixir and CEGA usernames: Replace @ with _ character
if strings.Contains(token.Subject(), "@") {
if strings.ReplaceAll(token.Subject(), "@", "_") != username {
return nil, fmt.Errorf("token supplied username %s but URL had %s", token.Subject(), username)
}
} else if token.Subject() != username {
return nil, fmt.Errorf("token supplied username %s but URL had %s", token.Subject(), username)
}

return token, nil

case r.Header.Get("Authorization") != "":
Expand Down
Loading

0 comments on commit 87b617f

Please sign in to comment.