Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MM-809]: Fixed the issue of getting errors when using github api with revoked/invalid token #832

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[MM-809]: Fixed the issue of getting errors when using github api with revoked/invalid token #832

wants to merge 7 commits into from
+370 −120

Conversation

Kshitij-Katiyar
Copy link
Contributor

@Kshitij-Katiyar Kshitij-Katiyar commented Sep 26, 2024 •
edited by raghavaggarwal2308
Loading

Summary

  • Fixed the issue of getting errors when using GitHub API with revoked/invalid tokens.
  • Created a wrapper function and used it on all the githubClient calls, in case of invalid token error, disconnected the user and DMed him to reconnect

Issue Link

Fixes #809

Testing steps

  • Do a complete sanity on a high level along with the below things on the side
  • Connect your mattermost to Github plugin
  • Revoke the user access token on the Github Oauth app as shown in the screenshot
    Screenshot from 2024-09-27 13-30-52
  • Perform any action which involves API call using the Github client like LHS refresh, LHS button click

@raghavaggarwal2308 raghavaggarwal2308 added this to the v2.4.0 milestone Sep 26, 2024
@wiggin77 wiggin77 added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Oct 22, 2024
@wiggin77
Copy link
Member

/update-branch

@mattermost-build
Copy link
Contributor

Error trying to update the PR.
Please do it manually.

@Kshitij-Katiyar
Copy link
Contributor Author

@wiggin77 I have synced this branch with the master

wiggin77
wiggin77 reviewed Oct 23, 2024
View reviewed changes
server/plugin/plugin.go Outdated Show resolved Hide resolved
wiggin77
wiggin77 reviewed Oct 28, 2024
View reviewed changes
server/plugin/plugin.go Outdated Show resolved Hide resolved
wiggin77
wiggin77 reviewed Oct 28, 2024
View reviewed changes
server/plugin/plugin.go Outdated Show resolved Hide resolved
wiggin77
wiggin77 approved these changes Oct 28, 2024
View reviewed changes
Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

raghavaggarwal2308
raghavaggarwal2308 requested changes Nov 12, 2024
View reviewed changes
server/plugin/api.go Show resolved Hide resolved
server/plugin/api.go Outdated Show resolved Hide resolved
server/plugin/api.go Show resolved Hide resolved
server/plugin/api.go Outdated Show resolved Hide resolved
server/plugin/api.go Outdated Show resolved Hide resolved
server/plugin/api.go Outdated Show resolved Hide resolved
server/plugin/api.go Show resolved Hide resolved
server/plugin/plugin.go Outdated Show resolved Hide resolved
server/plugin/plugin.go Outdated Show resolved Hide resolved
@raghavaggarwal2308 raghavaggarwal2308 removed the 2: Dev Review Requires review by a core committer label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raghavaggarwal2308 raghavaggarwal2308 raghavaggarwal2308 approved these changes

@wiggin77 wiggin77 wiggin77 approved these changes

@AayushChaudhary0001 AayushChaudhary0001 Awaiting requested review from AayushChaudhary0001

Assignees

@Kshitij-Katiyar Kshitij-Katiyar

Labels
3: QA Review Requires review by a QA tester
Projects
None yet
Milestone
v2.4.0
Development

Successfully merging this pull request may close these issues.

GitHub plugin issues on community after switching to SSO
4 participants
@Kshitij-Katiyar @wiggin77 @mattermost-build @raghavaggarwal2308