Issue/573 OAuth2 support for Bitbucket DC #607

mminns · 2022-01-12T15:26:59Z

(Bloody Hell it works.)

Bitbucket DC 7.20 now supports OAuth 2.0 connections from external applications.
https://confluence.atlassian.com/bitbucketserver/bitbucket-data-center-and-server-7-20-release-notes-1101934428.html

This means OAuth 2.0 access_tokens can be used as authentication between Git and Bitbucket DC.

The PR refactors the Bitbucket module provide a more generic framework for supporting OAuth 2.0 and providing implementations for both Bitbucket Cloud and Bitbucket DC as they are unfortunately not exactly the same.

Because each Bitbucket DC installation is a unique instance the OAuth 2.0 ClientId and ClientSecret are unique to each installation and therefore GCM must be configured to support each Bitbucket DC instance.

Documentation is updated to describe how to configure and use the feature.

Tests are extended to cover Bitbucket DC.

I've been dogfooding it for the last month.

mminns · 2022-01-26T11:20:07Z

I'm currently dogfooding this and awaiting some minor tweaks to the OAuth2 support in Bitbucket, once I'm happy it working and the OAuth2 support has been publicly released in Bitbucket I'll open up this PR.

mminns · 2022-03-11T13:33:40Z

I've been dogfooding this for a month and I believe its al working.

Apologies for the size. A significant chunk is refactoring and tests.

Thanks

mjcheetham

There's a lot to unpack in this PR and I'm still looking over some of it to give more feedback.
Here's some of my comments so far however.

docs/bitbucket-development.md

docs/configuration.md

src/shared/Atlassian.Bitbucket/DataCenter/BitbucketOAuth2Client.cs

src/shared/Atlassian.Bitbucket/AbstractBitbucketOAuth2Client.cs

src/shared/Atlassian.Bitbucket/Cloud/BitbucketOAuth2Client.cs

src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs

src/shared/Atlassian.Bitbucket/BitbucketHelper.cs

ldennington

Glad to see this happening! ⭐ Most of my comments center on general cleanup and removal of duplicated code where possible.

src/shared/Atlassian.Bitbucket/BitbucketAuthentication.cs

src/shared/Atlassian.Bitbucket/BitbucketHelper.cs

src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs

src/shared/Atlassian.Bitbucket/AbstractBitbucketOAuth2Client.cs

src/shared/Atlassian.Bitbucket/Cloud/BitbucketRestApi.cs

docs/environment.md

src/shared/Atlassian.Bitbucket/Cloud/BitbucketRestApi.cs

src/shared/Atlassian.Bitbucket.Tests/BitbucketHostProviderTest.cs

mminns · 2022-05-16T07:51:43Z

Thanks for the feedback, I'll start taking a look this week 👍

ldennington · 2022-05-16T18:22:26Z

Thanks for the feedback, I'll start taking a look this week 👍

Thanks so much Mike! We're also working on aligning with the Git project on commit formatting/organization. While you're making your changes, would you mind updating the format/organization of your commits per the guidance in this presentation? (There's also an awesome talk you can watch, but the slides are quicker if you're under time constraints 😊).

mminns · 2022-05-17T12:36:15Z

Rebased first 👍

mminns · 2022-05-17T13:09:18Z

Also apologies for the surprising number of typos/comments/TODOs etc 😬

mminns · 2022-05-20T13:49:58Z

OK, I think I've addressed the issues mentioned above, I'm just working on restructuring the commits.

mminns · 2022-05-20T19:24:30Z

OK,I think I have a much better structure for the commits.

I'm going to dogfood for a couple of days next week before pushing the rebase, fixes and revised commits.

Have a good weekend.

ldennington · 2022-05-23T03:22:08Z

Thanks so much for all your efforts here Mike. Take all the time you need to dogfood - I'll keep an eye out for your next push.

mminns · 2022-05-26T12:30:24Z

I've updated the PR.
I've covered the issues raised, hopefully.
I've restructured the commits, hopefully this will make the review easier.

But please flag anything you think still needs looking at.

mminns · 2022-07-07T16:01:03Z

src/shared/Core/Authentication/OAuth/OAuth2Client.cs

@@ -65,16 +65,20 @@ public class OAuth2Client : IOAuth2Client
        private readonly Uri _redirectUri;
        private readonly string _clientId;
        private readonly string _clientSecret;
+        private readonly ITrace _trace;
+        private readonly bool _addAuthHeader;


This was lost in the refactoring of this PR.

For Bitbucket we don't want to include authentication headers with the OAuth flow requests.

In the original implementation the request methods were overridden so that calls to CreateRequestMessage() could pass addAuthHeader = false.

This got lost.

I've re-instated this behaviour by making it a behaviour set when the OAuth2Client is created

mminns · 2022-07-07T16:02:04Z

FYI I think there is a bug after all the refactoring.

I'm just investigating

This should be fixed now, my dogfooding is working now.

mminns · 2022-10-05T12:34:48Z

Hi just checking in to see if there is any timeline for merging this PR?
Cheers 👍

src/shared/Core/Authentication/OAuth/OAuth2Client.cs

mjcheetham

Just a few comments on the OAuth2Client ctor and some broken links in the doc updates; but looks good other than that!

docs/environment.md

src/shared/Atlassian.Bitbucket/BitbucketHostProvider.cs

ldennington · 2022-10-06T01:01:10Z

Just a few comments on the OAuth2Client ctor and some broken links in the doc updates; but looks good other than that!

+1! We'll plan to go ahead and merge after the links are fixed up.

mminns · 2022-10-06T06:42:23Z

Thanks for the feedback. I'll address those issues for the start of next week. 👍

Use the WriteDictionarySecrets() method to safely log the dictionary of data that will be written back to the calling Git process When it is necessary to follow the processing in GCM during a call from Git the GCM_TRACE environment variable can be used to print trace information to standard error, however the ultimate output of GET commands would not be included in this trace information This lack of visibility is especially problematic during multi-stage authentication such as OAuth, adding safe logging of the final output makes it possible to follow the GCM process in realtime from inputs to outputs during Git GET requests

…of Bitbucket DC's OAuth2 implementation The BitbucketAuthentication and BitbucketHostProvider remain the single points of entry to processing authentication for all Bitbucket hosts. Bitbucket DC and Bitbucket Cloud do not share common REST APIs or common OAuth2 implementations. The interface IBitbucketRestApi and abstract class BitbucketOAuth2Client effectively hide the differences in implementation from BitbucketAuthentication and BitbucketHostProvider The interface IBitbucketRestApi and abstract class BitbucketOAuth2Client provide clear extension points to implement OAuth2 support for Bitbucket DC

New implementations of the interface IBitbucketRestApi and abstract class BitbucketOAuth2Client provide client code for interacting with the Bitbucket DC REST API and OAuth2 implementation. Extending the BitbucketRestApiRegistry and OAuth2ClientRegistry to provide these Bitbucket DC implementations when a DC host is detected means BitbucketAuthentication and BitbucketHostProvider remain agnostic about which form of Bitbcuket host they are interacting with Prior to this feature users were limited to using Basic Auth or HTTP Access Tokens to interact securely with a Bitbucket DC instance. OAuth2 provides a better solution for SSO environments

…ket DC Update docs/environment.md and docs/configuration.md to document the new configuration options for managing Bitbucket DC OAuth2 Client ID and Client Secrets Update docs//bitbucket-development.md to document how to create a test Bitbucket DC environment and OAuth2 setup

mminns · 2022-10-06T21:35:50Z

🤞

😄

mjcheetham

🎉 Great stuff @mminns!

Glad we could get this over the finish line 😁

mminns · 2022-10-10T20:43:57Z

Awesome, thank you.

I started messing around with this in 2018 and started looking at it in Git Credential Manager for Windows in 2019
microsoft/Git-Credential-Manager-for-Windows#837
https://github.com/mminns/Git-Credential-Manager-for-Windows/tree/mminns/issue-837-Bitbucket-Server-Support

So it's brilliant to finally get it done.

Thanks for all your help, @ldennington , @mjcheetham 👏

🙇

Changes: - Check for broken links in documentation (#700) - Support macOS `arm64` installs via Homebrew (#798) - Validate installers before publishing (#813) - Auto-generate maintainer away notification issues (#842) - Install dotnet via Jammy feeds on Ubuntu 22.04 and greater (#839) - Access Azure storage account using service principle credentials (#851) - Update documentation to use reference-style links (#680) - Unify documentation line length (#862) - Add generic username/password UI (#871) - Bitbucket DC OAuth support (#607) - Distribute GCM as a dotnet tool (#886) - Drop `-core` suffix from entry executable #551 - Speed up build graph (#924)

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch 3 times, most recently from e636ce1 to 2b90f17 Compare January 17, 2022 13:46

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from f62e992 to 2a80e22 Compare February 15, 2022 09:38

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from 2a80e22 to 13cbcab Compare February 22, 2022 10:23

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from 13cbcab to d36e9e5 Compare March 7, 2022 14:11

mminns marked this pull request as ready for review March 11, 2022 09:59

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from d36e9e5 to 7ee209c Compare March 11, 2022 10:00

mminns mentioned this pull request Mar 17, 2022

Add Test Coverage metrics to GCMC code repository #498

Closed

wolf99 mentioned this pull request Apr 15, 2022

Lint markdown #668

Merged

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch 3 times, most recently from c14a342 to c470a10 Compare April 28, 2022 19:24

mjcheetham requested review from ldennington and mjcheetham May 11, 2022 16:02

mjcheetham assigned ldennington May 11, 2022

mjcheetham reviewed May 13, 2022

View reviewed changes

ldennington requested changes May 13, 2022

View reviewed changes

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from c470a10 to 6982a44 Compare May 17, 2022 12:36

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from 6982a44 to 6b6bec7 Compare May 26, 2022 12:19

mminns commented Jul 7, 2022

View reviewed changes

mjcheetham reviewed Oct 5, 2022

View reviewed changes

src/shared/Core/Authentication/OAuth/OAuth2Client.cs Outdated Show resolved Hide resolved

mjcheetham requested changes Oct 5, 2022

View reviewed changes

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from 4e9abf1 to 3ff94e8 Compare October 6, 2022 20:32

mminns added 3 commits October 6, 2022 22:42

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch 5 times, most recently from d40c543 to 72dcf59 Compare October 6, 2022 21:27

mminns force-pushed the issue/573-OAuth2-support-for-Bitbucket-DC branch from 72dcf59 to 6c28d76 Compare October 6, 2022 21:29

mminns requested a review from mjcheetham October 6, 2022 21:35

mjcheetham approved these changes Oct 10, 2022

View reviewed changes

mjcheetham merged commit 228f7cd into git-ecosystem:main Oct 10, 2022

mminns deleted the issue/573-OAuth2-support-for-Bitbucket-DC branch October 10, 2022 20:39

ldennington mentioned this pull request Nov 3, 2022

Release 2.0.866 #944

Merged

This comment has been minimized.

Sign in to view

This was referenced Feb 13, 2023

Support SAML authentication on Bitbucket Server (DataCenter) #151

Closed

Add support for OAuth 2 in Bitbucket DC #573

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/573 OAuth2 support for Bitbucket DC #607

Issue/573 OAuth2 support for Bitbucket DC #607

mminns commented Jan 12, 2022 •

edited

Loading

mminns commented Jan 26, 2022

mminns commented Mar 11, 2022

mjcheetham left a comment

ldennington left a comment

mminns commented May 16, 2022

ldennington commented May 16, 2022

mminns commented May 17, 2022

mminns commented May 17, 2022

mminns commented May 20, 2022

mminns commented May 20, 2022

ldennington commented May 23, 2022

mminns commented May 26, 2022 •

edited

Loading

mminns Jul 7, 2022

mminns commented Jul 7, 2022

mminns commented Oct 5, 2022

mjcheetham left a comment

ldennington commented Oct 6, 2022

mminns commented Oct 6, 2022

mminns commented Oct 6, 2022

mjcheetham left a comment

mminns commented Oct 10, 2022

This comment has been minimized.

This comment has been minimized.

Issue/573 OAuth2 support for Bitbucket DC #607

Issue/573 OAuth2 support for Bitbucket DC #607

Conversation

mminns commented Jan 12, 2022 • edited Loading

mminns commented Jan 26, 2022

mminns commented Mar 11, 2022

mjcheetham left a comment

Choose a reason for hiding this comment

ldennington left a comment

Choose a reason for hiding this comment

mminns commented May 16, 2022

ldennington commented May 16, 2022

mminns commented May 17, 2022

mminns commented May 17, 2022

mminns commented May 20, 2022

mminns commented May 20, 2022

ldennington commented May 23, 2022

mminns commented May 26, 2022 • edited Loading

mminns Jul 7, 2022

Choose a reason for hiding this comment

mminns commented Jul 7, 2022

mminns commented Oct 5, 2022

mjcheetham left a comment

Choose a reason for hiding this comment

ldennington commented Oct 6, 2022

mminns commented Oct 6, 2022

mminns commented Oct 6, 2022

mjcheetham left a comment

Choose a reason for hiding this comment

mminns commented Oct 10, 2022

This comment has been minimized.

This comment has been minimized.

mminns commented Jan 12, 2022 •

edited

Loading

mminns commented May 26, 2022 •

edited

Loading