feat: add optional stricter password requirements (#24857) (#24890)

Allow password length and character class checking.

closes #24856

(cherry picked from commit 7d8884b)

closes #24884

cmd/influxd/launcher/cmd.go

@@ -191,6 +191,7 @@ type InfluxdOpts struct {
 	Viper *viper.Viper
 
 	HardeningEnabled bool
+	StrongPasswords  bool
 }
 
 // NewOpts constructs options with default values.
@@ -243,6 +244,7 @@ func NewOpts(viper *viper.Viper) *InfluxdOpts {
 		TestingAlwaysAllowSetup: false,
 
 		HardeningEnabled: false,
+		StrongPasswords:  false,
 	}
 }
 
@@ -657,6 +659,12 @@ func (o *InfluxdOpts) BindCliOpts() []cli.Opt {
 			Default: o.HardeningEnabled,
 			Desc:    "enable hardening options (disallow private IPs within flux and templates HTTP requests)",
 		},
+		{
+			DestP:   &o.StrongPasswords,
+			Flag:    "strong-passwords",
+			Default: o.StrongPasswords,
+			Desc:    "enable password strength enforcement",
+		},
 	}
 }
 

cmd/influxd/launcher/launcher.go

@@ -259,7 +259,7 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
 	m.reg.MustRegister(infprom.NewInfluxCollector(procID, info))
 
 	tenantStore := tenant.NewStore(m.kvStore)
-	ts := tenant.NewSystem(tenantStore, m.log.With(zap.String("store", "new")), m.reg, metric.WithSuffix("new"))
+	ts := tenant.NewSystem(tenantStore, m.log.With(zap.String("store", "new")), m.reg, opts.StrongPasswords, metric.WithSuffix("new"))
 
 	serviceConfig := kv.ServiceConfig{
 		FluxLanguageService: fluxlang.DefaultService,
@@ -644,7 +644,7 @@ func (m *Launcher) run(ctx context.Context, opts *InfluxdOpts) (err error) {
 			return err
 		}
 
-		authSvcV1 = authv1.NewService(authStore, ts)
+		authSvcV1 = authv1.NewService(authStore, ts, authv1.WithPasswordChecking(opts.StrongPasswords))
 		passwordV1 = authv1.NewCachingPasswordsService(authSvcV1)
 	}
 

cmd/influxd/recovery/user/user_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/influxdata/influxdb/v2/cmd/influxd/recovery/testhelper"
+	"github.com/influxdata/influxdb/v2/kit/platform/errors"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -20,7 +21,7 @@ func Test_User_Basic(t *testing.T) {
 		"user with name testuser already exists")
 
 	// user needs a long-ish password
-	assert.EqualError(t, testhelper.RunCommand(t, NewUserCommand(), "create", "--bolt-path", db.Name(), "--username", "testuser2", "--password", "foo"), "passwords must be at least 8 characters long")
+	assert.EqualError(t, testhelper.RunCommand(t, NewUserCommand(), "create", "--bolt-path", db.Name(), "--username", "testuser2", "--password", "foo"), errors.EPasswordLength.Error())
 	assert.NoError(t, testhelper.RunCommand(t, NewUserCommand(), "create", "--bolt-path", db.Name(), "--username", "testuser2", "--password", "my_password"), "")
 
 	// at least run the update code

cmd/influxd/upgrade/upgrade.go

@@ -634,7 +634,7 @@ func newInfluxDBv2(ctx context.Context, opts *optionsV2, log *zap.Logger) (svc *
 
 	// Create Tenant service (orgs, buckets, )
 	svc.tenantStore = tenant.NewStore(svc.kvStore)
-	svc.ts = tenant.NewSystem(svc.tenantStore, log.With(zap.String("store", "new")), reg, metric.WithSuffix("new"))
+	svc.ts = tenant.NewSystem(svc.tenantStore, log.With(zap.String("store", "new")), reg, false, metric.WithSuffix("new"))
 
 	svc.meta = meta.NewClient(meta.NewConfig(), svc.kvStore)
 	if err := svc.meta.Open(); err != nil {

tenant/error_user.go → kit/platform/errors/error_user.go

@@ -1,103 +1,115 @@
-package tenant
+package errors
 
 import (
 	"fmt"
-
-	"github.com/influxdata/influxdb/v2/kit/platform/errors"
 )
 
 const MinPasswordLen int = 8
+const MaxPasswordLen = 72
+const SpecialChars = `!@#$%^&*()_+`
 
 var (
 	// ErrUserNotFound is used when the user is not found.
-	ErrUserNotFound = &errors.Error{
+	ErrUserNotFound = &Error{
 		Msg:  "user not found",
-		Code: errors.ENotFound,
+		Code: ENotFound,
 	}
 
 	// EIncorrectPassword is returned when any password operation fails in which
 	// we do not want to leak information.
-	EIncorrectPassword = &errors.Error{
-		Code: errors.EForbidden,
+	EIncorrectPassword = &Error{
+		Code: EForbidden,
 		Msg:  "your username or password is incorrect",
 	}
 
 	// EIncorrectUser is returned when any user is failed to be found which indicates
 	// the userID provided is for a user that does not exist.
-	EIncorrectUser = &errors.Error{
-		Code: errors.EForbidden,
+	EIncorrectUser = &Error{
+		Code: EForbidden,
 		Msg:  "your userID is incorrect",
 	}
 
-	// EShortPassword is used when a password is less than the minimum
-	// acceptable password length.
-	EShortPassword = &errors.Error{
-		Code: errors.EInvalid,
-		Msg:  fmt.Sprintf("passwords must be at least %d characters long", MinPasswordLen),
+	// EPasswordLength is used when a password is less than the minimum
+	// acceptable password length or longer than the maximum acceptable password length
+	EPasswordLength = &Error{
+		Code: EInvalid,
+		Msg:  fmt.Sprintf("passwords must be between %d and %d characters long", MinPasswordLen, MaxPasswordLen),
+	}
+
+	EPasswordChars = &Error{
+		Code: EInvalid,
+		Msg: fmt.Sprintf(
+			"passwords must contain at least three of the following character types: uppercase, lowercase, numbers, and special characters: %s",
+			SpecialChars),
+	}
+
+	EPasswordChangeRequired = &Error{
+		Code: EForbidden,
+		Msg:  "password change required",
 	}
 )
 
 // UserAlreadyExistsError is used when attempting to create a user with a name
 // that already exists.
-func UserAlreadyExistsError(n string) *errors.Error {
-	return &errors.Error{
-		Code: errors.EConflict,
+func UserAlreadyExistsError(n string) *Error {
+	return &Error{
+		Code: EConflict,
 		Msg:  fmt.Sprintf("user with name %s already exists", n),
 	}
 }
 
 // UserIDAlreadyExistsError is used when attempting to create a user with an ID
 // that already exists.
-func UserIDAlreadyExistsError(id string) *errors.Error {
-	return &errors.Error{
-		Code: errors.EConflict,
+func UserIDAlreadyExistsError(id string) *Error {
+	return &Error{
+		Code: EConflict,
 		Msg:  fmt.Sprintf("user with ID %s already exists", id),
 	}
 }
 
 // UnexpectedUserBucketError is used when the error comes from an internal system.
-func UnexpectedUserBucketError(err error) *errors.Error {
-	return &errors.Error{
-		Code: errors.EInternal,
+func UnexpectedUserBucketError(err error) *Error {
+	return &Error{
+		Code: EInternal,
 		Msg:  fmt.Sprintf("unexpected error retrieving user bucket; Err: %v", err),
 		Op:   "kv/userBucket",
 	}
 }
 
 // UnexpectedUserIndexError is used when the error comes from an internal system.
-func UnexpectedUserIndexError(err error) *errors.Error {
-	return &errors.Error{
-		Code: errors.EInternal,
+func UnexpectedUserIndexError(err error) *Error {
+	return &Error{
+		Code: EInternal,
 		Msg:  fmt.Sprintf("unexpected error retrieving user index; Err: %v", err),
 		Op:   "kv/userIndex",
 	}
 }
 
 // InvalidUserIDError is used when a service was provided an invalid ID.
 // This is some sort of internal server error.
-func InvalidUserIDError(err error) *errors.Error {
-	return &errors.Error{
-		Code: errors.EInvalid,
+func InvalidUserIDError(err error) *Error {
+	return &Error{
+		Code: EInvalid,
 		Msg:  "user id provided is invalid",
 		Err:  err,
 	}
 }
 
 // ErrCorruptUser is used when the user cannot be unmarshalled from the bytes
 // stored in the kv.
-func ErrCorruptUser(err error) *errors.Error {
-	return &errors.Error{
-		Code: errors.EInternal,
+func ErrCorruptUser(err error) *Error {
+	return &Error{
+		Code: EInternal,
 		Msg:  "user could not be unmarshalled",
 		Err:  err,
 		Op:   "kv/UnmarshalUser",
 	}
 }
 
 // ErrUnprocessableUser is used when a user is not able to be processed.
-func ErrUnprocessableUser(err error) *errors.Error {
-	return &errors.Error{
-		Code: errors.EUnprocessableEntity,
+func ErrUnprocessableUser(err error) *Error {
+	return &Error{
+		Code: EUnprocessableEntity,
 		Msg:  "user could not be marshalled",
 		Err:  err,
 		Op:   "kv/MarshalUser",
@@ -107,9 +119,9 @@ func ErrUnprocessableUser(err error) *errors.Error {
 // UnavailablePasswordServiceError is used if we aren't able to add the
 // password to the store, it means the store is not available at the moment
 // (e.g. network).
-func UnavailablePasswordServiceError(err error) *errors.Error {
-	return &errors.Error{
-		Code: errors.EUnavailable,
+func UnavailablePasswordServiceError(err error) *Error {
+	return &Error{
+		Code: EUnavailable,
 		Msg:  fmt.Sprintf("Unable to connect to password service. Please try again; Err: %v", err),
 		Op:   "kv/setPassword",
 	}

session/http_server.go

@@ -2,6 +2,7 @@ package session
 
 import (
 	"context"
+	eBase "errors"
 	"net/http"
 
 	"github.com/go-chi/chi"
@@ -95,7 +96,10 @@ func (h *SessionHandler) handleSignin(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err := h.passSvc.ComparePassword(ctx, u.ID, req.Password); err != nil {
-		h.api.Err(w, r, ErrUnauthorized)
+		if !eBase.Is(err, errors.EPasswordChangeRequired) {
+			err = ErrUnauthorized
+		}
+		h.api.Err(w, r, err)
 		return
 	}
 

tenant/service.go

@@ -28,6 +28,8 @@ func isInternal(ctx context.Context) bool {
 
 type Service struct {
 	store *Store
+	// Store raw version (not interface) for test purposes.
+	userSvc *UserSvc
 	influxdb.UserService
 	influxdb.PasswordsService
 	influxdb.UserResourceMappingService
@@ -44,21 +46,25 @@ func (s *Service) RUnlock() {
 }
 
 // NewService creates a new base tenant service.
-func NewService(st *Store) *Service {
+func NewService(st *Store, UserSvcOptFns ...func(svc *UserSvc)) *Service {
 	svc := &Service{store: st}
-	userSvc := NewUserSvc(st, svc)
-	svc.UserService = userSvc
-	svc.PasswordsService = userSvc
+	svc.userSvc = NewUserSvc(st, svc, UserSvcOptFns...)
+	svc.UserService = svc.userSvc
+	svc.PasswordsService = svc.userSvc
 	svc.UserResourceMappingService = NewUserResourceMappingSvc(st, svc)
 	svc.OrganizationService = NewOrganizationSvc(st, svc)
 	svc.BucketService = NewBucketSvc(st, svc)
 
 	return svc
 }
 
+func (s *Service) SetUserOptions(opts ...func(*UserSvc)) {
+	s.userSvc.SetOptions(opts...)
+}
+
 // creates a new Service with logging and metrics middleware wrappers.
-func NewSystem(store *Store, log *zap.Logger, reg prometheus.Registerer, metricOpts ...metric.ClientOptFn) *Service {
-	ts := NewService(store)
+func NewSystem(store *Store, log *zap.Logger, reg prometheus.Registerer, strongPasswords bool, metricOpts ...metric.ClientOptFn) *Service {
+	ts := NewService(store, WithPasswordChecking(strongPasswords))
 	ts.UserService = NewUserLogger(log, NewUserMetrics(reg, ts.UserService, metricOpts...))
 	ts.PasswordsService = NewPasswordLogger(log, NewPasswordMetrics(reg, ts.PasswordsService, metricOpts...))
 	ts.UserResourceMappingService = NewURMLogger(log, NewUrmMetrics(reg, ts.UserResourceMappingService, metricOpts...))

tenant/service_onboarding_test.go

@@ -9,10 +9,12 @@ import (
 	"github.com/influxdata/influxdb/v2"
 	"github.com/influxdata/influxdb/v2/authorization"
 	icontext "github.com/influxdata/influxdb/v2/context"
+	influx_errors "github.com/influxdata/influxdb/v2/kit/platform/errors"
 	"github.com/influxdata/influxdb/v2/kv"
 	"github.com/influxdata/influxdb/v2/pkg/testing/assert"
 	"github.com/influxdata/influxdb/v2/tenant"
 	influxdbtesting "github.com/influxdata/influxdb/v2/testing"
+	assert2 "github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -252,5 +254,5 @@ func TestOnboardService_WeakPassword(t *testing.T) {
 		Org:      "name",
 		Bucket:   "name",
 	})
-	assert.Equal(t, err, tenant.EShortPassword)
+	assert2.ErrorIs(t, err, influx_errors.EPasswordLength)
 }