separate user and admin notifications

The current state is a mess of user and admin notifications, which will become worse after implementing the new user notification methods like a telegram. This change makes things simpler for the remark42 users.
umputun · Jun 3, 2021 · c0b392a · c0b392a
1 parent 97934e2
commit c0b392a
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 63 deletions.
diff --git a/README.md b/README.md
@@ -156,14 +156,14 @@ _this is the recommended way to run remark42_
 | auth.email.subj         | AUTH_EMAIL_SUBJ         | `remark42 confirmation`  | email subject                                   |
 | auth.email.content-type | AUTH_EMAIL_CONTENT_TYPE | `text/html`              | email content type                              |
 | auth.email.template     | AUTH_EMAIL_TEMPLATE     | none (predefined)        | custom email message template file              |
-| notify.type             | NOTIFY_TYPE             | none                     | type of notification (telegram, slack and/or email) |
+| notify.users            | NOTIFY_USERS            | none                     | type of user notifications (email)              |
+| notify.admins           | NOTIFY_ADMINS           | none                     | type of admin notifications (telegram, slack and/or email) |
 | notify.queue            | NOTIFY_QUEUE            | `100`                    | size of notification queue                      |
 | notify.telegram.chan    | NOTIFY_TELEGRAM_CHAN    |                          | telegram channel                                |
 | notify.slack.token      | NOTIFY_SLACK_TOKEN      |                          | slack token                                     |
 | notify.slack.chan       | NOTIFY_SLACK_CHAN       | `general`                | slack channel                                   |
 | notify.email.fromAddress | NOTIFY_EMAIL_FROM      |                          | from email address                              |
 | notify.email.verification_subj | NOTIFY_EMAIL_VERIFICATION_SUBJ | `Email verification` | verification message subject          |
-| notify.email.notify_admin | NOTIFY_EMAIL_ADMIN    | `false`                  | notify admin on new comments via ADMIN_SHARED_EMAIL |
 | telegram.token          | TELEGRAM_TOKEN          |                          | telegram token (used for auth and telegram notifications) |
 | telegram.timeout        | TELEGRAM_TIMEOUT        | `5s`                     | telegram connection timeout                     |
 | smtp.host               | SMTP_HOST               |                          | SMTP host                                       |
@@ -226,6 +226,8 @@ trouble with unrecognized command-line options in the future.
 | auth.email.tls     | smtp.tls      | AUTH_EMAIL_TLS     | SMTP_TLS      | `false` | enable TLS     | 1.5.0               |
 | auth.email.timeout | smtp.timeout  | AUTH_EMAIL_TIMEOUT | SMTP_TIMEOUT  | `10s`   | smtp timeout   | 1.5.0               |
 | img-proxy          | image-proxy.http2https | IMG_PROXY | IMAGE_PROXY_HTTP2HTTPS | `false` | enable http->https proxy for images | 1.5.0 |
+| notify.type        | notify.users, notify.admins | NOTIFY_TYPE       | NOTIFY_ADMINS, NOTIFY_USERS | 1.9.0               |
+| notify.email.notify_admin| notify.admins=email | NOTIFY_EMAIL_ADMIN          | NOTIFY_ADMINS=email | 1.9.0               |
 | notify.telegram.token | telegram.token | NOTIFY_TELEGRAM_TOKEN | TELEGRAM_TOKEN   | telegram token | 1.9.0               |
 | notify.telegram.timeout | telegram.timeout | NOTIFY_TELEGRAM_TIMEOUT | TELEGRAM_TIMEOUT | telegram timeout | 1.9.0       |
 </details>

diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go
@@ -210,7 +210,9 @@ type SMTPGroup struct {
 
 // NotifyGroup defines options for notification
 type NotifyGroup struct {
-	Type      []string `long:"type" env:"TYPE" description:"type of notification" choice:"none" choice:"telegram" choice:"email" choice:"slack" default:"none" env-delim:","` //nolint
+	Type      []string `long:"type" env:"TYPE" description:"[deprecated, use user and admin types instead] types of notifications" choice:"none" choice:"telegram" choice:"email" choice:"slack" default:"none" env-delim:","` //nolint
+	Users     []string `long:"users" env:"USERS" description:"types of user notifications" choice:"none" choice:"email" default:"none" env-delim:","`                                                                          //nolint
+	Admins    []string `long:"admins" env:"ADMINS" description:"types of admin notifications" choice:"none" choice:"telegram" choice:"email" choice:"slack" default:"none" env-delim:","`                                      //nolint
 	QueueSize int      `long:"queue" env:"QUEUE" description:"size of notification queue" default:"100"`
 	Telegram  struct {
 		Channel string        `long:"chan" env:"CHAN" description:"telegram channel"`
@@ -221,7 +223,7 @@ type NotifyGroup struct {
 	Email struct {
 		From                string `long:"from_address" env:"FROM" description:"from email address"`
 		VerificationSubject string `long:"verification_subj" env:"VERIFICATION_SUBJ" description:"verification message subject"`
-		AdminNotifications  bool   `long:"notify_admin" env:"ADMIN" description:"notify admin on new comments via ADMIN_SHARED_EMAIL"`
+		AdminNotifications  bool   `long:"notify_admin" env:"ADMIN" description:"[deprecated, use --notify.admins=email] notify admin on new comments via ADMIN_SHARED_EMAIL"`
 	} `group:"email" namespace:"email" env-namespace:"EMAIL"`
 	Slack struct {
 		Token   string `long:"token" env:"TOKEN" description:"slack token"`
@@ -342,6 +344,14 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) {
 		s.ImageProxy.HTTP2HTTPS = s.LegacyImageProxy
 		result = append(result, DeprecatedFlag{Old: "img-proxy", New: "image-proxy.http2https", Version: "1.5"})
 	}
+	if len(s.Notify.Type) != 0 && (len(s.Notify.Users) != 0 || len(s.Notify.Admins) != 0) {
+		s.handleDeprecatedNotifications()
+		result = append(result, DeprecatedFlag{Old: "notify.type", New: "notify.(users|admins)", Version: "1.9"})
+	}
+	if s.Notify.Email.AdminNotifications && !contains("email", s.Notify.Admins) {
+		s.Notify.Admins = append(s.Notify.Admins, "email")
+		result = append(result, DeprecatedFlag{Old: "notify.email.notify_admin", New: "notify.admins=email", Version: "1.9"})
+	}
 	if s.Notify.Telegram.Token != "" && s.Telegram.Token == "" {
 		s.Telegram.Token = s.Notify.Telegram.Token
 		result = append(result, DeprecatedFlag{Old: "notify.telegram.token", New: "telegram.token", Version: "1.9"})
@@ -354,6 +364,26 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) {
 	return result
 }
 
+func (s *ServerCommand) handleDeprecatedNotifications() {
+	for _, t := range s.Notify.Type {
+		if t == "email" && !contains(t, s.Notify.Users) {
+			s.Notify.Users = append(s.Notify.Users, t)
+		}
+		if (t == "telegram" || t == "slack") && !contains(t, s.Notify.Admins) {
+			s.Notify.Admins = append(s.Notify.Admins, t)
+		}
+	}
+}
+
+func contains(s string, a []string) bool {
+	for _, t := range a {
+		if t == s {
+			return true
+		}
+	}
+	return false
+}
+
 // newServerApp prepares application and return it with all active parts
 // doesn't start anything
 func (s *ServerCommand) newServerApp() (*serverApp, error) {
@@ -431,11 +461,8 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) {
 	var emailNotifications bool
 	notifyService, err := s.makeNotify(dataService, authenticator)
 
-	for _, t := range s.Notify.Type {
-		switch t {
-		case "email":
-			emailNotifications = true
-		}
+	if contains("email", s.Notify.Users) {
+		emailNotifications = true
 	}
 
 	if err != nil {
@@ -841,7 +868,7 @@ func (s *ServerCommand) loadEmailTemplate() (string, error) {
 func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator *auth.Service) (*notify.Service, error) {
 	var notifyService *notify.Service
 	var destinations []notify.Destination
-	for _, t := range s.Notify.Type {
+	for _, t := range s.Notify.Admins {
 		switch t {
 		case "slack":
 			slack, err := notify.NewSlack(s.Notify.Slack.Token, s.Notify.Slack.Channel)
@@ -857,56 +884,71 @@ func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator *
 			}
 			destinations = append(destinations, tg)
 		case "email":
-			emailParams := notify.EmailParams{
-				MsgTemplatePath:          s.emailMsgTemplatePath,
-				VerificationTemplatePath: s.emailVerificationTemplatePath,
-				From:                     s.Notify.Email.From,
-				VerificationSubject:      s.Notify.Email.VerificationSubject,
-				UnsubscribeURL:           s.RemarkURL + "/email/unsubscribe.html",
-				// TODO: uncomment after #560 frontend part is ready and URL is known
-				// SubscribeURL:        s.RemarkURL + "/subscribe.html?token=",
-				TokenGenFn: func(userID, email, site string) (string, error) {
-					claims := token.Claims{
-						Handshake: &token.Handshake{ID: userID + "::" + email},
-						StandardClaims: jwt.StandardClaims{
-							Audience:  site,
-							ExpiresAt: time.Now().Add(100 * 365 * 24 * time.Hour).Unix(),
-							NotBefore: time.Now().Add(-1 * time.Minute).Unix(),
-							Issuer:    "remark42",
-						},
-					}
-					tkn, err := authenticator.TokenService().Token(claims)
-					if err != nil {
-						return "", errors.Wrapf(err, "failed to make unsubscription token")
-					}
-					return tkn, nil
-				},
-			}
-			if s.Notify.Email.AdminNotifications {
-				emailParams.AdminEmails = s.Admin.Shared.Email
-			}
-			smtpParams := notify.SMTPParams{
-				Host:     s.SMTP.Host,
-				Port:     s.SMTP.Port,
-				TLS:      s.SMTP.TLS,
-				Username: s.SMTP.Username,
-				Password: s.SMTP.Password,
-				TimeOut:  s.SMTP.TimeOut,
-			}
-			emailService, err := notify.NewEmail(emailParams, smtpParams)
-			if err != nil {
-				return nil, errors.Wrap(err, "failed to create email notification destination")
-			}
-			destinations = append(destinations, emailService)
 		case "none":
 			notifyService = notify.NopService
 		default:
 			return nil, errors.Errorf("unsupported notification type %q", s.Notify.Type)
 		}
 	}
 
+	for _, t := range s.Notify.Users {
+		switch t {
+		case "email":
+		case "none":
+			notifyService = notify.NopService
+		default:
+			return nil, errors.Errorf("unsupported notification type %q", s.Notify.Type)
+		}
+	}
+
+	// with logic below admin notifications enable notifications for users on the backend even if they
+	// are not enabled explicitly, however they won't be visible to the users in the frontend
+	// because api.Rest.EmailNotifications would be set to false.
+	if contains("email", s.Notify.Users) || contains("email", s.Notify.Admins) {
+		emailParams := notify.EmailParams{
+			MsgTemplatePath:          s.emailMsgTemplatePath,
+			VerificationTemplatePath: s.emailVerificationTemplatePath, From: s.Notify.Email.From,
+			VerificationSubject: s.Notify.Email.VerificationSubject,
+			UnsubscribeURL:      s.RemarkURL + "/email/unsubscribe.html",
+			// TODO: uncomment after #560 frontend part is ready and URL is known
+			// SubscribeURL:        s.RemarkURL + "/subscribe.html?token=",
+			TokenGenFn: func(userID, email, site string) (string, error) {
+				claims := token.Claims{
+					Handshake: &token.Handshake{ID: userID + "::" + email},
+					StandardClaims: jwt.StandardClaims{
+						Audience:  site,
+						ExpiresAt: time.Now().Add(100 * 365 * 24 * time.Hour).Unix(),
+						NotBefore: time.Now().Add(-1 * time.Minute).Unix(),
+						Issuer:    "remark42",
+					},
+				}
+				tkn, err := authenticator.TokenService().Token(claims)
+				if err != nil {
+					return "", errors.Wrapf(err, "failed to make unsubscription token")
+				}
+				return tkn, nil
+			},
+		}
+		if contains("email", s.Notify.Admins) {
+			emailParams.AdminEmails = s.Admin.Shared.Email
+		}
+		smtpParams := notify.SMTPParams{
+			Host:     s.SMTP.Host,
+			Port:     s.SMTP.Port,
+			TLS:      s.SMTP.TLS,
+			Username: s.SMTP.Username,
+			Password: s.SMTP.Password,
+			TimeOut:  s.SMTP.TimeOut,
+		}
+		emailService, err := notify.NewEmail(emailParams, smtpParams)
+		if err != nil {
+			return nil, errors.Wrap(err, "failed to create email notification destination")
+		}
+		destinations = append(destinations, emailService)
+	}
+
 	if len(destinations) > 0 {
-		log.Printf("[INFO] make notify, types=%s", s.Notify.Type)
+		log.Printf("[INFO] make notify, for users: %s, for admins: %s", s.Notify.Users, s.Notify.Admins)
 		notifyService = notify.NewService(dataStore, s.Notify.QueueSize, destinations...)
 	}
 	return notifyService, nil

diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go
@@ -222,7 +222,7 @@ func TestServerApp_WithSSL(t *testing.T) {
 	p := flags.NewParser(&opts, flags.Default)
 	port := chooseRandomUnusedPort()
 	_, err := p.ParseArgs([]string{"--admin-passwd=password", "--port=" + strconv.Itoa(port), "--store.bolt.path=/tmp/xyz", "--backup=/tmp",
-		"--avatar.type=bolt", "--avatar.bolt.file=/tmp/ava-test.db", "--notify.type=none",
+		"--avatar.type=bolt", "--avatar.bolt.file=/tmp/ava-test.db",
 		"--ssl.type=static", "--ssl.cert=testdata/cert.pem", "--ssl.key=testdata/key.pem",
 		"--ssl.port=" + strconv.Itoa(sslPort), "--image.fs.path=/tmp"})
 	require.NoError(t, err)
@@ -392,7 +392,7 @@ func TestServerApp_MainSignal(t *testing.T) {
 	p := flags.NewParser(&s, flags.Default)
 	port := chooseRandomUnusedPort()
 	args := []string{"test", "--store.bolt.path=/tmp/xyz", "--backup=/tmp", "--avatar.type=bolt",
-		"--avatar.bolt.file=/tmp/ava-test.db", "--port=" + strconv.Itoa(port), "--notify.type=none", "--image.fs.path=/tmp"}
+		"--avatar.bolt.file=/tmp/ava-test.db", "--port=" + strconv.Itoa(port), "--image.fs.path=/tmp"}
 	defer os.Remove("/tmp/ava-test.db")
 	_, err := p.ParseArgs(args)
 	require.NoError(t, err)
@@ -436,6 +436,7 @@ func TestServerApp_DeprecatedArgs(t *testing.T) {
 			{Old: "auth.email.passwd", New: "smtp.password", Version: "1.5"},
 			{Old: "auth.email.timeout", New: "smtp.timeout", Version: "1.5"},
 			{Old: "auth.email.template", Version: "1.5"},
+			{Old: "notify.type", New: "notify.(users|admins)", Version: "1.9"},
 		},
 		deprecatedFlags)
 	assert.Equal(t, "smtp.example.org", s.SMTP.Host)
@@ -670,7 +671,8 @@ func prepServerApp(t *testing.T, fn func(o ServerCommand) ServerCommand) (*serve
 	cmd.Auth.Email.Enable = true
 	cmd.Auth.Email.MsgTemplate = "testdata/email.tmpl"
 	cmd.BackupLocation = "/tmp"
-	cmd.Notify.Type = []string{"email"}
+	cmd.Notify.Users = []string{"email"}
+	cmd.Notify.Admins = []string{"email"}
 	cmd.Notify.Email.From = "[email protected]"
 	cmd.Notify.Email.VerificationSubject = "test verification email subject"
 	cmd.SMTP.Host = "127.0.0.1"

diff --git a/compose-dev-backend.yml b/compose-dev-backend.yml
@@ -41,7 +41,8 @@ services:
             - ADMIN_PASSWD=password
             - AUTH_DEV=true # activate local oauth "dev"
             - ADMIN_SHARED_ID=dev_user # set admin flag for default user on local ouath2
-            - NOTIFY_TYPE
+            - NOTIFY_USERS
+            - NOTIFY_ADMINS
             - TELEGRAM_TOKEN
             - NOTIFY_TELEGRAM_CHAN
             - NOTIFY_EMAIL_FROM

diff --git a/docs/latest/email.md b/docs/latest/email.md
@@ -198,18 +198,18 @@ See [verified-authentication](https://github.com/go-pkgz/auth#verified-authentic
 Here is the list of variables which affect email notifications:
 
 ```yaml
-NOTIFY_TYPE
+NOTIFY_USERS
 NOTIFY_EMAIL_FROM
 NOTIFY_EMAIL_VERIFICATION_SUBJ
 # for administrator notifications for new comments on their site
 ADMIN_SHARED_EMAIL
-NOTIFY_EMAIL_ADMIN
+NOTIFY_ADMINS
 ```
 
 After you set `SMTP_` variables, you can allow email notifications by setting these two variables:
 
 ```yaml
-- NOTIFY_TYPE=email
-# - NOTIFY_TYPE=email,telegram # this is in case you want to have both email and telegram notifications enabled
+- NOTIFY_USERS=email
+# - NOTIFY_USERS=email,telegram # this is in case you want to have both email and telegram notifications enabled
 - [email protected]
 ```
diff --git a/docs/latest/slack.md b/docs/latest/slack.md
@@ -20,12 +20,12 @@ In order to integrate notifications from remark42 with the [slack](https://slack
 
 The slack token which you obtained before should be used as `NOTIFY_SLACK_TOKEN`. 
 
-You also need to set `NOTIFY_TYPE=slack` for the slack notification to be active.
+You also need to set `NOTIFY_ADMINS=slack` for the slack notification to be active.
 
 By default, the notification are sent to the `general` channel on slack. If you need another channel, you can specify it, for instance with `NOTIFY_SLACK_CHAN=random`.
 
 ```
-    - NOTIFY_TYPE=slack
+    - NOTIFY_ADMINS=slack
     - NOTIFY_SLACK_CHAN=general
     - NOTIFY_SLACK_TOKEN=xoxb-....
 ```

diff --git a/docs/latest/telegram.md b/docs/latest/telegram.md
@@ -4,7 +4,7 @@ title: Telegram
 
 ## Telegram notifications
 
-In order to integrate notifications from remark42 with the [telegram](https://telegram.org), you should make [a channel](https://telegram.org/faq_channels) and obtain a token. This token should be used as `TELEGRAM_TOKEN`. You also need to set `NOTIFY_TYPE=telegram` and set `NOTIFY_TELEGRAM_CHAN` to your channel.
+In order to integrate notifications from remark42 with the [telegram](https://telegram.org), you should make [a channel](https://telegram.org/faq_channels) and obtain a token. This token should be used as `TELEGRAM_TOKEN`. You also need to set `NOTIFY_ADMINS=telegram` and set `NOTIFY_TELEGRAM_CHAN` to your channel.
 
 In order to get token "just talk to [BotFather](https://core.telegram.org/bots#6-botfather)". All you need is to send `/newbot` command, and choose the name for your bot (it must end in `bot`). This is it, you got a token.