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.

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

feat: optimize github/gitee token process #919

Merged

Conversation

frank-zsy
Copy link
Contributor

Brief Information

This pull request is in the type of (more info about types):

  • build
  • ci
  • docs
  • feat
  • fix
  • perf
  • refactor
  • test

Related issues (all available keywords):

Details

This PR will optimize the GitHub token retrieve process. Originally, we requires users to generate the token manually and set into the HyperCRX settings, this is not friendly enough for some users especially non-tech users.

Now, we are using HyperCRX GitHub OAuth App to do this, user only need to click the button and HyperCRX will redirect to GitHub OAuth page for users to authorize HyperCRX App to get personal token automatically. Although this requires an OAuth server to get the token and get back to users, the process is much more easy for users.

Checklist

Others

@frank-zsy
Copy link
Contributor Author

Currently, we only has a very basic version which will log the information into console but all is good now.

@wangyantong2000 Could you keep working on this PR and make it available for production?

@frank-zsy frank-zsy marked this pull request as draft November 12, 2024 12:50
@frank-zsy
Copy link
Contributor Author

I also added the Gitee token process. Gitee is a little bit different than GitHub, GitHub OAuth App will return a permanent token without expire date ( maybe, as far as I know).

But Gitee will return an object with token and expire_in and refresh_token, if the token is expired, we can directly refresh the token with refresh_token from HyperCRX without auth or connect to server.

The doc is here: https://gitee.com/api/v5/oauth_doc#/

@frank-zsy frank-zsy changed the title feat: optimize github token process feat: optimize github/gitee token process Nov 12, 2024
@frank-zsy frank-zsy force-pushed the feat/optimize-github-token-process branch from 74af854 to d152f39 Compare November 12, 2024 14:20
@wangyantong2000
Copy link
Collaborator

The current issues with running this part locally are as follows.

image image

Moreover, in the Edge browser, chrome.identity seems to be ineffective, and Edge may not have this API, which is null.

@wangyantong2000
Copy link
Collaborator

Sorry, after updating the Edge browser, chrome. identity is also valid.

@frank-zsy
Copy link
Contributor Author

The redirect_uri should be fixed for OAuth App, so I changed the procedure, the OAuth App will directly redirect to server side and server will get the token and redirect to extension so extension can get the token from the URL.

@frank-zsy frank-zsy assigned frank-zsy and unassigned frank-zsy Nov 13, 2024
Signed-off-by: frank-zsy <[email protected]>
@frank-zsy
Copy link
Contributor Author

@wangyantong2000 I think I've finished all the functionality of new token retrieval procedure. You can test it in your local env and report any bug you find, and feel free to optimize the code as you like.

And the PR will not be ready until hypercrx.cn domain is valid.

@frank-zsy
Copy link
Contributor Author

@Xsy41 Could you also test this functionality?

@Xsy41
Copy link
Collaborator

Xsy41 commented Nov 13, 2024

Good work! I think it's a great feature.

@Xsy41
Copy link
Collaborator

Xsy41 commented Nov 13, 2024

Whether user consent is required for each binding?There is no problem in the implementation of the function.

@frank-zsy
Copy link
Contributor Author

frank-zsy commented Nov 13, 2024

Whether user consent is required for each binding?

No, the authorization action will be only required for the first time. Actually this action means user authorizes the HyperCRX OAuth App to access his account. So if the user finish the authorization in first time, he will not need to consent again even in a different device. So after you unbind the account, re-bind account is totally automated and there will not be a pop-up page.

However there will be pop-up page in following situations:

  • User needs to login. So if you did not login GitHub/Gitee in current browser, the page will pop up and requires login action in the first place. And if you logout GitHub/Gitee, bind action will requires login too.
  • User did not authorize the App. So after login, authorization page will pop up for user to finish the authorization with all the setting details.
  • The authorization revoked. After the initial authorization, both users and the App could revoke the authorization in GitHub/Gitee, so users need to authorize again after revoke.
  • The App setting updated. If the OAuth App update its setting, like the scope of the token or callback URL, this means the App is different than the former one, so users need to authorize again with the new settings.

Except the above situations, if users keep login on GitHub/Gitee, the bind action will be fully automated after the first time.

Signed-off-by: frank-zsy <[email protected]>
@Xsy41
Copy link
Collaborator

Xsy41 commented Nov 14, 2024

This setup sounds user-friendly, especially with automated re-binding in most cases. Thanks for your thorough response!

Signed-off-by: frank-zsy <[email protected]>
@frank-zsy
Copy link
Contributor Author

The hypercrx.cn domain has been ready and already changed the online URL, this PR is ready for merge now. @wangyantong2000

@frank-zsy frank-zsy marked this pull request as ready for review November 18, 2024 07:01
@wangyantong2000
Copy link
Collaborator

OK

@wangyantong2000 wangyantong2000 merged commit c47073c into hypertrons:master Nov 18, 2024
3 checks passed
@frank-zsy frank-zsy deleted the feat/optimize-github-token-process branch November 18, 2024 12:22
@wangyantong2000
Copy link
Collaborator

@Xsy41 After the refactoring work on the options page is merged, you can update and modify the comments on the token related part of the FastPR process in 911 again~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants