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

Add support for GitHub enterprise-managed user accounts #1190

Merged
merged 3 commits into from
Jun 23, 2023

Conversation

mjcheetham
Copy link
Collaborator

@mjcheetham mjcheetham commented Apr 7, 2023

Add support for GitHub enterprise-manage users (EMU) to the GitHub host provider.

Accounts in an 'EMU' enterprise/business are siloed from the regular, public GitHub.com accounts. EMU accounts are identified by the _shortcode suffix, where the shortcode is a moniker for the enterprise/business, for example alice_contoso.

When asked to recall credentials for the GitHub.com host we now attempt to filter stored accounts by the shortcode, given information provided in WWW-Authenticate headers from upcoming versions of Git that support these headers (as of gitgitgadget/git@92c56da).

The format of the header is:

WWW-Authenticate: Basic realm="GitHub" [domain_hint="X"] [enterprise_hint="Y"]

..where X is the shortcode, and Y is the enterprise name.

If multiple accounts are available for the given 'domain' then we present an account selection prompt. Users can avoid this prompt in the case of multiple user accounts by specifying the desired account in the remote URL (e.g. https://alice@github.com/mona/test to always use the alice account).

Note that GitHub.com does not yet return such WWW-Authenticate headers, except always Basic realm="GitHub", so this may be subject to fixes later. In the case of realm="GitHub", i.e., public accounts, there is no change.

Testing

To test the new behaviour before GitHub.com returns such headers, it's possible to fake the server response using mitmproxy and the following script:

"""Add an HTTP header to each response."""

class AddHeader:
    # initialize a dict with shortcodes and paths
    def __init__(self):
        org1 = ("domain1", "enterprise1")
        org2 = ("domain2", "enterprise2")

        self.orgMap = {
            "org1" : enterprise1,
            "org2" : enterprise1,
            "org3" : enterprise2,
        }

    def response(self, flow):
        if flow.response.status_code == 401:
            # lookup the correct shortcode based on the org
            org = flow.request.path.split("/")[1]
            if org not in self.orgMap:
                return
            domain_hint = self.orgMap[org][0]
            enterprise_hint = self.orgMap[org][1]
            # build the header
            header = "Basic realm=\"GitHub\" enterprise_hint=\"" + enterprise_hint + "\" domain_hint=\"" + domain_hint + "\""
            # set the header
            flow.response.headers["WWW-Authenticate"] = header

addons = [
    AddHeader()
]

Replace orgN with the org names that are backed by an EMU Enterprise, and fill domainN for the shortcode, and enterpriseN for the enterprise slug/name.

Configure Git to use the proxy and run mitmproxy with the --scripts argument:

git config --global http.proxy 'http://127.0.0.1:8080'
mitmproxy --scripts <SCRIPT>

Now all Git interactions that touch orgN will include the domain_hint and enterprise_hints as defined.

I use these two helpful aliases to quickly add and remove the local proxy from Git's config:

[alias]
	mitm = "!f(){ git config --global http.proxy 'http://127.0.0.1:8080'; }; f"
	unmitm = "!f(){ git config --global --unset http.proxy; }; f"

@mjcheetham mjcheetham added enhancement New feature or request host:github Specific to the GitHub host provider labels Apr 7, 2023
@mjcheetham mjcheetham requested a review from ldennington April 7, 2023 16:53
Jekyle944

This comment was marked as spam.

@mjcheetham mjcheetham force-pushed the github-emu branch 3 times, most recently from 696c36a to 90ff39d Compare May 26, 2023 03:18
@mjcheetham mjcheetham force-pushed the github-emu branch 7 times, most recently from 75bdc87 to bab4d3d Compare June 13, 2023 16:31
@mjcheetham mjcheetham force-pushed the github-emu branch 2 times, most recently from 29eaa0c to eee841a Compare June 15, 2023 18:19
@mjcheetham mjcheetham marked this pull request as ready for review June 15, 2023 18:19
Copy link
Contributor

@ldennington ldennington left a comment

Choose a reason for hiding this comment

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

Looks great! So excited this is finally happening! Just had a couple minor questions.

src/shared/GitHub/GitHubConstants.cs Show resolved Hide resolved
src/shared/GitHub/GitHubHostProvider.cs Outdated Show resolved Hide resolved
Add parsing of GitHub.com's WWW-Authenticate header, with the upcoming
enterprise_hint and domain_hint properties that can be used to indicate
when a resource (repository) requires a specific EMU account.
If we have been given a domain_hint in the WWW-Authenticate headers we
should use that value to filter any existing accounts we have stored.

The header format is:

WWW-Authenticate: Basic realm="GitHub" [enterprise_hint="X"] [domain_hint="Y"]

..where X is the enterprise slug/name, and Y is the enterprise 'shortcode'.

The shortcode is the suffix applied to GitHub.com accounts that are
EMUs (Enterprise Managed Users). That is to say they are backed by an
external IdP (Identity Provider).

If we have not been given any WWW-Authenticate header (such as with
older versions of Git), do not do any filtering. Likewise, if the remote
is not GitHub.com (the only place EMUs mingle with other account types)
then do no filtering.
Copy link
Contributor

@ldennington ldennington left a comment

Choose a reason for hiding this comment

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

Went through all the changes + re-did the testing workflows and all looks good! 🎉

@mjcheetham mjcheetham merged commit a211bab into git-ecosystem:main Jun 23, 2023
@mjcheetham mjcheetham deleted the github-emu branch June 23, 2023 20:23
@mjcheetham mjcheetham mentioned this pull request Jun 26, 2023
mjcheetham added a commit that referenced this pull request Jun 26, 2023
**Changes:**

- Use in-proc methods for getting OS version number (#1240, #1264)
- Update System.CommandLine (#1265)
- Suppress GUI from command-line argument (#1267)
- Add github (login|logout|list) commands (#1267)
- cURL Cookie file support (#1251)
- Update target framework on Mac/Linux to .NET 7 (#1274, #1282)
- Replace JSON.NET with System.Text.Json (#1274)
- Preserve exact redirect URI formatting in OAuth requests (#1281)
- Use IP localhost redirect for GitHub (#1286)
- Use WWW-Authenticate headers from Git for Azure Repos authority
(#1288)
- Better GitHub Enterprise Managed User (EMU) account support (#1190)
{
if (!IsGitHubDotCom(remoteUri))
{
_context.Trace.WriteLine("No account filtering outside of GitHub.com.");

Choose a reason for hiding this comment

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

Should everything below be skipped if remoteUri is not a github.com by returning a false? Like, in case there is a link to the GHES.

@matthewarkin
Copy link

Sorry to bump an old topic, but I'm curious if there's been any plans for Github.com to support the enterprise/domain hints that have been implemented here?

@mjcheetham
Copy link
Collaborator Author

mjcheetham commented Jun 3, 2024

Sorry to bump an old topic, but I'm curious if there's been any plans for Github.com to support the enterprise/domain hints that have been implemented here?

Hello! 👋 When you say "plans for Github.com to support the .. hints" I'm not sure what you mean? GitHub.com will return domain_hint and enterprise_hint for EMU repos in the WWW-Authenticate header.

Do you mean hints for non-EMU/public repos?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request host:github Specific to the GitHub host provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants