search

mattermost / mattermost-plugin-mscalendar

Mattermost plugin for Microsoft Office365
Apache License 2.0
13 stars 21 forks source link

Handle case where a user's refresh token expires #198

Open mickmister opened 3 years ago

mickmister commented 3 years ago

The task here is to do the following on any request to the msgraph API:

https://learn.microsoft.com/en-us/answers/questions/340205/for-how-long-i-can-keep-using-the-refresh-token

Per the discussion in https://github.com/mattermost/mattermost-plugin-mscalendar/issues/15#issuecomment-585033566, it was determined that we should allow the OAuth2 library to handle access tokens and refresh tokens with the library's default behavior.

The refresh token is used to retrieve an access token whenever we need to perform an operation in Microsoft's system. A customer's log reports an error that says The refresh token has expired due to inactivity. This error is occurring during the Renew Subscription job. The customer said this error is printed to the logs every night at midnight.

(log ERROR) Error renewing subscription. err=msgraph RenewSubscription: Patch https://graph.microsoft.com/v1.0/subscriptions/REDACTED: oauth2: cannot fetch token: 400 Bad Request Response: {"error":"invalid_grant","error_description":"AADSTS700082: The refresh token has expired due to inactivity. The token was issued on 2020-07-23T12:22:48.1096407Z and was inactive for 90.00:00:00.\r\nTrace ID: b40f4014-73bb-40f3-b90d-cd42d78c9600\r\nCorrelation ID: 1bb86522-a8e8-45a2-9966-2d11a892c925\r\nTimestamp: 2020-11-05 00:00:00Z","error_codes":[700082],"timestamp":"2020-11-05 00:00:00Z","trace_id":"b40f4014-73bb-40f3-b90d-cd42d78c9600","correlation_id":"1bb86522-a8e8-45a2-9966-2d11a892c925","error_uri":https://login.microsoftonline.com/error?code=700082}

https://learn.microsoft.com/en-us/answers/questions/340205/for-how-long-i-can-keep-using-the-refresh-token

More context here https://community-daily.mattermost.com/core/pl/u7jyh7dokpdtfcq49keu6peofe

_Issue created from a Mattermost message by @mickmister._

coltoneshaw commented 3 years ago

Debug logs related to this issue. All users related are active within Mattermost. Pending a check to see if the user ID above the error is able to access mscalendar. 

mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"debug","ts":1604966400.1092687,"caller":"mlog/sugar.go:15","msg":"Renewing for user: 3eehtsajybya7jwo33i71mrcca","plugin_id":"com.mattermost.mscalendar"}
mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"debug","ts":1604966400.1607916,"caller":"mlog/sugar.go:15","msg":"Renewing for user: n9stjpxqi3b1tjkyg7ooh4qrfe","plugin_id":"com.mattermost.mscalendar"}
mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"debug","ts":1604966400.2122893,"caller":"mlog/sugar.go:15","msg":"Renewing for user: c8duuyswbfgj3koy6sm5s5e7ka","plugin_id":"com.mattermost.mscalendar"}
mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"info","ts":1604966400.3001952,"caller":"mlog/sugar.go:19","msg":"Processed daily summary for 0 users","plugin_id":"com.mattermost.mscalendar"}
mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"debug","ts":1604966400.3004143,"caller":"mlog/sugar.go:15","msg":"Daily summary job finished","plugin_id":"com.mattermost.mscalendar"}
mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"error","ts":1604966400.8778589,"caller":"mlog/sugar.go:23","msg":"Error renewing subscription. err=msgraph RenewSubscription: Patch \"https://graph.microsoft.com/v1.0/subscriptions/ffb99374-4357-48fd-b364-594d5d3872c3\": oauth2: cannot fetch token: 400 Bad Request\nResponse: {\"error\":\"invalid_grant\",\"error_description\":\"AADSTS700082: The refresh token has expired due to inactivity. The token was issued on 2020-07-23T12:22:48.1096407Z and was inactive for 90.00:00:00.\\r\\nTrace ID: 857d4e41-65f5-4a28-a91a-99244ab7c301\\r\\nCorrelation ID: 46a992f8-b571-4b3a-b8db-405746912db7\\r\\nTimestamp: 2020-11-10 00:00:00Z\",\"error_codes\":[700082],\"timestamp\":\"2020-11-10 00:00:00Z\",\"trace_id\":\"857d4e41-65f5-4a28-a91a-99244ab7c301\",\"correlation_id\":\"46a992f8-b571-4b3a-b8db-405746912db7\",\"error_uri\":\"https://login.microsoftonline.com/error?code=700082\"}","plugin_id":"com.mattermost.mscalendar"}
mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"debug","ts":1604966400.8910735,"caller":"mlog/sugar.go:15","msg":"User status sync job finished","plugin_id":"com.mattermost.mscalendar"}
mattermost-2020-11-10T00-17-01.872.log.gz:{"level":"debug","ts":1604966400.9391136,"caller":"mlog/sugar.go:15","msg":"Renewing for user: dducu6kbg7bptpm7crj1bbeybe","plugin_id":"com.mattermost.mscalendar"}
levb commented 3 years ago

