From f183783baa67e7da0b0ae0909d3d6cb3045c0501 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Mon, 9 Sep 2024 17:05:16 -0400 Subject: [PATCH] Save initial signup information for users to aid in spam prevention (#31852) This will allow instance admins to view signup pattern patterns for public instances. It is modelled after discourse, mastodon, and MediaWiki's approaches. Note: This has privacy implications, but as the above-stated open-source projects take this approach, especially MediaWiki, which I have no doubt looked into this thoroughly, it is likely okay for us, too. However, I would be appreciative of any feedback on how this could be improved. --------- Co-authored-by: Giteabot --- cmd/admin_user_create.go | 2 +- custom/conf/app.example.ini | 3 ++ models/user/setting_keys.go | 4 +++ models/user/user.go | 34 ++++++++++++++++--- models/user/user_test.go | 8 ++--- modules/setting/security.go | 3 ++ routers/api/v1/admin/user.go | 2 +- routers/install/install.go | 2 +- routers/web/admin/users.go | 2 +- routers/web/auth/auth.go | 6 +++- services/auth/reverseproxy.go | 2 +- .../auth/source/ldap/source_authenticate.go | 2 +- services/auth/source/ldap/source_sync.go | 2 +- .../auth/source/oauth2/source_sync_test.go | 2 +- .../auth/source/pam/source_authenticate.go | 2 +- .../auth/source/smtp/source_authenticate.go | 2 +- services/auth/sspi.go | 2 +- services/user/user_test.go | 4 +-- 18 files changed, 61 insertions(+), 23 deletions(-) diff --git a/cmd/admin_user_create.go b/cmd/admin_user_create.go index f328b753d8..106d14b25a 100644 --- a/cmd/admin_user_create.go +++ b/cmd/admin_user_create.go @@ -158,7 +158,7 @@ func runCreateUser(c *cli.Context) error { IsRestricted: restricted, } - if err := user_model.CreateUser(ctx, u, overwriteDefault); err != nil { + if err := user_model.CreateUser(ctx, u, &user_model.Meta{}, overwriteDefault); err != nil { return fmt.Errorf("CreateUser: %w", err) } diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 0f70a1a3d0..7009df54db 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -507,6 +507,9 @@ INTERNAL_TOKEN = ;; stemming from cached/logged plain-text API tokens. ;; In future releases, this will become the default behavior ;DISABLE_QUERY_AUTH_TOKEN = false +;; +;; On user registration, record the IP address and user agent of the user to help identify potential abuse. +;; RECORD_USER_SIGNUP_METADATA = false ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/models/user/setting_keys.go b/models/user/setting_keys.go index 72b3974eee..3149aae18b 100644 --- a/models/user/setting_keys.go +++ b/models/user/setting_keys.go @@ -14,4 +14,8 @@ const ( UserActivityPubPrivPem = "activitypub.priv_pem" // UserActivityPubPubPem is user's public key UserActivityPubPubPem = "activitypub.pub_pem" + // SignupIP is the IP address that the user signed up with + SignupIP = "signup.ip" + // SignupUserAgent is the user agent that the user signed up with + SignupUserAgent = "signup.user_agent" ) diff --git a/models/user/user.go b/models/user/user.go index 2a3c1833b9..f93fba8ae0 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -150,6 +150,14 @@ type User struct { KeepActivityPrivate bool `xorm:"NOT NULL DEFAULT false"` } +// Meta defines the meta information of a user, to be stored in the K/V table +type Meta struct { + // Store the initial registration of the user, to aid in spam prevention + // Ensure that one IP isn't creating many accounts (following mediawiki approach) + InitialIP string + InitialUserAgent string +} + func init() { db.RegisterModel(new(User)) } @@ -615,17 +623,17 @@ type CreateUserOverwriteOptions struct { } // CreateUser creates record of a new user. -func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { - return createUser(ctx, u, false, overwriteDefault...) +func CreateUser(ctx context.Context, u *User, meta *Meta, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { + return createUser(ctx, u, meta, false, overwriteDefault...) } // AdminCreateUser is used by admins to manually create users -func AdminCreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { - return createUser(ctx, u, true, overwriteDefault...) +func AdminCreateUser(ctx context.Context, u *User, meta *Meta, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { + return createUser(ctx, u, meta, true, overwriteDefault...) } // createUser creates record of a new user. -func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { +func createUser(ctx context.Context, u *User, meta *Meta, createdByAdmin bool, overwriteDefault ...*CreateUserOverwriteOptions) (err error) { if err = IsUsableUsername(u.Name); err != nil { return err } @@ -745,6 +753,22 @@ func createUser(ctx context.Context, u *User, createdByAdmin bool, overwriteDefa return err } + if setting.RecordUserSignupMetadata { + // insert initial IP and UserAgent + if err = SetUserSetting(ctx, u.ID, SignupIP, meta.InitialIP); err != nil { + return err + } + + // trim user agent string to a reasonable length, if necessary + userAgent := strings.TrimSpace(meta.InitialUserAgent) + if len(userAgent) > 255 { + userAgent = userAgent[:255] + } + if err = SetUserSetting(ctx, u.ID, SignupUserAgent, userAgent); err != nil { + return err + } + } + // insert email address if err := db.Insert(ctx, &EmailAddress{ UID: u.ID, diff --git a/models/user/user_test.go b/models/user/user_test.go index 26cda3f89e..67efb3859f 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -227,7 +227,7 @@ func TestCreateUserInvalidEmail(t *testing.T) { MustChangePassword: false, } - err := user_model.CreateUser(db.DefaultContext, user) + err := user_model.CreateUser(db.DefaultContext, user, &user_model.Meta{}) assert.Error(t, err) assert.True(t, user_model.IsErrEmailCharIsNotSupported(err)) } @@ -241,7 +241,7 @@ func TestCreateUserEmailAlreadyUsed(t *testing.T) { user.Name = "testuser" user.LowerName = strings.ToLower(user.Name) user.ID = 0 - err := user_model.CreateUser(db.DefaultContext, user) + err := user_model.CreateUser(db.DefaultContext, user, &user_model.Meta{}) assert.Error(t, err) assert.True(t, user_model.IsErrEmailAlreadyUsed(err)) } @@ -258,7 +258,7 @@ func TestCreateUserCustomTimestamps(t *testing.T) { user.ID = 0 user.Email = "unique@example.com" user.CreatedUnix = creationTimestamp - err := user_model.CreateUser(db.DefaultContext, user) + err := user_model.CreateUser(db.DefaultContext, user, &user_model.Meta{}) assert.NoError(t, err) fetched, err := user_model.GetUserByID(context.Background(), user.ID) @@ -283,7 +283,7 @@ func TestCreateUserWithoutCustomTimestamps(t *testing.T) { user.Email = "unique@example.com" user.CreatedUnix = 0 user.UpdatedUnix = 0 - err := user_model.CreateUser(db.DefaultContext, user) + err := user_model.CreateUser(db.DefaultContext, user, &user_model.Meta{}) assert.NoError(t, err) timestampEnd := time.Now().Unix() diff --git a/modules/setting/security.go b/modules/setting/security.go index 3d7b1f9ce7..3d12fcf8d9 100644 --- a/modules/setting/security.go +++ b/modules/setting/security.go @@ -37,6 +37,7 @@ var ( DisableQueryAuthToken bool CSRFCookieName = "_csrf" CSRFCookieHTTPOnly = true + RecordUserSignupMetadata = false ) // loadSecret load the secret from ini by uriKey or verbatimKey, only one of them could be set @@ -164,6 +165,8 @@ func loadSecurityFrom(rootCfg ConfigProvider) { // TODO: default value should be true in future releases DisableQueryAuthToken = sec.Key("DISABLE_QUERY_AUTH_TOKEN").MustBool(false) + RecordUserSignupMetadata = sec.Key("RECORD_USER_SIGNUP_METADATA").MustBool(false) + // warn if the setting is set to false explicitly if sectionHasDisableQueryAuthToken && !DisableQueryAuthToken { log.Warn("Enabling Query API Auth tokens is not recommended. DISABLE_QUERY_AUTH_TOKEN will default to true in gitea 1.23 and will be removed in gitea 1.24.") diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 3e2fefc6da..b0f40084da 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -133,7 +133,7 @@ func CreateUser(ctx *context.APIContext) { u.UpdatedUnix = u.CreatedUnix } - if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil { + if err := user_model.AdminCreateUser(ctx, u, &user_model.Meta{}, overwriteDefault); err != nil { if user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err) || db.IsErrNameReserved(err) || diff --git a/routers/install/install.go b/routers/install/install.go index fde8b37ed5..e420d36da5 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -554,7 +554,7 @@ func SubmitInstall(ctx *context.Context) { IsActive: optional.Some(true), } - if err = user_model.CreateUser(ctx, u, overwriteDefault); err != nil { + if err = user_model.CreateUser(ctx, u, &user_model.Meta{}, overwriteDefault); err != nil { if !user_model.IsErrUserAlreadyExist(err) { setting.InstallLock = false ctx.Data["Err_AdminName"] = true diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 34bb1dfe26..48ff8ea04b 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -177,7 +177,7 @@ func NewUserPost(ctx *context.Context) { u.MustChangePassword = form.MustChangePassword } - if err := user_model.AdminCreateUser(ctx, u, overwriteDefault); err != nil { + if err := user_model.AdminCreateUser(ctx, u, &user_model.Meta{}, overwriteDefault); err != nil { switch { case user_model.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index f295cf039f..b86c1ff1c2 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -541,7 +541,11 @@ func createAndHandleCreatedUser(ctx *context.Context, tpl base.TplName, form any // createUserInContext creates a user and handles errors within a given context. // Optionally a template can be specified. func createUserInContext(ctx *context.Context, tpl base.TplName, form any, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, gothUser *goth.User, allowLink bool) (ok bool) { - if err := user_model.CreateUser(ctx, u, overwrites); err != nil { + meta := &user_model.Meta{ + InitialIP: ctx.RemoteAddr(), + InitialUserAgent: ctx.Req.UserAgent(), + } + if err := user_model.CreateUser(ctx, u, meta, overwrites); err != nil { if allowLink && (user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err)) { if setting.OAuth2Client.AccountLinking == setting.OAuth2AccountLinkingAuto { var user *user_model.User diff --git a/services/auth/reverseproxy.go b/services/auth/reverseproxy.go index b6aeb0aed2..36b4ef68f4 100644 --- a/services/auth/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -164,7 +164,7 @@ func (r *ReverseProxy) newUser(req *http.Request) *user_model.User { IsActive: optional.Some(true), } - if err := user_model.CreateUser(req.Context(), user, &overwriteDefault); err != nil { + if err := user_model.CreateUser(req.Context(), user, &user_model.Meta{}, &overwriteDefault); err != nil { // FIXME: should I create a system notice? log.Error("CreateUser: %v", err) return nil diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index 6ebd3ea50a..01cb743720 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/services/auth/source/ldap/source_authenticate.go @@ -89,7 +89,7 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u IsActive: optional.Some(true), } - err := user_model.CreateUser(ctx, user, overwriteDefault) + err := user_model.CreateUser(ctx, user, &user_model.Meta{}, overwriteDefault) if err != nil { return user, err } diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index 2a95326b9e..a6d6d2a0f2 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/services/auth/source/ldap/source_sync.go @@ -129,7 +129,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { IsActive: optional.Some(true), } - err = user_model.CreateUser(ctx, usr, overwriteDefault) + err = user_model.CreateUser(ctx, usr, &user_model.Meta{}, overwriteDefault) if err != nil { log.Error("SyncExternalUsers[%s]: Error creating user %s: %v", source.authSource.Name, su.Username, err) } diff --git a/services/auth/source/oauth2/source_sync_test.go b/services/auth/source/oauth2/source_sync_test.go index e2f04bcb25..25408e8727 100644 --- a/services/auth/source/oauth2/source_sync_test.go +++ b/services/auth/source/oauth2/source_sync_test.go @@ -36,7 +36,7 @@ func TestSource(t *testing.T) { Email: "external@example.com", } - err := user_model.CreateUser(context.Background(), user, &user_model.CreateUserOverwriteOptions{}) + err := user_model.CreateUser(context.Background(), user, &user_model.Meta{}, &user_model.CreateUserOverwriteOptions{}) assert.NoError(t, err) e := &user_model.ExternalLoginUser{ diff --git a/services/auth/source/pam/source_authenticate.go b/services/auth/source/pam/source_authenticate.go index addd1bd2c9..6fd02dc29f 100644 --- a/services/auth/source/pam/source_authenticate.go +++ b/services/auth/source/pam/source_authenticate.go @@ -63,7 +63,7 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u IsActive: optional.Some(true), } - if err := user_model.CreateUser(ctx, user, overwriteDefault); err != nil { + if err := user_model.CreateUser(ctx, user, &user_model.Meta{}, overwriteDefault); err != nil { return user, err } diff --git a/services/auth/source/smtp/source_authenticate.go b/services/auth/source/smtp/source_authenticate.go index 1f0a61c789..b2e94933a6 100644 --- a/services/auth/source/smtp/source_authenticate.go +++ b/services/auth/source/smtp/source_authenticate.go @@ -79,7 +79,7 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u IsActive: optional.Some(true), } - if err := user_model.CreateUser(ctx, user, overwriteDefault); err != nil { + if err := user_model.CreateUser(ctx, user, &user_model.Meta{}, overwriteDefault); err != nil { return user, err } diff --git a/services/auth/sspi.go b/services/auth/sspi.go index 64a127e97a..7f8a03a4c6 100644 --- a/services/auth/sspi.go +++ b/services/auth/sspi.go @@ -176,7 +176,7 @@ func (s *SSPI) newUser(ctx context.Context, username string, cfg *sspi.Source) ( KeepEmailPrivate: optional.Some(true), EmailNotificationsPreference: &emailNotificationPreference, } - if err := user_model.CreateUser(ctx, user, overwriteDefault); err != nil { + if err := user_model.CreateUser(ctx, user, &user_model.Meta{}, overwriteDefault); err != nil { return nil, err } diff --git a/services/user/user_test.go b/services/user/user_test.go index bd6019a14f..cd0f597501 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -92,7 +92,7 @@ func TestCreateUser(t *testing.T) { MustChangePassword: false, } - assert.NoError(t, user_model.CreateUser(db.DefaultContext, user)) + assert.NoError(t, user_model.CreateUser(db.DefaultContext, user, &user_model.Meta{})) assert.NoError(t, DeleteUser(db.DefaultContext, user, false)) } @@ -177,7 +177,7 @@ func TestCreateUser_Issue5882(t *testing.T) { for _, v := range tt { setting.Admin.DisableRegularOrgCreation = v.disableOrgCreation - assert.NoError(t, user_model.CreateUser(db.DefaultContext, v.user)) + assert.NoError(t, user_model.CreateUser(db.DefaultContext, v.user, &user_model.Meta{})) u, err := user_model.GetUserByEmail(db.DefaultContext, v.user.Email) assert.NoError(t, err)