From db52d1a8ff8e633304028fe1ef19fdb025fb6b25 Mon Sep 17 00:00:00 2001 From: Joakim Bygdell Date: Sun, 6 Nov 2022 12:02:36 +0100 Subject: [PATCH] Remove file based authentication This has not been in use since the first beta versions --- config.go | 10 +--- dev_utils/config.yaml | 1 - dev_utils/docker-compose.yml | 2 - dev_utils/users.csv | 3 - userauth.go | 113 ----------------------------------- userauth_test.go | 68 --------------------- 6 files changed, 2 insertions(+), 195 deletions(-) delete mode 100644 dev_utils/users.csv diff --git a/config.go b/config.go index 7ae40f7..1ef7f34 100644 --- a/config.go +++ b/config.go @@ -53,7 +53,6 @@ type BrokerConfig struct { type ServerConfig struct { cert string key string - users string jwtpubkeypath string jwtpubkeyurl string } @@ -200,13 +199,8 @@ func (c *Config) readConfig() error { // Setup server s := ServerConfig{} - if !(viper.IsSet("server.users") || viper.IsSet("server.jwtpubkeypath") || viper.IsSet("server.jwtpubkeyurl")) { - return errors.New("either server.users or server.pubkey should be present to start the service") - } - - // User file authentication - if viper.IsSet("server.users") { - s.users = viper.GetString("server.users") + if !(viper.IsSet("server.jwtpubkeypath") || viper.IsSet("server.jwtpubkeyurl")) { + return errors.New("either server.pubkeypath or server.jwtpubkeyurl should be present to start the service") } // Token authentication diff --git a/dev_utils/config.yaml b/dev_utils/config.yaml index 6b7d19d..8d6ce1a 100644 --- a/dev_utils/config.yaml +++ b/dev_utils/config.yaml @@ -29,7 +29,6 @@ broker: server: cert: "/certs/proxy.crt" key: "/certs/proxy.key" - users: "./dev_utils/users.csv" jwtpubkeypath: "./dev_utils/keys/" jwtpubkeyurl: "https://login.elixir-czech.org/oidc/jwk" diff --git a/dev_utils/docker-compose.yml b/dev_utils/docker-compose.yml index 31b1a46..ec32320 100644 --- a/dev_utils/docker-compose.yml +++ b/dev_utils/docker-compose.yml @@ -105,13 +105,11 @@ services: - BROKER_VERIFYPEER=true - SERVER_CERT=/certs/proxy.crt - SERVER_KEY=/certs/proxy.key - - SERVER_USERS=/users.csv - SERVER_JWTPUBKEYPATH=/keys/ - SERVER_JWTPUBEYURL=https://login.elixir-czech.org/oidc/jwk - LOG_FORMAT=json volumes: - proxy_certs:/certs - - ./users.csv:/users.csv - ./keys:/keys ports: - "8000:8000" diff --git a/dev_utils/users.csv b/dev_utils/users.csv deleted file mode 100644 index 587c4da..0000000 --- a/dev_utils/users.csv +++ /dev/null @@ -1,3 +0,0 @@ -ElixirID,987654321 -anotherid,testpass -username,testpass diff --git a/userauth.go b/userauth.go index dee598d..5765265 100644 --- a/userauth.go +++ b/userauth.go @@ -1,9 +1,7 @@ package main import ( - "bufio" "crypto/x509" - "encoding/csv" "encoding/json" "encoding/pem" "fmt" @@ -16,7 +14,6 @@ import ( "github.com/golang-jwt/jwt/v4" "github.com/lestrrat/go-jwx/jwk" - "github.com/minio/minio-go/v6/pkg/s3signer" log "github.com/sirupsen/logrus" ) @@ -41,18 +38,6 @@ func (u *AlwaysAllow) Authenticate(r *http.Request) (jwt.MapClaims, error) { return nil, nil } -// ValidateFromFile is an Authenticator that reads client ids and secret ids -// from a file. -type ValidateFromFile struct { - filename string -} - -// NewValidateFromFile returns a new ValidateFromFile, reading users from the -// supplied file. -func NewValidateFromFile(filename string) *ValidateFromFile { - return &ValidateFromFile{filename} -} - // ValidateFromToken is an Authenticator that reads the public key from // supplied file type ValidateFromToken struct { @@ -65,104 +50,6 @@ func NewValidateFromToken(pubkeys map[string][]byte) *ValidateFromToken { return &ValidateFromToken{pubkeys} } -// Authenticate checks whether the http.Request is signed by any of the users -// in the supplied file. -func (u *ValidateFromFile) Authenticate(r *http.Request) error { - re := regexp.MustCompile("Credential=([^/]+)/") - auth := r.Header.Get("Authorization") - curAccessKey := "" - if tmp := re.FindStringSubmatch(auth); tmp != nil { - // Check if user requested own bucket - curAccessKey = tmp[1] - re := regexp.MustCompile("/([^/]+)/") - if curAccessKey != re.FindStringSubmatch(r.URL.Path)[1] { - return fmt.Errorf("user not authorized to access location") - } - } else { - log.Debugf("No credentials in Authorization header (%s)", auth) - - return fmt.Errorf("authorization header had no credentials") - } - //nolint:nestif - if curSecretKey, err := u.secretFromID(curAccessKey); err == nil { - if r.Method == http.MethodGet { - re := regexp.MustCompile("Signature=(.*)") - - signature := re.FindStringSubmatch(auth) - if signature == nil || len(signature) < 2 { - - return fmt.Errorf("signature not found in Authorization header (%s)", auth) - } - - // Create signing request - nr, e := http.NewRequest(r.Method, r.URL.String(), r.Body) - if e != nil { - log.Debug("error creating the new request") - log.Debug(e) - } - - // Add required headers - nr.Header.Set("X-Amz-Date", r.Header.Get("X-Amz-Date")) - nr.Header.Set("X-Amz-Content-Sha256", r.Header.Get("X-Amz-Content-Sha256")) - nr.Host = r.Host - nr.URL.RawQuery = r.URL.RawQuery - - // Sign the new request - s3signer.SignV4(*nr, curAccessKey, curSecretKey, "", "us-east-1") - curSignature := re.FindStringSubmatch(nr.Header.Get("Authorization")) - - if curSignature == nil || len(signature) < 2 { - return fmt.Errorf("generated outgoing signature not found or unexpected (header wass %s)", - nr.Header.Get("Authorization")) - } - - // Compare signatures - if curSignature[1] != signature[1] { - return fmt.Errorf("signature for outgoing (%s)request does not match incoming (%s", - curSignature[1], signature[1]) - } - } - } else { - log.Debugf("Found no secret for user %s", curAccessKey) - - return fmt.Errorf("no secret for user %s found", curAccessKey) - } - - return nil -} - -func (u *ValidateFromFile) secretFromID(id string) (string, error) { - f, e := os.Open(u.filename) - if e != nil { - log.Panicf("Error opening users file (%s): %v", - u.filename, - e) - } - - defer func() { - if err := f.Close(); err != nil { - log.Debugf("Error on close: %v", err) - } - }() - - r := csv.NewReader(bufio.NewReader(f)) - for { - record, e := r.Read() - if e == io.EOF { - break - } - if record[0] == id { - log.Debugf("Returning secret for id %s", id) - - return record[1], nil - } - } - - log.Debugf("No secret found for id %s in %s", id, u.filename) - - return "", fmt.Errorf("cannot find id %s in %s", id, u.filename) -} - // Authenticate verifies that the token included in the http.Request // is valid func (u *ValidateFromToken) Authenticate(r *http.Request) (claims jwt.MapClaims, err error) { diff --git a/userauth_test.go b/userauth_test.go index bca2cb1..6fb5b2f 100644 --- a/userauth_test.go +++ b/userauth_test.go @@ -19,74 +19,6 @@ func TestAlwaysAuthenticator(t *testing.T) { assert.Nil(t, err) } -func TestUserFileAuthenticator_ReadFile(t *testing.T) { - a := NewValidateFromFile("dev_utils/users.csv") - - assert := assert.New(t) - - r, err := a.secretFromID("ElixirID") - if assert.Nil(err) { - assert.Equal(r, "987654321") - } - r, err = a.secretFromID("anotherid") - if assert.Nil(err) { - assert.Equal(r, "testpass") - } - r, err = a.secretFromID("username") - if assert.Nil(err) { - assert.Equal(r, "testpass") - } - - _, err = a.secretFromID("nonexistentuser") - assert.NotNil(err) -} - -func TestUserFileAuthenticator_NoFile(t *testing.T) { - a := NewValidateFromFile("1298379somerandomfilenamethatwedonthaveinthefilesystem1928739") - assert.Panics(t, func() { _, _ = a.secretFromID("random") }) -} - -func TestUserFileAuthenticator_ValidateSignature(t *testing.T) { - // These tests should be possible to reuse with all correct authenticators somehow - a := NewValidateFromFile("dev_utils/users.csv") - - // Set up request defaults - r, _ := http.NewRequest("", "/", nil) - r.Host = "localhost" - r.Header.Set("X-Amz-Content-Sha256", "Just needs to be here") - - // Test that a user can access their own bucket - r.URL.Path = "/username/" - s3signer.SignV4(*r, "username", "testpass", "", "us-east-1") - assert.Nil(t, a.Authenticate(r)) - - // Test that a valid user can't access someone elses bucket - r.URL.Path = "/notvalid/" - s3signer.SignV4(*r, "username", "testpass", "", "us-east-1") - assert.Error(t, a.Authenticate(r)) - - // Test that incorrect secret don't validate - r.URL.Path = "/username/" - s3signer.SignV4(*r, "username", "incorrect", "", "us-east-1") - assert.Error(t, a.Authenticate(r)) - - // Test that nonexistant user can't log in - r.URL.Path = "/snubbe/" - s3signer.SignV4(*r, "snubbe", "incorrect", "", "us-east-1") - assert.Error(t, a.Authenticate(r)) - - // Test that nonexistant user can't log in to other bucket - r.URL.Path = "/username/" - s3signer.SignV4(*r, "snubbe", "incorrect", "", "us-east-1") - assert.Error(t, a.Authenticate(r)) - - r, _ = http.NewRequest("", "/", nil) - r.Host = "localhost" - r.Header.Set("X-Amz-Content-Sha256", "Just needs to be here") - r.URL.Path = "/username/" - assert.Error(t, a.Authenticate(r)) -} - func TestUserTokenAuthenticator_NoFile(t *testing.T) { var pubkeys map[string][]byte a := NewValidateFromToken(pubkeys)