Repository: owncloud/ocis
Pull request #2495 (2021-09-14 07:25:38)

Add option to generate the demo users
<!--
Thanks for submitting a change to ownCloud!

This is the bug tracker for the oCIS component. Find other components at https://github.com/owncloud/core/blob/master/.github/CONTRIBUTING.md#guidelines

For fixing potential security issues please see https://owncloud.org/security/

To make it possible for us to get your change reviewed and merged please carefully fill out the requested information below.

Please note that any kind of change needs first be submitted to the master branch which holds the next version of oCIS.

Please set the following labels:

- Set label "Status:Needs-Review" for review or "Status:In-Progress" if the PR still has open tasks.
- Assignment: assign to self
- Reviewers: pick at least one
-->

## Description
Include an option to generate the demo users and groups. Only a minimal set of users and groups will be generated automatically unless the new option is enabled.

So far the minimal set includes:
* for users -> the reva IOP, the Kopano IDP, and the admin user. (passwords remain unchanged)
* for groups -> the sysusers group, and the users group.

## Related Issue
https://github.com/owncloud/ocis/issues/2484

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?
* Start a fresh installation with the new option activated -> the demo users will be generated as it has been happening until now.
* Start a fresh installation without the new option -> only a minimal set of users and groups will be generated.

## Screenshots (if appropriate):

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Technical debt
- [ ] Tests only (no source changes)

## Checklist:
<!-- Tick the checkboxes when done. -->
<!-- Raise documentation ticket in owncloud.github.io/ -->
- [x] Code changes
- [ ] Unit tests added
- [ ] Acceptance tests added
- [ ] Documentation ticket raised: <link> 


update-docs[bot]: Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a [changelog](https://github.com/owncloud/ocis/blob/master/changelog/README.md) item based on your changes.

wkloucek: @jvillafanez I'm not sure if we should stick with the default to create demo users as long as we are in the tech preview stage.
We also should mention this new config option in the docs here https://owncloud.dev/ocis/deployment/#secure-an-ocis-instance

jvillafanez: I agree. We can merge this (it seems there are tests to adjust) once we get close to the actual product release
wkloucek: > I agree. We can merge this (it seems there are tests to adjust) once we get close to the actual product release

No, I would love to have this right now. I just wanted to have the demo users as a default as of now ;-) So basically an opt-out instead of an opt-in to demo users...
jvillafanez: From a product perspective, I think this option is better. Usually, you don't want to create demo users that you need to delete afterwards. This is why we don't want to create the demo users by default.
The demo users are a convenience now during the tech preview, but later they'll be in the way.

I'd vote for keeping this behavior.
C0rby: > From a product perspective, I think this option is better. Usually, you don't want to create demo users that you need to delete afterwards.

That is true but currently we are still heavily in dev mode and are using the demo users during development. I think it would make developing a little bit more annoying if we have to explicitly generate the accounts every time we delete the ocis storage.

So I'm agreeing with @wkloucek here. I'd like to have an opt-out option for now and later on when we have more "productive" deployments we can always (and should) switch to opt-in.

We could then also move this code to a command in the accounts service like `./ocis accounts create-demo-users` or something like that.
jvillafanez: Turns out it's easier than initially thought... The default value is set to true, and we only need to change it to false (or remove it) when we no longer want to create the demo users by default. In any case, the environment variable will be respected if present.

Patch:
diff --git a/accounts/pkg/config/config.go b/accounts/pkg/config/config.go
index 8863919429b..c63c289f7ea 100644
--- a/accounts/pkg/config/config.go
+++ b/accounts/pkg/config/config.go
@@ -42,9 +42,10 @@ type GRPC struct {
 
 // Server configures a server.
 type Server struct {
-	Version        string
-	Name           string
-	HashDifficulty int
+	Version            string
+	Name               string
+	HashDifficulty     int
+	DemoUsersAndGroups bool
 }
 
 // Asset defines the available asset configuration.
diff --git a/accounts/pkg/flagset/flagset.go b/accounts/pkg/flagset/flagset.go
index be9cc3fc9af..4a7954f6626 100644
--- a/accounts/pkg/flagset/flagset.go
+++ b/accounts/pkg/flagset/flagset.go
@@ -134,6 +134,13 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
 			EnvVars:     []string{"ACCOUNTS_HASH_DIFFICULTY"},
 			Destination: &cfg.Server.HashDifficulty,
 		},
