Skip to content

Commit

Permalink
Revert "[GH-14] Add at-rest encryption for OAuth2 token (#143)" (#157)
Browse files Browse the repository at this point in the history
This reverts commit 0dd13f8.
  • Loading branch information
mickmister authored Jun 1, 2020
1 parent b549fad commit 75fc9b5
Show file tree
Hide file tree
Showing 11 changed files with 19 additions and 162 deletions.
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,3 @@ Copy the `Client ID` and `Tenant ID` from the Azure portal
- `tenantID` - copy from Azure App
- `clientID` - copy from Azure App
- `Client Secret` - copy from Azure App (Generated in `Certificates & secrets`, earlier in these instructions)
- `At Rest Token Encryption Key` - press the `Regenerate` button
7 changes: 0 additions & 7 deletions plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,6 @@
"type": "text",
"help_text": "Microsoft Office Client Secret.",
"default": ""
},
{
"key": "TokenEncryptionKey",
"display_name": "At Rest Token Encryption Key",
"type": "generated",
"help_text": "The AES encryption key used to encrypt stored access tokens.",
"regenerate_help_text": "Regenerates the encryption key for Microsoft Calendar OAuth Token. Regenerating the key invalidates your existing Microsoft Calendar OAuth."
}
]
}
Expand Down
2 changes: 0 additions & 2 deletions server/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ type StoredConfig struct {
EnableStatusSync bool
EnableDailySummary bool

TokenEncryptionKey string

bot.BotConfig
}

Expand Down
6 changes: 1 addition & 5 deletions server/mscalendar/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ func (m *mscalendar) MakeClient() (remote.Client, error) {
return nil, err
}

token, err := decryptToken(m.actingUser.OAuth2Token, m.Config.TokenEncryptionKey)
if err != nil {
return nil, err
}
return m.Remote.MakeClient(context.Background(), token), nil
return m.Remote.MakeClient(context.Background(), m.actingUser.OAuth2Token), nil
}