I've seen this issue early on, I thought we already implemented auto-renewal, or at least there was a ticket for it. Will look.

mickmister commented 3 years ago

@levb The ticket linked at the top of the description is likely the one you're referring to. It was determined that we don't need to refresh the refresh tokens because Microsoft's refresh tokens do not expire for server-based applications. It seems that may not be true.

We implemented auto-renewal for the event subscriptions, but not the OAuth2 refresh tokens.

levb commented 3 years ago

Ack. We should probably get a recommendation from the security team (cc @jupenur) but it feels that option (b) dis-activating the user locally makes sense. We'd need to disable all API activity for the user, and have clear re-activation events to facilitate the OAuth flow? DMs to user?

jupenur commented 3 years ago

I don't have a strong opinion on this either way. Both options seem like valid approaches.

mickmister commented 3 years ago

@levb I've updated the ticket's description to include the task at hand. Should this be made into HW?

coltoneshaw commented 3 years ago

Quick question, is there a suggested work around to fix this in the short term?

mickmister commented 3 years ago

@coltoneshaw Can we have the affected user reconnect their account by running /mscalendar disconnect, then /mscalendar connect?

coltoneshaw commented 3 years ago

@mickmister, thanks! The user actually reported to me that none of their uses are having an issue connecting to MSCalendar, just the one error. Do you know what could cause the error and how we can stop it?

mickmister commented 3 years ago

@coltoneshaw Does the admin know which MM account is being affected? I'm thinking that reconnecting the account for the user that is showing up in the logs should resolve the error, as it will re-sync any stored data pertaining to the user's Microsoft account.

The user actually reported to me that none of their uses are having an issue connecting to MSCalendar, just the one error.

Does this mean that they have reconnected their account and the error is still occurring?

coltoneshaw commented 3 years ago

They have not reconnected anything, all users are reporting that the plugin works fine, and the error still occurs.

coltoneshaw commented 3 years ago

@mickmister , We found the first item that's officially broken due to this token. I'm going to have the customer connect/disconnect this and see if it resolves it, but I wanted to share it with you.

mmCalendarIssue
mickmister commented 3 years ago

@levb I'm thinking we should just store the refresh token returned from Microsoft on every access token refresh. I'm sure golang's oauth2 library has a straightforward way to do this.

https://github.com/mattermost/mattermost-plugin-mscalendar/issues/15#issuecomment-585033566 relevant discussion

hanzei commented 3 years ago

Similar issue as https://github.com/mattermost/mattermost-plugin-zoom/issues/213

mickmister commented 3 years ago

@deepakdemiwal If you are the developer working on the ticket, you can comment on this ticket then I can add you as the assignee

maisnamrajusingh commented 3 years ago

@mickmister I'm taking this up

maisnamrajusingh commented 3 years ago

@mickmister I've spoken to siba about this, please assign this to me instead. The reason I wanted to pick this up was because of the settings which we resolved this morning. He can take it up now. For some reason I can't tag him

mickmister commented 3 years ago

please assign this to me instead

He can take it up now

Who am I assigning this to? They need to comment on the ticket so I can assign them

mickmister commented 3 years ago

@maisnamrajusingh ^

sibasankarnayak commented 3 years ago

@mickmister I am picking this up

sibasankarnayak commented 3 years ago

@mickmister i need some help regarding this issue

currently we were storing the token and missed out the refresh token , so tried to save the token can find here and feel refetch of refresh token need to be implemented here correct if i am wrong

mickmister commented 3 years ago

and feel refetch of refresh token need to be implemented here

The problem with waiting to refresh at this point is that the current refresh token has already expired. The refresh token needs to be saved every time we get a new refresh token from typical API usage.

currently we were storing the token and missed out the refresh token , so tried to save the token can find here

Is the refresh token available in the response anywhere? Maybe the oauth token struct we use gets updated?

maisnamrajusingh commented 3 years ago

@levb according to the discussion with @mickmister the oauth library already refreshes the token for the user. So a bit confused about how this could be resolved. Would appreciate some inputs.

rey-hernandez commented 1 year ago

Hope this helps: https://www.microsoftpartnercommunity.com/t5/Multi-Factor-Authentication-MFA/OAuth-Refresh-token-has-expired-after-90-days/m-p/9200

I am seeing this in the /mscalendar integration for some commands.

wiggin77 commented 1 month ago

@Kshitij-Katiyar Is this still a valid issue?

wiggin77 commented 3 weeks ago

@raghavaggarwal2308 Is this issue still valid? Jira: https://mattermost.atlassian.net/browse/MM-33777

raghavaggarwal2308 commented 3 weeks ago

@wiggin77 There is already a PR created for his issue. Once this is approved from your end, we can get it reviewed by a QA and merge it https://github.com/mattermost/mattermost-plugin-mscalendar/pull/256