+		&cli.BoolFlag{
+			Name:        "demo-users-and-groups",
+			Value:       flags.OverrideDefaultBool(cfg.Server.DemoUsersAndGroups, true),
+			Usage:       "Enable demo users and groups",
+			EnvVars:     []string{"ACCOUNTS_DEMO_USERS_AND_GROUPS"},
+			Destination: &cfg.Server.DemoUsersAndGroups,
+		},
 		&cli.StringFlag{
 			Name:        "asset-path",
 			Value:       flags.OverrideDefaultString(cfg.Asset.Path, ""),
diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go
index dae86a5d254..ff6059f143e 100644
--- a/accounts/pkg/service/v0/service.go
+++ b/accounts/pkg/service/v0/service.go
@@ -73,11 +73,11 @@ func New(opts ...Option) (s *Service, err error) {
 		return nil, err
 	}
 
-	if err = s.createDefaultAccounts(); err != nil {
+	if err = s.createDefaultAccounts(cfg.Server.DemoUsersAndGroups); err != nil {
 		return nil, err
 	}
 
-	if err = s.createDefaultGroups(); err != nil {
+	if err = s.createDefaultGroups(cfg.Server.DemoUsersAndGroups); err != nil {
 		return nil, err
 	}
 	return
@@ -152,7 +152,7 @@ func configFromSvc(cfg *config.Config) (*idxcfg.Config, error) {
 	return c, nil
 }
 
-func (s Service) createDefaultAccounts() (err error) {
+func (s Service) createDefaultAccounts(withDemoAccounts bool) (err error) {
 	accounts := []proto.Account{
 		{
 			Id:                       "4c510ada-c86b-4815-8820-42cdf82c3d51",
@@ -278,8 +278,19 @@ func (s Service) createDefaultAccounts() (err error) {
 			},
 		},
 	}
+
+	mustHaveAccounts := map[string]bool{
+		"bc596f3c-c955-4328-80a0-60d018b4ad57": true, // Reva IOP
+		"820ba2a1-3f54-4538-80a4-2d73007e30bf": true, // Kopano IDP
+		"ddc2004c-0977-11eb-9d3f-a793888cd0f8": true, // admin
+	}
+
 	// this only deals with the metadata service.
 	for i := range accounts {
+		if !withDemoAccounts && !mustHaveAccounts[accounts[i].Id] {
+			continue
+		}
+
 		a := &proto.Account{}
 		err := s.repo.LoadAccount(context.Background(), accounts[i].Id, a)
 		if !storage.IsNotFoundErr(err) {
@@ -323,7 +334,7 @@ func (s Service) createDefaultAccounts() (err error) {
 	return nil
 }
 
-func (s Service) createDefaultGroups() (err error) {
+func (s Service) createDefaultGroups(withDemoGroups bool) (err error) {
 	groups := []proto.Group{
 		{Id: "34f38767-c937-4eb6-b847-1c175829a2a0", GidNumber: 15000, OnPremisesSamAccountName: "sysusers", DisplayName: "Technical users", Description: "A group for technical users. They should not show up in sharing dialogs.", Members: []*proto.Account{
 			{Id: "820ba2a1-3f54-4538-80a4-2d73007e30bf"}, // idp
@@ -358,7 +369,17 @@ func (s Service) createDefaultGroups() (err error) {
 			{Id: "932b4540-8d16-481e-8ef4-588e4b6b151c"}, // feynman
 		}},
 	}
+
+	mustHaveGroups := map[string]bool{
+		"34f38767-c937-4eb6-b847-1c175829a2a0": true, // sysusers
+		"509a9dcd-bb37-4f4f-a01a-19dca27d9cfa": true, // users
+	}
+
 	for i := range groups {
+		if !withDemoGroups && !mustHaveGroups[groups[i].Id] {
+			continue
+		}
+
 		g := &proto.Group{}
 		err := s.repo.LoadGroup(context.Background(), groups[i].Id, g)
 		if !storage.IsNotFoundErr(err) {
diff --git a/changelog/unreleased/skip-demo-users.md b/changelog/unreleased/skip-demo-users.md
new file mode 100644
index 00000000000..bd0790af44f
--- /dev/null
+++ b/changelog/unreleased/skip-demo-users.md
@@ -0,0 +1,6 @@
+Enhancement: Demo users and groups won't be generated unless requested
+
+We've added a new environment variable to decide whether we should generate the demo users and groups or not. This environment variable is false by default, so the demo users and groups won't be generated by default.
+There are still some users and groups automatically generated: for users: Reva IOP, Kopano IDP, admin; for groups: sysusers and users.
+
+https://github.com/owncloud/ocis/pull/2495


Test patch:
diff --git a/accounts/pkg/proto/v0/accounts.pb.micro_test.go b/accounts/pkg/proto/v0/accounts.pb.micro_test.go
index 22328f2ce71..0f5cf724ed7 100644
--- a/accounts/pkg/proto/v0/accounts.pb.micro_test.go
+++ b/accounts/pkg/proto/v0/accounts.pb.micro_test.go
@@ -81,6 +81,7 @@ func init() {
 
 	cfg := config.New()
 	cfg.Repo.Disk.Path = dataPath
+	cfg.Server.DemoUsersAndGroups = true
 	var hdlr *svc.Service
 	var err error
 
diff --git a/ocs/pkg/server/http/svc_test.go b/ocs/pkg/server/http/svc_test.go
index 965edd3973a..7c46558f0ba 100644
--- a/ocs/pkg/server/http/svc_test.go
+++ b/ocs/pkg/server/http/svc_test.go
@@ -538,7 +538,9 @@ func init() {
 	)
 
 	c := &accountsCfg.Config{
-		Server: accountsCfg.Server{},
+		Server: accountsCfg.Server{
+			DemoUsersAndGroups: true,
+		},
 		Repo: accountsCfg.Repo{
 			Disk: accountsCfg.Disk{
 				Path: dataPath,