17170: Add config keys to enable shell access for users/admins.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 2aa53a4329a9091ef1c5d10a91c0f134568f314a..7748ea4aabc2cb5038add656334c042ee559e161 100644 (file)
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -870,6 +870,26 @@ Clusters:
         # period.
         LogUpdateSize: 32MiB
 
+      ShellAccess:
+        # An admin user can use "arvados-client shell" to start an
+        # interactive shell (with any user ID) in any running
+        # container.
+        Admin: false
+
+        # Any user can use "arvados-client shell" to start an
+        # interactive shell (with any user ID) in any running
+        # container that they started, provided it isn't also
+        # associated with a different user's container request.
+        #
+        # Interactive sessions make it easy to alter the container's
+        # runtime environment in ways that aren't recorded or
+        # reproducible. Consider the implications for automatic
+        # container reuse before enabling and using this feature. In
+        # particular, note that starting an interactive session does
+        # not disqualify a container from being reused by a different
+        # user/workflow in the future.
+        User: false
+
       SLURM:
         PrioritySpread: 0
         SbatchArgumentsList: []

diff --git a/lib/config/export.go b/lib/config/export.go
index e4917032ffe06ca72d5a016fe7cd747a4cc12e02..2d666e638bea225d5ec1f4e1dc974a6695481494 100644 (file)
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -121,6 +121,9 @@ var whitelist = map[string]bool{
        "Containers.MaxRetryAttempts":                  true,
        "Containers.MinRetryPeriod":                    true,
        "Containers.ReserveExtraRAM":                   true,
+       "Containers.ShellAccess":                       true,
+       "Containers.ShellAccess.Admin":                 true,
+       "Containers.ShellAccess.User":                  true,
        "Containers.SLURM":                             false,
        "Containers.StaleLockTimeout":                  false,
        "Containers.SupportedDockerImageFormats":       true,

diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 34f0a0c92b24d0c8f28a10b4fb1da66c0717edca..fd5723b1396ebe4721988accaeb91a765109be00 100644 (file)
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -876,6 +876,26 @@ Clusters:
         # period.
         LogUpdateSize: 32MiB
 
+      ShellAccess:
+        # An admin user can use "arvados-client shell" to start an
+        # interactive shell (with any user ID) in any running
+        # container.
+        Admin: false
+
+        # Any user can use "arvados-client shell" to start an
+        # interactive shell (with any user ID) in any running
+        # container that they started, provided it isn't also
+        # associated with a different user's container request.
+        #
+        # Interactive sessions make it easy to alter the container's
+        # runtime environment in ways that aren't recorded or
+        # reproducible. Consider the implications for automatic
+        # container reuse before enabling and using this feature. In
+        # particular, note that starting an interactive session does
+        # not disqualify a container from being reused by a different
+        # user/workflow in the future.
+        User: false
+
       SLURM:
         PrioritySpread: 0
         SbatchArgumentsList: []

diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index ca968cf20d04c07c8b5392e09232a73cc78c3b50..b8d85516a2369366b6daed5d8aa064ed722f29e9 100644 (file)
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -38,7 +38,11 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
        if err != nil {
                return
        }
-       if !user.IsAdmin {
+       if !user.IsAdmin || !conn.cluster.Containers.ShellAccess.Admin {
+               if !conn.cluster.Containers.ShellAccess.User {
+                       err = httpserver.ErrorWithStatus(errors.New("shell access is disabled in config"), http.StatusServiceUnavailable)
+                       return
+               }
                ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
                var crs arvados.ContainerRequestList
                crs, err = conn.railsProxy.ContainerRequestList(ctxRoot, arvados.ListOptions{Limit: -1, Filters: []arvados.Filter{{"container_uuid", "=", opts.UUID}}})

diff --git a/lib/controller/localdb/container_gateway_test.go b/lib/controller/localdb/container_gateway_test.go
index 336d5b63463e67ee521126340a03c6d014006ebe..54c1b2149a0fd1d0d8a9ac01c12519c02ee88ead 100644 (file)
--- a/lib/controller/localdb/container_gateway_test.go
+++ b/lib/controller/localdb/container_gateway_test.go
@@ -74,6 +74,50 @@ func (s *ContainerGatewaySuite) SetUpSuite(c *check.C) {
        c.Assert(err, check.IsNil)
 }
 
+func (s *ContainerGatewaySuite) SetUpTest(c *check.C) {
+       s.cluster.Containers.ShellAccess.Admin = true
+       s.cluster.Containers.ShellAccess.User = true
+}
+
+func (s *ContainerGatewaySuite) TestConfig(c *check.C) {
+       for _, trial := range []struct {
+               configAdmin bool
+               configUser  bool
+               sendToken   string
+               errorCode   int
+       }{
+               {true, true, arvadostest.ActiveTokenV2, 0},
+               {true, false, arvadostest.ActiveTokenV2, 503},
+               {false, true, arvadostest.ActiveTokenV2, 0},
+               {false, false, arvadostest.ActiveTokenV2, 503},
+               {true, true, arvadostest.AdminToken, 0},
+               {true, false, arvadostest.AdminToken, 0},
+               {false, true, arvadostest.AdminToken, 403},
+               {false, false, arvadostest.AdminToken, 503},
+       } {
+               c.Logf("trial %#v", trial)
+               s.cluster.Containers.ShellAccess.Admin = trial.configAdmin
+               s.cluster.Containers.ShellAccess.User = trial.configUser
+               ctx := auth.NewContext(s.ctx, &auth.Credentials{Tokens: []string{trial.sendToken}})
+               sshconn, err := s.localdb.ContainerSSH(ctx, arvados.ContainerSSHOptions{UUID: s.ctrUUID})
+               if trial.errorCode == 0 {
+                       if !c.Check(err, check.IsNil) {
+                               continue
+                       }
+                       if !c.Check(sshconn.Conn, check.NotNil) {
+                               continue
+                       }
+                       sshconn.Conn.Close()
+               } else {
+                       c.Check(err, check.NotNil)
+                       err, ok := err.(interface{ HTTPStatus() int })
+                       if c.Check(ok, check.Equals, true) {
+                               c.Check(err.HTTPStatus(), check.Equals, trial.errorCode)
+                       }
+               }
+       }
+}
+
 func (s *ContainerGatewaySuite) TestConnect(c *check.C) {
        c.Logf("connecting to %s", s.gw.Address)
        sshconn, err := s.localdb.ContainerSSH(s.ctx, arvados.ContainerSSHOptions{UUID: s.ctrUUID})

diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 9dc9e17dd815b1818bc17bb20c074d69982e146d..8222035a330c8212b5c705ffe5160656ec51ab73 100644 (file)
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -429,6 +429,10 @@ type ContainersConfig struct {
                LogUpdatePeriod              Duration
                LogUpdateSize                ByteSize
        }
+       ShellAccess struct {
+               Admin bool
+               User  bool
+       }
        SLURM struct {
                PrioritySpread             int64
                SbatchArgumentsList        []string

diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 6a0f7d9f49d0820476449998030b951688aab7a6..953021f0e7a18bc622920a19e8d7e73cc764ae13 100644 (file)
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -802,6 +802,10 @@ def setup_config():
                         "GitInternalDir": os.path.join(SERVICES_SRC_DIR, 'api', 'tmp', 'internal.git'),
                     },
                     "SupportedDockerImageFormats": {"v1": {}},
+                    "ShellAccess": {
+                        "Admin": True,
+                        "User": True,
+                    },
                 },
                 "Volumes": {
                     "zzzzz-nyw5e-%015d"%n: {