From 0c9262c4d145b0ec8bd6c3ad4a96807717431584 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 28 Jan 2022 12:37:02 +0530 Subject: [PATCH] MM-41236: Sentry crash: Fix nil reference to token (#19417) The AND condition would mean that it would try to dereference token.Valid if there was an error. And there's no guarantee to always have a non-nil token in case of an error. We need to track those conditions separately. https://mattermost.atlassian.net/browse/MM-41236 ```release-note NONE ``` --- app/license.go | 5 ++++- app/license_test.go | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/license.go b/app/license.go index 00f88b404e..7ffce9a025 100644 --- a/app/license.go +++ b/app/license.go @@ -358,9 +358,12 @@ func (s *Server) renewalTokenValid(tokenString, signingKey string) (bool, error) token, err := jwt.ParseWithClaims(tokenString, claims, func(token *jwt.Token) (interface{}, error) { return []byte(signingKey), nil }) - if err != nil && !token.Valid { + 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 diff --git a/app/license_test.go b/app/license_test.go index 3b04e398b5..803a5d718b 100644 --- a/app/license_test.go +++ b/app/license_test.go @@ -4,9 +4,11 @@ package app import ( + "errors" "testing" "time" + "github.com/dgrijalva/jwt-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -80,6 +82,12 @@ 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)