FEATURE: Allow for multiple GitHub onebox tokens (#27887)

Followup 560e8aff75

GitHub auth tokens cannot be made with permissions to
access multiple organisations. This is quite limiting.
This commit changes the site setting to be a "secret list"
type, which allows for a key/value mapping where the value
is treated like a password in the UI.

Now when a GitHub URL is requested for oneboxing, the
org name from the URL is used to determine which token
to use for the request.

Just in case anyone used the old site setting already,
there is a migration to create a `default` entry
with that token in the new list setting, and for
a period of time we will consider that token valid to
use for all GitHub oneboxes as well.

app/assets/javascripts/admin/addon/components/site-setting.hbs

@@ -50,12 +50,24 @@
 
 {{#if this.dirty}}
   <div class="setting-controls">
-    <DButton class="ok" @action={{this.update}} @icon="check" />
-    <DButton class="cancel" @action={{this.cancel}} @icon="times" />
+    <DButton
+      @action={{this.update}}
+      @icon="check"
+      class="ok setting-controls__ok"
+    />
+    <DButton
+      @action={{this.cancel}}
+      @icon="times"
+      class="cancel setting-controls__cancel"
+    />
   </div>
 {{else if this.overridden}}
   {{#if this.setting.secret}}
-    <DButton @action={{this.toggleSecret}} @icon="far-eye-slash" />
+    <DButton
+      @action={{this.toggleSecret}}
+      @icon="far-eye-slash"
+      class="setting-toggle-secret"
+    />
   {{/if}}
 
   <DButton

config/locales/server.en.yml

@@ -1758,7 +1758,7 @@ en:
     force_custom_user_agent_hosts: "Hosts for which to use the custom onebox user agent on all requests. (Especially useful for hosts that limit access by user agent)."
     max_oneboxes_per_post: "Set the maximum number of oneboxes that can be included in a single post. Oneboxes provide a preview of linked content within the post."
     facebook_app_access_token: "A token generated from your Facebook app ID and secret. Used to generate Instagram oneboxes."
-    github_onebox_access_token: "A GitHub access token which is used to generate GitHub oneboxes for private repos, commits, pull requests, issues, and file contents. Without this, only public GitHub URLs will be oneboxed."
+    github_onebox_access_tokens: "A mapping of a GitHub organisation or user to a GitHub access token which is used to generate GitHub oneboxes for private repos, commits, pull requests, issues, and file contents. Without this, only public GitHub URLs will be oneboxed."
 
     logo: "The logo image at the top left of your site. Use a wide rectangular image with a height of 120 and an aspect ratio greater than 3:1. If left blank, the site title text will be shown."
     logo_small: "The small logo image at the top left of your site, seen when scrolling down. Use a square 120 × 120 image. If left blank, a home glyph will be shown."

config/site_settings.yml

@@ -2125,9 +2125,11 @@ onebox:
   cache_onebox_user_agent:
     default: ""
     hidden: true
-  github_onebox_access_token:
+  github_onebox_access_tokens:
     default: ""
+    type: list
     secret: true
+    list_type: secret
 spam:
   add_rel_nofollow_to_user_content: true
   hide_post_sensitivity:

db/migrate/20240712050324_default_github_onebox_token.rb

@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class DefaultGithubOneboxToken < ActiveRecord::Migration[7.1]
+  def up
+    existing_token =
+      DB.query_single(
+        "SELECT value FROM site_settings WHERE name = 'github_onebox_access_token'",
+      ).first
+
+    # 8 is the data type for a list
+    execute <<~SQL if existing_token.present?
+      INSERT INTO site_settings (name, data_type, value, created_at, updated_at)
+      VALUES ('github_onebox_access_tokens', 8, 'default|#{existing_token}', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
+      ON CONFLICT DO NOTHING
+    SQL
+
+    execute <<~SQL
+      DELETE FROM site_settings WHERE name = 'github_onebox_access_token'
+    SQL
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

lib/onebox/engine/github_actions_onebox.rb

@@ -65,7 +65,7 @@ module Onebox
       end
 
       def data
-        result = raw(github_auth_header).clone
+        result = raw(github_auth_header(match[:org])).clone
 
         status = "unknown"
         if result["status"] == "completed"

lib/onebox/engine/github_blob_onebox.rb

@@ -28,19 +28,19 @@ module Onebox
       end
 
       def raw_regexp
-        %r{github\.com/(?<user>[^/]+)/(?<repo>[^/]+)/blob/(?<sha1>[^/]+)/(?<file>[^#]+)(#(L(?<from>[^-]*)(-L(?<to>.*))?))?}mi
+        %r{github\.com/(?<org>[^/]+)/(?<repo>[^/]+)/blob/(?<sha1>[^/]+)/(?<file>[^#]+)(#(L(?<from>[^-]*)(-L(?<to>.*))?))?}mi
       end
 
-      def raw_template(m)
-        "https://raw.githubusercontent.com/#{m[:user]}/#{m[:repo]}/#{m[:sha1]}/#{m[:file]}"
+      def raw_template(match)
+        "https://raw.githubusercontent.com/#{match[:org]}/#{match[:repo]}/#{match[:sha1]}/#{match[:file]}"
       end
 
       def title
         Sanitize.fragment(Onebox::Helpers.uri_unencode(link).sub(%r{^https?\://github\.com/}, ""))
       end
 
-      def auth_headers
-        github_auth_header
+      def auth_headers(match)
+        github_auth_header(match[:org])
       end
     end
   end

lib/onebox/engine/github_commit_onebox.rb

@@ -16,7 +16,7 @@ module Onebox
       always_https
 
       def url
-        "https://api.github.com/repos/#{match[:owner]}/#{match[:repository]}/commits/#{match[:sha]}"
+        "https://api.github.com/repos/#{match[:org]}/#{match[:repository]}/commits/#{match[:sha]}"
       end
 
       private
@@ -24,18 +24,17 @@ module Onebox
       def match
         return @match if defined?(@match)
 
-        @match =
-          @url.match(%{github\.com/(?<owner>[^/]+)/(?<repository>[^/]+)/commit/(?<sha>[^/]+)})
+        @match = @url.match(%{github\.com/(?<org>[^/]+)/(?<repository>[^/]+)/commit/(?<sha>[^/]+)})
         @match ||=
           @url.match(
-            %{github\.com/(?<owner>[^/]+)/(?<repository>[^/]+)/pull/(?<pr>[^/]+)/commit/(?<sha>[^/]+)},
+            %{github\.com/(?<org>[^/]+)/(?<repository>[^/]+)/pull/(?<pr>[^/]+)/commit/(?<sha>[^/]+)},
           )
 
         @match
       end
 
       def data
-        result = raw(github_auth_header).clone
+        result = raw(github_auth_header(match[:org])).clone
 
         lines = result["commit"]["message"].split("\n")
         result["title"] = lines.first

lib/onebox/engine/github_issue_onebox.rb

@@ -37,7 +37,7 @@ module Onebox
       end
 
       def data
-        result = raw(github_auth_header).clone
+        result = raw(github_auth_header(match[:org])).clone
         created_at = Time.parse(result["created_at"])
         closed_at = Time.parse(result["closed_at"]) if result["closed_at"]
         body, excerpt = compute_body(result["body"])

lib/onebox/engine/github_pull_request_onebox.rb

@@ -18,18 +18,18 @@ module Onebox
       always_https
 
       def url
-        "https://api.github.com/repos/#{match[:owner]}/#{match[:repository]}/pulls/#{match[:number]}"
+        "https://api.github.com/repos/#{match[:org]}/#{match[:repository]}/pulls/#{match[:number]}"
       end
 
       private
 
       def match
         @match ||=
-          @url.match(%r{github\.com/(?<owner>[^/]+)/(?<repository>[^/]+)/pull/(?<number>[^/]+)})
+          @url.match(%r{github\.com/(?<org>[^/]+)/(?<repository>[^/]+)/pull/(?<number>[^/]+)})
       end
 
       def data
-        result = raw(github_auth_header).clone
+        result = raw(github_auth_header(match[:org])).clone
         result["link"] = link
 
         created_at = Time.parse(result["created_at"])
@@ -78,7 +78,7 @@ module Onebox
       def load_commit(link)
         if commit_match = link.match(%r{commits/(\h+)})
           load_json(
-            "https://api.github.com/repos/#{match[:owner]}/#{match[:repository]}/commits/#{commit_match[1]}",
+            "https://api.github.com/repos/#{match[:org]}/#{match[:repository]}/commits/#{commit_match[1]}",
           )
         end
       end
@@ -86,7 +86,7 @@ module Onebox
       def load_comment(link)
         if comment_match = link.match(/#issuecomment-(\d+)/)
           load_json(
-            "https://api.github.com/repos/#{match[:owner]}/#{match[:repository]}/issues/comments/#{comment_match[1]}",
+            "https://api.github.com/repos/#{match[:org]}/#{match[:repository]}/issues/comments/#{comment_match[1]}",
           )
         end
       end
@@ -94,7 +94,7 @@ module Onebox
       def load_review(link)
         if review_match = link.match(/#discussion_r(\d+)/)
           load_json(
-            "https://api.github.com/repos/#{match[:owner]}/#{match[:repository]}/pulls/comments/#{review_match[1]}",
+            "https://api.github.com/repos/#{match[:org]}/#{match[:repository]}/pulls/comments/#{review_match[1]}",
           )
         end
       end

lib/onebox/engine/gitlab_blob_onebox.rb

@@ -34,7 +34,7 @@ module Onebox
         Sanitize.fragment(Onebox::Helpers.uri_unencode(link).sub(%r{^https?\://gitlab\.com/}, ""))
       end
 
-      def auth_headers
+      def auth_headers(_match)
         {}
       end
     end

lib/onebox/mixins/git_blob_onebox.rb

@@ -173,7 +173,7 @@ module Onebox
               contents =
                 URI
                   .parse(self.raw_template(m))
-                  .open({ read_timeout: timeout }.merge(self.auth_headers))
+                  .open({ read_timeout: timeout }.merge(self.auth_headers(m)))
                   .read
 
               if contents.encoding == Encoding::BINARY || contents.bytes.include?(0)

lib/onebox/mixins/github_auth_header.rb

@@ -3,9 +3,18 @@
 module Onebox
   module Mixins
     module GithubAuthHeader
-      def github_auth_header
-        return {} if SiteSetting.github_onebox_access_token.blank?
-        { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}" }
+      def github_auth_header(github_org)
+        return {} if SiteSetting.github_onebox_access_tokens.blank?
+        org_tokens =
+          SiteSetting.github_onebox_access_tokens.split("\n").map { |line| line.split("|") }.to_h
+
+        # Use the default token if no token is found for the org,
+        # this will be the token that used to be stored in the old
+        # github_onebox_access_token site setting if it was configured.
+        token = org_tokens[github_org] || org_tokens["default"]
+        return {} if token.blank?
+
+        { "Authorization" => "Bearer #{token}" }
       end
     end
   end

lib/oneboxer.rb

@@ -685,7 +685,7 @@ module Oneboxer
     # FinalDestination to request the final URL because no auth headers
     # are sent. In this case we can ignore redirects and go straight to
     # using Onebox.preview
-    if SiteSetting.github_onebox_access_token.present? && uri.hostname == "github.com"
+    if SiteSetting.github_onebox_access_tokens.present? && uri.hostname == "github.com"
       fd_options[:ignore_redirects] << "https://github.com"
     end
 

spec/lib/onebox/engine/github_actions_onebox_spec.rb

@@ -35,13 +35,13 @@ RSpec.describe Onebox::Engine::GithubActionsOnebox do
     end
 
     context "when github_onebox_access_token is configured" do
-      before { SiteSetting.github_onebox_access_token = "1234" }
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
 
       it "sends it as part of the request" do
         html
         expect(WebMock).to have_requested(:get, run_uri).with(
           headers: {
-            "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}",
+            "Authorization" => "Bearer github_pat_1234",
           },
         )
       end
@@ -78,13 +78,13 @@ RSpec.describe Onebox::Engine::GithubActionsOnebox do
     end
 
     context "when github_onebox_access_token is configured" do
-      before { SiteSetting.github_onebox_access_token = "1234" }
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
 
       it "sends it as part of the request" do
         html
         expect(WebMock).to have_requested(:get, pr_run_uri).with(
           headers: {
-            "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}",
+            "Authorization" => "Bearer github_pat_1234",
           },
         )
       end

spec/lib/onebox/engine/github_blob_onebox_spec.rb

@@ -46,13 +46,13 @@ RSpec.describe Onebox::Engine::GithubBlobOnebox do
     end
 
     context "when github_onebox_access_token is configured" do
-      before { SiteSetting.github_onebox_access_token = "1234" }
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
 
       it "sends it as part of the request" do
         html
         expect(WebMock).to have_requested(:get, raw_uri).with(
           headers: {
-            "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}",
+            "Authorization" => "Bearer github_pat_1234",
           },
         )
       end

spec/lib/onebox/engine/github_commit_onebox_spec.rb

@@ -55,14 +55,14 @@ RSpec.describe Onebox::Engine::GithubCommitOnebox do
     end
 
     context "when github_onebox_access_token is configured" do
-      before { SiteSetting.github_onebox_access_token = "1234" }
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
 
       it "sends it as part of the request" do
         html
         expect(WebMock).to have_requested(
           :get,
           "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919",
-        ).with(headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}" })
+        ).with(headers: { "Authorization" => "Bearer github_pat_1234" })
       end
     end
   end
@@ -122,14 +122,14 @@ RSpec.describe Onebox::Engine::GithubCommitOnebox do
     end
 
     context "when github_onebox_access_token is configured" do
-      before { SiteSetting.github_onebox_access_token = "1234" }
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
 
       it "sends it as part of the request" do
         html
         expect(WebMock).to have_requested(
           :get,
           "https://api.github.com/repos/discourse/discourse/commits/803d023e2307309f8b776ab3b8b7e38ba91c0919",
-        ).with(headers: { "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}" })
+        ).with(headers: { "Authorization" => "Bearer github_pat_1234" })
       end
     end
   end

spec/lib/onebox/engine/github_issue_onebox_spec.rb

@@ -24,13 +24,13 @@ RSpec.describe Onebox::Engine::GithubIssueOnebox do
     end
 
     context "when github_onebox_access_token is configured" do
-      before { SiteSetting.github_onebox_access_token = "1234" }
+      before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
 
       it "sends it as part of the request" do
         html
         expect(WebMock).to have_requested(:get, issue_uri).with(
           headers: {
-            "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}",
+            "Authorization" => "Bearer github_pat_1234",
           },
         )
       end

spec/lib/onebox/engine/github_pull_request_onebox_spec.rb

@@ -92,13 +92,13 @@ RSpec.describe Onebox::Engine::GithubPullRequestOnebox do
   end
 
   context "when github_onebox_access_token is configured" do
-    before { SiteSetting.github_onebox_access_token = "1234" }
+    before { SiteSetting.github_onebox_access_tokens = "discourse|github_pat_1234" }
 
     it "sends it as part of the request" do
       html
       expect(WebMock).to have_requested(:get, api_uri).with(
         headers: {
-          "Authorization" => "Bearer #{SiteSetting.github_onebox_access_token}",
+          "Authorization" => "Bearer github_pat_1234",
         },
       )
     end