Merge pull request #174 from kbase/credentials-for-dbs

Created an auth.Credential type and threaded it through DTS.

Author: jeff-cohere

auth/auth.go

@@ -21,25 +21,7 @@
 
 package auth
 
-// A record containing information about a DTS client. A DTS client is a KBase
-// user whose KBase developer token is used to authorize with the DTS.
-type Client struct {
-	// client name (human-readable and display-friendly)
-	Name string
-	// KBase username (if any) used by client to access DTS
-	Username string
-	// client email address
-	Email string
-	// ORCID identifier associated with this client
-	Orcid string
-	// organization with which this client is affiliated
-	Organization string
-}
-
-// A record containing information about a DTS user using a DTS client to
-// request file transfers. A DTS user need not have a KBase developer token
-// (but should have a KBase account if they are requesting files be transferred
-// to KBase).
+// A record containing information about a DTS user using a DTS client to request file transfers.
 type User struct {
 	// name (human-readable and display-friendly)
 	Name string
@@ -52,3 +34,12 @@ type User struct {
 	// true if this user is a Superuser
 	IsSuper bool
 }
+
+// A credential used for authorization and authentication
+type Credential struct {
+	// the ID used for authorization (username or UUID)
+	Id string `yaml:"id"`
+	// the secret used for authentication (e.g. password)
+	// DO NOT STORE THIS IN A CONFIG FILE! Use an environment variable instead
+	Secret string `yaml:"secret"`
+}

auth/authenticator.go

@@ -26,13 +26,10 @@ import (
 	"encoding/csv"
 	"errors"
 	"os"
-	"path/filepath"
 	"strings"
 	"time"
 
 	"github.com/fernet/fernet-go"
-
-	"github.com/kbase/dts/config"
 )
 
 // This type accepts a valid access token in exchange for a user record. It is
@@ -44,19 +41,20 @@ type Authenticator struct {
 	TimeOfLastRead  time.Time
 	RereadInterval  time.Duration
 	AccessTokenFile string
+	Secret          string
 }
 
 const (
 	// how often to reread the access token file, in minutes
 	defaultRereadInterval = time.Minute
-	// name of the access token file
-	defaultAccessTokenFile = "access.dat"
 )
 
-func NewAuthenticator() (*Authenticator, error) {
+// Creates a new authenticator by reading an access token file and decrypting it with a secret.
+func NewAuthenticator(accessTokenFile, secret string) (*Authenticator, error) {
 	var a Authenticator
 	a.RereadInterval = defaultRereadInterval
-	a.AccessTokenFile = defaultAccessTokenFile
+	a.AccessTokenFile = accessTokenFile
+	a.Secret = secret
 	err := a.readAccessTokenFile()
 	if err != nil {
 		return nil, err
@@ -83,13 +81,12 @@ func (a *Authenticator) GetUser(accessToken string) (User, error) {
 }
 
 func (a *Authenticator) readAccessTokenFile() error {
-	tokenFilePath := filepath.Join(config.Service.DataDirectory, a.AccessTokenFile)
-	key, err := fernet.DecodeKey(config.Service.Secret)
+	key, err := fernet.DecodeKey(a.Secret)
 	if err != nil {
 		return err
 	}
 
-	cipherText, err := os.ReadFile(tokenFilePath)
+	cipherText, err := os.ReadFile(a.AccessTokenFile)
 	if err != nil {
 		return err
 	}

auth/authenticator_test.go

@@ -27,16 +27,14 @@ package auth
 import (
 	"fmt"
 	"log"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/fernet/fernet-go"
 	"github.com/stretchr/testify/assert"
-
-	"github.com/kbase/dts/config"
-	"github.com/kbase/dts/dtstest"
 )
 
 // runs setup, runs all tests, and does breakdown
@@ -51,7 +49,6 @@ func TestMain(m *testing.M) {
 func TestRunner(t *testing.T) {
 	tester := SerialTests{Test: t}
 	tester.TestNewAuthenticator()
-	tester.TestInvalidDataDirectory()
 	tester.TestGetUser()
 	tester.TestGetUserAfterReread()
 	tester.TestGetUserAfterBadReread()
@@ -64,6 +61,9 @@ var TestKey fernet.Key
 // temporary testing directory
 var TestDir string
 
+// testing access token file
+var TestAccessTokenFile string
+
 // testing access token
 var TestAccessToken string
 
@@ -77,21 +77,19 @@ var TestUser = User{
 }
 
 func setup() {
-	dtstest.EnableDebugLogging()
+	enableDebugLogging()
 
 	log.Print("Creating testing directory...\n")
 	var err error
 	TestDir, err = os.MkdirTemp(os.TempDir(), "data-transfer-service-tests-")
 	if err != nil {
 		log.Panicf("Couldn't create testing directory: %s", err.Error())
 	}
-	config.Service.DataDirectory = TestDir
 
 	err = TestKey.Generate()
 	if err != nil {
 		log.Panicf("Couldn't generate encryption key: %s", err.Error())
 	}
-	config.Service.Secret = TestKey.Encode()
 
 	TestAccessToken = "7029c1877e9c2dd3dab814cc0f2763af"
 
@@ -106,7 +104,8 @@ func setup() {
 		log.Panicf("Couldn't encrypt test access data: %s", err.Error())
 	}
 
-	output, err := os.Create(filepath.Join(TestDir, "access.dat"))
+	TestAccessTokenFile = filepath.Join(TestDir, "access.dat")
+	output, err := os.Create(TestAccessTokenFile)
 	if err != nil {
 		log.Panicf("Couldn't open test access data file: %s", err.Error())
 	}
@@ -119,6 +118,13 @@ func setup() {
 	setupKBaseAuthServerTests()
 }
 
+func enableDebugLogging() {
+	logLevel := new(slog.LevelVar)
+	logLevel.Set(slog.LevelDebug)
+	h := slog.NewJSONHandler(os.Stderr, &slog.HandlerOptions{Level: logLevel})
+	slog.SetDefault(slog.New(h))
+}
+
 // To run the tests serially, we attach them to a SerialTests type and
 // have them run by a a single test runner.
 type SerialTests struct{ Test *testing.T }
@@ -127,27 +133,16 @@ type SerialTests struct{ Test *testing.T }
 // constructed
 func (t *SerialTests) TestNewAuthenticator() {
 	assert := assert.New(t.Test)
-	auth, err := NewAuthenticator()
+	auth, err := NewAuthenticator(TestAccessTokenFile, TestKey.Encode())
 	assert.NotNil(auth, "Authenticator not created")
 	assert.Nil(err, "Authenticator constructor triggered an error")
 }
 
-// tests the case in which a directory without an encrpyted access.dat file has
-// been configured for the authenticator
-func (t *SerialTests) TestInvalidDataDirectory() {
-	assert := assert.New(t.Test)
-	config.Service.DataDirectory = os.Getenv("HOME")
-	auth, err := NewAuthenticator()
-	assert.Nil(auth, "Authenticator created with invalid data directory")
-	assert.NotNil(err, "Invalid data directory for authenticator triggered no error")
-	config.Service.DataDirectory = TestDir
-}
-
 // tests whether the authenticator server can return information for the
 // the user associated with a valid ORCID
 func (t *SerialTests) TestGetUser() {
 	assert := assert.New(t.Test)
-	auth, err := NewAuthenticator()
+	auth, err := NewAuthenticator(TestAccessTokenFile, TestKey.Encode())
 	assert.NotNil(auth)
 	assert.Nil(err)
 
@@ -166,7 +161,7 @@ func (t *SerialTests) TestGetUser() {
 // user after enough time has passed to trigger a re-read of the access file
 func (t *SerialTests) TestGetUserAfterReread() {
 	assert := assert.New(t.Test)
-	auth, err := NewAuthenticator()
+	auth, err := NewAuthenticator(TestAccessTokenFile, TestKey.Encode())
 	assert.NotNil(auth)
 	assert.Nil(err)
 
@@ -188,7 +183,7 @@ func (t *SerialTests) TestGetUserAfterReread() {
 // tests whether the authenticator server handles a bad re-read correctly
 func (t *SerialTests) TestGetUserAfterBadReread() {
 	assert := assert.New(t.Test)
-	auth, err := NewAuthenticator()
+	auth, err := NewAuthenticator(TestAccessTokenFile, TestKey.Encode())
 	assert.NotNil(auth)
 	assert.Nil(err)
 
@@ -209,7 +204,7 @@ func (t *SerialTests) TestGetUserAfterBadReread() {
 // (fictitious ORCID: https://orcid.org/0000-0001-5109-3700)
 func (t *SerialTests) TestGetInvalidUser() {
 	assert := assert.New(t.Test)
-	auth, _ := NewAuthenticator()
+	auth, _ := NewAuthenticator(TestAccessTokenFile, TestKey.Encode())
 	badAccessToken := "c5683570c1412b77eabcb9d6eb0aae2a"
 	_, err := auth.GetUser(badAccessToken)
 	assert.NotNil(err)

auth/kbase_auth_server.go

@@ -97,24 +97,23 @@ func NewKBaseAuthServer(accessToken string, options ...KBaseAuthServerOption) (*
 }
 
 // returns a normalized user record for the current KBase user
-func (server KBaseAuthServer) Client() (Client, error) {
+func (server KBaseAuthServer) User() (User, error) {
 	kbUser, err := server.kbaseUser()
 	if err != nil {
-		return Client{}, err
+		return User{}, err
 	}
-	client := Client{
-		Name:     kbUser.Display,
-		Username: kbUser.Username,
-		Email:    kbUser.Email,
+	user := User{
+		Name:  kbUser.Display,
+		Email: kbUser.Email,
 	}
 	for _, pid := range kbUser.Idents {
 		// grab the first ORCID associated with the user
 		if pid.Provider == "OrcID" {
-			client.Orcid = pid.UserName
+			user.Orcid = pid.UserName
 			break
 		}
 	}
-	return client, nil
+	return user, nil
 }
 
 //-----------

auth/kbase_auth_server_test.go

@@ -200,12 +200,11 @@ func TestClient(t *testing.T) {
 	if len(devToken) > 0 {
 		server, _ := NewKBaseAuthServer(devToken)
 		assert.NotNil(server)
-		client, err := server.Client()
+		user, err := server.User()
 		assert.Nil(err)
 
-		assert.True(len(client.Username) > 0)
-		assert.True(len(client.Email) > 0)
-		assert.Equal(os.Getenv("DTS_KBASE_TEST_ORCID"), client.Orcid)
+		assert.True(len(user.Email) > 0)
+		assert.Equal(os.Getenv("DTS_KBASE_TEST_ORCID"), user.Orcid)
 	}
 
 	// test with the mock server
@@ -214,11 +213,10 @@ func TestClient(t *testing.T) {
 			cfg.BaseURL = mockKBaseServer.URL
 		})
 	assert.NotNil(server, "Authentication server not created with valid token")
-	client, err := server.Client()
-	assert.Nil(err, "Client() triggered an error with valid token")
+	user, err := server.User()
+	assert.Nil(err, "User() triggered an error with valid token")
 
-	assert.Equal("testuser", client.Username)
-	assert.Equal("Test User", client.Name)
-	assert.Equal("[email protected]", client.Email)
-	assert.Equal("testuser", client.Orcid)
+	assert.Equal("Test User", user.Name)
+	assert.Equal("[email protected]", user.Email)
+	assert.Equal("testuser", user.Orcid)
 }

config/config.go

@@ -29,6 +29,8 @@ import (
 
 	"github.com/google/uuid"
 	"gopkg.in/yaml.v3"
+
+	"github.com/kbase/dts/auth"
 )
 
 // a type with service configuration parameters
@@ -66,7 +68,7 @@ type serviceConfig struct {
 
 // global config variables
 var Service serviceConfig
-var Credentials map[string]credentialConfig
+var Credentials map[string]auth.Credential
 var Endpoints map[string]endpointConfig
 var Databases map[string]databaseConfig
 
@@ -77,10 +79,10 @@ var Databases map[string]databaseConfig
 // data around internally, but this is not yet complete. Once that is done, the
 // global variables above can be removed.
 type Config struct {
-	Service     serviceConfig               `yaml:"service"`
-	Credentials map[string]credentialConfig `yaml:"credentials"`
-	Databases   map[string]databaseConfig   `yaml:"databases"`
-	Endpoints   map[string]endpointConfig   `yaml:"endpoints"`
+	Service     serviceConfig              `yaml:"service"`
+	Credentials map[string]auth.Credential `yaml:"credentials"`
+	Databases   map[string]databaseConfig  `yaml:"databases"`
+	Endpoints   map[string]endpointConfig  `yaml:"endpoints"`
 }
 
 // This helper locates and reads the selected sections in a configuration file,
@@ -167,16 +169,6 @@ func (params serviceConfig) Validate() error {
 	return nil
 }
 
-func (credential credentialConfig) Validate(name string) error {
-	if credential.Id == "" {
-		return &InvalidCredentialConfigError{
-			Credential: name,
-			Message:    "Invalid credential ID",
-		}
-	}
-	return nil
-}
-
 func (endpoint endpointConfig) Validate(name string) error {
 	if endpoint.Id == uuid.Nil { // invalid endpoint UUID
 		return &InvalidEndpointConfigError{
@@ -236,15 +228,6 @@ func (c Config) Validate(service, credentials, databases, endpoints bool) error
 		}
 	}
 
-	if credentials {
-		for name, credential := range c.Credentials {
-			err = credential.Validate(name)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
 	if endpoints {
 		if len(c.Endpoints) == 0 {
 			return &InvalidServiceConfigError{