1
0
mirror of https://github.com/pocketbase/pocketbase.git synced 2025-03-19 06:07:48 +02:00

added record.SetRandomPassword() helper and updated oauth2 autogenerated password handling

This commit is contained in:
Gani Georgiev 2024-12-26 13:24:03 +02:00
parent d8c0b11271
commit d34c8ec048
8 changed files with 126 additions and 37 deletions

View File

@ -18,6 +18,9 @@
- Added auth collection select for the settings "Send test email" popup ([#6166](https://github.com/pocketbase/pocketbase/issues/6166)).
- Added `record.SetRandomPassword()` to simplify random password generation usually used in the OAuth2 or OTP record creation flows.
_The generated ~30 chars random password is assigned directly as bcrypt hash and ignores the `password` field plain value validators like min/max length or regex pattern._
- ⚠️ Removed the "dry submit" when executing the collections Create API rule
(you can find more details why this change was introduced and how it could affect your app in https://github.com/pocketbase/pocketbase/discussions/6073).
For most users it should be non-breaking change, BUT if you have Create API rules that uses self-references or view counters you may have to adjust them manually.

View File

@ -16,7 +16,6 @@ import (
"github.com/pocketbase/pocketbase/tools/auth"
"github.com/pocketbase/pocketbase/tools/dbutils"
"github.com/pocketbase/pocketbase/tools/filesystem"
"github.com/pocketbase/pocketbase/tools/security"
"golang.org/x/oauth2"
)
@ -224,12 +223,6 @@ func oauth2Submit(e *core.RecordAuthWithOAuth2RequestEvent, optExternalAuth *cor
payload[core.FieldNameEmail] = e.OAuth2User.Email
// set a random password if none is set
if v, _ := payload[core.FieldNamePassword].(string); v == "" {
payload[core.FieldNamePassword] = security.RandomString(30)
payload[core.FieldNamePassword+"Confirm"] = payload[core.FieldNamePassword]
}
// map known fields (unless the field was explicitly submitted as part of CreateData)
if _, ok := payload[e.Collection.OAuth2.MappedFields.Id]; !ok && e.Collection.OAuth2.MappedFields.Id != "" {
payload[e.Collection.OAuth2.MappedFields.Id] = e.OAuth2User.Id
@ -292,7 +285,7 @@ func oauth2Submit(e *core.RecordAuthWithOAuth2RequestEvent, optExternalAuth *cor
// set random password for users with unverified email
// (this is in case a malicious actor has registered previously with the user email)
if !isLoggedAuthRecord && e.Record.Email() != "" && !e.Record.Verified() {
e.Record.SetPassword(security.RandomString(30))
e.Record.SetRandomPassword()
needUpdate = true
}

View File

@ -958,6 +958,8 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
"createData": {
"email": "invalid",
"emailVisibility": true,
"password": "1234567890",
"passwordConfirm": "1234567890",
"name": "test_name",
"username": "test_username",
"rel": "0yxhwia2amd8gec"
@ -1027,6 +1029,16 @@ func TestRecordAuthWithOAuth2(t *testing.T) {
"OnModelValidate": 4,
"OnRecordValidate": 4,
},
AfterTestFunc: func(t testing.TB, app *tests.TestApp, res *http.Response) {
user, err := app.FindFirstRecordByData("users", "username", "test_username")
if err != nil {
t.Fatal(err)
}
if !user.ValidatePassword("1234567890") {
t.Fatalf("Expected password %q to be valid", "1234567890")
}
},
},
{
Name: "creating user (with mapped OAuth2 fields and avatarURL->file field)",

View File

@ -94,9 +94,9 @@ func recordsList(e *core.RequestEvent) error {
// Add a randomized throttle in case of too many empty search filter attempts.
//
// This is just for extra precaution since security researches raised concern regarding the possibity of eventual
// This is just for extra precaution since security researches raised concern regarding the possibility of eventual
// timing attacks because the List API rule acts also as filter and executes in a single run with the client-side filters.
// This is by design and it is an accepted tradeoff between performance, usability and correctness.
// This is by design and it is an accepted trade off between performance, usability and correctness.
//
// While technically the below doesn't fully guarantee protection against filter timing attacks, in practice combined with the network latency it makes them even less feasible.
// A properly configured rate limiter or individual fields Hidden checks are better suited if you are really concerned about eventual information disclosure by side-channel attacks.
@ -220,6 +220,16 @@ func recordCreate(optFinalizer func(data any) error) func(e *core.RequestEvent)
return firstApiError(err, e.BadRequestError("Failed to read the submitted data.", err))
}
// set a random password for the OAuth2 ignoring its plain password validators
var skipPlainPasswordRecordValidators bool
if requestInfo.Context == core.RequestInfoContextOAuth2 {
if _, ok := data[core.FieldNamePassword]; !ok {
data[core.FieldNamePassword] = security.RandomString(30)
data[core.FieldNamePassword+"Confirm"] = data[core.FieldNamePassword]
skipPlainPasswordRecordValidators = true
}
}
// replace modifiers fields so that the resolved value is always
// available when accessing requestInfo.Body
requestInfo.Body = data
@ -230,6 +240,13 @@ func recordCreate(optFinalizer func(data any) error) func(e *core.RequestEvent)
}
form.Load(data)
if skipPlainPasswordRecordValidators {
// unset the plain value to skip the plain password field validators
if raw, ok := record.GetRaw(core.FieldNamePassword).(*core.PasswordFieldValue); ok {
raw.Plain = ""
}
}
var isOptFinalizerCalled bool
event := new(core.RecordRequestEvent)

View File

@ -1,5 +1,7 @@
package core
import "github.com/pocketbase/pocketbase/tools/security"
// Email returns the "email" record field value (usually available with Auth collections).
func (m *Record) Email() string {
return m.GetString(FieldNameEmail)
@ -51,6 +53,25 @@ func (m *Record) SetPassword(password string) {
m.Set(FieldNamePassword, password)
}
// SetRandomPassword sets the "password" auth record field to a random autogenerated value.
//
// The autogenerated password is ~30 characters and it is set directly as hash,
// aka. the field plain password value validators (length, pattern, etc.) are ignored
// (this is usually used as part of the auto created OTP or OAuth2 user flows).
func (m *Record) SetRandomPassword() string {
pass := security.RandomString(30)
m.Set(FieldNamePassword, pass)
m.RefreshTokenKey() // manually refresh the token key because the plain password is resetted
// unset the plain value to skip the field validators
if raw, ok := m.GetRaw(FieldNamePassword).(*PasswordFieldValue); ok {
raw.Plain = ""
}
return pass
}
// ValidatePassword validates a plain password against the "password" record field.
//
// Returns false if the password is incorrect.

View File

@ -1,9 +1,11 @@
package core_test
import (
"context"
"testing"
"github.com/pocketbase/pocketbase/core"
"github.com/pocketbase/pocketbase/tests"
"github.com/pocketbase/pocketbase/tools/security"
)
@ -117,3 +119,42 @@ func TestRecordPassword(t *testing.T) {
})
}
}
func TestRecordSetRandomPassword(t *testing.T) {
t.Parallel()
app, _ := tests.NewTestApp()
defer app.Cleanup()
oldTokenKey := "old_tokenKey"
record := core.NewRecord(core.NewAuthCollection("test"))
record.SetTokenKey(oldTokenKey)
pass := record.SetRandomPassword()
if pass == "" {
t.Fatal("Expected non-empty generated random password")
}
if !record.ValidatePassword(pass) {
t.Fatal("Expected the generated random password to be valid")
}
if record.TokenKey() == oldTokenKey {
t.Fatal("Expected token key to change")
}
f, ok := record.Collection().Fields.GetByName(core.FieldNamePassword).(*core.PasswordField)
if !ok {
t.Fatal("Expected *core.PasswordField")
}
// ensure that the field validators will be ignored
f.Min = 1
f.Max = 2
f.Pattern = `\d+`
if err := f.ValidateValue(context.Background(), app, record); err != nil {
t.Fatalf("Expected password field plain value validators to be ignored, got %v", err)
}
}

View File

@ -27,9 +27,9 @@ type RecordUpsert struct {
accessLevel int
// extra password fields
Password string `form:"password" json:"password"`
PasswordConfirm string `form:"passwordConfirm" json:"passwordConfirm"`
OldPassword string `form:"oldPassword" json:"oldPassword"`
password string
passwordConfirm string
oldPassword string
}
// NewRecordUpsert creates a new [RecordUpsert] form from the provided [core.App] and [core.Record] instances
@ -93,13 +93,13 @@ func (form *RecordUpsert) Load(data map[string]any) {
// load the special auth form fields
if isAuth {
if v, ok := data["password"]; ok {
form.Password = cast.ToString(v)
form.password = cast.ToString(v)
}
if v, ok := data["passwordConfirm"]; ok {
form.PasswordConfirm = cast.ToString(v)
form.passwordConfirm = cast.ToString(v)
}
if v, ok := data["oldPassword"]; ok {
form.OldPassword = cast.ToString(v)
form.oldPassword = cast.ToString(v)
}
excludeFields = append(excludeFields, "passwordConfirm", "oldPassword") // skip non-schema password fields
@ -137,9 +137,9 @@ func (form *RecordUpsert) validateFormFields() error {
validateData := map[string]any{
"email": form.record.Email(),
"verified": form.record.Verified(),
"password": form.Password,
"passwordConfirm": form.PasswordConfirm,
"oldPassword": form.OldPassword,
"password": form.password,
"passwordConfirm": form.passwordConfirm,
"oldPassword": form.oldPassword,
}
return validation.Validate(validateData,
@ -165,17 +165,17 @@ func (form *RecordUpsert) validateFormFields() error {
validation.Key(
"password",
validation.When(
(isNew || form.PasswordConfirm != "" || form.OldPassword != ""),
(isNew || form.passwordConfirm != "" || form.oldPassword != ""),
validation.Required,
),
),
validation.Key(
"passwordConfirm",
validation.When(
(isNew || form.Password != "" || form.OldPassword != ""),
(isNew || form.password != "" || form.oldPassword != ""),
validation.Required,
),
validation.By(validators.Equal(form.Password)),
validation.By(validators.Equal(form.password)),
),
validation.Key(
"oldPassword",
@ -183,7 +183,7 @@ func (form *RecordUpsert) validateFormFields() error {
// - form.HasManageAccess() is not satisfied
// - changing the existing password
validation.When(
!isNew && !form.HasManageAccess() && (form.Password != "" || form.PasswordConfirm != ""),
!isNew && !form.HasManageAccess() && (form.password != "" || form.passwordConfirm != ""),
validation.Required,
validation.By(form.checkOldPassword),
),

View File

@ -64,15 +64,15 @@ func TestRecordUpsertLoad(t *testing.T) {
`"text":"test_text"`,
`"number":456`,
`"select_many":["optionB","optionC"]`,
`"password":""`,
`"oldPassword":""`,
`"passwordConfirm":""`,
`"created":""`,
`"updated":""`,
`"json":null`,
},
notExpected: []string{
`"custom"`,
`"password"`,
`"oldPassword"`,
`"passwordConfirm"`,
`"select_many-"`,
`"select_many+"`,
},
@ -89,9 +89,11 @@ func TestRecordUpsertLoad(t *testing.T) {
record: core.NewRecord(usersCol),
expected: []string{
`"email":"test@example.com"`,
`"oldPassword":"123"`,
`"password":"456"`,
`"passwordConfirm":"789"`,
},
notExpected: []string{
`"oldPassword"`,
`"passwordConfirm"`,
},
},
{
@ -110,8 +112,10 @@ func TestRecordUpsertLoad(t *testing.T) {
`"email":"test@example.com"`,
`"tokenKey":""`,
`"password":"456"`,
`"oldPassword":"123"`,
`"passwordConfirm":"789"`,
},
notExpected: []string{
`"oldPassword"`,
`"passwordConfirm"`,
},
},
{
@ -130,8 +134,10 @@ func TestRecordUpsertLoad(t *testing.T) {
`"email":"test@example.com"`,
`"tokenKey":"abc"`,
`"password":"456"`,
`"oldPassword":"123"`,
`"passwordConfirm":"789"`,
},
notExpected: []string{
`"oldPassword"`,
`"passwordConfirm"`,
},
},
{
@ -168,11 +174,7 @@ func TestRecordUpsertLoad(t *testing.T) {
form.Load(s.data)
loaded := map[string]any{
"oldPassword": form.OldPassword,
"password": form.Password,
"passwordConfirm": form.PasswordConfirm,
}
loaded := map[string]any{}
maps.Copy(loaded, s.record.FieldsData())
maps.Copy(loaded, s.record.CustomData())