From 12ea290fb4b8a47ea1a51525bb09040b54c6dbb2 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Thu, 15 Sep 2022 22:17:15 -0400 Subject: [PATCH] 19518: Check account access permission during pam auth. Also fix test. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../localdb/login_ldap_docker_test.sh | 29 ++++++++++++++++++- lib/controller/localdb/login_pam.go | 10 +++++++ lib/controller/localdb/login_pam_test.go | 26 +++++++++++++++-- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/lib/controller/localdb/login_ldap_docker_test.sh b/lib/controller/localdb/login_ldap_docker_test.sh index 43f2ec0d77..6fc6dd9444 100755 --- a/lib/controller/localdb/login_ldap_docker_test.sh +++ b/lib/controller/localdb/login_ldap_docker_test.sh @@ -160,7 +160,7 @@ objectClass: inetOrgPerson objectClass: posixAccount objectClass: top objectClass: shadowAccount -shadowMax: 180 +shadowMax: -1 shadowMin: 1 shadowWarning: 7 shadowLastChange: 10701 @@ -169,6 +169,26 @@ uidNumber: 11111 gidNumber: 11111 homeDirectory: /home/foo-bar userPassword: ${passwordhash} + +dn: uid=expired,dc=example,dc=org +uid: expired +cn: "Exp Ired" +givenName: Exp +sn: Ired +mail: expired@example.com +objectClass: inetOrgPerson +objectClass: posixAccount +objectClass: top +objectClass: shadowAccount +shadowMax: 180 +shadowMin: 1 +shadowWarning: 7 +shadowLastChange: 10701 +loginShell: /bin/bash +uidNumber: 11112 +gidNumber: 11111 +homeDirectory: /home/expired +userPassword: ${passwordhash} EOF echo >&2 "Adding example user entry user=foo-bar pass=secret (retrying until server comes up)" @@ -227,6 +247,13 @@ else check_contains "${resp}" '{"errors":["PAM: Authentication failure (with username \"foo-bar\" and password)"]}' fi +if [[ "${config_method}" = pam ]]; then + echo >&2 "Testing expired credentials" + resp="$(set -x; curl -s --include -d username=expired -d password=secret "http://0.0.0.0:${ctrlport}/arvados/v1/users/authenticate" | tee $debug)" + check_contains "${resp}" "HTTP/1.1 401" + check_contains "${resp}" '{"errors":["PAM: Authentication failure; \"You are required to change your LDAP password immediately.\""]}' +fi + echo >&2 "Testing authentication success" resp="$(set -x; curl -s --include -d username=foo-bar -d password=secret "http://0.0.0.0:${ctrlport}/arvados/v1/users/authenticate" | tee $debug)" check_contains "${resp}" "HTTP/1.1 200" diff --git a/lib/controller/localdb/login_pam.go b/lib/controller/localdb/login_pam.go index 14e0a582c1..4669122543 100644 --- a/lib/controller/localdb/login_pam.go +++ b/lib/controller/localdb/login_pam.go @@ -57,6 +57,7 @@ func (ctrl *pamLoginController) UserAuthenticate(ctx context.Context, opts arvad if err != nil { return arvados.APIClientAuthorization{}, err } + // Check that the given credentials are valid. err = tx.Authenticate(pam.DisallowNullAuthtok) if err != nil { err = fmt.Errorf("PAM: %s", err) @@ -77,6 +78,15 @@ func (ctrl *pamLoginController) UserAuthenticate(ctx context.Context, opts arvad if errorMessage != "" { return arvados.APIClientAuthorization{}, httpserver.ErrorWithStatus(errors.New(errorMessage), http.StatusUnauthorized) } + // Check that the account/user is permitted to access this host. + err = tx.AcctMgmt(pam.DisallowNullAuthtok) + if err != nil { + err = fmt.Errorf("PAM: %s", err) + if errorMessage != "" { + err = fmt.Errorf("%s; %q", err, errorMessage) + } + return arvados.APIClientAuthorization{}, httpserver.ErrorWithStatus(err, http.StatusUnauthorized) + } user, err := tx.GetItem(pam.User) if err != nil { return arvados.APIClientAuthorization{}, err diff --git a/lib/controller/localdb/login_pam_test.go b/lib/controller/localdb/login_pam_test.go index c5876bbfad..0282b566f1 100644 --- a/lib/controller/localdb/login_pam_test.go +++ b/lib/controller/localdb/login_pam_test.go @@ -13,9 +13,11 @@ import ( "git.arvados.org/arvados.git/lib/config" "git.arvados.org/arvados.git/lib/controller/rpc" + "git.arvados.org/arvados.git/lib/ctrlctx" "git.arvados.org/arvados.git/sdk/go/arvados" "git.arvados.org/arvados.git/sdk/go/arvadostest" "git.arvados.org/arvados.git/sdk/go/ctxlog" + "github.com/jmoiron/sqlx" check "gopkg.in/check.v1" ) @@ -25,6 +27,9 @@ type PamSuite struct { cluster *arvados.Cluster ctrl *pamLoginController railsSpy *arvadostest.Proxy + db *sqlx.DB + ctx context.Context + rollback func() error } func (s *PamSuite) SetUpSuite(c *check.C) { @@ -39,10 +44,24 @@ func (s *PamSuite) SetUpSuite(c *check.C) { Cluster: s.cluster, Parent: &Conn{railsProxy: rpc.NewConn(s.cluster.ClusterID, s.railsSpy.URL, true, rpc.PassthroughTokenProvider)}, } + s.db = arvadostest.DB(c, s.cluster) +} + +func (s *PamSuite) SetUpTest(c *check.C) { + tx, err := s.db.Beginx() + c.Assert(err, check.IsNil) + s.ctx = ctrlctx.NewWithTransaction(context.Background(), tx) + s.rollback = tx.Rollback +} + +func (s *PamSuite) TearDownTest(c *check.C) { + if s.rollback != nil { + s.rollback() + } } func (s *PamSuite) TestLoginFailure(c *check.C) { - resp, err := s.ctrl.UserAuthenticate(context.Background(), arvados.UserAuthenticateOptions{ + resp, err := s.ctrl.UserAuthenticate(s.ctx, arvados.UserAuthenticateOptions{ Username: "bogususername", Password: "boguspassword", }) @@ -57,6 +76,9 @@ func (s *PamSuite) TestLoginFailure(c *check.C) { // This test only runs if the ARVADOS_TEST_PAM_CREDENTIALS_FILE env // var is set. The credentials file should contain a valid username // and password, separated by \n. +// +// Depending on the host config, this test succeeds only if the test +// credentials are for the same account being used to run tests. func (s *PamSuite) TestLoginSuccess(c *check.C) { testCredsFile := os.Getenv("ARVADOS_TEST_PAM_CREDENTIALS_FILE") if testCredsFile == "" { @@ -69,7 +91,7 @@ func (s *PamSuite) TestLoginSuccess(c *check.C) { c.Assert(len(lines), check.Equals, 2, check.Commentf("credentials file %s should contain \"username\\npassword\"", testCredsFile)) u, p := lines[0], lines[1] - resp, err := s.ctrl.UserAuthenticate(context.Background(), arvados.UserAuthenticateOptions{ + resp, err := s.ctrl.UserAuthenticate(s.ctx, arvados.UserAuthenticateOptions{ Username: u, Password: p, }) -- 2.30.2