From 4329a231a8c211e300953002545671661f3009dd Mon Sep 17 00:00:00 2001 From: Keith James Date: Fri, 25 Oct 2024 18:38:52 +0100 Subject: [PATCH 1/3] Add TOML config file and update docs Add TOML config file. Allow command line arguments to override the config file for backward compatibility. Allow environment variables to supply missing config values for backward compatibility, except for the OIDC client secret. Remove the possibility of using secrets set in the environment. --- README.md | 99 +++++++++----- cmd/sqyrrl.go | 101 +++++++++++--- config/sqyrrl.toml | 11 ++ docker-compose.yml | 15 +- go.mod | 1 + go.sum | 2 + server/handlers_test.go | 41 +++--- server/irods.go | 47 +++---- server/server.go | 213 +++++++++++++++++++++-------- server/server_suite_test.go | 18 ++- server/server_test.go | 54 +++++--- server/testdata/config/sqyrrl.toml | 7 + 12 files changed, 417 insertions(+), 192 deletions(-) create mode 100644 config/sqyrrl.toml create mode 100644 server/testdata/config/sqyrrl.toml diff --git a/README.md b/README.md index 87c0c23..8b740f5 100644 --- a/README.md +++ b/README.md @@ -11,77 +11,114 @@ serve data directly from iRODS. Sqyrrl is a available as and `amd64` binary for Linux, macOS and Windows, or as a Docker image. Copy the file to the desired location and run it. -## Limitations - -Sqyrrl is an early development version and has the following limitations: - -- Does not authenticate users to the HTTP endpoint; anyone can access the data it serves. -- Only serves files that have public access in iRODS. - ## Running Sqyrrl Sqyrrl authenticates to iRODS using the standard method for an iRODS client i.e. -using the iRODS environment file. It respects the `IRODS_ENVIRONMENT_FILE` environment -variable, and if that is not set, it will look for the file in the standard location -`$HOME/.irods/irods_environment.json`. Alternatively, command line option `--irods-env` -may be used to set the environment file location explicitly. +using the iRODS environment file. -Since Sqyrrl will serve any data that it can access, it's important to use an iRODS user -with appropriate authorization. The chosen iRODS user should have access only to public -(unrestricted) data. +Since Sqyrrl may serve any data that it can access, it's important to use an iRODS user +with appropriate authorisation. In addition to the limitations imposed by the iRODS account +used directly by the server, the server itself may be configured use OpenID Connect for +HTTP client authentication. In this case, the user must also be authenticated by the OIDC +provider and Sqyrrl will only serve data that the user has access to. OIDC user identity is +mapped to and iRODS user account by user name. + +If the server is started without OIDC enabled, it will serve only data that is explicitly +set to be readable by the `public` iRODS user. To start the server, use the following command: ```sh +Configure and start the server. + Usage: sqyrrl start [flags] Flags: --cert-file string Path to the SSL certificate file + --config string Path to a TOML configuration file + --enable-oidc Enable OpenID Connect authentication -h, --help help for start --host string Address on which to listen, host part (default "localhost") --index-interval duration Interval at which update the index (default 1m0s) - --irods-env string Path to the iRODS environment file (default "/Users/kdj/.irods/irods_environment.json") + --irods-env string Path to the iRODS environment file --key-file string Path to the SSL private key file --port string Port on which to listen (default "3333") Global Flags: --log-level string Set the log level (trace, debug, info, warn, error) (default "info") - ``` +For additional options, use the `--help` flag. + To stop the server, send `SIGINT` or `SIGTERM` to the process. The server will wait for active connections to close before shutting down. -For additional options, use the `--help` flag. -## Authentication - WARNING: This feature is not yet fully implemented +### Configuration + +The preferred way to configure Sqyrrl is to use a TOML configuration file. This file may be +specified using the `--config` flag. This file may be used to provide all the necessary +configuration options and is the only way to pass secrets (OIDC client secret and iRODS +password) to the server. + +An example configuration file is provided in the repository. The following fields are recognised: + +```toml +Host = "" +Port = "" +IRODSEnvFilePath = "" +IRODSPassword = "" +CertFilePath = "" +KeyFilePath = "" +EnableOIDC = true # Boolean value +OIDCClientID = "" +OIDCClientSecret = "" +OIDCIssuerURL = "" +OIDCRedirectURL = "" +IndexInterval = "" # e.g. "1m0s", "30s" +``` + +If `EnableOIDC` is set to false, the OIDC fields are not required and will be ignored, if present. -Sqyrrl supports OpenID Connect for authentication. To enable OpenID Connect, use the -`--enable-oidc` flag. The following environment variables are then required: +Command line options and environment variables may also be used to configure the server +for all settings except secrets. The configuration file has highest precedence, followed +by command line options, and finally environment variables. + +Sqyrrl respects the `IRODS_ENVIRONMENT_FILE` environment variable, and if that is not set, it will +look for the file in the standard location `$HOME/.irods/irods_environment.json`. Alternatively, +command line option `--irods-env` may be used to set the environment file location explicitly. + +If an iRODS authentication file (default `~/.irods/.irodsA`) is present, Sqyrrl will use it +and the iRODS password field is not required and will be ignored, if present. + +For backwards compatibility, it's possible to set some OIDC configuration options using +environment variables. The following environment variables are recognised: - `OIDC_CLIENT_ID` - the client ID for the OIDC provider -- `OIDC_CLIENT_SECRET` - the client secret for the OIDC provider - `OIDC_ISSUER_URL` - the URL of the OIDC provider +- `OIDC_REDIRECT_URL` - the URL to which the OIDC provider should redirect after authentication + +## Authentication + +Sqyrrl supports OpenID Connect for authentication. To enable OpenID Connect, use the +`EnableOIDC` field in the configuration file (or the `--enable-oidc` command line flag). Sqyrrl will then redirect users to the OIDC provider for authentication. The user will be redirected back to Sqyrrl after authentication. -**Currently this feature does nothing more than enable the Login / Logout buttons on the -home page.** - ## iRODS authentication Sqyrrl uses the standard iRODS environment file to authenticate to iRODS. If the user has been authenticated with `iinit` before starting Sqyrrl, the server will use the existing iRODS auth file created by `iinit`. If the user has not been authenticated, Sqyrrl will require the iRODS -password to be supplied using the environment variable `IRODS_PASSWORD`. Sqyrrl will then create -the iRODS auth file itself, without requiring `iinit` to be used. +password to be supplied using the `IRODSPassword` field of the Sqyrrl configuration file. Sqyrrl +will then create the iRODS auth file itself, without requiring `iinit` to be used. ## Running in a container -When running Sqyrrl in a Docker container, configuration files (iRODS environment file, any -existing auth file, SSL certificates) should be mounted into the container. +When running Sqyrrl in a Docker container, configuration files (Sqyrrl configuration file, iRODS +environment file, any existing auth file, SSL certificates) should be mounted into the container. The docker-compose.yml file in the repository contains an example configuration for running Sqyrrl in a container. @@ -94,8 +131,9 @@ add a metadata attribute `sqyrrl:index` with value `1`. Data objects may be gro on the page, under a title, known as a "category". To specify a category for a data object, add a metadata attribute `sqyrrl:category` with the value being the category name. -The home page will be re-indexed at the interval specified by the `--index-interval` flag. The -home page auto-refreshes every 30 seconds. +The home page will be re-indexed at the interval specified by the `IndexInterval` field in the +configuration file (or the `--index-interval` command line flag). The home page auto-refreshes +every 30 seconds. N.B. As go-irodsclient does not support metadata queries across federated zones, this feature is limited to data objects in the same zone as the iRODS user. @@ -103,4 +141,3 @@ is limited to data objects in the same zone as the iRODS user. ## Dependencies Sqyrrl uses [go-irodsclient](https://github.com/cyverse/go-irodsclient) to connect to iRODS. - diff --git a/cmd/sqyrrl.go b/cmd/sqyrrl.go index e279f65..31f763e 100644 --- a/cmd/sqyrrl.go +++ b/cmd/sqyrrl.go @@ -19,6 +19,7 @@ package cmd import ( "fmt" + "github.com/BurntSushi/toml" "io" "os" "strings" @@ -35,9 +36,10 @@ import ( var mainLogger = zerolog.New(zerolog.ConsoleWriter{Out: os.Stderr}) type cliFlags struct { - certFilePath string // Path to the certificate file - keyFilePath string // Path to the key file - envFilePath string // Path to the iRODS environment file + certFilePath string // Path to the certificate file + keyFilePath string // Path to the key file + envFilePath string // Path to the iRODS environment file + configFilePath string // Path to a TOML configuration file host string // Address to listen on, host part level string // Logging level @@ -48,9 +50,7 @@ type cliFlags struct { enableOIDC bool // Enable OpenID Connect authentication } -var cliFlagsSelected = cliFlags{ - host: "localhost", -} +var cliFlagsSelected = cliFlags{} // configureRootLogger configures the root logger for the application. It sets up common // fields for the application name, version, and process ID, and it sets the default log @@ -118,18 +118,78 @@ func printHelp(cmd *cobra.Command, args []string) { } } -func startServer(cmd *cobra.Command, args []string) error { +func startServer(cmd *cobra.Command, args []string) (err error) { // NRV logger := configureRootLogger(&cliFlagsSelected) - return server.ConfigureAndStart(logger, server.Config{ - Host: cliFlagsSelected.host, - Port: cliFlagsSelected.port, - CertFilePath: cliFlagsSelected.certFilePath, - KeyFilePath: cliFlagsSelected.keyFilePath, - EnvFilePath: cliFlagsSelected.envFilePath, - EnableOIDC: cliFlagsSelected.enableOIDC, - IndexInterval: cliFlagsSelected.indexInterval, - }) + var config server.Config + if cliFlagsSelected.configFilePath != "" { + var tomlData []byte + if tomlData, err = os.ReadFile(cliFlagsSelected.configFilePath); err != nil { + return err + } + + _, err = toml.Decode(string(tomlData), config) + if err != nil { + return err + } + logger.Info().Str("path", cliFlagsSelected.configFilePath). + Str("config", fmt.Sprintf("%v", config)).Msg("Config loaded") + config.ConfigFilePath = cliFlagsSelected.configFilePath + } + + if cliFlagsSelected.host != "" { + config.Host = cliFlagsSelected.host + logger.Info().Str("host", config.Host).Msg( + "Configured host overridden on command line") + } + if cliFlagsSelected.port != "" { + config.Port = cliFlagsSelected.port + logger.Info().Str("port", config.Port).Msg( + "Configured port overridden on command line") + } + if cliFlagsSelected.certFilePath != "" { + config.CertFilePath = cliFlagsSelected.certFilePath + logger.Info().Str("path", config.CertFilePath).Msg( + "Configured certificate file path overridden on command line") + } + if cliFlagsSelected.keyFilePath != "" { + config.KeyFilePath = cliFlagsSelected.keyFilePath + logger.Info().Str("path", config.KeyFilePath).Msg( + "Configured key file path overridden on command line") + } + if cliFlagsSelected.envFilePath != "" { + config.IRODSEnvFilePath = cliFlagsSelected.envFilePath + logger.Info().Str("path", config.IRODSEnvFilePath).Msg( + "Configured iRODS environment file path overridden on command line") + } + if cliFlagsSelected.enableOIDC { + config.EnableOIDC = cliFlagsSelected.enableOIDC + logger.Info().Bool("enabled", config.EnableOIDC).Msg( + "Configured OpenID Connect authentication overridden on command line") + } + if cliFlagsSelected.indexInterval != 0 { + config.IndexInterval = cliFlagsSelected.indexInterval + logger.Info().Dur("interval", config.IndexInterval).Msg( + "Configured index interval overridden on command line") + } + + err = server.Configure(logger, &config) + if err != nil { + return err + } + + var srv *server.SqyrrlServer + srv, err = server.NewSqyrrlServer(logger, &config) + if err != nil { + return err + } + + err = srv.Start() + if err != nil { + return err + } + + return err } func CLI() { @@ -152,10 +212,10 @@ func CLI() { RunE: startServer, } startCmd.Flags().StringVar(&cliFlagsSelected.host, - "host", "localhost", + "host", "", "Address on which to listen, host part") startCmd.Flags().StringVar(&cliFlagsSelected.port, - "port", "3333", + "port", "", "Port on which to listen") startCmd.Flags().StringVar(&cliFlagsSelected.certFilePath, "cert-file", "", @@ -164,8 +224,11 @@ func CLI() { "key-file", "", "Path to the SSL private key file") startCmd.Flags().StringVar(&cliFlagsSelected.envFilePath, - "irods-env", server.LookupIRODSEnvFilePath(), + "irods-env", "", "Path to the iRODS environment file") + startCmd.Flags().StringVar(&cliFlagsSelected.configFilePath, + "config", "", + "Path to a TOML configuration file") startCmd.Flags().DurationVar(&cliFlagsSelected.indexInterval, "index-interval", server.DefaultIndexInterval, "Interval at which update the index") diff --git a/config/sqyrrl.toml b/config/sqyrrl.toml new file mode 100644 index 0000000..7cd1956 --- /dev/null +++ b/config/sqyrrl.toml @@ -0,0 +1,11 @@ +Host = "0.0.0.0" +Port = "3333" +IRODSEnvFilePath = "/app/config/app_irods_environment.json" +CertFilePath = "/app/config/localhost.crt" +KeyFilePath = "/app/config/localhost.key" +EnableOIDC = true +OIDCClientID = "0oafha8j3cQCmfxRP417" +OIDCClientSecret = "pjDn3100j2eJaqg9uOYYZ3uDkHdHHQI9ku4Nkn9vx_6twZELwA_euVQeIg3iyVoJ" +OIDCIssuerURL = "" +OIDCRedirectURL = "" +IndexInterval = "60s" diff --git a/docker-compose.yml b/docker-compose.yml index 59fa9a8..c90e116 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -20,15 +20,9 @@ services: context: . dockerfile: Dockerfile command: ["start", - "--host", "0.0.0.0", - "--port", "3333", - "--cert-file", "/app/config/localhost.crt", - "--key-file", "/app/config/localhost.key", - "--irods-env", "/app/config/app_irods_environment.json", - "--enable-oidc", - "--index-interval", "60s", + "--config", "/app/config/sqyrrl.toml", "--log-level", "trace"] - # Set the following environment variables in a .env file (files named .env + # The following environment variables may be set in a .env file (files named .env # are declared in .gitignore): # # If no iRODS auth file is provided: @@ -38,9 +32,12 @@ services: # And if using OIDC: # # OIDC_CLIENT_ID - # OIDC_CLIENT_SECRET # OIDC_ISSUER_URL # OIDC_CALLBACK_URL + # + # The OIDC client secret may not be set in the environment. Instead, it should be + # provided in the TOML config file mounted into the container at the path specified + # by the --config option.. env_file: .env ports: - "3333:3333" diff --git a/go.mod b/go.mod index 5aa9ed1..0cff857 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( ) require ( + github.com/BurntSushi/toml v1.4.0 // indirect github.com/aymerick/douceur v0.2.0 // indirect github.com/go-jose/go-jose/v4 v4.0.2 // indirect github.com/go-logr/logr v1.4.2 // indirect diff --git a/go.sum b/go.sum index 511e8cb..a6b37b2 100644 --- a/go.sum +++ b/go.sum @@ -1,3 +1,5 @@ +github.com/BurntSushi/toml v1.4.0 h1:kuoIxZQy2WRRk1pttg9asf+WVv6tWQuBNVmK8+nqPr0= +github.com/BurntSushi/toml v1.4.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/alexedwards/scs/v2 v2.8.0 h1:h31yUYoycPuL0zt14c0gd+oqxfRwIj6SOjHdKRZxhEw= github.com/alexedwards/scs/v2 v2.8.0/go.mod h1:ToaROZxyKukJKT/xLcVQAChi5k6+Pn1Gvmdl7h3RRj8= github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk= diff --git a/server/handlers_test.go b/server/handlers_test.go index 01c60ff..bca00b0 100644 --- a/server/handlers_test.go +++ b/server/handlers_test.go @@ -43,7 +43,7 @@ var _ = Describe("iRODS Get Handler", func() { var testConfig server.Config var testServer *server.SqyrrlServer - BeforeEach(func() { + BeforeEach(func(ctx SpecContext) { // Put a test file into iRODS testZone = "testZone" rootColl = fmt.Sprintf("/%s/home/irods", testZone) @@ -59,7 +59,13 @@ var _ = Describe("iRODS Get Handler", func() { } var err error - testServer, err = server.NewSqyrrlServer(suiteLogger, testConfig) + err = server.Configure(suiteLogger, &testConfig) + Expect(err).NotTo(HaveOccurred()) + + testServer, err = server.NewSqyrrlServer(suiteLogger, &testConfig) + Expect(err).NotTo(HaveOccurred()) + + err = testServer.StartBackground() Expect(err).NotTo(HaveOccurred()) err = irodsFS.MakeDir(workColl, true) @@ -71,9 +77,10 @@ var _ = Describe("iRODS Get Handler", func() { err = irodsFS.UploadFile(localPath, remotePath, "", false, nil) Expect(err).NotTo(HaveOccurred()) - }) + }, NodeTimeout(time.Second*5)) AfterEach(func() { + testServer.Stop() // Remove the test file from iRODS err := irodsFS.RemoveDir(workColl, true, true) Expect(err).NotTo(HaveOccurred()) @@ -84,7 +91,7 @@ var _ = Describe("iRODS Get Handler", func() { var handler http.Handler var err error - BeforeEach(func() { + BeforeEach(func(ctx SpecContext) { handler, err = testServer.GetHandler(server.EndpointIRODS) Expect(err).NotTo(HaveOccurred()) @@ -94,14 +101,14 @@ var _ = Describe("iRODS Get Handler", func() { r, err = http.NewRequest("GET", getURL, nil) Expect(err).NotTo(HaveOccurred()) - }) + }, NodeTimeout(time.Second*2)) - It("should return NotFound", func() { + It("should return NotFound", func(ctx SpecContext) { rec := httptest.NewRecorder() handler.ServeHTTP(rec, r) Expect(rec.Code).To(Equal(http.StatusNotFound)) - }) + }, SpecTimeout(time.Second*2)) }) When("a valid data object path is given", func() { @@ -109,7 +116,7 @@ var _ = Describe("iRODS Get Handler", func() { var handler http.Handler var err error - BeforeEach(func() { + BeforeEach(func(ctx SpecContext) { handler, err = testServer.GetHandler(server.EndpointIRODS) Expect(err).NotTo(HaveOccurred()) @@ -119,22 +126,22 @@ var _ = Describe("iRODS Get Handler", func() { r, err = http.NewRequest("GET", getURL, nil) Expect(err).NotTo(HaveOccurred()) - }) + }, NodeTimeout(time.Second*2)) When("the data object does not have public read permissions", func() { - It("should return Forbidden", func() { + It("should return Forbidden", func(ctx SpecContext) { rec := httptest.NewRecorder() handler.ServeHTTP(rec, r) Expect(rec.Code).To(Equal(http.StatusForbidden)) - }) + }, SpecTimeout(time.Second*2)) }) When("the data object has public read permissions", func() { var conn *connection.IRODSConnection var acl []*types.IRODSAccess - BeforeEach(func() { + BeforeEach(func(ctx SpecContext) { handler, err = testServer.GetHandler(server.EndpointIRODS) Expect(err).NotTo(HaveOccurred()) @@ -166,26 +173,26 @@ var _ = Describe("iRODS Get Handler", func() { } } Expect(publicAccess).To(BeTrue()) - }) + }, NodeTimeout(time.Second*5)) AfterEach(func() { irodsFS.ReturnIOConnection(conn) }) - It("should return OK", func() { + It("should return OK", func(ctx SpecContext) { rec := httptest.NewRecorder() handler.ServeHTTP(rec, r) Expect(rec.Code).To(Equal(http.StatusOK)) - }) + }, SpecTimeout(time.Second*2)) - It("should serve the correct body content", func() { + It("should serve the correct body content", func(ctx SpecContext) { rec := httptest.NewRecorder() handler.ServeHTTP(rec, r) Expect(rec.Code).To(Equal(http.StatusOK)) Expect(rec.Body.String()).To(Equal("test\n")) - }) + }, SpecTimeout(time.Second*2)) }) }) }) diff --git a/server/irods.go b/server/irods.go index f3170fc..0145c27 100644 --- a/server/irods.go +++ b/server/irods.go @@ -34,7 +34,6 @@ import ( const ( IRODSEnvFileDefault = "~/.irods/irods_environment.json" IRODSEnvFileEnvVar = "IRODS_ENVIRONMENT_FILE" - IRODSPasswordEnvVar = "IRODS_PASSWORD" IRODSPublicUser = "public" ) @@ -69,7 +68,13 @@ func LookupIRODSEnvFilePath() string { // InitIRODS initialises the iRODS environment by creating a populated auth file if it // does not already exist. This avoids the need to have `iinit` present on the server // host. -func InitIRODS(logger zerolog.Logger, manager *icommands.ICommandsEnvironmentManager) (err error) { +func InitIRODS(logger zerolog.Logger, manager *icommands.ICommandsEnvironmentManager, + password string) (err error) { + if password == "" { + return fmt.Errorf("password was empty: %w", ErrInvalidArgument) + } + manager.Password = password + authFilePath := manager.GetPasswordFilePath() if _, err = os.Stat(authFilePath); err != nil && os.IsNotExist(err) { logger.Info(). @@ -116,43 +121,28 @@ func NewICommandsEnvironmentManager(logger zerolog.Logger, Str("path", iRODSEnvFilePath). Msg("Loaded iRODS environment file") - authFilePath := manager.GetPasswordFilePath() - - // An existing auth file takes precedence over the environment variable - if _, err = os.Stat(authFilePath); err != nil && os.IsNotExist(err) { - password, ok := os.LookupEnv(IRODSPasswordEnvVar) - if !ok { - return nil, fmt.Errorf("iRODS auth file '%s' was not present "+ - "and the '%s' environment variable needed to create it was not set: %w", - authFilePath, IRODSPasswordEnvVar, ErrMissingArgument) - } - if password == "" { - return nil, fmt.Errorf("iRODS auth file '%s' was not present "+ - "and the '%s' environment variable needed to set it was empty: %w", - authFilePath, IRODSPasswordEnvVar, ErrInvalidArgument) - } - - manager.Password = password // The password is propagated to the iRODS account - } - return manager, nil } // NewIRODSAccount returns an iRODS account instance using the iRODS environment for // configuration. The environment file path is obtained from the iRODS environment -// manager. +// manager. If the iRODS password is an empty string, it is assumed that the iRODS +// auth file is already present. func NewIRODSAccount(logger zerolog.Logger, - manager *icommands.ICommandsEnvironmentManager) (account *types.IRODSAccount, err error) { // NRV + manager *icommands.ICommandsEnvironmentManager, + password string) (account *types.IRODSAccount, err error) { // NRV if account, err = manager.ToIRODSAccount(); err != nil { logger.Err(err).Msg("Failed to obtain an iRODS account instance") return nil, err } - if err = InitIRODS(logger, manager); err != nil { - logger.Err(err). - Str("path", manager.GetPasswordFilePath()). - Msg("Failed to initialise iRODS") - return nil, err + if password != "" { + if err = InitIRODS(logger, manager, password); err != nil { + logger.Err(err). + Str("path", manager.GetPasswordFilePath()). + Msg("Failed to initialise iRODS") + return nil, err + } } logger.Info(). @@ -162,6 +152,7 @@ func NewIRODSAccount(logger zerolog.Logger, Str("user", account.ClientUser). Str("env_file", manager.GetEnvironmentFilePath()). Str("auth_file", manager.GetPasswordFilePath()). + Bool("password", password != ""). Str("auth_scheme", string(account.AuthenticationScheme)). Bool("cs_neg_required", account.ClientServerNegotiation). Str("cs_neg_policy", string(account.CSNegotiationPolicy)). diff --git a/server/server.go b/server/server.go index d51d2b5..fad57ff 100644 --- a/server/server.go +++ b/server/server.go @@ -51,7 +51,7 @@ type ContextKey string // SqyrrlServer is an HTTP server which contains an embedded iRODS client. type SqyrrlServer struct { http.Server - sqyrrlConfig Config + sqyrrlConfig *Config oauth2Config *oauth2.Config oidcConfig *oidc.Config oidcProvider *oidc.Provider @@ -66,13 +66,19 @@ type SqyrrlServer struct { } type Config struct { - Host string - Port string - EnvFilePath string // Path to the iRODS environment file - CertFilePath string - KeyFilePath string - EnableOIDC bool - IndexInterval time.Duration + Host string + Port string + IRODSEnvFilePath string // Path to the iRODS environment file + IRODSPassword string // Password for the iRODS account + CertFilePath string + KeyFilePath string + ConfigFilePath string // Path to a TOML configuration file + EnableOIDC bool + OIDCClientID string + OIDCClientSecret string + OIDCIssuerURL string + OIDCRedirectURL string + IndexInterval time.Duration } const AppName = "sqyrrl" @@ -84,7 +90,6 @@ const ( const ( EnvClientID = "OIDC_CLIENT_ID" - EnvClientSecret = "OIDC_CLIENT_SECRET" EnvOIDCIssuerURL = "OIDC_ISSUER_URL" EnvOIDCRedirectURL = "OIDC_CALLBACK_URL" ) @@ -125,7 +130,11 @@ func init() { // // The logger should be the root logger of the application. The server will create // sub-loggers for its components. -func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer, err error) { // NRV +// +// The config argument should be initialised by calling Configure before passing it to +// this function. +func NewSqyrrlServer(logger zerolog.Logger, config *Config) (server *SqyrrlServer, + err error) { // NRV if config.Host == "" { return nil, fmt.Errorf("server sqyrrlConfig %w: host", ErrMissingArgument) } @@ -136,6 +145,7 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer return nil, fmt.Errorf("server sqyrrlConfig %w: certificate file path", ErrMissingArgument) } + if config.IndexInterval < MinIndexInterval { logger.Warn(). Dur("interval", config.IndexInterval). @@ -164,50 +174,43 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer var oidcConfig *oidc.Config var oidcProvider *oidc.Provider var oauth2Config *oauth2.Config - var clientID, clientSecret, oidcIssuerURL, oidcRedirectURL string - var issuerURL, redirectURL *url.URL if config.EnableOIDC { - if clientID, err = getEnv(EnvClientID); err != nil { - return nil, err + if config.OIDCClientID == "" { + return nil, fmt.Errorf("server config %w: OIDC client ID", + ErrMissingArgument) } - if clientSecret, err = getEnv(EnvClientSecret); err != nil { - return nil, err + if config.OIDCClientSecret == "" { + return nil, fmt.Errorf("server config %w: OIDC client secret", + ErrMissingArgument) } - if oidcIssuerURL, err = getEnv(EnvOIDCIssuerURL); err != nil { - return nil, err + if config.OIDCIssuerURL == "" { + return nil, fmt.Errorf("server config %w: OIDC issuer URL", + ErrMissingArgument) } - if oidcRedirectURL, err = getEnv(EnvOIDCRedirectURL); err != nil { - return nil, err + if config.OIDCRedirectURL == "" { + return nil, fmt.Errorf("server config %w: OIDC redirect URL", + ErrMissingArgument) } oidcConfig = &oidc.Config{ - ClientID: clientID, + ClientID: config.OIDCClientID, } - // Parse the provided URLs to ensure they are valid - issuerURL, err = url.Parse(oidcIssuerURL) - if err != nil { - return nil, err - } - redirectURL, err = url.Parse(oidcRedirectURL) - if err != nil { - return nil, err - } - redirectURL, err = url.Parse(redirectURL.Scheme + "://" + - net.JoinHostPort(redirectURL.Hostname(), config.Port)) + oidcProvider, err = oidc.NewProvider(context.Background(), config.OIDCIssuerURL) if err != nil { return nil, err } - oidcProvider, err = oidc.NewProvider(context.Background(), issuerURL.String()) + var redirectURL *url.URL + redirectURL, err = url.Parse(config.OIDCRedirectURL) if err != nil { return nil, err } oauth2Config = &oauth2.Config{ - ClientID: clientID, - ClientSecret: clientSecret, + ClientID: config.OIDCClientID, + ClientSecret: config.OIDCClientSecret, Endpoint: oidcProvider.Endpoint(), RedirectURL: redirectURL.JoinPath(EndpointAuthCallback).String(), Scopes: []string{oidc.ScopeOpenID, "profile", "email"}, @@ -220,17 +223,13 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer } var iRODSEnvManager *icommands.ICommandsEnvironmentManager - if config.EnvFilePath == "" { - config.EnvFilePath = LookupIRODSEnvFilePath() - } - - if iRODSEnvManager, err = NewICommandsEnvironmentManager(subLogger, config.EnvFilePath); err != nil { + if iRODSEnvManager, err = NewICommandsEnvironmentManager(subLogger, config.IRODSEnvFilePath); err != nil { logger.Err(err).Msg("Failed to create an iRODS environment manager") return nil, err } var iRODSAccount *types.IRODSAccount - if iRODSAccount, err = NewIRODSAccount(subLogger, iRODSEnvManager); err != nil { + if iRODSAccount, err = NewIRODSAccount(subLogger, iRODSEnvManager, config.IRODSPassword); err != nil { logger.Err(err).Msg("Failed to get an iRODS account") return nil, err } @@ -284,7 +283,7 @@ func NewSqyrrlServer(logger zerolog.Logger, config Config) (server *SqyrrlServer Str("port", config.Port). Str("cert_file", config.CertFilePath). Str("key_file", config.KeyFilePath). - Str("irods_env", config.EnvFilePath). + Str("irods_env", config.IRODSEnvFilePath). Bool("oidc_enabled", config.EnableOIDC). Str("cwd", cwd). Dur("index_interval", config.IndexInterval).Msg("Server configured") @@ -300,8 +299,10 @@ func (server *SqyrrlServer) IRODSAuthFilePath() string { return server.iRODSEnvManager.GetPasswordFilePath() } -// GetHandler returns the handler for the named endpoint. This is used for ease of testing -// because it will return a handler configured with the server's session manager. +// GetHandler returns the handler for the named endpoint. +// +// This is used for ease of testing because it will return a handler configured with the +// server's session manager. func (server *SqyrrlServer) GetHandler(endpoint string) (http.Handler, error) { // If the named handler is not in the handlers map, return an error if handler, ok := server.handlers[endpoint]; ok { @@ -318,6 +319,14 @@ func (server *SqyrrlServer) GetHandler(endpoint string) (http.Handler, error) { func (server *SqyrrlServer) Start() error { var serveErr, shutErr error + config := server.sqyrrlConfig + for _, path := range []string{config.CertFilePath, config.KeyFilePath, + config.IRODSEnvFilePath} { + if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("server config %w: %s", err, path) + } + } + go func() { logger := server.logger @@ -360,6 +369,20 @@ func (server *SqyrrlServer) Start() error { return errors.Join(serveErr, shutErr) } +// StartBackground starts the server in a goroutine. This function returns immediately. +// The error is returned only if the server fails to start, otherwise it is nil. The +// server can be stopped by calling the server's Stop function. +func (server *SqyrrlServer) StartBackground() (err error) { // NRV + go func() { + err = server.Start() + if err != nil { + server.logger.Err(err).Msg("Error starting server") + } + }() + + return err +} + // Stop stops the server. It provides a public means to call the server's cancel function. func (server *SqyrrlServer) Stop() { server.cancel() @@ -470,34 +493,112 @@ func (server *SqyrrlServer) waitAndShutdown() (err error) { // NRV return err } -func ConfigureAndStart(logger zerolog.Logger, config Config) error { +// Configure sets up the server configuration. This function reads the configuration +// from the provided Config struct and updates it with values from the environment, if +// they are not already set. This is for backwards compatibility with previous versions +// of the server. It also validates and URLs provided in the configuration. If the +// configuration is invalid, it returns an error. +func Configure(logger zerolog.Logger, config *Config) error { + // Fall back to environment variables if the configuration file does not specify them + // and they are not overridden on the command line. This is for backwards compatibility + var err error + + if config.EnableOIDC { + if config.OIDCClientID == "" { + var clientID string + clientID, err = getEnv(EnvClientID) + if err != nil { + return err + } + logger.Info().Str("client_id", + clientID).Msg("Configured OpenID Connect client ID from the environment") + config.OIDCClientID = clientID + } + if config.OIDCIssuerURL == "" { + var issuerURL string + issuerURL, err = getEnv(EnvOIDCIssuerURL) + if err != nil { + return err + } + logger.Info().Str("issuer_url", + issuerURL).Msg("Configured OpenID Connect issuer URL from the environment") + config.OIDCIssuerURL = issuerURL + } + if config.OIDCRedirectURL == "" { + var redirectURL string + redirectURL, err = getEnv(EnvOIDCRedirectURL) + if err != nil { + return err + } + logger.Info().Str("redirect_url", + redirectURL).Msg("Configured OpenID Connect redirect URL from the environment") + config.OIDCRedirectURL = redirectURL + } + + // Parse the provided URLs to ensure they are valid + if config.OIDCIssuerURL != "" { + var issuerURL *url.URL + issuerURL, err = url.Parse(config.OIDCIssuerURL) + if err != nil { + return err + } + config.OIDCIssuerURL = issuerURL.String() + } + if config.OIDCRedirectURL != "" { + var redirectURL *url.URL + redirectURL, err = url.Parse(config.OIDCRedirectURL) + if err != nil { + return err + } + + redirectURL, err = url.Parse(redirectURL.Scheme + + "://" + net.JoinHostPort(redirectURL.Hostname(), config.Port)) + if err != nil { + return err + } + config.OIDCRedirectURL = redirectURL.String() + } + } + + if config.IRODSEnvFilePath == "" { + path := LookupIRODSEnvFilePath() + if path == "" { + logger.Error(). + Msg("Failed to find the iRODS environment file path from the environment") + } else { + logger.Info(). + Str("path", path). + Msg("Configured iRODS environment file path from the environment") + } + config.IRODSEnvFilePath = path + } + if config.Host == "" { - return fmt.Errorf("server sqyrrlConfig %w: address", ErrMissingArgument) + return fmt.Errorf("server config %w: address", ErrMissingArgument) } if config.Port == "" { - return fmt.Errorf("server sqyrrlConfig %w: port", ErrMissingArgument) + return fmt.Errorf("server config %w: port", ErrMissingArgument) } if config.CertFilePath == "" { - return fmt.Errorf("server sqyrrlConfig %w: certificate file path", ErrMissingArgument) + return fmt.Errorf("server config %w: certificate file path", ErrMissingArgument) } if config.KeyFilePath == "" { - return fmt.Errorf("server sqyrrlConfig %w: key file path", ErrMissingArgument) + return fmt.Errorf("server config %w: key file path", ErrMissingArgument) + } + if config.IRODSEnvFilePath == "" { + return fmt.Errorf("server config %w: iRODS environment file path", ErrMissingArgument) } if !(config.IndexInterval > 0) { - return fmt.Errorf("server sqyrrlConfig %w: index interval", ErrMissingArgument) + return fmt.Errorf("server config %w: index interval", ErrMissingArgument) } - if server, err := NewSqyrrlServer(logger, config); err != nil { - return err - } else { - return server.Start() - } + return err } func getEnv(envVar string) (string, error) { val := os.Getenv(envVar) if val == "" { - return "", fmt.Errorf("server sqyrrlConfig %w: %s", + return "", fmt.Errorf("server config %w: %s", ErrEnvironmentVariableNotSet, envVar) } return val, nil diff --git a/server/server_suite_test.go b/server/server_suite_test.go index 926340a..8f60fb6 100644 --- a/server/server_suite_test.go +++ b/server/server_suite_test.go @@ -50,19 +50,17 @@ var ( func TestSuite(t *testing.T) { writer := zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: time.RFC3339} - suiteLogger = zerolog.New(writer).With().Timestamp().Logger().Level(zerolog.InfoLevel) + suiteLogger = zerolog.New(writer).With().Timestamp().Logger().Level(zerolog. + DebugLevel) RegisterFailHandler(Fail) RunSpecs(t, suiteName) } // Set up the iRODS environment and create a new iRODS filesystem -var _ = BeforeSuite(func() { +var _ = BeforeSuite(func(ctx SpecContext) { var err error - err = os.Setenv(server.IRODSPasswordEnvVar, iRODSPassword) - Expect(err).NotTo(HaveOccurred()) - err = os.Setenv(server.IRODSEnvFileEnvVar, iRODSEnvFilePath) Expect(err).NotTo(HaveOccurred()) @@ -74,18 +72,18 @@ var _ = BeforeSuite(func() { manager, err := server.NewICommandsEnvironmentManager(suiteLogger, iRODSEnvFilePath) Expect(err).NotTo(HaveOccurred()) - Expect(manager.GetEnvironmentFilePath()).To(Equal(iRODSEnvFilePath)) - Expect(manager.Password).To(Equal(iRODSPassword)) - err = server.InitIRODS(suiteLogger, manager) + err = server.InitIRODS(suiteLogger, manager, iRODSPassword) Expect(err).NotTo(HaveOccurred()) + Expect(manager.GetEnvironmentFilePath()).To(Equal(iRODSEnvFilePath)) + Expect(manager.Password).To(Equal(iRODSPassword)) - account, err = server.NewIRODSAccount(suiteLogger, manager) + account, err = server.NewIRODSAccount(suiteLogger, manager, iRODSPassword) Expect(err).NotTo(HaveOccurred()) irodsFS, err = fs.NewFileSystemWithDefault(account, suiteName) Expect(err).NotTo(HaveOccurred()) -}) +}, NodeTimeout(time.Second*20)) // Release the iRODS filesystem var _ = AfterSuite(func() { diff --git a/server/server_test.go b/server/server_test.go index 428ed4e..4992913 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -6,7 +6,6 @@ import ( "net/http" "net/url" "os" - "path/filepath" "sync" . "github.com/onsi/ginkgo/v2" @@ -15,6 +14,11 @@ import ( "sqyrrl/server" ) +var ( + certFilePath = "testdata/config/localhost.crt" + keyFilePath = "testdata/config/localhost.key" +) + var _ = Describe("Server startup and shutdown", func() { var host, port = "localhost", "3333" var config server.Config @@ -23,20 +27,19 @@ var _ = Describe("Server startup and shutdown", func() { // Test server configuration uses a self-signed certificate for localhost and // respected the IRODS_ENVIRONMENT_FILE environment variable to determine the // test iRODS server to use. - configDir := filepath.Join("testdata", "config") config = server.Config{ - Host: host, - Port: port, - CertFilePath: filepath.Join(configDir, "localhost.crt"), - KeyFilePath: filepath.Join(configDir, "localhost.key"), - EnvFilePath: filepath.Join(configDir, "test_irods_environment.json"), - IndexInterval: server.DefaultIndexInterval, + Host: host, + Port: port, + CertFilePath: certFilePath, + KeyFilePath: keyFilePath, + IRODSEnvFilePath: iRODSEnvFilePath, + IndexInterval: server.DefaultIndexInterval, } }) When("a server is created", func() { It("can be started and stopped", func() { - srv, err := server.NewSqyrrlServer(suiteLogger, config) + srv, err := server.NewSqyrrlServer(suiteLogger, &config) Expect(err).NotTo(HaveOccurred()) var startStopErr error @@ -73,37 +76,44 @@ var _ = Describe("Server startup and shutdown", func() { When("no iRODS environment file is provided on the command line", func() { When("no IRODS_ENVIRONMENT_FILE environment variable is set", func() { - It("uses the default iRODS environment file path", func() { - config.EnvFilePath = "" - srv, err := server.NewSqyrrlServer(suiteLogger, config) + It("falls back to the default", func() { + config.IRODSEnvFilePath = "" + + serr := os.Unsetenv("IRODS_ENVIRONMENT_FILE") + Expect(serr).NotTo(HaveOccurred()) + + err := server.Configure(suiteLogger, &config) Expect(err).NotTo(HaveOccurred()) - Expect(srv.IRODSEnvFilePath()).To(Equal(server.LookupIRODSEnvFilePath())) + envRoot, err := os.UserHomeDir() + Expect(err).NotTo(HaveOccurred()) + + Expect(config.IRODSEnvFilePath).To(Equal(envRoot + "/.irods/irods_environment.json")) }) }) When("an IRODS_ENVIRONMENT_FILE environment variable is set", func() { It("uses the IRODS_ENVIRONMENT_FILE environment variable", func() { - envFilePath := config.EnvFilePath - config.EnvFilePath = "" + envFilePath := config.IRODSEnvFilePath + config.IRODSEnvFilePath = "" serr := os.Setenv("IRODS_ENVIRONMENT_FILE", envFilePath) Expect(serr).NotTo(HaveOccurred()) - srv, err := server.NewSqyrrlServer(suiteLogger, config) + err := server.Configure(suiteLogger, &config) Expect(err).NotTo(HaveOccurred()) - - Expect(srv.IRODSEnvFilePath()).To(Equal(envFilePath)) + Expect(config.IRODSEnvFilePath).To(Equal(envFilePath)) }) }) }) When("the configured iRODS environment file is not found", func() { It("returns an error", func() { - config.EnvFilePath = "nonexistent.json" - srv, err := server.NewSqyrrlServer(suiteLogger, config) - Expect(err).To(HaveOccurred()) - Expect(srv).To(BeNil()) + config.IRODSEnvFilePath = "nonexistent.json" + err := server.Configure(suiteLogger, &config) + + _, err = server.NewSqyrrlServer(suiteLogger, &config) + Expect(err).To(MatchError("stat nonexistent.json: no such file or directory")) }) }) }) diff --git a/server/testdata/config/sqyrrl.toml b/server/testdata/config/sqyrrl.toml new file mode 100644 index 0000000..5e3d0e1 --- /dev/null +++ b/server/testdata/config/sqyrrl.toml @@ -0,0 +1,7 @@ +Host = "0.0.0.0" +Port = "3333" +CertFilePath = "/app/config/localhost.crt" +KeyFilePath = "/app/config/localhost.key" +IRODSEnvFilePath = "/app/config/app_irods_environment.json" +EnableOIDC = false +IndexInterval = "60s" From 6d834c02b65f74770c2d7cc4a693731560257ea4 Mon Sep 17 00:00:00 2001 From: Keith James <47220353+kjsanger@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:06:10 +0000 Subject: [PATCH 2/3] Fix typo Co-authored-by: dkj --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8b740f5..7707b24 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ with appropriate authorisation. In addition to the limitations imposed by the iR used directly by the server, the server itself may be configured use OpenID Connect for HTTP client authentication. In this case, the user must also be authenticated by the OIDC provider and Sqyrrl will only serve data that the user has access to. OIDC user identity is -mapped to and iRODS user account by user name. +mapped to an iRODS user account by user name. If the server is started without OIDC enabled, it will serve only data that is explicitly set to be readable by the `public` iRODS user. From 8e07c437f4a11b110784e267ca13c186e7c5a7eb Mon Sep 17 00:00:00 2001 From: Keith James <47220353+kjsanger@users.noreply.github.com> Date: Mon, 4 Nov 2024 11:15:13 +0000 Subject: [PATCH 3/3] Fix incorrect Decode argument Co-authored-by: dkj --- cmd/sqyrrl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/sqyrrl.go b/cmd/sqyrrl.go index 31f763e..53a6c15 100644 --- a/cmd/sqyrrl.go +++ b/cmd/sqyrrl.go @@ -128,7 +128,7 @@ func startServer(cmd *cobra.Command, args []string) (err error) { // NRV return err } - _, err = toml.Decode(string(tomlData), config) + _, err = toml.Decode(string(tomlData), &config) if err != nil { return err }