Merge pull request #295 from neicnordic/refactor/auth

Remove file based authentication

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

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"
 

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"

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) {

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)