-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Gitee.com support #1000
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
Gitee.com support #1000
Conversation
…s Git and SVN, and offers free private repository hosting. More than 8 million developers currently choose Gitee, which is the most used Git repository hosting provider in China.
dscho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is great to see this effort!
To make things easier to review (and we really want to make sure that it is well-reviewed, this program is highly security-relevant, after all), I would recommend to break things up into as-easily-reviewable pieces as possible.
ldennington
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this looks very nice, and works well with my local testing (which, albeit, I limited to macOS). I have a few workflow notes, though:
- Typically you will not merge to
GitCredentialManager:mainfrom themainbranch of your fork. For this PR, it's fine, but in the future (likely before you start working), you will want to create a new branch locally by runninggit branch <branch name>while onmainand then merge that branch. - I'm still seeing some updated GUIDs in the solution file, as originally called out by @dscho. Were those changed purposefully?
- I see a couple of your commit messages are in Chinese. If you could change them to English, it would be much appreciated. You can use these instructions to learn how to do an interactive rebase in order to update your commit messages.
src/shared/Git-Credential-Manager/Git-Credential-Manager.csproj
Outdated
Show resolved
Hide resolved
At least in GitHub-flavored Markdown, that section title gets transformed into the anchor `net-tool`, not `.NET-tool`. Signed-off-by: Johannes Schindelin <[email protected]>
Thanks for the tip,
I've modified the solution.
I've made the changes and pushed them After the New Year's holidays, take time to review the code. Thank. |
| public const string OAuthClientId = "09776da975878a86ed655d5ada62a5d4e6faf8e89514ec982794eb61f4092d01"; | ||
| public const string OAuthClientSecret = "1d52e7bb3c53b396ad3c4e256ca1eaff27df20a782cd4f1f3dfefa32f44bafc9"; | ||
|
|
||
| public static readonly Uri OAuthRedirectUri = new Uri("http://127.0.0.1:52344/"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explicit port shouldn't be necessary, assuming the server correctly implements OAuth RFC 8252
The authorization server MUST allow any port to be specified at the time of the request for loopback IP redirect URIs
Have you reported a bug to Gitee?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already reported! After the Chinese New Year holiday, I will keep in touch with Gitee.Com .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant, thanks for reporting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maikebing any news?
|
Nice work @maikebing ! I wasn't previously familiar with https://gitee.com If it helps anyone, you can also use https://github.com/hickford/git-credential-oauth with the following [credential "https://gitee.com"]
oauthClientId = b5da229f85d94f5d728c43e429c69527fe2dcc387d4ffcb677b91c93983adaa3
oauthClientSecret = a9f88c722f66e54184204efa0f49b960f2666947e0b14bd0d73a8c9c1c33c340
oauthScopes = projects gists
oauthAuthURL = /oauth/authorize
oauthTokenURL = /oauth/token
# workaround server confused by random ports
oauthRedirectURL = http://127.0.0.1:53119 |
|
Hello @maikebing! Thanks for your PR here! Given the number of different requests we've had over time to add OAuth-based support for new Git hosts/forges, we'd been planning to extend the generic provider to also support OAuth authentication. We plan to have this completed by the next Git release (2.40). In light of this we won't be accepting this PR or others to add OAuth-based Git hosts. Thank you again for your contribution, we love to see such great engagement. |
Gitee.com is a code hosting platform launched by OSCHINA.NET, supports Git and SVN, and offers free private repository hosting. More than 8 million developers currently choose Gitee, which is the most used Git repository hosting provider in China.