Added GitHub Enterprise and Private GitHub support #83
Conversation
|
Welcome! Congrats on your first pull request to Jekyll Remote Theme. If you haven't already, please be sure to check out the contributing guidelines. |
|
resolves #81 |
…mote_host as uri. Also added remote_headers config for providing headers such as authorization
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
| ``` | ||
| Where the personal autorization token is the token provided by the github repo setting page. | ||
|
|
||
| > :warning: **Storing credentials can lead to security issues** - As stated in the [Creating a personal access token on GitHub guides](https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token) Treat your tokens like passwords and keep them secret. When working with the API, use tokens as environment variables instead of hardcoding them into your programs. |
There was a problem hiding this comment.
Given that this is explicitly called out in the documentation, I'd be hesitant to create a workflow that requires committing the secret to the repository. Could we use an environmental variable as suggested?
There was a problem hiding this comment.
I fully agree that specifying an environment variable here would prevent "hardcoding" any credentials which can be a security hazard. Unfortunately, unless some changes are done on the server side to allow the GitHub Pages to specify credentials for a privately hosted themes, this was the easiest way to solve the issue.
There was a problem hiding this comment.
So does this mean that there is no way for GitHub Secrets to be accessed in the GitHub Pages environment?
| CONFIG_KEY = "remote_theme" | ||
| CONFIG_REPOSITORY_KEY = "repository" | ||
| CONFIG_THEME_KEY = "remote_theme" | ||
| CONFIG_HEADERS_KEY = "remote_headers" |
There was a problem hiding this comment.
Why would a user need to be able to specify custom headers?
There was a problem hiding this comment.
To allow downloading a private hosted theme and additional headers required in a large organization.
| end | ||
| end | ||
|
|
||
| def fetch(uri_str, limit = 10) |
There was a problem hiding this comment.
Can you describe the change here? I'm having trouble following.
There was a problem hiding this comment.
It takes the remote_headers from the configuration file and ensures that the custom headers are passed to the request of the download of the remote_theme. I had to reorganize the download function code for readability and usability.
More details are provided with additional comments of the relevant code.
| ENV["PAGES_GITHUB_HOSTNAME"], | ||
| ENV["GITHUB_HOSTNAME"], | ||
| host, | ||
| default_host, |
There was a problem hiding this comment.
As currently written, this is an anti-abuse mechanism, which it looks like this change removes. In an untrusted environment, a malicious actor could request a remote theme from a malicious Git server, leading to a denial of service or other attack. How would we limit hosts in untrusted environments?
There was a problem hiding this comment.
The problem for us is that since we are using custom DNS, privately hosted repos, the themes are not hosted on any of theses endpoints.
We are hosting our GitHub Enterprise Server (2.21) with a custom internal DNS so it is not using the default |
| :host => "codeload.#{theme.host}", | ||
| :path => [theme.owner, theme.name, "zip", theme.git_ref].join("/") | ||
| :host => theme.host, | ||
| :path => [theme.owner, theme.name, "archive", theme.git_ref + ".zip"].join("/") |
There was a problem hiding this comment.
According to the GitHub API documentation, host "api.github.com" and rest endpoint "zipball" is the proper endpoint for getting the repository archive zip file. As per the documentation, this requires support for redirections.
Using "archive" should extend the the support to other git private repository such as gitlab and still work for github repos.
| raise_unless_sucess(response) | ||
| enforce_max_file_size(response.content_length) | ||
| response.read_body do |chunk| | ||
| zip_file.write chunk |
There was a problem hiding this comment.
Reorganizing code for better readability and usability
| end | ||
|
|
||
| req | ||
| end |
There was a problem hiding this comment.
Create a request including the remote_headers configuration provided to the downloader
| response.read_body do |chunk| | ||
| zip_file.write chunk | ||
| end | ||
| end |
There was a problem hiding this comment.
Reading the archive file from the server as per the download code above
| when Net::HTTPSuccess | ||
| handle_response(response) | ||
| when Net::HTTPRedirection | ||
| fetch(response["location"], limit - 1) |
There was a problem hiding this comment.
Our organization is using a custom DNS, and the REST Api are creating some redirections. This new download code supports HTTP Redirections to a maximum of 10 hops.
A naive and working attempt to support both Github Enterprise Private and GitHub Private Repositories