From 6ea95630b17cb3b9dfdc3b2837e13039b90ca79e Mon Sep 17 00:00:00 2001 From: Richard Nienaber Date: Sat, 26 Jun 2021 01:06:31 +0100 Subject: [PATCH 1/6] add configuration option to restrict users by default --- custom/conf/app.example.ini | 3 +++ .../doc/advanced/config-cheat-sheet.en-us.md | 1 + integrations/mssql.ini.tmpl | 1 + integrations/mysql.ini.tmpl | 1 + integrations/mysql8.ini.tmpl | 1 + integrations/pgsql.ini.tmpl | 1 + integrations/sqlite.ini.tmpl | 1 + models/user.go | 1 + models/user_test.go | 22 +++++++++++++++++++ modules/setting/service.go | 2 ++ 10 files changed, 34 insertions(+) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 5adfb0546f0bf..edf7347934f59 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -649,6 +649,9 @@ PATH = ;; Default value for AllowCreateOrganization ;; Every new user will have rights set to create organizations depending on this setting ;DEFAULT_ALLOW_CREATE_ORGANIZATION = true +;; Default value for IsRestricted +;; Every new user will have restricted permissions depending on this setting +;DEFAULT_USER_IS_RESTRICTED = true ;; ;; Either "public", "limited" or "private", default is "public" ;; Limited is for signed user only diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 5e976174fb19b..d45d899d263a9 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -499,6 +499,7 @@ relation to port exhaustion. - `HCAPTCHA_SITEKEY`: **""**: Sign up at https://www.hcaptcha.com/ to get a sitekey for hcaptcha. - `DEFAULT_KEEP_EMAIL_PRIVATE`: **false**: By default set users to keep their email address private. - `DEFAULT_ALLOW_CREATE_ORGANIZATION`: **true**: Allow new users to create organizations by default. +- `DEFAULT_USER_IS_RESTRICTED`: **false**: Give new users restricted permissions by default - `DEFAULT_ENABLE_DEPENDENCIES`: **true**: Enable this to have dependencies enabled by default. - `ALLOW_CROSS_REPOSITORY_DEPENDENCIES` : **true** Enable this to allow dependencies on issues from any repository where the user is granted access. - `ENABLE_USER_HEATMAP`: **true**: Enable this to display the heatmap on users profiles. diff --git a/integrations/mssql.ini.tmpl b/integrations/mssql.ini.tmpl index 1867070ff5ef2..6532add36a4b7 100644 --- a/integrations/mssql.ini.tmpl +++ b/integrations/mssql.ini.tmpl @@ -65,6 +65,7 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true +DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org ENABLE_NOTIFY_MAIL = true diff --git a/integrations/mysql.ini.tmpl b/integrations/mysql.ini.tmpl index 176992cb26d0a..dd6620863bf76 100644 --- a/integrations/mysql.ini.tmpl +++ b/integrations/mysql.ini.tmpl @@ -85,6 +85,7 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true +DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org ENABLE_NOTIFY_MAIL = true diff --git a/integrations/mysql8.ini.tmpl b/integrations/mysql8.ini.tmpl index 7c5bcb58dc5f5..90dc674ab923b 100644 --- a/integrations/mysql8.ini.tmpl +++ b/integrations/mysql8.ini.tmpl @@ -63,6 +63,7 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true +DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org [picture] diff --git a/integrations/pgsql.ini.tmpl b/integrations/pgsql.ini.tmpl index 3a4a5e6c4fe52..b91b10beb786b 100644 --- a/integrations/pgsql.ini.tmpl +++ b/integrations/pgsql.ini.tmpl @@ -66,6 +66,7 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true +DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org ENABLE_NOTIFY_MAIL = true diff --git a/integrations/sqlite.ini.tmpl b/integrations/sqlite.ini.tmpl index 4a796e9317871..8e5662f2bc2d9 100644 --- a/integrations/sqlite.ini.tmpl +++ b/integrations/sqlite.ini.tmpl @@ -62,6 +62,7 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true +DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org [picture] diff --git a/models/user.go b/models/user.go index 5998341422197..5f066c47c1331 100644 --- a/models/user.go +++ b/models/user.go @@ -843,6 +843,7 @@ func CreateUser(u *User) (err error) { } u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization && !setting.Admin.DisableRegularOrgCreation u.EmailNotificationsPreference = setting.Admin.DefaultEmailNotification + u.IsRestricted = setting.Service.DefaultUserIsRestricted u.MaxRepoCreation = -1 u.Theme = setting.UI.DefaultTheme diff --git a/models/user_test.go b/models/user_test.go index 39a1b3c989c05..5f123c7d5366d 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -322,6 +322,28 @@ func TestCreateUser(t *testing.T) { assert.NoError(t, DeleteUser(user)) } +func TestCreateUserWithRestrictedUserByDefault(t *testing.T) { + user := &User{ + Name: "GiteaBot", + Email: "GiteaBot@gitea.io", + Passwd: ";p['////..-++']", + IsAdmin: false, + Theme: setting.UI.DefaultTheme, + MustChangePassword: false, + } + + setting.Service.DefaultUserIsRestricted = true + + assert.NoError(t, CreateUser(user)) + + savedUser, err := GetUserByEmail(user.Email) + assert.NoError(t, err) + + assert.Equal(t, savedUser.IsRestricted, true) + + assert.NoError(t, DeleteUser(savedUser)) +} + func TestCreateUserInvalidEmail(t *testing.T) { user := &User{ Name: "GiteaBot", diff --git a/modules/setting/service.go b/modules/setting/service.go index 41e834e8e61ef..17cc8764a57aa 100644 --- a/modules/setting/service.go +++ b/modules/setting/service.go @@ -44,6 +44,7 @@ var Service struct { HcaptchaSitekey string DefaultKeepEmailPrivate bool DefaultAllowCreateOrganization bool + DefaultUserIsRestricted bool EnableTimetracking bool DefaultEnableTimetracking bool DefaultEnableDependencies bool @@ -105,6 +106,7 @@ func newService() { Service.HcaptchaSitekey = sec.Key("HCAPTCHA_SITEKEY").MustString("") Service.DefaultKeepEmailPrivate = sec.Key("DEFAULT_KEEP_EMAIL_PRIVATE").MustBool() Service.DefaultAllowCreateOrganization = sec.Key("DEFAULT_ALLOW_CREATE_ORGANIZATION").MustBool(true) + Service.DefaultUserIsRestricted = sec.Key("DEFAULT_USER_IS_RESTRICTED").MustBool(false) Service.EnableTimetracking = sec.Key("ENABLE_TIMETRACKING").MustBool(true) if Service.EnableTimetracking { Service.DefaultEnableTimetracking = sec.Key("DEFAULT_ENABLE_TIMETRACKING").MustBool(true) From 0a271212ef834125ad05337d136a230cd1b32ea5 Mon Sep 17 00:00:00 2001 From: Richard Nienaber Date: Sat, 26 Jun 2021 15:52:37 +0100 Subject: [PATCH 2/6] default IsRestricted permission only set on sign up setting this in the model messes with other workflows (e.g. syncing LDAP users) where the IsRestricted permission needs to be explicitly set and not overridden by a config value --- integrations/signup_test.go | 19 +++++++++++++++++++ models/user.go | 1 - models/user_test.go | 22 ---------------------- routers/web/user/auth.go | 9 +++++---- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/integrations/signup_test.go b/integrations/signup_test.go index 5208a42ce5918..0b8cd194b963e 100644 --- a/integrations/signup_test.go +++ b/integrations/signup_test.go @@ -33,6 +33,25 @@ func TestSignup(t *testing.T) { MakeRequest(t, req, http.StatusOK) } +func TestSignupAsRestricted(t *testing.T) { + defer prepareTestEnv(t)() + + setting.Service.EnableCaptcha = false + setting.Service.DefaultUserIsRestricted = true + + req := NewRequestWithValues(t, "POST", "/user/sign_up", map[string]string{ + "user_name": "restrictedUser", + "email": "restrictedUser@example.com", + "password": "examplePassword!1", + "retype": "examplePassword!1", + }) + MakeRequest(t, req, http.StatusFound) + + // should be able to view new user's page + req = NewRequest(t, "GET", "/restrictedUser") + MakeRequest(t, req, http.StatusOK) +} + func TestSignupEmail(t *testing.T) { defer prepareTestEnv(t)() diff --git a/models/user.go b/models/user.go index 5f066c47c1331..5998341422197 100644 --- a/models/user.go +++ b/models/user.go @@ -843,7 +843,6 @@ func CreateUser(u *User) (err error) { } u.AllowCreateOrganization = setting.Service.DefaultAllowCreateOrganization && !setting.Admin.DisableRegularOrgCreation u.EmailNotificationsPreference = setting.Admin.DefaultEmailNotification - u.IsRestricted = setting.Service.DefaultUserIsRestricted u.MaxRepoCreation = -1 u.Theme = setting.UI.DefaultTheme diff --git a/models/user_test.go b/models/user_test.go index 5f123c7d5366d..39a1b3c989c05 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -322,28 +322,6 @@ func TestCreateUser(t *testing.T) { assert.NoError(t, DeleteUser(user)) } -func TestCreateUserWithRestrictedUserByDefault(t *testing.T) { - user := &User{ - Name: "GiteaBot", - Email: "GiteaBot@gitea.io", - Passwd: ";p['////..-++']", - IsAdmin: false, - Theme: setting.UI.DefaultTheme, - MustChangePassword: false, - } - - setting.Service.DefaultUserIsRestricted = true - - assert.NoError(t, CreateUser(user)) - - savedUser, err := GetUserByEmail(user.Email) - assert.NoError(t, err) - - assert.Equal(t, savedUser.IsRestricted, true) - - assert.NoError(t, DeleteUser(savedUser)) -} - func TestCreateUserInvalidEmail(t *testing.T) { user := &User{ Name: "GiteaBot", diff --git a/routers/web/user/auth.go b/routers/web/user/auth.go index 827b7cdef0651..10608841b3fc7 100644 --- a/routers/web/user/auth.go +++ b/routers/web/user/auth.go @@ -1204,10 +1204,11 @@ func SignUpPost(ctx *context.Context) { } u := &models.User{ - Name: form.UserName, - Email: form.Email, - Passwd: form.Password, - IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), + Name: form.UserName, + Email: form.Email, + Passwd: form.Password, + IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), + IsRestricted: setting.Service.DefaultUserIsRestricted, } if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, false) { From e23a876447c20b8a0e0f48da837ecf89b3fbf570 Mon Sep 17 00:00:00 2001 From: Richard Nienaber Date: Sun, 4 Jul 2021 21:24:52 +0100 Subject: [PATCH 3/6] fix formatting --- integrations/sqlite.ini.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/sqlite.ini.tmpl b/integrations/sqlite.ini.tmpl index 8e5662f2bc2d9..c686b6df966c5 100644 --- a/integrations/sqlite.ini.tmpl +++ b/integrations/sqlite.ini.tmpl @@ -62,7 +62,7 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true -DEFAULT_USER_IS_RESTRICTED = false +DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org [picture] From 0c054048d5d64bc3526ad2ae00fcc390351a33be Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 6 Jul 2021 04:12:21 +0200 Subject: [PATCH 4/6] Apply suggestions from code review --- custom/conf/app.example.ini | 2 +- integrations/mssql.ini.tmpl | 1 - integrations/mysql.ini.tmpl | 1 - integrations/mysql8.ini.tmpl | 1 - integrations/pgsql.ini.tmpl | 1 - integrations/sqlite.ini.tmpl | 1 - 6 files changed, 1 insertion(+), 6 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index edf7347934f59..d7e1f05ba1bcc 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -651,7 +651,7 @@ PATH = ;DEFAULT_ALLOW_CREATE_ORGANIZATION = true ;; Default value for IsRestricted ;; Every new user will have restricted permissions depending on this setting -;DEFAULT_USER_IS_RESTRICTED = true +;DEFAULT_USER_IS_RESTRICTED = false ;; ;; Either "public", "limited" or "private", default is "public" ;; Limited is for signed user only diff --git a/integrations/mssql.ini.tmpl b/integrations/mssql.ini.tmpl index 6532add36a4b7..1867070ff5ef2 100644 --- a/integrations/mssql.ini.tmpl +++ b/integrations/mssql.ini.tmpl @@ -65,7 +65,6 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true -DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org ENABLE_NOTIFY_MAIL = true diff --git a/integrations/mysql.ini.tmpl b/integrations/mysql.ini.tmpl index dd6620863bf76..176992cb26d0a 100644 --- a/integrations/mysql.ini.tmpl +++ b/integrations/mysql.ini.tmpl @@ -85,7 +85,6 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true -DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org ENABLE_NOTIFY_MAIL = true diff --git a/integrations/mysql8.ini.tmpl b/integrations/mysql8.ini.tmpl index 90dc674ab923b..7c5bcb58dc5f5 100644 --- a/integrations/mysql8.ini.tmpl +++ b/integrations/mysql8.ini.tmpl @@ -63,7 +63,6 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true -DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org [picture] diff --git a/integrations/pgsql.ini.tmpl b/integrations/pgsql.ini.tmpl index b91b10beb786b..3a4a5e6c4fe52 100644 --- a/integrations/pgsql.ini.tmpl +++ b/integrations/pgsql.ini.tmpl @@ -66,7 +66,6 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true -DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org ENABLE_NOTIFY_MAIL = true diff --git a/integrations/sqlite.ini.tmpl b/integrations/sqlite.ini.tmpl index c686b6df966c5..4a796e9317871 100644 --- a/integrations/sqlite.ini.tmpl +++ b/integrations/sqlite.ini.tmpl @@ -62,7 +62,6 @@ ENABLE_CAPTCHA = false REQUIRE_SIGNIN_VIEW = false DEFAULT_KEEP_EMAIL_PRIVATE = false DEFAULT_ALLOW_CREATE_ORGANIZATION = true -DEFAULT_USER_IS_RESTRICTED = false NO_REPLY_ADDRESS = noreply.example.org [picture] From 607ccd4ffbaf36559801ea035bbf9604d62f20da Mon Sep 17 00:00:00 2001 From: Richard Nienaber Date: Tue, 6 Jul 2021 07:50:07 +0100 Subject: [PATCH 5/6] ensure newly created user is set to restricted --- integrations/signup_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/integrations/signup_test.go b/integrations/signup_test.go index 0b8cd194b963e..a70f998d9b293 100644 --- a/integrations/signup_test.go +++ b/integrations/signup_test.go @@ -5,6 +5,7 @@ package integrations import ( + "code.gitea.io/gitea/models" "fmt" "net/http" "strings" @@ -50,6 +51,9 @@ func TestSignupAsRestricted(t *testing.T) { // should be able to view new user's page req = NewRequest(t, "GET", "/restrictedUser") MakeRequest(t, req, http.StatusOK) + + user2 := models.AssertExistsAndLoadBean(t, &models.User{Name: "restrictedUser"}).(*models.User) + assert.True(t, user2.IsRestricted) } func TestSignupEmail(t *testing.T) { From 32717a8c0d096e9a7f6b572f2b5e6df160a99494 Mon Sep 17 00:00:00 2001 From: Richard Nienaber Date: Tue, 6 Jul 2021 08:25:15 +0100 Subject: [PATCH 6/6] ensure imports are in the correct order --- integrations/signup_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/signup_test.go b/integrations/signup_test.go index a70f998d9b293..66ff8ac2d72ff 100644 --- a/integrations/signup_test.go +++ b/integrations/signup_test.go @@ -5,12 +5,12 @@ package integrations import ( - "code.gitea.io/gitea/models" "fmt" "net/http" "strings" "testing" + "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" "github.com/unknwon/i18n"