func (m *mscalendar) MakeSuperuserClient() (remote.Client, error) {
Expand Down
7 changes: 1 addition & 6 deletions server/mscalendar/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,7 @@ func (processor *notificationProcessor) processNotification(n *remote.Notificati
n.Subscription = sub.Remote
n.SubscriptionCreator = creator.Remote

token, err := decryptToken(creator.OAuth2Token, processor.Config.TokenEncryptionKey)
if err != nil {
return err
}

client := processor.Remote.MakeClient(context.Background(), token)
client := processor.Remote.MakeClient(context.Background(), creator.OAuth2Token)

if n.RecommendRenew {
var renewed *remote.Subscription
Expand Down
11 changes: 2 additions & 9 deletions server/mscalendar/notification_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"github.com/mattermost/mattermost-plugin-mscalendar/server/remote/mock_remote"
"github.com/mattermost/mattermost-plugin-mscalendar/server/store"
"github.com/mattermost/mattermost-plugin-mscalendar/server/store/mock_store"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/bot/mock_bot"
)
Expand Down Expand Up @@ -66,14 +65,13 @@ func newTestSubscription() *store.Subscription {
}

func newTestUser() *store.User {
accessToken, _ := utils.Encrypt([]byte(fakeEncryptionKey), "creator_oauth_token")
return &store.User{
Settings: store.Settings{
EventSubscriptionID: "remote_subscription_id",
},
Remote: &remote.User{ID: "remote_user_id"},
OAuth2Token: &oauth2.Token{
AccessToken: accessToken,
AccessToken: "creator_oauth_token",
},
MattermostUserID: "creator_mm_id",
}
Expand Down Expand Up @@ -133,12 +131,7 @@ func TestProcessNotification(t *testing.T) {
mockPluginAPI := mock_plugin_api.NewMockPluginAPI(ctrl)
mockClient := mock_remote.NewMockClient(ctrl)

conf := &config.Config{
PluginVersion: "x.x.x",
StoredConfig: config.StoredConfig{
TokenEncryptionKey: fakeEncryptionKey,
},
}
conf := &config.Config{PluginVersion: "x.x.x"}
env := Env{
Config: conf,
Dependencies: &Dependencies{
Expand Down
36 changes: 1 addition & 35 deletions server/mscalendar/oauth2.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

"github.com/mattermost/mattermost-plugin-mscalendar/server/config"
"github.com/mattermost/mattermost-plugin-mscalendar/server/store"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils"
"github.com/mattermost/mattermost-plugin-mscalendar/server/utils/oauth2connect"
)

Expand Down Expand Up @@ -92,16 +91,11 @@ func (app *oauth2App) CompleteOAuth2(authedUserID, code, state string) error {
}
}

encryptedToken, err := encryptToken(tok, app.Config.TokenEncryptionKey)
if err != nil {
return err
}

u := &store.User{
PluginVersion: app.Config.PluginVersion,
MattermostUserID: mattermostUserID,
Remote: me,
OAuth2Token: encryptedToken,
OAuth2Token: tok,
}

mailboxSettings, err := client.GetMailboxSettings(me.ID)
Expand Down Expand Up @@ -129,31 +123,3 @@ func (app *oauth2App) CompleteOAuth2(authedUserID, code, state string) error {

return nil
}

func decryptToken(tok *oauth2.Token, encryptionKey string) (*oauth2.Token, error) {
unencryptedToken, err := utils.Decrypt([]byte(encryptionKey), tok.AccessToken)
if err != nil {
return nil, errors.New("cannot decrypt token")
}

newToken := &oauth2.Token{
AccessToken: unencryptedToken,
TokenType: tok.TokenType,
RefreshToken: tok.RefreshToken,
}
return newToken, nil
}

func encryptToken(tok *oauth2.Token, encryptionKey string) (*oauth2.Token, error) {
encryptedToken, err := utils.Encrypt([]byte(encryptionKey), tok.AccessToken)
if err != nil {
return nil, err
}

newToken := &oauth2.Token{
AccessToken: encryptedToken,
TokenType: tok.TokenType,
RefreshToken: tok.RefreshToken,
}
return newToken, nil
}
12 changes: 5 additions & 7 deletions server/mscalendar/oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,11 @@ import (
)

const (
fakeID = "fake@mattermost.com"
fakeRemoteID = "user-remote-id"
fakeBotID = "bot-user-id"
fakeBotRemoteID = "bot-remote-id"
fakeCode = "fakecode"
fakeEncryptionKey = "00000000000000000000000000000000"
fakeID = "fake@mattermost.com"
fakeRemoteID = "user-remote-id"
fakeBotID = "bot-user-id"
fakeBotRemoteID = "bot-remote-id"
fakeCode = "fakecode"
)

func TestCompleteOAuth2Happy(t *testing.T) {
Expand Down Expand Up @@ -366,7 +365,6 @@ func newOAuth2TestApp(ctrl *gomock.Controller) (oauth2connect.App, Env) {
OAuth2Authority: "common",
OAuth2ClientID: "fakeclientid",
OAuth2ClientSecret: "fakeclientsecret",
TokenEncryptionKey: fakeEncryptionKey,
},
PluginURL: "http://localhost",
}
Expand Down
19 changes: 9 additions & 10 deletions server/mscalendar/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,12 @@ func (user *User) Markdown() string {

func (m *mscalendar) DisconnectUser(mattermostUserID string) error {
m.AfterDisconnect(mattermostUserID)
err := m.Filter(
withClient,
)
if err != nil {
return err
}

storedUser, err := m.Store.LoadUser(mattermostUserID)
if err != nil {
Expand All @@ -130,16 +136,9 @@ func (m *mscalendar) DisconnectUser(mattermostUserID string) error {
return errors.WithMessagef(err, "failed to delete subscription %s", eventSubscriptionID)
}

err = m.Filter(
withClient,
)
if err == nil {
err = m.client.DeleteSubscription(eventSubscriptionID)
if err != nil {
m.Logger.Warnf("failed to delete remote subscription %s. err=%v", eventSubscriptionID, err)
}
} else {
m.Logger.Errorf("failed to create client %s. err=%v", err.Error())
err = m.client.DeleteSubscription(eventSubscriptionID)
if err != nil {
m.Logger.Warnf("failed to delete remote subscription %s. err=%v", eventSubscriptionID, err)
}
}

Expand Down
4 changes: 0 additions & 4 deletions server/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ func (p *Plugin) OnConfigurationChange() (err error) {
return errors.New("failed to configure: OAuth2 credentials to be set in the config")
}

if stored.TokenEncryptionKey == "" {
return errors.New("token encryption key not generated")
}

mattermostSiteURL := p.API.GetConfig().ServiceSettings.SiteURL
if mattermostSiteURL == nil {
return errors.New("plugin requires Mattermost Site URL to be set")
Expand Down
76 changes: 0 additions & 76 deletions server/utils/encryption.go

This file was deleted.

0 comments on commit 75fc9b5

Please sign in to comment.