Fix OAuth token refresh (#253)

* avoid refreshing token if an error occurs while fetching current token * pass in firstConnect bool * lint * explicitly assign firstConnect boolean to local variable * ensure oauth token is decrypted when fetched from the kv store * rename functions * wrap account level token refresh in firstConnect block as well * Cherrypick "Correct docs and cleanup screenshots" * Revert "Cherrypick "Correct docs and cleanup screenshots"" This reverts commit 7164075. * preserve access token when saving encrypted version * use refresh token for check Co-authored-by: Mattermod <mattermod@users.noreply.github.com> Co-authored-by: Ben Schumacher <ben.schumacher@mattermost.com> Co-authored-by: Daniel Espino García <larkox@gmail.com>
mattermost · Aug 5, 2022 · dec2ad9 · dec2ad9
1 parent 38cd3c1
commit dec2ad9
Show file tree

Hide file tree

Showing 6 changed files with 88 additions and 28 deletions.
diff --git a/server/http.go b/server/http.go
@@ -135,7 +135,8 @@ func (p *Plugin) completeUserOAuthToZoom(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	zoomUser, authErr := client.GetUser(user)
+	firstConnect := true
+	zoomUser, authErr := client.GetUser(user, firstConnect)
 	if authErr != nil {
 		if p.configuration.AccountLevelApp && !justConnect {
 			http.Error(w, "Connection completed but there was an error creating the meeting. "+authErr.Message, http.StatusInternalServerError)

diff --git a/server/plugin.go b/server/plugin.go
@@ -165,12 +165,6 @@ func (p *Plugin) getActiveClient(user *model.User) (zoom.Client, string, error)
 		return nil, message, errors.Wrap(err, "could not fetch Zoom OAuth info")
 	}
 
-	plainToken, err := decrypt([]byte(config.EncryptionKey), info.OAuthToken.AccessToken)
-	if err != nil {
-		return nil, message, errors.New("could not decrypt OAuth access token")
-	}
-
-	info.OAuthToken.AccessToken = plainToken
 	conf := p.getOAuthConfig()
 	return zoom.NewOAuthClient(info.OAuthToken, conf, p.siteURL, p.getZoomAPIURL(), false, p), "", nil
 }
@@ -201,7 +195,8 @@ func (p *Plugin) authenticateAndFetchZoomUser(user *model.User) (*zoom.User, *zo
 		}
 	}
 
-	return zoomClient.GetUser(user)
+	firstConnect := false
+	return zoomClient.GetUser(user, firstConnect)
 }
 
 func (p *Plugin) sendDirectMessage(userID string, message string) error {
@@ -241,6 +236,28 @@ func (p *Plugin) SetZoomSuperUserToken(token *oauth2.Token) error {
 	return nil
 }
 
+func (p *Plugin) GetZoomOAuthUserInfo(userID string) (*zoom.OAuthUserInfo, error) {
+	info, err := p.fetchOAuthUserInfo(zoomUserByMMID, userID)
+	if err != nil {
+		return nil, errors.Wrap(err, "could not get token")
+	}
+	if info == nil {
+		return nil, errors.New("zoom app not connected")
+	}
+
+	return info, nil
+}
+
+func (p *Plugin) UpdateZoomOAuthUserInfo(userID string, info *zoom.OAuthUserInfo) error {
+	if err := p.storeOAuthUserInfo(info); err != nil {
+		msg := "unable to update user token"
+		p.API.LogWarn(msg, "error", err.Error())
+		return errors.Wrap(err, msg)
+	}
+
+	return nil
+}
+
 func (p *Plugin) isCloudLicense() bool {
 	license := p.API.GetLicense()
 	return license != nil && *license.Features.Cloud

diff --git a/server/store.go b/server/store.go
@@ -32,6 +32,8 @@ func (p *Plugin) storeOAuthUserInfo(info *zoom.OAuthUserInfo) error {
 	if err != nil {
 		return errors.Wrap(err, "could not encrypt OAuth token")
 	}
+
+	original := info.OAuthToken.AccessToken
 	info.OAuthToken.AccessToken = encryptedToken
 
 	encoded, err := json.Marshal(info)
@@ -47,10 +49,13 @@ func (p *Plugin) storeOAuthUserInfo(info *zoom.OAuthUserInfo) error {
 		return err
 	}
 
+	info.OAuthToken.AccessToken = original
 	return nil
 }
 
 func (p *Plugin) fetchOAuthUserInfo(tokenKey, userID string) (*zoom.OAuthUserInfo, error) {
+	config := p.getConfiguration()
+
 	encoded, appErr := p.API.KVGet(tokenKey + userID)
 	if appErr != nil || encoded == nil {
 		return nil, errors.New("must connect user account to Zoom first")
@@ -61,6 +66,13 @@ func (p *Plugin) fetchOAuthUserInfo(tokenKey, userID string) (*zoom.OAuthUserInf
 		return nil, errors.New("could not to parse OAauth access token")
 	}
 
+	plainToken, err := decrypt([]byte(config.EncryptionKey), info.OAuthToken.AccessToken)
+	if err != nil {
+		return nil, errors.New("could not decrypt OAuth access token")
+	}
+
+	info.OAuthToken.AccessToken = plainToken
+
 	return &info, nil
 }
 

diff --git a/server/zoom/client.go b/server/zoom/client.go
@@ -27,11 +27,13 @@ var errNotFound = errors.New("not found")
 // Client interface for Zoom
 type Client interface {
 	GetMeeting(meetingID int) (*Meeting, error)
-	GetUser(user *model.User) (*User, *AuthError)
+	GetUser(user *model.User, firstConnect bool) (*User, *AuthError)
 	CreateMeeting(user *User, topic string) (*Meeting, error)
 }
 
 type PluginAPI interface {
 	GetZoomSuperUserToken() (*oauth2.Token, error)
 	SetZoomSuperUserToken(*oauth2.Token) error
+	GetZoomOAuthUserInfo(userID string) (*OAuthUserInfo, error)
+	UpdateZoomOAuthUserInfo(userID string, info *OAuthUserInfo) error
 }
diff --git a/server/zoom/jwt.go b/server/zoom/jwt.go
@@ -45,7 +45,7 @@ func (c *JWTClient) GetMeeting(meetingID int) (*Meeting, error) {
 }
 
 // GetUser returns the Zoom user via JWT authentication.
-func (c *JWTClient) GetUser(user *model.User) (*User, *AuthError) {
+func (c *JWTClient) GetUser(user *model.User, firstConnect bool) (*User, *AuthError) {
 	var zoomUser User
 	if err := c.request(http.MethodGet, fmt.Sprintf("/users/%v", user.Email), "", &zoomUser); err != nil {
 		return nil, &AuthError{fmt.Sprintf(zoomEmailMismatch, user.Email), err}

diff --git a/server/zoom/oauth.go b/server/zoom/oauth.go
@@ -47,8 +47,8 @@ func NewOAuthClient(token *oauth2.Token, config *oauth2.Config, siteURL, apiURL
 }
 
 // GetUser returns the Zoom user via OAuth.
-func (c *OAuthClient) GetUser(user *model.User) (*User, *AuthError) {
-	zoomUser, err := c.getUserViaOAuth(user)
+func (c *OAuthClient) GetUser(user *model.User, firstConnect bool) (*User, *AuthError) {
+	zoomUser, err := c.getUserViaOAuth(user, firstConnect)
 	if err != nil {
 		if c.isAccountLevel {
 			if err == errNotFound {
@@ -127,29 +127,57 @@ func (c *OAuthClient) CreateMeeting(user *User, topic string) (*Meeting, error)
 	return &ret, err
 }
 
-func (c *OAuthClient) getUserViaOAuth(user *model.User) (*User, error) {
+func (c *OAuthClient) getUserViaOAuth(user *model.User, firstConnect bool) (*User, error) {
 	url := fmt.Sprintf("%s/users/me", c.apiURL)
 	if c.isAccountLevel {
 		url = fmt.Sprintf("%s/users/%s", c.apiURL, user.Email)
-		currentToken, err := c.api.GetZoomSuperUserToken()
-		if err != nil {
-			return nil, errors.Wrap(err, "error getting zoom super user token")
-		}
+	}
 
-		tokenSource := c.config.TokenSource(context.Background(), currentToken)
-		updatedToken, err := tokenSource.Token()
-		if err != nil {
-			return nil, errors.Wrap(err, "error getting token from token source")
-		}
+	if !firstConnect {
+		if c.isAccountLevel {
+			currentToken, err := c.api.GetZoomSuperUserToken()
+			if err != nil {
+				return nil, errors.Wrap(err, "error getting zoom super user token")
+			}
 
-		if updatedToken.AccessToken != currentToken.AccessToken {
-			kvErr := c.api.SetZoomSuperUserToken(updatedToken)
-			if kvErr != nil {
-				return nil, errors.Wrap(kvErr, "error setting new token")
+			tokenSource := c.config.TokenSource(context.Background(), currentToken)
+			updatedToken, err := tokenSource.Token()
+			if err != nil {
+				return nil, errors.Wrap(err, "error getting token from token source")
+			}
+
+			if updatedToken.RefreshToken != currentToken.RefreshToken {
+				kvErr := c.api.SetZoomSuperUserToken(updatedToken)
+				if kvErr != nil {
+					return nil, errors.Wrap(kvErr, "error setting new token")
+				}
 			}
-		}
 
-		c.token = updatedToken
+			c.token = updatedToken
+		} else {
+			info, err := c.api.GetZoomOAuthUserInfo(user.Id)
+			if err != nil {
+				return nil, errors.Wrap(err, "error getting zoom user token")
+			}
+
+			currentToken := info.OAuthToken
+
+			tokenSource := c.config.TokenSource(context.Background(), currentToken)
+			updatedToken, err := tokenSource.Token()
+			if err != nil {
+				return nil, errors.Wrap(err, "error getting token from token source")
+			}
+
+			if updatedToken.RefreshToken != currentToken.RefreshToken {
+				info.OAuthToken = updatedToken
+				kvErr := c.api.UpdateZoomOAuthUserInfo(user.Id, info)
+				if kvErr != nil {
+					return nil, errors.Wrap(kvErr, "error setting new token")
+				}
+			}
+
+			c.token = updatedToken
+		}
 	}
 
 	client := c.config.Client(context.Background(), c.token)