feat: Add https auth #748

jonathanmorley · 2024-04-05T18:55:06Z

Shepherd currently uses ssh git urls to clone repositories.
This assumes that ssh auth is available in the environment for all repositories being cloned.

We are trying to use GitHub Actions to run Shepherd, with a GitHub App providing authentication.
This works for the API (we can have the GitHub App's API token get used by shepherd), but GitHub Apps do not have SSH keys associated with them to allow git pushes over ssh.

They can however use http authentication with their GitHub token to perform git pushes, which this PR allows for using (SHEPHERD_GITHUB_ENTERPRISE_BASE_URL=x-access-token:${GITHUB_PAT}@github.com)

kavitha186 · 2024-04-06T19:01:13Z

README.md

@@ -26,7 +26,7 @@ npm install -g @nerdwallet/shepherd
 If using GitHub Enterprise, ensure the following environment variables are exported:

 ```
-export SHEPHERD_GITHUB_ENTERPRISE_BASE_URL={company_github_enterprise_base_url} # e.g., api.github.com
+export SHEPHERD_GITHUB_ENTERPRISE_BASE_URL={company_github_enterprise_base_url} # e.g., github.com


Should we add both as example Github enterprise server (github.com) and api.github.com (Cloud github enterprise)? @jonathanmorley

Im not sure api.github.com is ever appropriate here.
I am using GitHub Enterprise Cloud, and github.com is the right value for this setting.
I would expect GitHub Enterprise Server to be installation-dependent.

When working with GitHub, the API endpoints differ based on whether you are using GitHub's public cloud service, GitHub Enterprise Cloud, or GitHub Enterprise Server (on-premises). Here’s how the endpoints vary:

GitHub Public (GitHub.com): https://api.github.com/.

GitHub Enterprise Cloud: https://api.github.com/enterprise/[enterprise-name]

GitHub Enterprise Server (On-Premises): https://[hostname]/api/v3/

In each case, the API functionality and how you interact with it are largely the same, but the base URL changes based on where your GitHub instance is hosted.

I think what you are describing is the API endpoint for GitHub, which is configurable in shepherd with SHEPHERD_GITHUB_ENTERPRISE_URL.

However, this environment variable is SHEPHERD_GITHUB_ENTERPRISE_BASE_URL, which appears to only be used as a hostname to generate git clone URLs.
I have observed both public github.com and github enterprise cloud using github.com for these values.

I've observed this as well using Enterprise Cloud

kavitha186 · 2024-04-06T19:12:14Z

src/adapters/github.ts

@@ -309,7 +305,16 @@ class GithubAdapter extends GitAdapter {
  }

  protected getRepositoryUrl(repo: IRepo): string {
-    return `git@${gitHubEnterpriseBaseUrl}:${repo.owner}/${repo.name}.git`;
+    const gitHubEnterpriseBaseUrl = process.env.SHEPHERD_GITHUB_ENTERPRISE_BASE_URL ?? 'github.com';


How moving the gitHubEnterpriseBaseUrl initialized outside class?

This is moved for a couple of reasons:

Having it here improves testability (the tests would otherwise need to reimport the file within the tests)

This reduces the scope of the variable (from file, to method), without loss, since the variable is only used in this method.

As per my other comment, we should default to api.github.com. Additionally, as this has already been rolled out to consumers, we would want to avoid breaking consumers that depend on it being defaulted to api.github.com.

Yeah I like having the callout in the readme

aorinevo · 2024-04-10T22:41:00Z

src/adapters/github.ts

@@ -309,7 +305,16 @@ class GithubAdapter extends GitAdapter {
  }

  protected getRepositoryUrl(repo: IRepo): string {
-    return `git@${gitHubEnterpriseBaseUrl}:${repo.owner}/${repo.name}.git`;
+    const gitHubEnterpriseBaseUrl = process.env.SHEPHERD_GITHUB_ENTERPRISE_BASE_URL ?? 'github.com';
+    const githubProtocol = process.env.SHEPHERD_GITHUB_PROTOCOL ?? 'ssh';


Curious, what's the reasoning for defaulting to ssh instead of https?

The only option right now is ssh. Setting the default to ssh maintains the backwards compatability

Hi @jonathanmorley, I've finally got some time to circle back on this. Changes make sense and you're correct that as the code currently exists, it only supports ssh, though it's rather odd as I've certainly used it for https without making any changes.

That said, the changes in this PR explicitly support https protocol and I'll work to land these changes this week.

At the moment I'm working through some dependency upgrades and ESM/CommonJS transpilation issues. Once complete, this will be a fast follow.

aorinevo · 2024-04-10T22:44:19Z

README.md

@@ -26,7 +26,7 @@ npm install -g @nerdwallet/shepherd
 If using GitHub Enterprise, ensure the following environment variables are exported:

 ```
-export SHEPHERD_GITHUB_ENTERPRISE_BASE_URL={company_github_enterprise_base_url} # e.g., api.github.com
+export SHEPHERD_GITHUB_ENTERPRISE_BASE_URL={company_github_enterprise_base_url} # e.g., github.com


When working with GitHub, the API endpoints differ based on whether you are using GitHub's public cloud service, GitHub Enterprise Cloud, or GitHub Enterprise Server (on-premises). Here’s how the endpoints vary:

GitHub Public (GitHub.com): https://api.github.com/.

GitHub Enterprise Cloud: https://api.github.com/enterprise/[enterprise-name]

GitHub Enterprise Server (On-Premises): https://[hostname]/api/v3/

In each case, the API functionality and how you interact with it are largely the same, but the base URL changes based on where your GitHub instance is hosted.

aorinevo · 2024-04-10T22:46:06Z

src/adapters/github.ts

@@ -309,7 +305,16 @@ class GithubAdapter extends GitAdapter {
  }

  protected getRepositoryUrl(repo: IRepo): string {
-    return `git@${gitHubEnterpriseBaseUrl}:${repo.owner}/${repo.name}.git`;
+    const gitHubEnterpriseBaseUrl = process.env.SHEPHERD_GITHUB_ENTERPRISE_BASE_URL ?? 'github.com';


As per my other comment, we should default to api.github.com. Additionally, as this has already been rolled out to consumers, we would want to avoid breaking consumers that depend on it being defaulted to api.github.com.

aorinevo · 2024-04-10T22:47:38Z

@jonathanmorley, thank you for contributing. When you get a chance, can you update the PR description with context for these changes?

cal4 · 2024-07-26T14:08:15Z

README.md

Think it'd be useful to add a blurb to the Environment Variables table describing the new option for more exposure, something like:
| SHEPHERD_GITHUB_PROTOCOL | ssh | (github adapter) is the protocol to use when cloning repos. Can be https or ssh. https is useful when ssh is firewalled. |

cal4 · 2024-07-26T14:10:38Z

Thanks for contributing this!

aorinevo · 2024-10-03T05:50:05Z

While CI failures are valid, landing on feature branch so that we can collaborate on upstream branch to resolve them.

aorinevo · 2024-10-03T05:52:26Z

Please refer to #847 to track progress towards landing on upstream.

* feat: Add https auth (#748) --------- Co-authored-by: Jonathan Morley <morley.jonathan@gmail.com>

# [2.6.0](v2.5.0...v2.6.0) (2024-10-03) ### Features * **auth:** add support for https protocol ([#748](#748)) ([#847](#847)) ([c0e0596](c0e0596))

jonathanmorley added 2 commits April 5, 2024 14:53

feat: Enable https auth for github

cdd2bf4

fix: update SHEPHERD_GITHUB_ENTERPRISE_BASE_URL

42bda81

aorinevo added the enhancement New feature or request label Apr 5, 2024

aorinevo requested review from kavitha186 and aorinevo April 5, 2024 20:27

kavitha186 reviewed Apr 6, 2024

View reviewed changes

aorinevo reviewed Apr 10, 2024

View reviewed changes

aorinevo assigned jonathanmorley Apr 10, 2024

jonathanmorley changed the title ~~Add https auth~~ feat: Add https auth Apr 16, 2024

cal4 reviewed Jul 26, 2024

View reviewed changes

cal4 mentioned this pull request Jul 26, 2024

Checkout repos using https versus ssh #820

Closed

aorinevo closed this Oct 3, 2024

aorinevo reopened this Oct 3, 2024

aorinevo changed the base branch from main to feat/add-https-auth October 3, 2024 05:48

aorinevo self-requested a review October 3, 2024 05:50

aorinevo merged commit 0dd67d6 into NerdWalletOSS:feat/add-https-auth Oct 3, 2024
0 of 7 checks passed

aorinevo added a commit that referenced this pull request Oct 3, 2024

feat(auth): add support for https protocol (#748) (#847)

c0e0596

* feat: Add https auth (#748) --------- Co-authored-by: Jonathan Morley <morley.jonathan@gmail.com>

aorinevo pushed a commit that referenced this pull request Oct 3, 2024

chore(release): 2.6.0

70aa2fb

# [2.6.0](v2.5.0...v2.6.0) (2024-10-03) ### Features * **auth:** add support for https protocol ([#748](#748)) ([#847](#847)) ([c0e0596](c0e0596))

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add https auth #748

feat: Add https auth #748

jonathanmorley commented Apr 5, 2024 •

edited

Loading

kavitha186 Apr 6, 2024

jonathanmorley Apr 9, 2024

aorinevo Apr 10, 2024 •

edited

Loading

jonathanmorley Apr 16, 2024

cal4 Jul 26, 2024

kavitha186 Apr 6, 2024

jonathanmorley Apr 9, 2024

aorinevo Apr 10, 2024

cal4 Jul 26, 2024

aorinevo Apr 10, 2024

jonathanmorley Apr 16, 2024

aorinevo Sep 30, 2024

aorinevo Apr 10, 2024 •

edited

Loading

aorinevo Apr 10, 2024

aorinevo commented Apr 10, 2024 •

edited

Loading

cal4 Jul 26, 2024 •

edited

Loading

cal4 commented Jul 26, 2024

aorinevo commented Oct 3, 2024

aorinevo commented Oct 3, 2024

feat: Add https auth #748

feat: Add https auth #748

Conversation

jonathanmorley commented Apr 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aorinevo Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aorinevo Apr 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aorinevo commented Apr 10, 2024 • edited Loading

cal4 Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

cal4 commented Jul 26, 2024

aorinevo commented Oct 3, 2024

aorinevo commented Oct 3, 2024

jonathanmorley commented Apr 5, 2024 •

edited

Loading

aorinevo Apr 10, 2024 •

edited

Loading

aorinevo Apr 10, 2024 •

edited

Loading

aorinevo commented Apr 10, 2024 •

edited

Loading

cal4 Jul 26, 2024 •

edited

Loading