[MM-40962-2] - Fix unnecessary logging in license expiration check (#19826)

* [MM-40962-2] - Fix unnecessary logging in license expiration check

* skip check for TE

* document

* feedback impl-1

* fix logic

* remove saving of renewal token

* fix lint

* remove commented code

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>
Этот коммит содержится в:
Allan Guwatudde
2022-03-31 18:25:46 +03:00
коммит произвёл GitHub
родитель fa56ee9d9a
Коммит c17e210dd0
4 изменённых файлов: 20 добавлений и 103 удалений

Просмотреть файл

@@ -361,15 +361,10 @@ func (s *Server) RequestTrialLicense(trialRequest *model.TrialLicenseRequest) *m
return nil
}
// GenerateRenewalToken returns the current active token or generate a new one if
// the current active one has expired
// GenerateRenewalToken returns a renewal token that expires after duration expiration
func (s *Server) GenerateRenewalToken(expiration time.Duration) (string, *model.AppError) {
license := s.License()
if license == nil {
// Clean renewal token if there is no license present
if _, err := s.Store.System().PermanentDeleteByName(model.SystemLicenseRenewalToken); err != nil {
mlog.Warn("error removing the renewal token", mlog.Err(err))
}
return "", model.NewAppError("GenerateRenewalToken", "app.license.generate_renewal_token.no_license", nil, "", http.StatusBadRequest)
}
@@ -377,14 +372,6 @@ func (s *Server) GenerateRenewalToken(expiration time.Duration) (string, *model.
return "", model.NewAppError("GenerateRenewalToken", "app.license.generate_renewal_token.bad_license", nil, "", http.StatusBadRequest)
}
currentToken, _ := s.Store.System().GetByName(model.SystemLicenseRenewalToken)
if currentToken != nil {
tokenIsValid, _ := s.renewalTokenValid(currentToken.Value, license.Customer.Email)
if currentToken.Value != "" && tokenIsValid {
return currentToken.Value, nil
}
}
activeUsers, err := s.Store.User().Count(model.UserCountOptions{})
if err != nil {
return "", model.NewAppError("GenerateRenewalToken", "app.license.generate_renewal_token.app_error",
@@ -405,35 +392,10 @@ func (s *Server) GenerateRenewalToken(expiration time.Duration) (string, *model.
if err != nil {
return "", model.NewAppError("GenerateRenewalToken", "app.license.generate_renewal_token.app_error", nil, err.Error(), http.StatusInternalServerError)
}
err = s.Store.System().SaveOrUpdate(&model.System{
Name: model.SystemLicenseRenewalToken,
Value: tokenString,
})
if err != nil {
return "", model.NewAppError("GenerateRenewalToken", "app.license.generate_renewal_token.app_error", nil, err.Error(), http.StatusInternalServerError)
}
return tokenString, nil
}
func (s *Server) renewalTokenValid(tokenString, signingKey string) (bool, error) {
claims := &JWTClaims{}
token, err := jwt.ParseWithClaims(tokenString, claims, func(token *jwt.Token) (interface{}, error) {
return []byte(signingKey), nil
})
if err != nil {
return false, errors.Wrapf(err, "Error validating JWT token")
}
if !token.Valid {
return false, errors.New("invalid JWT token")
}
expirationTime := time.Unix(claims.ExpiresAt, 0)
if expirationTime.Before(time.Now().UTC()) {
return false, nil
}
return true, nil
}
// GenerateLicenseRenewalLink returns a link that points to the CWS where clients can renew license
func (s *Server) GenerateLicenseRenewalLink() (string, string, *model.AppError) {
renewalToken, err := s.GenerateRenewalToken(JWTDefaultTokenExpiration)

Просмотреть файл

@@ -4,11 +4,8 @@
package app
import (
"errors"
"testing"
"time"
"github.com/dgrijalva/jwt-go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -82,35 +79,11 @@ func TestGenerateRenewalToken(t *testing.T) {
th := Setup(t)
defer th.TearDown()
t.Run("test invalid token", func(t *testing.T) {
_, err := th.App.Srv().renewalTokenValid("badtoken", "")
var vErr *jwt.ValidationError
require.True(t, errors.As(err, &vErr))
})
t.Run("renewal token generated correctly", func(t *testing.T) {
setLicense(th, nil)
token, appErr := th.App.Srv().GenerateRenewalToken(JWTDefaultTokenExpiration)
require.Nil(t, appErr)
require.NotEmpty(t, token)
defer th.App.Srv().Store.System().PermanentDeleteByName(model.SystemLicenseRenewalToken)
customerEmail := th.App.Srv().License().Customer.Email
validToken, err := th.App.Srv().renewalTokenValid(token, customerEmail)
require.NoError(t, err)
require.True(t, validToken)
})
t.Run("only one token should be active", func(t *testing.T) {
setLicense(th, nil)
token, appErr := th.App.Srv().GenerateRenewalToken(JWTDefaultTokenExpiration)
require.Nil(t, appErr)
require.NotEmpty(t, token)
defer th.App.Srv().Store.System().PermanentDeleteByName(model.SystemLicenseRenewalToken)
newToken, appErr := th.App.Srv().GenerateRenewalToken(JWTDefaultTokenExpiration)
require.Nil(t, appErr)
require.Equal(t, token, newToken)
})
t.Run("return error if there is no active license", func(t *testing.T) {
@@ -118,35 +91,6 @@ func TestGenerateRenewalToken(t *testing.T) {
_, appErr := th.App.Srv().GenerateRenewalToken(JWTDefaultTokenExpiration)
require.NotNil(t, appErr)
})
t.Run("return another token if the license owner change", func(t *testing.T) {
setLicense(th, nil)
token, appErr := th.App.Srv().GenerateRenewalToken(JWTDefaultTokenExpiration)
require.Nil(t, appErr)
require.NotEmpty(t, token)
defer th.App.Srv().Store.System().PermanentDeleteByName(model.SystemLicenseRenewalToken)
setLicense(th, &model.Customer{
Name: "another customer",
Email: "another@example.com",
})
newToken, appErr := th.App.Srv().GenerateRenewalToken(JWTDefaultTokenExpiration)
require.Nil(t, appErr)
require.NotEqual(t, token, newToken)
})
t.Run("return another token if the active one has expired", func(t *testing.T) {
setLicense(th, nil)
token, appErr := th.App.Srv().GenerateRenewalToken(1 * time.Second)
require.Nil(t, appErr)
require.NotEmpty(t, token)
defer th.App.Srv().Store.System().PermanentDeleteByName(model.SystemLicenseRenewalToken)
// The small time unit for expiration we're using is seconds
time.Sleep(1 * time.Second)
newToken, appErr := th.App.Srv().GenerateRenewalToken(JWTDefaultTokenExpiration)
require.Nil(t, appErr)
require.NotEqual(t, token, newToken)
})
}
func setLicense(th *TestHelper, customer *model.Customer) {

Просмотреть файл

@@ -1721,6 +1721,15 @@ func (s *Server) sendLicenseUpForRenewalEmail(users map[string]*model.User, lice
func (s *Server) doLicenseExpirationCheck() {
s.LoadLicense()
// This takes care of a rare edge case reported here https://mattermost.atlassian.net/browse/MM-40962
// To reproduce that case locally, attach a license to a server that was started with enterprise enabled
// Then restart using BUILD_ENTERPRISE=false make restart-server to enter Team Edition
if model.BuildEnterpriseReady != "true" {
mlog.Debug("Skipping license expiration check because no license is expected on Team Edition")
return
}
license := s.License()
if license == nil {
@@ -1744,6 +1753,7 @@ func (s *Server) doLicenseExpirationCheck() {
if appErr != nil {
mlog.Debug(appErr.Error())
}
return
}
if !license.IsPastGracePeriod() {
@@ -1751,6 +1761,12 @@ func (s *Server) doLicenseExpirationCheck() {
return
}
renewalLink, _, appErr := s.GenerateLicenseRenewalLink()
if appErr != nil {
mlog.Error("Error while sending the license expired email.", mlog.Err(appErr))
return
}
//send email to admin(s)
for _, user := range users {
user := user
@@ -1761,7 +1777,7 @@ func (s *Server) doLicenseExpirationCheck() {
mlog.Debug("Sending license expired email.", mlog.String("user_email", user.Email))
s.Go(func() {
if err := s.SendRemoveExpiredLicenseEmail(user.Email, user.Locale, *s.Config().ServiceSettings.SiteURL); err != nil {
if err := s.SendRemoveExpiredLicenseEmail(user.Email, renewalLink, user.Locale, *s.Config().ServiceSettings.SiteURL); err != nil {
mlog.Error("Error while sending the license expired email.", mlog.String("user_email", user.Email), mlog.Err(err))
}
})
@@ -1773,11 +1789,7 @@ func (s *Server) doLicenseExpirationCheck() {
// SendRemoveExpiredLicenseEmail formats an email and uses the email service to send the email to user with link pointing to CWS
// to renew the user license
func (s *Server) SendRemoveExpiredLicenseEmail(email string, locale, siteURL string) *model.AppError {
renewalLink, _, err := s.GenerateLicenseRenewalLink()
if err != nil {
return err
}
func (s *Server) SendRemoveExpiredLicenseEmail(email string, renewalLink, locale, siteURL string) *model.AppError {
if err := s.EmailService.SendRemoveExpiredLicenseEmail(renewalLink, email, locale, siteURL); err != nil {
return model.NewAppError("SendRemoveExpiredLicenseEmail", "api.license.remove_expired_license.failed.error", nil, err.Error(), http.StatusInternalServerError)

Просмотреть файл

@@ -12,7 +12,6 @@ const (
SystemRanUnitTests = "RanUnitTests"
SystemLastSecurityTime = "LastSecurityTime"
SystemActiveLicenseId = "ActiveLicenseId"
SystemLicenseRenewalToken = "LicenseRenewalToken"
SystemLastComplianceTime = "LastComplianceTime"
SystemAsymmetricSigningKeyKey = "AsymmetricSigningKey"
SystemPostActionCookieSecretKey = "PostActionCookieSecret"