Proposal: Support looking up GitHub users keeping their email address private

[carlgleisner]

Current behaviour

Users are fetched using GitHub's /search/users endpoint with the parameter q as contributor@example.com in:email.

git_ops/lib/git_ops/github.ex

Lines 40 to 43 in dad5612

	case Req.get("#{GitOps.Config.github_api_base_url()}/search/users",
	headers: github_headers(),
	params: [q: "#{email} in:email", per_page: 2]
	) do

This works fine for user exposing their email addresses. However, the search endpoint will not return any results for the @users.noreply.github.com addresses on commits made by users using the "Keep my email address private" feature.

As a result of the endpoint not returning any records, the lookup for users using this feature fails.

Proposal

Since the username is in included in the email address – such as: carlgleisner@users.noreply.github.com – it is possible to extract that username and call the /users/<username> endpoint instead in those cases.

@doc """
Find a GitHub user by their email address.
Returns {:ok, user} if found, where user contains :username, :id, and :url.
Returns {:error, reason} if not found or if there's an error.
"""
def fetch_user_from_api(nil) do
  {:error, "Error making GitHub API request: No email address"}
end

The above would be a new case to catch no email provided since the (proposed) if email do statement would be replaced with an if statement on whether the provided email address is of the "keep my email address private" variety.

def fetch_user_from_api(email) do
  Application.ensure_all_started(:req)

  if String.match?(email, ~r/@users.noreply.github.com$/) do
    case Req.get("#{GitOps.Config.github_api_base_url()}/users/#{username_from_email(email)}") do
      {:ok, %Req.Response{status: 200, body: user}} ->
        {:ok,
         %{
           username: user["login"],
           id: user["id"],
           url: user["html_url"]
         }}

      {:ok, %Req.Response{status: status, body: body}} ->
        {:error, "GitHub API request failed with status #{status}: #{inspect(body)}"}

      {:error, reason} ->
        {:error, "Error making GitHub API request: #{inspect(reason)}"}
    end

That's the proposed new part, then the rest is as before.

  else
    case Req.get("#{GitOps.Config.github_api_base_url()}/search/users",
           headers: github_headers(),
           params: [q: "#{email} in:email", per_page: 2]
         ) do
      {:ok, %Req.Response{status: 200, body: %{"items" => [first_user | _]}}} ->
        {:ok,
         %{
           username: first_user["login"],
           id: first_user["id"],
           url: first_user["html_url"]
         }}

      {:ok, %Req.Response{status: 200, body: %{"items" => []}}} ->
        {:error, "No user found with email #{email}"}

      {:ok, %Req.Response{status: status, body: body}} ->
        {:error, "GitHub API request failed with status #{status}: #{inspect(body)}"}

      {:error, reason} ->
        {:error, "Error making GitHub API request: #{inspect(reason)}"}
    end
  end
rescue
  error ->
    {:error, "Error making GitHub API request: #{inspect(error)}"}
end

Since the email addresses can have the format 1234567+username@ I propose a private helper function to extract the username:

defp username_from_email(email) do
  Regex.named_captures(~r/^(\d+\+){0,1}(?<username>\w+)@users.noreply.github.com$/, email)
  |> Map.get("username")
end

I've got a working branch of this in my own fork, for what it's worth.

This proposal is provided most humbly of course! 😇

[zachdaniel]

This makes perfect sense to me, and seems like it is only a benefit w/ no downside. PR welcome!

[carlgleisner]

Wilco 🫡

I don't think the GitHub module is under test, right?

Also, in the meantime I found existing logic for extracting username from noreply addresses (duh!) with slightly different regexes:

git_ops/lib/git_ops/commit.ex

Lines 141 to 144 in dad5612

	# Match GitHub noreply emails like 12345678+username@users.noreply.github.com
	String.match?(email, ~r/\d+\+(.+)@users\.noreply\.github\.com/) ->
	captures =
	Regex.named_captures(~r/\d+\+(?<username>.+)@users\.noreply\.github\.com/, email)

I don't know if you want to make something reusable, dryer. Otherwise I'll provide a PR with identical regexes at least.

[zachdaniel]

In this case it would make sense to DRY it up yeah 😄

[carlgleisner]

I'll have a look at it this weekend 🥸

I trust the future of the world doesn't depend on this! Hopefully.