Skip to content

Commit

Permalink
Skip validating minimum password length for existing user logins (gra…
Browse files Browse the repository at this point in the history
…vitational#36579)

The minimum password length was being checked against all password checks,
both for validating existing passwords and new passwords. This meant that
if the minimum password length was increased, any existing passwords that
were under the minimum length would be rejected.

Follow-up to gravitational#36389.

Ref gravitational#1936.
Ref gravitational#7687.
  • Loading branch information
reedloden authored Jan 12, 2024
1 parent b7551f3 commit 710a7d2
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 5 deletions.
5 changes: 0 additions & 5 deletions lib/auth/password.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,6 @@ func (a *Server) ChangePassword(ctx context.Context, req *proto.ChangePasswordRe
func (a *Server) checkPasswordWOToken(user string, password []byte) error {
const errMsg = "invalid username or password"

err := services.VerifyPassword(password)
if err != nil {
return trace.BadParameter(errMsg)
}

hash, err := a.GetPasswordHash(user)
if err != nil && !trace.IsNotFound(err) {
return trace.Wrap(err)
Expand Down
34 changes: 34 additions & 0 deletions lib/auth/password_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/jonboulle/clockwork"
"github.com/pquerna/otp/totp"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/bcrypt"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/client/proto"
Expand All @@ -46,6 +47,7 @@ import (
"github.com/gravitational/teleport/lib/events/eventstest"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/suite"
"github.com/gravitational/teleport/lib/utils"
)

type passwordSuite struct {
Expand Down Expand Up @@ -126,6 +128,38 @@ func TestUserNotFound(t *testing.T) {
require.True(t, trace.IsBadParameter(err))
}

func TestPasswordLengthChange(t *testing.T) {
t.Parallel()
ctx := context.Background()
srv := newTestTLSServer(t)
authServer := srv.Auth()

ap, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{
Type: constants.Local,
SecondFactor: constants.SecondFactorOff,
})
require.NoError(t, err)

err = authServer.SetAuthPreference(ctx, ap)
require.NoError(t, err)

username := fmt.Sprintf("llama%[email protected]", rand.Int())
password := []byte("a")
_, _, err = CreateUserAndRole(authServer, username, []string{username}, nil)
require.NoError(t, err)

hash, err := utils.BcryptFromPassword(password, bcrypt.DefaultCost)
require.NoError(t, err)

// Set an initial password that is shorter than minimum length
err = authServer.UpsertPasswordHash(username, hash)
require.NoError(t, err)

// Ensure that a shorter password still works for auth
err = authServer.checkPasswordWOToken(username, password)
require.NoError(t, err)
}

func TestChangePassword(t *testing.T) {
t.Parallel()
ctx := context.Background()
Expand Down
37 changes: 37 additions & 0 deletions lib/services/identity_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/defaults"
)

func TestSAMLAuthRequest_Check(t *testing.T) {
Expand Down Expand Up @@ -326,3 +327,39 @@ func TestGithubAuthRequest_Check(t *testing.T) {
})
}
}

func TestVerifyPassword(t *testing.T) {
tests := []struct {
name string
pass []byte
wantErr bool
}{
{
name: "password too short",
pass: make([]byte, defaults.MinPasswordLength-1),
wantErr: true,
},
{
name: "password just right",
pass: make([]byte, defaults.MinPasswordLength),
wantErr: false,
},
{
name: "password too long",
pass: make([]byte, defaults.MaxPasswordLength+1),
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := VerifyPassword(tt.pass)
if tt.wantErr {
require.Error(t, err)
require.True(t, trace.IsBadParameter(err))
} else {
require.NoError(t, err)
}
})
}
}

0 comments on commit 710a7d2

Please sign in to comment.