[ARVADOS] updated: 1.3.0-2355-g40798c765

[Git user]

Summary of changes:
 lib/controller/localdb/login_pam.go             | 15 +++++++++++++++
 lib/controller/localdb/login_pam_docker_test.sh |  7 ++++++-
 lib/controller/localdb/login_pam_test.go        |  2 +-
 3 files changed, 22 insertions(+), 2 deletions(-)

       via  40798c7655139fdd96ffd67a5d66cfffe3e5091e (commit)
       via  892521c6c0189d2e6a8fffff77140183098db259 (commit)
      from  ac8dd29afc2536501a43231bbb143feff1d5f3b6 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 40798c7655139fdd96ffd67a5d66cfffe3e5091e
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Tue Mar 31 11:27:01 2020 -0400

    16212: Add clues/details to authentication error messages.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/controller/localdb/login_pam.go b/lib/controller/localdb/login_pam.go
index be90cb3d1..a9e60ccba 100644
--- a/lib/controller/localdb/login_pam.go
+++ b/lib/controller/localdb/login_pam.go
@@ -36,6 +36,7 @@ func (ctrl *pamLoginController) Login(ctx context.Context, opts arvados.LoginOpt
 
 func (ctrl *pamLoginController) UserAuthenticate(ctx context.Context, opts arvados.UserAuthenticateOptions) (arvados.APIClientAuthorization, error) {
 	errorMessage := ""
+	sentPassword := false
 	tx, err := pam.StartFunc(ctrl.Cluster.Login.PAMService, opts.Username, func(style pam.Style, message string) (string, error) {
 		ctxlog.FromContext(ctx).Debugf("pam conversation: style=%v message=%q", style, message)
 		switch style {
@@ -47,6 +48,7 @@ func (ctrl *pamLoginController) UserAuthenticate(ctx context.Context, opts arvad
 			ctxlog.FromContext(ctx).WithField("Message", message).Info("pam.TextInfo")
 			return "", nil
 		case pam.PromptEchoOn, pam.PromptEchoOff:
+			sentPassword = true
 			return opts.Password, nil
 		default:
 			return "", fmt.Errorf("unrecognized message style %d", style)
@@ -57,6 +59,19 @@ func (ctrl *pamLoginController) UserAuthenticate(ctx context.Context, opts arvad
 	}
 	err = tx.Authenticate(pam.DisallowNullAuthtok)
 	if err != nil {
+		err = fmt.Errorf("PAM: %s", err)
+		if errorMessage != "" {
+			// Perhaps the error message in the
+			// conversation is helpful.
+			err = fmt.Errorf("%s; %q", err, errorMessage)
+		}
+		if sentPassword {
+			err = fmt.Errorf("%s (with username %q and password)", err, opts.Username)
+		} else {
+			// This might hint that the username was
+			// invalid.
+			err = fmt.Errorf("%s (with username %q; password was never requested by PAM service)", err, opts.Username)
+		}
 		return arvados.APIClientAuthorization{}, httpserver.ErrorWithStatus(err, http.StatusUnauthorized)
 	}
 	if errorMessage != "" {
diff --git a/lib/controller/localdb/login_pam_docker_test.sh b/lib/controller/localdb/login_pam_docker_test.sh
index fe67f50dd..3feba58ec 100755
--- a/lib/controller/localdb/login_pam_docker_test.sh
+++ b/lib/controller/localdb/login_pam_docker_test.sh
@@ -181,7 +181,7 @@ check_contains() {
 echo >&2 "Testing authentication failure"
 resp="$(curl -s --include -d username=foo -d password=nosecret "http://${ctrlhostport}/arvados/v1/users/authenticate" | tee $debug)"
 check_contains "${resp}" "HTTP/1.1 401"
-check_contains "${resp}" '{"errors":["Authentication failure"]}'
+check_contains "${resp}" '{"errors":["PAM: Authentication failure (with username \"foo\" and password)"]}'
 
 echo >&2 "Testing authentication success"
 resp="$(curl -s --include -d username=foo -d password=secret "http://${ctrlhostport}/arvados/v1/users/authenticate" | tee $debug)"
diff --git a/lib/controller/localdb/login_pam_test.go b/lib/controller/localdb/login_pam_test.go
index 4ecead8b1..d32aa1f24 100644
--- a/lib/controller/localdb/login_pam_test.go
+++ b/lib/controller/localdb/login_pam_test.go
@@ -46,7 +46,7 @@ func (s *PamSuite) TestLoginFailure(c *check.C) {
 		Username: "bogususername",
 		Password: "boguspassword",
 	})
-	c.Check(err, check.ErrorMatches, "Authentication failure")
+	c.Check(err, check.ErrorMatches, `PAM: Authentication failure \(with username "bogususername" and password\)`)
 	hs, ok := err.(interface{ HTTPStatus() int })
 	if c.Check(ok, check.Equals, true) {
 		c.Check(hs.HTTPStatus(), check.Equals, http.StatusUnauthorized)

commit 892521c6c0189d2e6a8fffff77140183098db259
Author: Tom Clegg <tom at tomclegg.ca>
Date:   Tue Mar 31 11:26:01 2020 -0400

    16212: Make test timeout more predictable by pulling image first.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>

diff --git a/lib/controller/localdb/login_pam_docker_test.sh b/lib/controller/localdb/login_pam_docker_test.sh
index 932811a8d..fe67f50dd 100755
--- a/lib/controller/localdb/login_pam_docker_test.sh
+++ b/lib/controller/localdb/login_pam_docker_test.sh
@@ -37,6 +37,11 @@ cleanup() {
 }
 trap cleanup ERR
 
+if [[ -z "$(docker image ls -q osixia/openldap:1.3.0)" ]]; then
+    echo >&2 "Pulling docker image for ldap server"
+    docker pull osixia/openldap:1.3.0
+fi
+
 ldapctr=ldap-${RANDOM}
 echo >&2 "Starting ldap server in docker container ${ldapctr}"
 docker run --rm --detach \

-----------------------------------------------------------------------


hooks/post-receive
--