From 62db69f1797cf62f616317452cbd3755bc25b4d5 Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:53:12 +0800 Subject: [PATCH 01/10] feat(integrations): add a GitHub App API client Signs the app JWT with the private key, mints installation tokens scoped to one repository with issues:write, and caches them until five minutes before they expire. Also lists the installation's repositories, exchanges an OAuth code for a user token, and lists the installations that user can see. Co-Authored-By: Claude Opus 5.5 --- lib/integrations/github/app_client.rb | 91 +++++++++++++++ .../integrations/github/app_client_spec.rb | 108 ++++++++++++++++++ 2 files changed, 199 insertions(+) create mode 100644 lib/integrations/github/app_client.rb create mode 100644 spec/lib/integrations/github/app_client_spec.rb diff --git a/lib/integrations/github/app_client.rb b/lib/integrations/github/app_client.rb new file mode 100644 index 0000000000..573c65315f --- /dev/null +++ b/lib/integrations/github/app_client.rb @@ -0,0 +1,91 @@ +# Talks to GitHub as the "Pathors Inbox" GitHub App. Nothing long-lived is stored: +# the app's private key signs a short JWT, which buys an installation token that +# expires within the hour, so an uninstall on GitHub cuts us off immediately. +class Integrations::Github::AppClient + API_URL = 'https://api.github.com'.freeze + OAUTH_TOKEN_URL = 'https://github.com/login/oauth/access_token'.freeze + API_VERSION = '2022-11-28'.freeze + PER_PAGE = 100 + TOKEN_REFRESH_MARGIN = 5.minutes + # 422 is how the token endpoint says the requested repository is no longer + # granted to the installation; every one of these needs the admin to reconnect. + REAUTHORIZATION_STATUSES = [401, 403, 404, 422].freeze + + class Error < StandardError; end + class AuthorizationError < Error; end + + def installation_token(installation_id, repository: nil) + cache_key = ['github_app_installation_token', installation_id, repository&.downcase].compact.join(':') + cached = Rails.cache.read(cache_key) + return cached if cached.present? + + body = { repositories: [repository.split('/').last], permissions: { issues: 'write' } } if repository.present? + response = HTTParty.post("#{API_URL}/app/installations/#{installation_id}/access_tokens", + headers: headers(app_jwt), body: body&.to_json) + ensure_success!(response) + + expires_in = Time.zone.parse(response['expires_at']) - TOKEN_REFRESH_MARGIN - Time.current + Rails.cache.write(cache_key, response['token'], expires_in: expires_in) + response['token'] + end + + def repositories(installation_id) + paginate("#{API_URL}/installation/repositories", installation_token(installation_id), 'repositories') + .pluck('full_name') + end + + def exchange_code(code) + response = HTTParty.post(OAUTH_TOKEN_URL, + headers: { 'Accept' => 'application/json' }, + body: { client_id: config('GITHUB_APP_CLIENT_ID'), client_secret: config('GITHUB_APP_CLIENT_SECRET'), code: code }) + ensure_success!(response) + # GitHub answers a bad or expired code with 200 and an `error` field. + raise Error, "GitHub code exchange failed: #{response['error'] || response.code}" if response['access_token'].blank? + + response['access_token'] + end + + def user_installation_ids(user_token) + paginate("#{API_URL}/user/installations", user_token, 'installations').pluck('id') + end + + private + + def paginate(url, token, key) + results = [] + next_url = "#{url}?per_page=#{PER_PAGE}" + while next_url + response = HTTParty.get(next_url, headers: headers(token)) + ensure_success!(response) + results.concat(response[key]) + next_url = response.headers['link']&.match(/<([^>]+)>;\s*rel="next"/)&.captures&.first + end + results + end + + def ensure_success!(response) + return if response.success? + + error_class = REAUTHORIZATION_STATUSES.include?(response.code) ? AuthorizationError : Error + raise error_class, "GitHub request to #{response.request.last_uri} failed: #{response.code} #{response.body}" + end + + def app_jwt + now = Time.current.to_i + JWT.encode({ iss: config('GITHUB_APP_ID'), iat: now - 60, exp: now + 9.minutes.to_i }, + OpenSSL::PKey::RSA.new(config('GITHUB_APP_PRIVATE_KEY')), 'RS256') + end + + def headers(token) + { + 'Authorization' => "Bearer #{token}", + 'Accept' => 'application/vnd.github+json', + 'X-GitHub-Api-Version' => API_VERSION, + 'Content-Type' => 'application/json' + } + end + + def config(key) + GlobalConfigService.load(key, nil) + end +end diff --git a/spec/lib/integrations/github/app_client_spec.rb b/spec/lib/integrations/github/app_client_spec.rb new file mode 100644 index 0000000000..9b624fd1df --- /dev/null +++ b/spec/lib/integrations/github/app_client_spec.rb @@ -0,0 +1,108 @@ +require 'rails_helper' + +describe Integrations::Github::AppClient do + subject(:client) { described_class.new } + + let(:private_key) { OpenSSL::PKey::RSA.generate(2048) } + let(:tokens_url) { 'https://api.github.com/app/installations/4242/access_tokens' } + let(:token_response) { { token: 'ghs_installation_token', expires_at: 1.hour.from_now.iso8601 } } + let(:json_headers) { { 'Content-Type' => 'application/json' } } + + before do + allow(GlobalConfigService).to receive(:load).and_call_original + { + 'GITHUB_APP_ID' => '123456', + 'GITHUB_APP_CLIENT_ID' => 'Iv1.client', + 'GITHUB_APP_CLIENT_SECRET' => 'client-secret', + 'GITHUB_APP_PRIVATE_KEY' => private_key.to_pem + }.each { |key, value| allow(GlobalConfigService).to receive(:load).with(key, nil).and_return(value) } + end + + describe '#installation_token' do + it 'signs an app JWT with the private key and returns the installation token' do + stub_request(:post, tokens_url).to_return(status: 201, body: token_response.to_json, headers: json_headers) + + expect(client.installation_token(4242)).to eq('ghs_installation_token') + + expect(WebMock).to(have_requested(:post, tokens_url).with do |request| + jwt = request.headers['Authorization'].delete_prefix('Bearer ') + payload, header = JWT.decode(jwt, private_key.public_key, true, algorithm: 'RS256') + header['alg'] == 'RS256' && payload['iss'] == '123456' && request.body.blank? + end) + end + + it 'scopes the token to the repository and to issues when a repository is given' do + stub_request(:post, tokens_url).to_return(status: 201, body: token_response.to_json, headers: json_headers) + + client.installation_token(4242, repository: 'pathorsAI/pathors') + + expect(WebMock).to have_requested(:post, tokens_url) + .with(body: { repositories: ['pathors'], permissions: { issues: 'write' } }.to_json) + end + + it 'reuses a cached token until five minutes before it expires' do + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + stub_request(:post, tokens_url).to_return(status: 201, body: token_response.to_json, headers: json_headers) + + freeze_time do + 2.times { client.installation_token(4242) } + expect(WebMock).to have_requested(:post, tokens_url).once + + travel 54.minutes + client.installation_token(4242) + expect(WebMock).to have_requested(:post, tokens_url).once + + travel 2.minutes + client.installation_token(4242) + expect(WebMock).to have_requested(:post, tokens_url).twice + end + end + + it 'raises an authorization error when the installation is gone' do + stub_request(:post, tokens_url).to_return(status: 404, body: { message: 'Not Found' }.to_json, headers: json_headers) + + expect { client.installation_token(4242) }.to raise_error(described_class::AuthorizationError, /404/) + end + end + + describe '#repositories' do + it 'follows pagination and returns every full name' do + stub_request(:post, tokens_url).to_return(status: 201, body: token_response.to_json, headers: json_headers) + stub_request(:get, 'https://api.github.com/installation/repositories?per_page=100') + .with(headers: { 'Authorization' => 'Bearer ghs_installation_token' }) + .to_return(status: 200, body: { repositories: [{ full_name: 'pathorsAI/pathors' }] }.to_json, + headers: json_headers.merge('Link' => '; rel="next"')) + stub_request(:get, 'https://api.github.com/installation/repositories?per_page=100&page=2') + .to_return(status: 200, body: { repositories: [{ full_name: 'pathorsAI/inbox' }] }.to_json, headers: json_headers) + + expect(client.repositories(4242)).to eq(%w[pathorsAI/pathors pathorsAI/inbox]) + end + end + + describe '#exchange_code' do + it 'returns the user token' do + stub_request(:post, 'https://github.com/login/oauth/access_token') + .with(body: { client_id: 'Iv1.client', client_secret: 'client-secret', code: 'oauth-code' }) + .to_return(status: 200, body: { access_token: 'ghu_user_token' }.to_json, headers: json_headers) + + expect(client.exchange_code('oauth-code')).to eq('ghu_user_token') + end + + it 'raises when GitHub rejects the code' do + stub_request(:post, 'https://github.com/login/oauth/access_token') + .to_return(status: 200, body: { error: 'bad_verification_code' }.to_json, headers: json_headers) + + expect { client.exchange_code('stale') }.to raise_error(described_class::Error, /bad_verification_code/) + end + end + + describe '#user_installation_ids' do + it 'lists the installations the user can see' do + stub_request(:get, 'https://api.github.com/user/installations?per_page=100') + .with(headers: { 'Authorization' => 'Bearer ghu_user_token' }) + .to_return(status: 200, body: { installations: [{ id: 4242 }, { id: 7 }] }.to_json, headers: json_headers) + + expect(client.user_installation_ids('ghu_user_token')).to eq([4242, 7]) + end + end +end From 0408b8d937455014ffb52fc1ecd03f33c5cadcc5 Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:53:13 +0800 Subject: [PATCH 02/10] feat(integrations): open GitHub issues as the GitHub App The processor now authenticates with an installation token instead of a personal access token pasted into the hook settings. It skips hooks that have no installation (left over from the token setup), no repository, or a pending reconnect. When GitHub answers 401, 403 without rate-limit headers, or 404, it flags the hook for reconnect instead of raising. The hook settings schema drops access_token and the generic settings form; reference_id holds the installation id. Co-Authored-By: Claude Opus 5.5 --- config/integration/apps.yml | 29 +----- lib/integrations/github/processor_service.rb | 38 ++++++-- spec/factories/integrations/hooks.rb | 3 +- .../github/processor_service_spec.rb | 88 +++++++++++++++++-- 4 files changed, 118 insertions(+), 40 deletions(-) diff --git a/config/integration/apps.yml b/config/integration/apps.yml index e01e34ea0e..642dc61fc9 100644 --- a/config/integration/apps.yml +++ b/config/integration/apps.yml @@ -373,46 +373,21 @@ github: id: github logo: github.png i18n_key: github - action: /github hook_type: account allow_multiple_hooks: false + # The installation id lives in reference_id; GitHub tokens are minted per request and never stored. settings_json_schema: { 'type': 'object', 'properties': { - 'access_token': { 'type': 'string' }, 'repository': { 'type': 'string', 'pattern': '^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$' }, 'label': { 'type': 'string' }, }, - 'required': ['access_token', 'repository'], + 'required': [], 'additionalProperties': false, } - settings_form_schema: - [ - { - 'label': 'Personal Access Token', - 'type': 'password', - 'name': 'access_token', - 'validation': 'required', - 'help': 'A GitHub personal access token that is allowed to create issues in the repository below.', - }, - { - 'label': 'Repository', - 'type': 'text', - 'name': 'repository', - 'validation': 'required', - 'help': 'The repository issues are created in, in owner/repository format.', - }, - { - 'label': 'Label', - 'type': 'text', - 'name': 'label', - 'validation': '', - 'help': 'Optional. Applied to every issue this integration creates.', - }, - ] visible_properties: ['repository', 'label'] # Twenty CRM (fork). Links contacts to Twenty people, keeps both sides' blank diff --git a/lib/integrations/github/processor_service.rb b/lib/integrations/github/processor_service.rb index 89c5734911..0300b6add6 100644 --- a/lib/integrations/github/processor_service.rb +++ b/lib/integrations/github/processor_service.rb @@ -7,25 +7,37 @@ class Integrations::Github::ProcessorService include ::Rails.application.routes.url_helpers ISSUES_URL = 'https://api.github.com/repos/%s/issues'.freeze - API_VERSION = '2022-11-28'.freeze DESCRIPTION_LIMIT = 500 pattr_initialize [:hook!, :event_name!, :event_data!] def perform - return unless event_name == 'ticket.created' - # The event can reach us more than once (job re-enqueue, replayed dispatch); - # a case that already has an issue must not get a second one. - return if conversation.additional_attributes['github_issue'].present? + return unless issue_wanted? response = create_issue + return hook.prompt_reauthorization! if access_lost?(response) return log_failure(response) unless response.success? record_issue(response.parsed_response) + rescue Integrations::Github::AppClient::AuthorizationError => e + Rails.logger.warn("GitHub installation #{hook.reference_id} refused hook #{hook.id}: #{e.message}") + hook.prompt_reauthorization! end private + def issue_wanted? + return false unless event_name == 'ticket.created' + # Not installed through the app yet (a hook left over from the old token + # setup), no repository picked, or GitHub already told us the installation + # is gone: nothing to do until an admin finishes the setup. + return false if hook.reference_id.blank? || settings[:repository].blank? || hook.reauthorization_required? + + # The event can reach us more than once (job re-enqueue, replayed dispatch); + # a case that already has an issue must not get a second one. + conversation.additional_attributes['github_issue'].blank? + end + def ticket event_data[:ticket] end @@ -42,15 +54,19 @@ def create_issue HTTParty.post( format(ISSUES_URL, repository: settings[:repository]), headers: { - 'Authorization' => "Bearer #{settings[:access_token]}", + 'Authorization' => "Bearer #{installation_token}", 'Accept' => 'application/vnd.github+json', - 'X-GitHub-Api-Version' => API_VERSION, + 'X-GitHub-Api-Version' => Integrations::Github::AppClient::API_VERSION, 'Content-Type' => 'application/json' }, body: issue_payload.to_json ) end + def installation_token + Integrations::Github::AppClient.new.installation_token(hook.reference_id, repository: settings[:repository]) + end + def issue_payload payload = { title: ticket.subject, body: issue_body } payload[:labels] = [settings[:label]] if settings[:label].present? @@ -101,6 +117,14 @@ def record_issue(issue) ) end + # Uninstalled, suspended, or the repository left the installation. A 403 that + # carries rate-limit headers is throttling, which passes on its own. + def access_lost?(response) + return true if [401, 404].include?(response.code) + + response.code == 403 && response.headers['retry-after'].blank? && response.headers['x-ratelimit-remaining'] != '0' + end + def log_failure(response) Rails.logger.error("GitHub issue creation failed for hook #{hook.id} on #{settings[:repository]}: #{response.code} #{response.body}") end diff --git a/spec/factories/integrations/hooks.rb b/spec/factories/integrations/hooks.rb index 9ba7aeaa04..c2f9a4c7a3 100644 --- a/spec/factories/integrations/hooks.rb +++ b/spec/factories/integrations/hooks.rb @@ -47,7 +47,8 @@ trait :github do app_id { 'github' } - settings { { 'access_token' => 'github_pat_token', 'repository' => 'pathorsAI/chatwoot' } } + reference_id { '4242' } + settings { { 'repository' => 'pathorsAI/chatwoot' } } end trait :shopify do diff --git a/spec/lib/integrations/github/processor_service_spec.rb b/spec/lib/integrations/github/processor_service_spec.rb index 4f4680e11a..f5aa279739 100644 --- a/spec/lib/integrations/github/processor_service_spec.rb +++ b/spec/lib/integrations/github/processor_service_spec.rb @@ -9,10 +9,20 @@ let(:hook) { create(:integrations_hook, :github, account: account) } let(:ticket) { create(:ticket, conversation: conversation, subject: 'Refund never arrived', ticket_type: 'issue') } let(:issues_url) { 'https://api.github.com/repos/pathorsAI/chatwoot/issues' } + let(:tokens_url) { 'https://api.github.com/app/installations/4242/access_tokens' } let(:issue_response) do { 'html_url' => 'https://github.com/pathorsAI/chatwoot/issues/42', 'number' => 42 } end + before do + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_ID', nil).and_return('123456') + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_PRIVATE_KEY', nil).and_return(OpenSSL::PKey::RSA.generate(2048).to_pem) + stub_request(:post, tokens_url) + .to_return(status: 201, body: { token: 'ghs_installation_token', expires_at: 1.hour.from_now.iso8601 }.to_json, + headers: { 'Content-Type' => 'application/json' }) + end + describe '#perform' do context 'when a ticket is created' do before do @@ -27,7 +37,7 @@ described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform expect(WebMock).to have_requested(:post, issues_url) - .with(headers: { 'Authorization' => 'Bearer github_pat_token', 'X-GitHub-Api-Version' => '2022-11-28' }) { |request| + .with(headers: { 'Authorization' => 'Bearer ghs_installation_token', 'X-GitHub-Api-Version' => '2022-11-28' }) { |request| body = JSON.parse(request.body) expect(body['title']).to eq('Refund never arrived') expect(body['body']).to include('Ada Lovelace ') @@ -84,19 +94,87 @@ end end - context 'when github rejects the request' do - it 'logs the failure without raising or recording an issue' do + context 'when no repository has been picked yet' do + it 'does not call github at all' do + hook.update!(settings: {}) + + described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform + + expect(WebMock).not_to have_requested(:post, tokens_url) + expect(WebMock).not_to have_requested(:post, issues_url) + end + end + + context 'when the hook predates the GitHub App' do + it 'does not call github at all' do + hook.update!(reference_id: nil) + + described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform + + expect(WebMock).not_to have_requested(:post, tokens_url) + end + end + + context 'when the hook is waiting for a reconnect' do + it 'does not call github at all' do + hook.prompt_reauthorization! + + described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform + + expect(WebMock).not_to have_requested(:post, tokens_url) + end + end + + context 'when the app no longer reaches the repository' do + it 'asks for a reconnect without raising or recording an issue' do stub_request(:post, issues_url).to_return(status: 404, body: { message: 'Not Found' }.to_json, headers: { 'Content-Type' => 'application/json' }) - allow(Rails.logger).to receive(:error) expect do described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform end.not_to raise_error - expect(Rails.logger).to have_received(:error).with(/404.*Not Found/) + expect(hook.reauthorization_required?).to be(true) expect(conversation.reload.additional_attributes['github_issue']).to be_nil end end + + context 'when github throttles the request' do + it 'keeps the hook connected' do + stub_request(:post, issues_url).to_return(status: 403, body: { message: 'secondary rate limit' }.to_json, + headers: { 'Content-Type' => 'application/json', 'Retry-After' => '60' }) + + described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform + + expect(hook.reauthorization_required?).to be(false) + end + end + + context 'when the installation is gone' do + it 'asks for a reconnect without raising' do + stub_request(:post, tokens_url).to_return(status: 404, body: { message: 'Not Found' }.to_json, + headers: { 'Content-Type' => 'application/json' }) + + expect do + described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform + end.not_to raise_error + + expect(hook.reauthorization_required?).to be(true) + expect(WebMock).not_to have_requested(:post, issues_url) + end + end + + context 'when github rejects the issue itself' do + it 'logs the failure and keeps the hook connected' do + stub_request(:post, issues_url).to_return(status: 422, body: { message: 'Validation Failed' }.to_json, + headers: { 'Content-Type' => 'application/json' }) + allow(Rails.logger).to receive(:error) + + described_class.new(hook: hook, event_name: 'ticket.created', event_data: { ticket: ticket }).perform + + expect(Rails.logger).to have_received(:error).with(/422.*Validation Failed/) + expect(hook.reauthorization_required?).to be(false) + end + end end end From 7078e60bb8b03aa2f3493ee41ee9d6dad1a2363c Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:53:28 +0800 Subject: [PATCH 03/10] feat(integrations): connect GitHub through the app install flow The integration card links to the app's install page with a signed, 15-minute state naming the account. GitHub returns the browser to /github/callback, which only checks the state and forwards code, installation_id and state to the settings page. The page posts them to POST /integrations/github from the admin's own session. Binding happens there, not in the callback, for two reasons. The state must name the current account, so an admin cannot hand their state to another organization's owner and capture that installation. The user token from the code must list the installation, because installation_id comes through a browser redirect and anyone can forge it. Reconnecting keeps the label, and keeps the repository only if the new installation still grants it. Administrators can list the installation's repositories, choose one plus an optional label (422 for a repository outside the installation), and unbind the installation. Hook JSON now reports reauthorization_required. Co-Authored-By: Claude Opus 5.5 --- .../integrations/github_controller.rb | 96 ++++++++++ .../github/callbacks_controller.rb | 28 +++ app/helpers/github/integration_helper.rb | 30 ++++ app/models/integrations/app.rb | 16 +- app/policies/hook_policy.rb | 5 + .../integrations/github/update.json.jbuilder | 1 + app/views/api/v1/models/_hook.json.jbuilder | 1 + config/locales/en.yml | 5 + config/locales/zh_TW.yml | 5 + config/routes.rb | 9 + .../integrations/github_controller_spec.rb | 165 ++++++++++++++++++ .../github/callbacks_controller_spec.rb | 57 ++++++ spec/models/integrations/app_spec.rb | 53 ++++++ 13 files changed, 470 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/v1/accounts/integrations/github_controller.rb create mode 100644 app/controllers/github/callbacks_controller.rb create mode 100644 app/helpers/github/integration_helper.rb create mode 100644 app/views/api/v1/accounts/integrations/github/update.json.jbuilder create mode 100644 spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb create mode 100644 spec/controllers/github/callbacks_controller_spec.rb diff --git a/app/controllers/api/v1/accounts/integrations/github_controller.rb b/app/controllers/api/v1/accounts/integrations/github_controller.rb new file mode 100644 index 0000000000..eec49475bc --- /dev/null +++ b/app/controllers/api/v1/accounts/integrations/github_controller.rb @@ -0,0 +1,96 @@ +class Api::V1::Accounts::Integrations::GithubController < Api::V1::Accounts::Integrations::BaseController + include Github::IntegrationHelper + + before_action :check_authorization + before_action :fetch_hook, except: [:create] + + # Completes an install that GitHub redirected through Github::CallbacksController. + def create + return render_connect_error('connection_failed') unless verify_github_state(params[:state]) == Current.account.id + return render_connect_error('installation_not_verified') unless installation_visible_to_user? + + connect_installation + render :update + rescue Integrations::Github::AppClient::Error => e + Rails.logger.error("GitHub connect failed for account #{Current.account.id}: #{e.message}") + render_connect_error('connection_failed') + end + + def repositories + render json: installation_repositories + rescue Integrations::Github::AppClient::AuthorizationError => e + render_access_lost(e) + end + + def update + repository = installation_repositories.find { |name| name.casecmp?(params[:repository].to_s) } + return render json: { error: I18n.t('integration_apps.github.errors.repository_not_granted') }, status: :unprocessable_entity if repository.blank? + + @hook.update!(settings: { 'repository' => repository, 'label' => params[:label].presence }.compact) + rescue Integrations::Github::AppClient::AuthorizationError => e + render_access_lost(e) + end + + # Unbinds the installation from this account only. The app stays installed on + # GitHub, where the organization may still use it for other accounts. + def destroy + @hook.destroy! + head :ok + end + + private + + def fetch_hook + @hook = Current.account.hooks.find_by!(app_id: 'github') + end + + def installation_id + params[:installation_id].to_s + end + + # `installation_id` arrives through a browser redirect, so anyone can put any id + # there. Binding it unchecked would let one account open issues in another + # organization's repositories. The user who just authorized must be able to see + # the installation on GitHub's side. + def installation_visible_to_user? + return false if params[:code].blank? || installation_id.blank? + + user_token = app_client.exchange_code(params[:code]) + return true if app_client.user_installation_ids(user_token).map(&:to_s).include?(installation_id) + + Rails.logger.warn("GitHub connect for account #{Current.account.id} named installation #{installation_id}, which the authorizing user cannot see") + false + end + + def connect_installation + @hook = Current.account.hooks.find_or_initialize_by(app_id: 'github') + previous = @hook.settings.to_h + settings = { 'label' => previous['label'] } + settings['repository'] = previous['repository'] if repository_still_granted?(previous['repository']) + + @hook.update!(reference_id: installation_id, status: 'enabled', settings: settings.compact_blank) + @hook.reauthorized! + end + + def repository_still_granted?(repository) + repository.present? && app_client.repositories(installation_id).any? { |name| name.casecmp?(repository) } + end + + def installation_repositories + @installation_repositories ||= app_client.repositories(@hook.reference_id) + end + + def app_client + @app_client ||= Integrations::Github::AppClient.new + end + + def render_connect_error(reason) + render json: { error: I18n.t("integration_apps.github.errors.#{reason}"), reason: reason }, status: :unprocessable_entity + end + + def render_access_lost(error) + Rails.logger.warn("GitHub installation #{@hook.reference_id} refused account #{Current.account.id}: #{error.message}") + @hook.prompt_reauthorization! + render json: { error: I18n.t('integration_apps.github.errors.access_lost') }, status: :unprocessable_entity + end +end diff --git a/app/controllers/github/callbacks_controller.rb b/app/controllers/github/callbacks_controller.rb new file mode 100644 index 0000000000..32b4d3a555 --- /dev/null +++ b/app/controllers/github/callbacks_controller.rb @@ -0,0 +1,28 @@ +# GitHub sends the browser here after an install. Nothing is bound yet: the +# settings page posts `code`, `installation_id` and `state` back from inside the +# admin's authenticated session (Integrations::GithubController#create). Binding +# here instead would let an admin of one account hand their `state` to an org +# owner elsewhere and capture that org's installation when the owner installs. +class Github::CallbacksController < ApplicationController + include Github::IntegrationHelper + + def show + account_id = verify_github_state(params[:state]) + return redirect_to(frontend_url) if account_id.blank? + + query = if params[:setup_action] == 'request' + # An org member without admin rights only *requested* the install; + # GitHub waits for an org owner, and nothing is installed yet. + { setup_action: 'request' } + else + params.permit(:code, :installation_id, :state).to_h + end + redirect_to "#{frontend_url}/app/accounts/#{account_id}/settings/integrations/github?#{query.to_query}" + end + + private + + def frontend_url + ENV.fetch('FRONTEND_URL', 'http://localhost:3000') + end +end diff --git a/app/helpers/github/integration_helper.rb b/app/helpers/github/integration_helper.rb new file mode 100644 index 0000000000..87ddf1c07d --- /dev/null +++ b/app/helpers/github/integration_helper.rb @@ -0,0 +1,30 @@ +# Signs the account id into the `state` of the GitHub App install URL. GitHub +# hands `state` back on the callback, and it is the only thing that tells us +# which account started the install. +module Github::IntegrationHelper + STATE_TTL = 15.minutes + + def generate_github_state(account_id) + secret = github_client_secret + return if secret.blank? + + now = Time.current + JWT.encode({ sub: account_id, iat: now.to_i, exp: (now + STATE_TTL).to_i }, secret, 'HS256') + end + + def verify_github_state(token) + secret = github_client_secret + return if token.blank? || secret.blank? + + JWT.decode(token, secret, true, { algorithm: 'HS256', required_claims: %w[exp] }).first['sub'] + rescue JWT::DecodeError => e + Rails.logger.warn("Rejected GitHub install state: #{e.message}") + nil + end + + private + + def github_client_secret + GlobalConfigService.load('GITHUB_APP_CLIENT_SECRET', nil) + end +end diff --git a/app/models/integrations/app.rb b/app/models/integrations/app.rb index e5b716b248..cd56c26578 100644 --- a/app/models/integrations/app.rb +++ b/app/models/integrations/app.rb @@ -1,5 +1,6 @@ class Integrations::App include Linear::IntegrationHelper + include Github::IntegrationHelper include Pathors::IntegrationHelper # Pathors: never offered in the catalog. Each of these asks the customer to @@ -15,6 +16,10 @@ class Integrations::App # The presence of such a bot is what "connected to Pathors" means. PATHORS_CALLBACK_URL_FRAGMENT = '/integration/chatwoot/callback'.freeze + GITHUB_APP_CONFIG_KEYS = %w[ + GITHUB_APP_ID GITHUB_APP_SLUG GITHUB_APP_CLIENT_ID GITHUB_APP_CLIENT_SECRET GITHUB_APP_PRIVATE_KEY GITHUB_APP_WEBHOOK_SECRET + ].freeze + attr_accessor :params def initialize(params) @@ -62,6 +67,8 @@ def action "#{params[:action]}&client_id=#{client_id}&redirect_uri=#{self.class.slack_integration_url}" when 'linear' build_linear_action + when 'github' + build_github_action when 'pathors' build_pathors_action when 'shopify' @@ -81,12 +88,14 @@ def active?(account) # Whether the instance/account has what this app needs to be usable at all — # an OAuth client on the instance, a feature flag on the account, or both. - def credentials_available?(account) + def credentials_available?(account) # rubocop:disable Metrics/CyclomaticComplexity case params[:id] when 'slack' GlobalConfigService.load('SLACK_CLIENT_SECRET', nil).present? when 'linear' account.feature_enabled?('linear_integration') && GlobalConfigService.load('LINEAR_CLIENT_ID', nil).present? + when 'github' + GITHUB_APP_CONFIG_KEYS.all? { |key| GlobalConfigService.load(key, nil).present? } when 'shopify' shopify_enabled?(account) when 'leadsquared' @@ -113,6 +122,11 @@ def build_linear_action ].join('&') end + def build_github_action + slug = GlobalConfigService.load('GITHUB_APP_SLUG', nil) + "https://github.com/apps/#{slug}/installations/new?state=#{generate_github_state(Current.account.id)}" + end + def enabled?(account) case params[:id] when 'webhook' diff --git a/app/policies/hook_policy.rb b/app/policies/hook_policy.rb index 153a967989..5244b94a2b 100644 --- a/app/policies/hook_policy.rb +++ b/app/policies/hook_policy.rb @@ -15,6 +15,11 @@ def destroy? @account_user.administrator? end + # GitHub App repository picker (Integrations::GithubController) + def repositories? + @account_user.administrator? + end + # CRM sync log and backfill (Integrations::CrmSyncController) def sync_events? @account_user.administrator? diff --git a/app/views/api/v1/accounts/integrations/github/update.json.jbuilder b/app/views/api/v1/accounts/integrations/github/update.json.jbuilder new file mode 100644 index 0000000000..6c6f810010 --- /dev/null +++ b/app/views/api/v1/accounts/integrations/github/update.json.jbuilder @@ -0,0 +1 @@ +json.partial! 'api/v1/models/hook', formats: [:json], resource: @hook diff --git a/app/views/api/v1/models/_hook.json.jbuilder b/app/views/api/v1/models/_hook.json.jbuilder index 3b9b14029e..d902059d84 100644 --- a/app/views/api/v1/models/_hook.json.jbuilder +++ b/app/views/api/v1/models/_hook.json.jbuilder @@ -4,6 +4,7 @@ json.status resource.enabled? json.inbox resource.inbox&.slice(:id, :name) json.account_id resource.account_id json.hook_type resource.hook_type +json.reauthorization_required resource.reauthorization_required? if Current.account_user&.administrator? visible_properties = resource.app&.visible_properties || [] diff --git a/config/locales/en.yml b/config/locales/en.yml index 3d347408a8..86f87137cf 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -504,6 +504,11 @@ en: short_description: 'Open a GitHub issue whenever a ticket is created.' description: 'Hand cases over to engineering automatically. Whenever a ticket is created, this integration opens an issue in the GitHub repository you configure, carrying the contact, the ticket details and a link back to the conversation.' issue_created_note: 'GitHub issue created for this ticket: %{url}' + errors: + repository_not_granted: 'The GitHub App is not installed on that repository.' + access_lost: 'GitHub no longer accepts this installation. Reconnect GitHub to continue.' + installation_not_verified: 'Your GitHub account cannot see that installation, so it was not connected.' + connection_failed: 'Connecting GitHub failed. Try again.' issue: contact: 'Contact' ticket_type: 'Ticket type' diff --git a/config/locales/zh_TW.yml b/config/locales/zh_TW.yml index aadd529b88..5583da11fe 100644 --- a/config/locales/zh_TW.yml +++ b/config/locales/zh_TW.yml @@ -470,6 +470,11 @@ zh_TW: short_description: '建立工單時自動開一則 GitHub issue。' description: '自動把案件交接給工程團隊。每當有人建立工單，此整合就會在您設定的 GitHub 儲存庫開一則 issue，並帶上聯絡人、工單資訊與回到對話的連結。' issue_created_note: '已為這張工單建立 GitHub issue：%{url}' + errors: + repository_not_granted: 'GitHub App 沒有安裝在這個儲存庫上。' + access_lost: 'GitHub 已不再接受這個安裝，請重新連接 GitHub。' + installation_not_verified: '你的 GitHub 帳號看不到這個安裝，因此沒有連接。' + connection_failed: '連接 GitHub 失敗，請再試一次。' issue: contact: '聯絡人' ticket_type: '工單類型' diff --git a/config/routes.rb b/config/routes.rb index 512ed825f8..984c8ac859 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -453,6 +453,11 @@ get :linked_issues end end + resource :github, controller: 'github', only: [:create, :update, :destroy] do + collection do + get :repositories + end + end resource :notion, controller: 'notion', only: [] do collection do delete :destroy @@ -739,6 +744,10 @@ resource :callback, only: [:show] end + namespace :github do + resource :callback, only: [:show] + end + namespace :pathors do resource :callback, only: [:show] end diff --git a/spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb new file mode 100644 index 0000000000..86d13dd7bf --- /dev/null +++ b/spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb @@ -0,0 +1,165 @@ +require 'rails_helper' + +RSpec.describe 'GitHub Integration API', type: :request do + let(:account) { create(:account) } + let(:admin) { create(:user, account: account, role: :administrator) } + let(:agent) { create(:user, account: account, role: :agent) } + let!(:hook) { create(:integrations_hook, :github, account: account, settings: {}) } + let(:base_url) { "/api/v1/accounts/#{account.id}/integrations/github" } + let(:json_headers) { { 'Content-Type' => 'application/json' } } + let(:repositories_url) { 'https://api.github.com/installation/repositories?per_page=100' } + + before do + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_ID', nil).and_return('123456') + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_PRIVATE_KEY', nil).and_return(OpenSSL::PKey::RSA.generate(2048).to_pem) + stub_request(:post, 'https://api.github.com/app/installations/4242/access_tokens') + .to_return(status: 201, body: { token: 'ghs_token', expires_at: 1.hour.from_now.iso8601 }.to_json, headers: json_headers) + stub_request(:get, repositories_url) + .to_return(status: 200, body: { repositories: [{ full_name: 'pathorsAI/pathors' }, { full_name: 'pathorsAI/inbox' }] }.to_json, + headers: json_headers) + end + + describe 'POST /api/v1/accounts/:account_id/integrations/github' do + let(:client_secret) { 'github-client-secret' } + let(:state) { JWT.encode({ sub: account.id, iat: Time.current.to_i, exp: 10.minutes.from_now.to_i }, client_secret, 'HS256') } + let(:visible_installations) { [{ id: 4242 }] } + let(:connect_params) { { code: 'oauth-code', installation_id: '4242', state: state } } + + before do + hook.destroy! + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_CLIENT_ID', nil).and_return('Iv1.client') + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_CLIENT_SECRET', nil).and_return(client_secret) + stub_request(:post, 'https://github.com/login/oauth/access_token') + .to_return(status: 200, body: { access_token: 'ghu_user_token' }.to_json, headers: json_headers) + stub_request(:get, 'https://api.github.com/user/installations?per_page=100') + .with(headers: { 'Authorization' => 'Bearer ghu_user_token' }) + .to_return(status: 200, body: { installations: visible_installations }.to_json, headers: json_headers) + end + + it 'binds an installation the authorizing user can see' do + post base_url, params: connect_params, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:ok) + connected = account.hooks.find_by!(app_id: 'github') + expect(connected.reference_id).to eq('4242') + expect(connected.settings).to eq({}) + expect(response.parsed_body['reference_id']).to eq('4242') + end + + it 'refuses a forged installation id and writes nothing' do + post base_url, params: connect_params.merge(installation_id: '9999'), headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body['reason']).to eq('installation_not_verified') + expect(account.hooks.where(app_id: 'github')).to be_empty + end + + it 'refuses a state minted for another account' do + other_state = JWT.encode({ sub: create(:account).id, exp: 10.minutes.from_now.to_i }, client_secret, 'HS256') + + post base_url, params: connect_params.merge(state: other_state), headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body['reason']).to eq('connection_failed') + expect(account.hooks.where(app_id: 'github')).to be_empty + expect(WebMock).not_to have_requested(:post, 'https://github.com/login/oauth/access_token') + end + + it 'keeps the label and a repository the installation still grants when reconnecting' do + previous = create(:integrations_hook, :github, account: account, reference_id: '1', + settings: { 'repository' => 'pathorsai/inbox', 'label' => 'support' }) + previous.prompt_reauthorization! + + post base_url, params: connect_params, headers: admin.create_new_auth_token, as: :json + + previous.reload + expect(previous.reference_id).to eq('4242') + expect(previous.settings).to eq('repository' => 'pathorsai/inbox', 'label' => 'support') + expect(previous.reauthorization_required?).to be(false) + end + + it 'drops a repository the new installation does not grant' do + create(:integrations_hook, :github, account: account, reference_id: '1', settings: { 'repository' => 'someone/else', 'label' => 'support' }) + + post base_url, params: connect_params, headers: admin.create_new_auth_token, as: :json + + expect(account.hooks.find_by!(app_id: 'github').settings).to eq('label' => 'support') + end + + it 'reports a code GitHub rejects' do + stub_request(:post, 'https://github.com/login/oauth/access_token') + .to_return(status: 200, body: { error: 'bad_verification_code' }.to_json, headers: json_headers) + + post base_url, params: connect_params, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body['reason']).to eq('connection_failed') + end + + it 'is limited to administrators' do + post base_url, params: connect_params, headers: agent.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unauthorized) + expect(account.hooks.where(app_id: 'github')).to be_empty + end + end + + describe 'GET /api/v1/accounts/:account_id/integrations/github/repositories' do + it 'lists the repositories the installation grants' do + get "#{base_url}/repositories", headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:ok) + expect(response.parsed_body).to eq(%w[pathorsAI/pathors pathorsAI/inbox]) + end + + it 'is limited to administrators' do + get "#{base_url}/repositories", headers: agent.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unauthorized) + end + + it 'asks for a reconnect when GitHub no longer accepts the installation' do + stub_request(:post, 'https://api.github.com/app/installations/4242/access_tokens').to_return(status: 404, body: '{}', headers: json_headers) + + get "#{base_url}/repositories", headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(hook.reauthorization_required?).to be(true) + end + end + + describe 'PATCH /api/v1/accounts/:account_id/integrations/github' do + it 'saves a granted repository and the label' do + patch base_url, params: { repository: 'pathorsai/inbox', label: 'support' }, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:ok) + expect(hook.reload.settings).to eq('repository' => 'pathorsAI/inbox', 'label' => 'support') + expect(response.parsed_body['settings']).to eq('repository' => 'pathorsAI/inbox', 'label' => 'support') + end + + it 'refuses a repository outside the installation' do + patch base_url, params: { repository: 'someone/else' }, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(hook.reload.settings).to eq({}) + end + + it 'is limited to administrators' do + patch base_url, params: { repository: 'pathorsAI/inbox' }, headers: agent.create_new_auth_token, as: :json + + expect(response).to have_http_status(:unauthorized) + expect(hook.reload.settings).to eq({}) + end + end + + describe 'DELETE /api/v1/accounts/:account_id/integrations/github' do + it 'removes the hook without calling GitHub' do + delete base_url, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:ok) + expect(account.hooks.where(app_id: 'github')).to be_empty + expect(WebMock).not_to have_requested(:any, /github\.com/) + end + end +end diff --git a/spec/controllers/github/callbacks_controller_spec.rb b/spec/controllers/github/callbacks_controller_spec.rb new file mode 100644 index 0000000000..aecb65e7d6 --- /dev/null +++ b/spec/controllers/github/callbacks_controller_spec.rb @@ -0,0 +1,57 @@ +require 'rails_helper' + +RSpec.describe Github::CallbacksController, type: :request do + let(:account) { create(:account) } + let(:client_secret) { 'github-client-secret' } + let(:state) { JWT.encode({ sub: account.id, iat: Time.current.to_i, exp: 10.minutes.from_now.to_i }, client_secret, 'HS256') } + let(:settings_url) { "http://www.example.com/app/accounts/#{account.id}/settings/integrations/github" } + + around do |example| + with_modified_env(FRONTEND_URL: 'http://www.example.com') { example.run } + end + + before do + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_CLIENT_SECRET', nil).and_return(client_secret) + end + + describe 'GET /github/callback' do + it 'hands the install to the settings page of the account named in the state, without binding it' do + get github_callback_path, params: { code: 'oauth-code', installation_id: '4242', setup_action: 'install', state: state } + + expect(response).to redirect_to("#{settings_url}?#{{ code: 'oauth-code', installation_id: '4242', state: state }.to_query}") + expect(Integrations::Hook.where(app_id: 'github')).to be_empty + expect(WebMock).not_to have_requested(:any, /github\.com/) + end + + it 'sends a pending org approval back to the settings page' do + get github_callback_path, params: { setup_action: 'request', state: state } + + expect(response).to redirect_to("#{settings_url}?setup_action=request") + end + + it 'redirects to the app root without naming any account when the state is forged' do + forged = JWT.encode({ sub: account.id, exp: 10.minutes.from_now.to_i }, 'not-the-secret', 'HS256') + + get github_callback_path, params: { code: 'oauth-code', installation_id: '4242', state: forged } + + expect(response).to redirect_to('http://www.example.com') + end + + it 'rejects an expired state' do + expired = JWT.encode({ sub: account.id, exp: 1.minute.ago.to_i }, client_secret, 'HS256') + + get github_callback_path, params: { code: 'oauth-code', installation_id: '4242', state: expired } + + expect(response).to redirect_to('http://www.example.com') + end + + it 'rejects a state without an expiry' do + eternal = JWT.encode({ sub: account.id }, client_secret, 'HS256') + + get github_callback_path, params: { code: 'oauth-code', installation_id: '4242', state: eternal } + + expect(response).to redirect_to('http://www.example.com') + end + end +end diff --git a/spec/models/integrations/app_spec.rb b/spec/models/integrations/app_spec.rb index c121a467b6..6d6e92e24f 100644 --- a/spec/models/integrations/app_spec.rb +++ b/spec/models/integrations/app_spec.rb @@ -60,6 +60,24 @@ end end + context 'when the app is github' do + let(:app_name) { 'github' } + + it 'points at the app install page with a state that names the account and expires' do + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_SLUG', nil).and_return('pathors-inbox') + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_CLIENT_SECRET', nil).and_return('github-client-secret') + + uri = URI.parse(app.action) + state = CGI.parse(uri.query)['state'].first + payload = JWT.decode(state, 'github-client-secret', true, algorithm: 'HS256').first + + expect("#{uri.scheme}://#{uri.host}#{uri.path}").to eq('https://github.com/apps/pathors-inbox/installations/new') + expect(payload['sub']).to eq(account.id) + expect(payload['exp'] - payload['iat']).to eq(15.minutes.to_i) + end + end + context 'when the app is pathors' do let(:app_name) { 'pathors' } let(:connect_secret) { 'test_connect_secret' } @@ -196,6 +214,41 @@ end end + context 'when the app is github' do + let(:app_name) { 'github' } + let(:github_config) do + { + 'GITHUB_APP_ID' => '123456', + 'GITHUB_APP_SLUG' => 'pathors-inbox', + 'GITHUB_APP_CLIENT_ID' => 'Iv1.client', + 'GITHUB_APP_CLIENT_SECRET' => 'secret', + 'GITHUB_APP_PRIVATE_KEY' => 'pem', + 'GITHUB_APP_WEBHOOK_SECRET' => 'hook-secret' + } + end + + before do + allow(GlobalConfigService).to receive(:load).and_call_original + github_config.each { |key, value| allow(GlobalConfigService).to receive(:load).with(key, nil).and_return(value) } + end + + it 'returns true when every GitHub App credential is configured' do + expect(app.active?(account)).to be true + end + + it 'returns false when the private key is missing' do + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_PRIVATE_KEY', nil).and_return(nil) + + expect(app.active?(account)).to be false + end + + it 'returns false when the webhook secret is missing, since uninstalls would go unnoticed' do + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_WEBHOOK_SECRET', nil).and_return(nil) + + expect(app.active?(account)).to be false + end + end + context 'when the app is pathors' do let(:app_name) { 'pathors' } From de73a1fef602263c7e818a81ffa03575852a1d3f Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:53:37 +0800 Subject: [PATCH 04/10] feat(integrations): flag GitHub hooks for reconnect from app webhooks POST /webhooks/github verifies X-Hub-Signature-256 against GITHUB_APP_WEBHOOK_SECRET and answers 401 without it. An installation that is deleted or suspended, or that loses the hook's repository, marks every hook bound to it as needing a reconnect; unsuspend clears the flag. Replays converge on the same state, and other events are acknowledged. Co-Authored-By: Claude Opus 5.5 --- app/controllers/webhooks/github_controller.rb | 53 +++++++++++ config/routes.rb | 1 + .../webhooks/github_controller_spec.rb | 93 +++++++++++++++++++ 3 files changed, 147 insertions(+) create mode 100644 app/controllers/webhooks/github_controller.rb create mode 100644 spec/controllers/webhooks/github_controller_spec.rb diff --git a/app/controllers/webhooks/github_controller.rb b/app/controllers/webhooks/github_controller.rb new file mode 100644 index 0000000000..9e936f852c --- /dev/null +++ b/app/controllers/webhooks/github_controller.rb @@ -0,0 +1,53 @@ +class Webhooks::GithubController < ActionController::API + before_action :verify_signature! + + def events + case request.headers['X-GitHub-Event'] + when 'installation' + handle_installation + when 'installation_repositories' + handle_repositories_removed + end + + head :ok + end + + private + + def verify_signature! + secret = GlobalConfigService.load('GITHUB_APP_WEBHOOK_SECRET', nil) + signature = request.headers['X-Hub-Signature-256'] + return head :unauthorized if secret.blank? || signature.blank? + + expected = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, request.raw_post)}" + head :unauthorized unless ActiveSupport::SecurityUtils.secure_compare(expected, signature) + end + + def handle_installation + case payload['action'] + when 'deleted', 'suspend' + installation_hooks.find_each(&:prompt_reauthorization!) + when 'unsuspend' + installation_hooks.find_each(&:reauthorized!) + end + end + + def handle_repositories_removed + removed = Array(payload['repositories_removed']).map { |repository| repository['full_name'].downcase } + return if removed.empty? + + installation_hooks.find_each do |hook| + hook.prompt_reauthorization! if removed.include?(hook.settings['repository']&.downcase) + end + end + + def installation_hooks + Integrations::Hook.where(app_id: 'github', reference_id: payload.dig('installation', 'id').to_s) + end + + # Parsed from the raw body because Rails reserves params[:action] for the + # controller action, which would hide the GitHub event's own `action`. + def payload + @payload ||= JSON.parse(request.raw_post) + end +end diff --git a/config/routes.rb b/config/routes.rb index 984c8ac859..b880cba1b5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -735,6 +735,7 @@ post 'webhooks/instagram', to: 'webhooks/instagram#events' post 'webhooks/tiktok', to: 'webhooks/tiktok#events' post 'webhooks/shopify', to: 'webhooks/shopify#events' + post 'webhooks/github', to: 'webhooks/github#events' namespace :twitter do resource :callback, only: [:show] diff --git a/spec/controllers/webhooks/github_controller_spec.rb b/spec/controllers/webhooks/github_controller_spec.rb new file mode 100644 index 0000000000..309ab05f0e --- /dev/null +++ b/spec/controllers/webhooks/github_controller_spec.rb @@ -0,0 +1,93 @@ +require 'rails_helper' + +RSpec.describe Webhooks::GithubController, type: :request do + let(:account) { create(:account) } + let!(:hook) { create(:integrations_hook, :github, account: account, reference_id: '4242') } + let(:webhook_secret) { 'github-webhook-secret' } + let(:event) { 'installation' } + let(:payload) { { action: 'deleted', installation: { id: 4242 } } } + let(:body) { payload.to_json } + let(:signature) { "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', webhook_secret, body)}" } + let(:headers) { { 'CONTENT_TYPE' => 'application/json', 'X-GitHub-Event' => event, 'X-Hub-Signature-256' => signature } } + + before do + allow(GlobalConfigService).to receive(:load).and_call_original + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_WEBHOOK_SECRET', nil).and_return(webhook_secret) + end + + it 'rejects a payload signed with another secret' do + post '/webhooks/github', params: body, + headers: headers.merge('X-Hub-Signature-256' => "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', 'wrong', body)}") + + expect(response).to have_http_status(:unauthorized) + expect(hook.reauthorization_required?).to be(false) + end + + it 'rejects every payload when no webhook secret is configured' do + allow(GlobalConfigService).to receive(:load).with('GITHUB_APP_WEBHOOK_SECRET', nil).and_return(nil) + + post '/webhooks/github', params: body, headers: headers + + expect(response).to have_http_status(:unauthorized) + end + + it 'asks for a reconnect when the app is uninstalled, and stays that way on replay' do + 2.times { post '/webhooks/github', params: body, headers: headers } + + expect(response).to have_http_status(:ok) + expect(hook.reauthorization_required?).to be(true) + end + + it 'leaves hooks of other installations alone' do + other = create(:integrations_hook, :github, account: create(:account), reference_id: '7') + + post '/webhooks/github', params: body, headers: headers + + expect(other.reauthorization_required?).to be(false) + end + + context 'when a suspended installation is unsuspended' do + let(:payload) { { action: 'unsuspend', installation: { id: 4242 } } } + + it 'clears the reconnect prompt' do + hook.prompt_reauthorization! + + post '/webhooks/github', params: body, headers: headers + + expect(hook.reauthorization_required?).to be(false) + end + end + + context 'when repositories are removed from the installation' do + let(:event) { 'installation_repositories' } + let(:removed) { [{ full_name: 'PathorsAI/Chatwoot' }] } + let(:payload) { { action: 'removed', installation: { id: 4242 }, repositories_removed: removed } } + + it 'asks for a reconnect when the selected repository is among them' do + post '/webhooks/github', params: body, headers: headers + + expect(hook.reauthorization_required?).to be(true) + end + + context 'when the selected repository is still granted' do + let(:removed) { [{ full_name: 'pathorsAI/other' }] } + + it 'keeps the hook working' do + post '/webhooks/github', params: body, headers: headers + + expect(hook.reauthorization_required?).to be(false) + end + end + end + + context 'with an event the integration does not handle' do + let(:event) { 'push' } + + it 'acknowledges it' do + post '/webhooks/github', params: body, headers: headers + + expect(response).to have_http_status(:ok) + expect(hook.reauthorization_required?).to be(false) + end + end +end From 5028d5c42478fb3296f7ab86c0d08f6421a462b5 Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:36:15 +0800 Subject: [PATCH 05/10] feat(integrations): add the GitHub App settings page The GitHub card now opens a dedicated page instead of the generic hook form. The page derives one status from the account's hook (not connected, reconnect, choose repository, connected), links Connect to the GitHub App install URL, lets an administrator pick the repository and optional issue label, and shows the install redirect's setup_action/error notice once before clearing the query. Co-Authored-By: Claude Opus 5.5 --- .../dashboard/api/integrations/github.js | 19 ++ .../api/specs/integrations/github.spec.js | 52 +++ .../settings/integrations/Github.vue | 192 +++++++++++ .../integrations/Github/RepositoryForm.vue | 145 ++++++++ .../integrations/integrations.routes.js | 14 + .../integrations/specs/Github.spec.js | 320 ++++++++++++++++++ 6 files changed, 742 insertions(+) create mode 100644 app/javascript/dashboard/api/integrations/github.js create mode 100644 app/javascript/dashboard/api/specs/integrations/github.spec.js create mode 100644 app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue create mode 100644 app/javascript/dashboard/routes/dashboard/settings/integrations/Github/RepositoryForm.vue create mode 100644 app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js diff --git a/app/javascript/dashboard/api/integrations/github.js b/app/javascript/dashboard/api/integrations/github.js new file mode 100644 index 0000000000..bee51691e4 --- /dev/null +++ b/app/javascript/dashboard/api/integrations/github.js @@ -0,0 +1,19 @@ +/* global axios */ + +import ApiClient from '../ApiClient'; + +class GithubAPI extends ApiClient { + constructor() { + super('integrations/github', { accountScoped: true }); + } + + getRepositories() { + return axios.get(`${this.url}/repositories`); + } + + updateSettings({ repository, label }) { + return axios.patch(this.url, { repository, label }); + } +} + +export default new GithubAPI(); diff --git a/app/javascript/dashboard/api/specs/integrations/github.spec.js b/app/javascript/dashboard/api/specs/integrations/github.spec.js new file mode 100644 index 0000000000..061a3be253 --- /dev/null +++ b/app/javascript/dashboard/api/specs/integrations/github.spec.js @@ -0,0 +1,52 @@ +import GithubAPIClient from '../../integrations/github'; +import ApiClient from '../../ApiClient'; + +describe('#githubAPI', () => { + const originalAxios = window.axios; + const axiosMock = { + get: vi.fn(() => Promise.resolve()), + patch: vi.fn(() => Promise.resolve()), + }; + + beforeEach(() => { + window.axios = axiosMock; + }); + + afterEach(() => { + window.axios = originalAxios; + vi.clearAllMocks(); + }); + + it('creates correct instance', () => { + expect(GithubAPIClient).toBeInstanceOf(ApiClient); + }); + + it('fetches the repositories of the installation', () => { + GithubAPIClient.getRepositories(); + expect(axiosMock.get).toHaveBeenCalledWith( + '/api/v1/integrations/github/repositories' + ); + }); + + it('saves the repository and label', () => { + GithubAPIClient.updateSettings({ + repository: 'pathorsAI/inbox', + label: 'support', + }); + expect(axiosMock.patch).toHaveBeenCalledWith( + '/api/v1/integrations/github', + { repository: 'pathorsAI/inbox', label: 'support' } + ); + }); + + it('sends an empty label to clear it', () => { + GithubAPIClient.updateSettings({ + repository: 'pathorsAI/inbox', + label: '', + }); + expect(axiosMock.patch).toHaveBeenCalledWith( + '/api/v1/integrations/github', + { repository: 'pathorsAI/inbox', label: '' } + ); + }); +}); diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue new file mode 100644 index 0000000000..d2c9f7c41d --- /dev/null +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue @@ -0,0 +1,192 @@ + + + diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/Github/RepositoryForm.vue b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github/RepositoryForm.vue new file mode 100644 index 0000000000..fc5edf3314 --- /dev/null +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github/RepositoryForm.vue @@ -0,0 +1,145 @@ + + + diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js b/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js index 5087f1eb59..70d993aa7d 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js @@ -10,6 +10,7 @@ import Slack from './Slack.vue'; import Linear from './Linear.vue'; import Notion from './Notion.vue'; import Shopify from './Shopify.vue'; +import Github from './Github.vue'; export default { routes: [ @@ -99,6 +100,19 @@ export default { }, props: route => ({ error: route.query.error }), }, + { + path: 'github', + name: 'settings_integrations_github', + component: Github, + meta: { + featureFlag: FEATURE_FLAGS.INTEGRATIONS, + permissions: ['administrator'], + }, + props: route => ({ + setupAction: route.query.setup_action, + error: route.query.error, + }), + }, { path: ':integration_id', name: 'settings_applications_integration', diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js b/app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js new file mode 100644 index 0000000000..3bd172973c --- /dev/null +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js @@ -0,0 +1,320 @@ +import { flushPromises, mount } from '@vue/test-utils'; +import { createStore } from 'vuex'; +import { withFullI18n } from 'test-i18n'; +import { useAlert } from 'dashboard/composables'; +import Github from '../Github.vue'; +import Integration from '../Integration.vue'; +import ComboBox from 'dashboard/components-next/combobox/ComboBox.vue'; + +const { getRepositories, updateSettings, routerReplace } = vi.hoisted(() => ({ + getRepositories: vi.fn(), + updateSettings: vi.fn(), + routerReplace: vi.fn(), +})); + +vi.mock('dashboard/api/integrations/github', () => ({ + default: { getRepositories, updateSettings }, +})); + +vi.mock('dashboard/composables', () => ({ useAlert: vi.fn() })); + +vi.mock('vue-router', async importOriginal => ({ + ...(await importOriginal()), + useRouter: () => ({ replace: routerReplace }), + useRoute: () => ({ path: '/app/accounts/1/settings/integrations/github' }), +})); + +const i18n = withFullI18n('en'); + +const INSTALL_URL = + 'https://github.com/apps/pathors-inbox/installations/new?state=signed'; + +const installedHook = (overrides = {}) => ({ + id: 5, + app_id: 'github', + status: true, + reference_id: '81234567', + settings: {}, + reauthorization_required: false, + ...overrides, +}); + +const buildStore = hooks => { + const refreshedHooks = { value: null }; + const get = vi.fn(({ commit }) => { + if (refreshedHooks.value) commit('setHooks', refreshedHooks.value); + }); + const store = createStore({ + modules: { + integrations: { + namespaced: true, + state: { + records: [ + { + id: 'github', + name: 'GitHub', + description: 'Open GitHub issues from conversations.', + enabled: Boolean(hooks.length), + action: INSTALL_URL, + hooks, + }, + ], + }, + getters: { + getIntegration: state => id => + state.records.find(record => record.id === id) ?? {}, + }, + mutations: { + setHooks: (state, newHooks) => { + state.records[0].hooks = newHooks; + }, + }, + actions: { get }, + }, + }, + }); + return { store, get, refreshedHooks }; +}; + +const mountPage = async ({ hooks = [], props = {} } = {}) => { + const { store, get, refreshedHooks } = buildStore(hooks); + const wrapper = mount(Github, { + props, + global: { + plugins: [store], + stubs: { + Integration: true, + BaseSettingsHeader: true, + WootLoadingState: true, + }, + }, + }); + await flushPromises(); + return { wrapper, get, refreshedHooks }; +}; + +const integrationCard = wrapper => wrapper.findComponent(Integration); + +describe('Github settings page', () => { + beforeEach(() => { + i18n.global.locale.value = 'en'; + getRepositories.mockResolvedValue({ + data: ['pathorsAI/pathors', 'pathorsAI/inbox'], + }); + updateSettings.mockResolvedValue({ data: {} }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('offers the install link when nothing is connected', async () => { + const { wrapper } = await mountPage(); + + expect(integrationCard(wrapper).props('integrationEnabled')).toBe(false); + expect(integrationCard(wrapper).props('integrationAction')).toBe( + INSTALL_URL + ); + expect(wrapper.text()).not.toContain('Repository for new issues'); + expect(getRepositories).not.toHaveBeenCalled(); + }); + + it('treats a legacy token hook as not connected', async () => { + const { wrapper } = await mountPage({ + hooks: [installedHook({ reference_id: null })], + }); + + expect(integrationCard(wrapper).props('integrationEnabled')).toBe(false); + expect(integrationCard(wrapper).props('integrationAction')).toBe( + INSTALL_URL + ); + }); + + it('asks to reconnect when the installation lost access', async () => { + const { wrapper } = await mountPage({ + hooks: [ + installedHook({ + reauthorization_required: true, + settings: { repository: 'pathorsAI/inbox' }, + }), + ], + }); + + expect(wrapper.text()).toContain('GitHub needs to be reconnected'); + expect(wrapper.text()).toContain( + 'The GitHub App was uninstalled, suspended, or lost access to the selected repository.' + ); + expect(wrapper.find(`a[href="${INSTALL_URL}"]`).exists()).toBe(true); + expect(integrationCard(wrapper).props('integrationEnabled')).toBe(true); + expect(integrationCard(wrapper).props('integrationAction')).toBe( + 'disconnect' + ); + }); + + it('saves the chosen repository and label, then shows them', async () => { + const { wrapper, get, refreshedHooks } = await mountPage({ + hooks: [installedHook()], + }); + + expect(getRepositories).toHaveBeenCalledTimes(1); + expect(wrapper.text()).toContain( + 'Only repositories the GitHub App can access are listed.' + ); + const comboBox = wrapper.findComponent(ComboBox); + expect(comboBox.props('options')).toEqual([ + { value: 'pathorsAI/pathors', label: 'pathorsAI/pathors' }, + { value: 'pathorsAI/inbox', label: 'pathorsAI/inbox' }, + ]); + + comboBox.vm.$emit('update:modelValue', 'pathorsAI/inbox'); + await wrapper.find('input').setValue(' support '); + refreshedHooks.value = [ + installedHook({ + settings: { repository: 'pathorsAI/inbox', label: 'support' }, + }), + ]; + await wrapper + .findAll('button') + .find(button => button.text() === 'Save') + .trigger('click'); + await flushPromises(); + + expect(updateSettings).toHaveBeenCalledWith({ + repository: 'pathorsAI/inbox', + label: 'support', + }); + expect(useAlert).toHaveBeenCalledWith('GitHub settings saved'); + expect(get).toHaveBeenCalledTimes(2); + expect(wrapper.findComponent(ComboBox).exists()).toBe(false); + expect(wrapper.text()).toContain('Where issues are opened'); + expect( + wrapper.find('a[href="https://github.com/pathorsAI/inbox"]').text() + ).toBe('pathorsAI/inbox'); + expect(wrapper.text()).toContain('support'); + }); + + it('shows the server error when the repository is refused', async () => { + updateSettings.mockRejectedValue({ + response: { + status: 422, + data: { error: 'Repository is not part of this installation' }, + }, + }); + const { wrapper } = await mountPage({ hooks: [installedHook()] }); + + wrapper + .findComponent(ComboBox) + .vm.$emit('update:modelValue', 'pathorsAI/pathors'); + await wrapper.vm.$nextTick(); + await wrapper + .findAll('button') + .find(button => button.text() === 'Save') + .trigger('click'); + await flushPromises(); + + expect(wrapper.text()).toContain( + 'Repository is not part of this installation' + ); + expect(useAlert).not.toHaveBeenCalled(); + expect(wrapper.findComponent(ComboBox).exists()).toBe(true); + }); + + it('says so when the installation exposes no repositories', async () => { + getRepositories.mockResolvedValue({ data: [] }); + const { wrapper } = await mountPage({ hooks: [installedHook()] }); + + expect(wrapper.text()).toContain( + "The GitHub App can't access any repositories yet." + ); + }); + + it('refetches the hook when GitHub refuses the repository listing', async () => { + getRepositories.mockRejectedValue({ + response: { status: 422, data: { error: 'Bad credentials' } }, + }); + const { wrapper, get } = await mountPage({ hooks: [installedHook()] }); + + expect(wrapper.text()).toContain("Couldn't load repositories from GitHub."); + expect(get).toHaveBeenCalledTimes(2); + }); + + it('lets the repository be changed from the connected summary', async () => { + const { wrapper } = await mountPage({ + hooks: [ + installedHook({ + settings: { repository: 'pathorsAI/inbox', label: '' }, + }), + ], + }); + + expect(wrapper.text()).toContain('Where issues are opened'); + expect(wrapper.text()).toContain('None'); + expect(getRepositories).not.toHaveBeenCalled(); + + await wrapper + .findAll('button') + .find(button => button.text() === 'Change') + .trigger('click'); + await flushPromises(); + + expect(getRepositories).toHaveBeenCalledTimes(1); + expect(wrapper.findComponent(ComboBox).props('modelValue')).toBe( + 'pathorsAI/inbox' + ); + + await wrapper + .findAll('button') + .find(button => button.text() === 'Cancel') + .trigger('click'); + + expect(wrapper.findComponent(ComboBox).exists()).toBe(false); + expect(wrapper.text()).toContain('Where issues are opened'); + }); + + it.each([ + [ + { error: 'installation_not_verified' }, + "The GitHub account you signed in with can't see that installation", + ], + [ + { error: 'connection_failed' }, + 'Something went wrong while connecting GitHub.', + ], + [ + { setupAction: 'request' }, + 'Waiting for your GitHub organization owner to approve the installation.', + ], + ])( + 'shows the install redirect notice for %o and clears the query', + async (props, message) => { + const { wrapper } = await mountPage({ props }); + + expect(wrapper.text()).toContain(message); + expect(routerReplace).toHaveBeenCalledWith( + '/app/accounts/1/settings/integrations/github' + ); + } + ); + + it('leaves the URL alone without an install redirect query', async () => { + await mountPage(); + + expect(routerReplace).not.toHaveBeenCalled(); + }); + + it('renders the zh_TW copy', async () => { + i18n.global.locale.value = 'zh_TW'; + const { wrapper } = await mountPage({ + hooks: [ + installedHook({ + settings: { repository: 'pathorsAI/inbox', label: 'support' }, + }), + ], + props: { setupAction: 'request' }, + }); + + expect(wrapper.text()).toContain('Issue 建立位置'); + expect(wrapper.text()).toContain('儲存庫'); + expect(wrapper.text()).toContain('正在等待你的 GitHub 組織擁有者核准安裝'); + }); +}); From d2c05e7a09127d8c9d10fc4eb0d513565098c94b Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:36:17 +0800 Subject: [PATCH 06/10] feat(integrations): add GitHub App strings for en and zh_TW Co-Authored-By: Claude Opus 5.5 --- .../i18n/locale/en/integrations.json | 48 +++++++++++++++++++ .../i18n/locale/zh_TW/integrations.json | 48 +++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/app/javascript/dashboard/i18n/locale/en/integrations.json b/app/javascript/dashboard/i18n/locale/en/integrations.json index c08931e1da..20e8bc3c83 100644 --- a/app/javascript/dashboard/i18n/locale/en/integrations.json +++ b/app/javascript/dashboard/i18n/locale/en/integrations.json @@ -119,6 +119,54 @@ }, "LIST_SEPARATOR": ", " }, + "GITHUB": { + "HEADER": "GitHub", + "DELETE": { + "TITLE": "Disconnect GitHub?", + "MESSAGE": "Agents will stop opening GitHub issues from conversations. Issues that were already opened stay on GitHub." + }, + "FORM": { + "TITLE": "Repository for new issues", + "DESCRIPTION": "Issues opened from conversations are created in this repository." + }, + "REPOSITORY": { + "LABEL": "Repository", + "PLACEHOLDER": "Select a repository", + "SEARCH_PLACEHOLDER": "Search repositories", + "LOADING": "Loading repositories...", + "HELP": "Only repositories the GitHub App can access are listed.", + "EMPTY": "The GitHub App can't access any repositories yet. Grant it access to a repository on GitHub, then reload this page.", + "NO_MATCH": "No repositories match your search", + "LOAD_ERROR": "Couldn't load repositories from GitHub. Try again in a moment." + }, + "LABEL": { + "LABEL": "Issue label", + "PLACEHOLDER": "e.g. support", + "HELP": "Optional. Added to every issue opened from a conversation." + }, + "SAVE": "Save", + "CANCEL": "Cancel", + "CHANGE": "Change", + "UPDATE_SUCCESS": "GitHub settings saved", + "UPDATE_ERROR": "Couldn't save the GitHub settings. Try again.", + "CONNECTED": { + "TITLE": "Where issues are opened", + "DESCRIPTION": "Issues opened from conversations are created in this repository.", + "REPOSITORY": "Repository", + "LABEL": "Label", + "NO_LABEL": "None" + }, + "RECONNECT": { + "TITLE": "GitHub needs to be reconnected", + "DESCRIPTION": "The GitHub App was uninstalled, suspended, or lost access to the selected repository. Reconnect to resume opening issues.", + "BUTTON": "Reconnect GitHub" + }, + "AWAITING_APPROVAL": "Waiting for your GitHub organization owner to approve the installation. You'll be able to pick a repository once they approve.", + "ERRORS": { + "INSTALLATION_NOT_VERIFIED": "The GitHub account you signed in with can't see that installation, so it wasn't connected. Install the app with an account that has access to it, or ask an organization owner to connect it.", + "CONNECTION_FAILED": "Something went wrong while connecting GitHub. Try connecting again." + } + }, "SHOPIFY": { "HEADER": "Shopify", "DELETE": { diff --git a/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json b/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json index 5b45bbd568..e00b54cee6 100644 --- a/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json +++ b/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json @@ -119,6 +119,54 @@ }, "LIST_SEPARATOR": "、" }, + "GITHUB": { + "HEADER": "GitHub", + "DELETE": { + "TITLE": "要解除連接 GitHub 嗎？", + "MESSAGE": "客服人員將無法再從對話建立 GitHub Issue。已經建立的 Issue 會保留在 GitHub 上。" + }, + "FORM": { + "TITLE": "新 Issue 要開在哪個儲存庫", + "DESCRIPTION": "從對話建立的 Issue 會開在這個儲存庫。" + }, + "REPOSITORY": { + "LABEL": "儲存庫", + "PLACEHOLDER": "選擇儲存庫", + "SEARCH_PLACEHOLDER": "搜尋儲存庫", + "LOADING": "正在載入儲存庫...", + "HELP": "只會列出 GitHub App 有權限存取的儲存庫。", + "EMPTY": "GitHub App 目前無法存取任何儲存庫。請先到 GitHub 授權它存取至少一個儲存庫，再重新整理此頁面。", + "NO_MATCH": "找不到符合的儲存庫", + "LOAD_ERROR": "無法從 GitHub 載入儲存庫，請稍後再試。" + }, + "LABEL": { + "LABEL": "Issue 標籤", + "PLACEHOLDER": "例如：support", + "HELP": "選填。從對話建立的每個 Issue 都會加上這個標籤。" + }, + "SAVE": "儲存", + "CANCEL": "取消", + "CHANGE": "變更", + "UPDATE_SUCCESS": "已儲存 GitHub 設定", + "UPDATE_ERROR": "無法儲存 GitHub 設定，請再試一次。", + "CONNECTED": { + "TITLE": "Issue 建立位置", + "DESCRIPTION": "從對話建立的 Issue 會開在這個儲存庫。", + "REPOSITORY": "儲存庫", + "LABEL": "標籤", + "NO_LABEL": "無" + }, + "RECONNECT": { + "TITLE": "需要重新連接 GitHub", + "DESCRIPTION": "GitHub App 已被解除安裝、已被暫停，或失去所選儲存庫的存取權限。重新連接後即可繼續建立 Issue。", + "BUTTON": "重新連接 GitHub" + }, + "AWAITING_APPROVAL": "正在等待你的 GitHub 組織擁有者核准安裝。核准後，你就能在這裡選擇儲存庫。", + "ERRORS": { + "INSTALLATION_NOT_VERIFIED": "你登入的 GitHub 帳號看不到這個安裝，因此沒有完成連接。請改用有權限存取它的帳號安裝，或請組織擁有者來連接。", + "CONNECTION_FAILED": "連接 GitHub 時發生問題，請再試一次。" + } + }, "SHOPIFY": { "HEADER": "Shopify", "DELETE": { From 87fcc23bf66694c838f29e171e15acbffc76cc96 Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:47:39 +0800 Subject: [PATCH 07/10] feat(integrations): complete the GitHub install from the settings page The install callback no longer binds the installation, because doing it server-side allowed a cross-account CSRF. The settings page now posts the redirect's code, installation_id and state to the integrations endpoint itself, clears the single-use query, and then loads the hook. A refused install shows the notice for the returned reason. Co-Authored-By: Claude Opus 5.5 --- .../dashboard/api/integrations/github.js | 8 ++ .../api/specs/integrations/github.spec.js | 14 +++ .../i18n/locale/en/integrations.json | 1 + .../i18n/locale/zh_TW/integrations.json | 1 + .../settings/integrations/Github.vue | 41 ++++--- .../integrations/integrations.routes.js | 4 +- .../integrations/specs/Github.spec.js | 111 +++++++++++++++--- 7 files changed, 147 insertions(+), 33 deletions(-) diff --git a/app/javascript/dashboard/api/integrations/github.js b/app/javascript/dashboard/api/integrations/github.js index bee51691e4..29c10e56e6 100644 --- a/app/javascript/dashboard/api/integrations/github.js +++ b/app/javascript/dashboard/api/integrations/github.js @@ -7,6 +7,14 @@ class GithubAPI extends ApiClient { super('integrations/github', { accountScoped: true }); } + connect({ code, installationId, state }) { + return axios.post(this.url, { + code, + installation_id: installationId, + state, + }); + } + getRepositories() { return axios.get(`${this.url}/repositories`); } diff --git a/app/javascript/dashboard/api/specs/integrations/github.spec.js b/app/javascript/dashboard/api/specs/integrations/github.spec.js index 061a3be253..6763c94307 100644 --- a/app/javascript/dashboard/api/specs/integrations/github.spec.js +++ b/app/javascript/dashboard/api/specs/integrations/github.spec.js @@ -5,6 +5,7 @@ describe('#githubAPI', () => { const originalAxios = window.axios; const axiosMock = { get: vi.fn(() => Promise.resolve()), + post: vi.fn(() => Promise.resolve()), patch: vi.fn(() => Promise.resolve()), }; @@ -21,6 +22,19 @@ describe('#githubAPI', () => { expect(GithubAPIClient).toBeInstanceOf(ApiClient); }); + it('completes an installation', () => { + GithubAPIClient.connect({ + code: 'oauth-code', + installationId: '81234567', + state: 'signed-state', + }); + expect(axiosMock.post).toHaveBeenCalledWith('/api/v1/integrations/github', { + code: 'oauth-code', + installation_id: '81234567', + state: 'signed-state', + }); + }); + it('fetches the repositories of the installation', () => { GithubAPIClient.getRepositories(); expect(axiosMock.get).toHaveBeenCalledWith( diff --git a/app/javascript/dashboard/i18n/locale/en/integrations.json b/app/javascript/dashboard/i18n/locale/en/integrations.json index 20e8bc3c83..639e2aa82c 100644 --- a/app/javascript/dashboard/i18n/locale/en/integrations.json +++ b/app/javascript/dashboard/i18n/locale/en/integrations.json @@ -149,6 +149,7 @@ "CHANGE": "Change", "UPDATE_SUCCESS": "GitHub settings saved", "UPDATE_ERROR": "Couldn't save the GitHub settings. Try again.", + "CONNECTED_SUCCESS": "GitHub connected", "CONNECTED": { "TITLE": "Where issues are opened", "DESCRIPTION": "Issues opened from conversations are created in this repository.", diff --git a/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json b/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json index e00b54cee6..11992bbdfd 100644 --- a/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json +++ b/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json @@ -149,6 +149,7 @@ "CHANGE": "變更", "UPDATE_SUCCESS": "已儲存 GitHub 設定", "UPDATE_ERROR": "無法儲存 GitHub 設定，請再試一次。", + "CONNECTED_SUCCESS": "已連接 GitHub", "CONNECTED": { "TITLE": "Issue 建立位置", "DESCRIPTION": "從對話建立的 Issue 會開在這個儲存庫。", diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue index d2c9f7c41d..88a0665517 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue @@ -3,6 +3,8 @@ import { ref, computed, onMounted } from 'vue'; import { useRouter, useRoute } from 'vue-router'; import { useI18n } from 'vue-i18n'; import { useFunctionGetter, useStore } from 'dashboard/composables/store'; +import { useAlert } from 'dashboard/composables'; +import githubAPI from 'dashboard/api/integrations/github'; import Integration from './Integration.vue'; import RepositoryForm from './Github/RepositoryForm.vue'; @@ -13,7 +15,9 @@ import Button from 'dashboard/components-next/button/Button.vue'; const props = defineProps({ setupAction: { type: String, default: '' }, - error: { type: String, default: '' }, + code: { type: String, default: '' }, + installationId: { type: String, default: '' }, + state: { type: String, default: '' }, }); const STATUS = { @@ -35,7 +39,7 @@ const SETUP_ACTION_NOTICES = { }, }; -const ERROR_NOTICES = { +const CONNECT_ERROR_NOTICES = { installation_not_verified: { color: 'ruby', message: t('INTEGRATION_SETTINGS.GITHUB.ERRORS.INSTALLATION_NOT_VERIFIED'), @@ -71,22 +75,31 @@ const isConnected = computed(() => status.value !== STATUS.NOT_CONNECTED); const repositoryUrl = computed(() => `https://github.com/${repository.value}`); -const initializeGithubIntegration = async () => { - await store.dispatch('integrations/get', 'github'); - integrationLoaded.value = true; +const completeInstall = async ({ code, installationId, state }) => { + try { + await githubAPI.connect({ code, installationId, state }); + useAlert(t('INTEGRATION_SETTINGS.GITHUB.CONNECTED_SUCCESS')); + } catch (error) { + notice.value = + CONNECT_ERROR_NOTICES[error?.response?.data?.reason] ?? + CONNECT_ERROR_NOTICES.connection_failed; + } }; -onMounted(() => { - // The install redirect's query is cleared below so a reload does not repeat - // it, which also clears these props; keep the notice it asked for. - notice.value = - ERROR_NOTICES[props.error] ?? - SETUP_ACTION_NOTICES[props.setupAction] ?? - null; - if (props.error || props.setupAction) { +onMounted(async () => { + // Clearing the install redirect's query also clears these props, so read + // them first. + const { setupAction, code, installationId, state } = props; + notice.value = SETUP_ACTION_NOTICES[setupAction] ?? null; + if (code && installationId && state) { + await completeInstall({ code, installationId, state }); + } + // The code is single-use; a reload must not submit it again. + if (setupAction || code || installationId || state) { router.replace(route.path); } - initializeGithubIntegration(); + await store.dispatch('integrations/get', 'github'); + integrationLoaded.value = true; }); diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js b/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js index 70d993aa7d..77bdf8ebfe 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js @@ -110,7 +110,9 @@ export default { }, props: route => ({ setupAction: route.query.setup_action, - error: route.query.error, + code: route.query.code, + installationId: route.query.installation_id, + state: route.query.state, }), }, { diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js b/app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js index 3bd172973c..8a2fdc7770 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/specs/Github.spec.js @@ -6,14 +6,17 @@ import Github from '../Github.vue'; import Integration from '../Integration.vue'; import ComboBox from 'dashboard/components-next/combobox/ComboBox.vue'; -const { getRepositories, updateSettings, routerReplace } = vi.hoisted(() => ({ - getRepositories: vi.fn(), - updateSettings: vi.fn(), - routerReplace: vi.fn(), -})); +const { connect, getRepositories, updateSettings, routerReplace } = vi.hoisted( + () => ({ + connect: vi.fn(), + getRepositories: vi.fn(), + updateSettings: vi.fn(), + routerReplace: vi.fn(), + }) +); vi.mock('dashboard/api/integrations/github', () => ({ - default: { getRepositories, updateSettings }, + default: { connect, getRepositories, updateSettings }, })); vi.mock('dashboard/composables', () => ({ useAlert: vi.fn() })); @@ -271,34 +274,106 @@ describe('Github settings page', () => { expect(wrapper.text()).toContain('Where issues are opened'); }); + const INSTALL_QUERY = { + code: 'oauth-code', + installationId: '81234567', + state: 'signed-state', + }; + + it('completes the install from the redirect query before loading', async () => { + let finishConnect; + connect.mockReturnValue( + new Promise(resolve => { + finishConnect = resolve; + }) + ); + const { store, get, refreshedHooks } = buildStore([]); + refreshedHooks.value = [installedHook()]; + const wrapper = mount(Github, { + props: INSTALL_QUERY, + global: { + plugins: [store], + stubs: { + Integration: true, + BaseSettingsHeader: true, + WootLoadingState: true, + }, + }, + }); + await flushPromises(); + + expect(connect).toHaveBeenCalledWith(INSTALL_QUERY); + expect(wrapper.findComponent(Integration).exists()).toBe(false); + expect(routerReplace).not.toHaveBeenCalled(); + expect(get).not.toHaveBeenCalled(); + + finishConnect({ data: installedHook() }); + await flushPromises(); + + expect(useAlert).toHaveBeenCalledWith('GitHub connected'); + expect(routerReplace).toHaveBeenCalledWith( + '/app/accounts/1/settings/integrations/github' + ); + expect(get).toHaveBeenCalledTimes(1); + expect(routerReplace.mock.invocationCallOrder[0]).toBeLessThan( + get.mock.invocationCallOrder[0] + ); + expect(wrapper.findComponent(ComboBox).exists()).toBe(true); + }); + it.each([ [ - { error: 'installation_not_verified' }, + 'installation_not_verified', "The GitHub account you signed in with can't see that installation", ], - [ - { error: 'connection_failed' }, - 'Something went wrong while connecting GitHub.', - ], - [ - { setupAction: 'request' }, - 'Waiting for your GitHub organization owner to approve the installation.', - ], + ['connection_failed', 'Something went wrong while connecting GitHub.'], + ['something_new', 'Something went wrong while connecting GitHub.'], ])( - 'shows the install redirect notice for %o and clears the query', - async (props, message) => { - const { wrapper } = await mountPage({ props }); + 'shows the notice for a refused install with reason %s', + async (reason, message) => { + connect.mockRejectedValue({ + response: { + status: 422, + data: { error: 'Installation could not be bound', reason }, + }, + }); + const { wrapper, get } = await mountPage({ props: INSTALL_QUERY }); expect(wrapper.text()).toContain(message); + expect(useAlert).not.toHaveBeenCalled(); expect(routerReplace).toHaveBeenCalledWith( '/app/accounts/1/settings/integrations/github' ); + expect(get).toHaveBeenCalledTimes(1); + expect(integrationCard(wrapper).props('integrationEnabled')).toBe(false); } ); + it('clears an incomplete install query without connecting', async () => { + await mountPage({ props: { code: 'oauth-code' } }); + + expect(connect).not.toHaveBeenCalled(); + expect(routerReplace).toHaveBeenCalledWith( + '/app/accounts/1/settings/integrations/github' + ); + }); + + it('shows the pending approval notice and clears the query', async () => { + const { wrapper } = await mountPage({ props: { setupAction: 'request' } }); + + expect(wrapper.text()).toContain( + 'Waiting for your GitHub organization owner to approve the installation.' + ); + expect(connect).not.toHaveBeenCalled(); + expect(routerReplace).toHaveBeenCalledWith( + '/app/accounts/1/settings/integrations/github' + ); + }); + it('leaves the URL alone without an install redirect query', async () => { await mountPage(); + expect(connect).not.toHaveBeenCalled(); expect(routerReplace).not.toHaveBeenCalled(); }); From cb60ee1b78be00139f4be7e00d5ec943d19ca994 Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 16:53:47 +0800 Subject: [PATCH 08/10] feat(integrations): configure the GitHub App from super admin Adds GITHUB_APP_ID, GITHUB_APP_SLUG, GITHUB_APP_CLIENT_ID, GITHUB_APP_CLIENT_SECRET, GITHUB_APP_PRIVATE_KEY and GITHUB_APP_WEBHOOK_SECRET as installation configs with a GitHub card in super admin. The private key is a code field, because a password input drops the PEM's newlines. The card stays hidden until all six are set; a missing webhook secret would leave uninstalls unnoticed. The integration description now says to connect the GitHub App. Co-Authored-By: Claude Opus 5.5 --- .../super_admin/app_configs_controller.rb | 2 ++ app/helpers/super_admin/features.yml | 6 ++++ .../super_admin/application/_icons.html.erb | 4 +++ config/installation_config.yml | 36 +++++++++++++++++++ config/locales/en.yml | 2 +- config/locales/zh_TW.yml | 2 +- 6 files changed, 50 insertions(+), 2 deletions(-) diff --git a/app/controllers/super_admin/app_configs_controller.rb b/app/controllers/super_admin/app_configs_controller.rb index 7883e0b712..884fb6228c 100644 --- a/app/controllers/super_admin/app_configs_controller.rb +++ b/app/controllers/super_admin/app_configs_controller.rb @@ -62,6 +62,8 @@ def allowed_configs 'microsoft' => %w[AZURE_APP_ID AZURE_APP_SECRET], 'email' => %w[MAILER_INBOUND_EMAIL_DOMAIN ACCOUNT_EMAILS_LIMIT ACCOUNT_EMAILS_PLAN_LIMITS], 'linear' => %w[LINEAR_CLIENT_ID LINEAR_CLIENT_SECRET], + 'github' => %w[GITHUB_APP_ID GITHUB_APP_SLUG GITHUB_APP_CLIENT_ID GITHUB_APP_CLIENT_SECRET GITHUB_APP_PRIVATE_KEY + GITHUB_APP_WEBHOOK_SECRET], 'slack' => %w[SLACK_CLIENT_ID SLACK_CLIENT_SECRET SLACK_SIGNING_SECRET], 'instagram' => %w[INSTAGRAM_APP_ID INSTAGRAM_APP_SECRET INSTAGRAM_VERIFY_TOKEN INSTAGRAM_API_VERSION ENABLE_INSTAGRAM_CHANNEL_HUMAN_AGENT], 'tiktok' => %w[TIKTOK_APP_ID TIKTOK_APP_SECRET TIKTOK_API_VERSION], diff --git a/app/helpers/super_admin/features.yml b/app/helpers/super_admin/features.yml index 0fb6499486..12824cbbf2 100644 --- a/app/helpers/super_admin/features.yml +++ b/app/helpers/super_admin/features.yml @@ -124,6 +124,12 @@ linear: enabled: true icon: 'icon-linear' config_key: 'linear' +github: + name: 'GitHub' + description: 'Configuration for the GitHub App that opens issues from tickets' + enabled: true + icon: 'icon-github' + config_key: 'github' notion: name: 'Notion' description: 'Configuration for setting up Notion Integration' diff --git a/app/views/super_admin/application/_icons.html.erb b/app/views/super_admin/application/_icons.html.erb index 39253c9695..1e98faee29 100644 --- a/app/views/super_admin/application/_icons.html.erb +++ b/app/views/super_admin/application/_icons.html.erb @@ -170,6 +170,10 @@ + + + + diff --git a/config/installation_config.yml b/config/installation_config.yml index 319402166a..826d235607 100644 --- a/config/installation_config.yml +++ b/config/installation_config.yml @@ -413,6 +413,42 @@ description: 'Linear client secret' type: secret ## ------ End of Configs added for Linear ------ ## +## ------ Configs added for the GitHub App ------ ## +- name: GITHUB_APP_ID + display_title: 'GitHub App ID' + value: + locked: false + description: 'Numeric App ID from the GitHub App settings page' +- name: GITHUB_APP_SLUG + display_title: 'GitHub App Slug' + value: + locked: false + description: 'URL name of the GitHub App, as in github.com/apps/' +- name: GITHUB_APP_CLIENT_ID + display_title: 'GitHub App Client ID' + value: + locked: false + description: 'Client ID of the GitHub App' +- name: GITHUB_APP_CLIENT_SECRET + display_title: 'GitHub App Client Secret' + value: + locked: false + description: 'Client secret of the GitHub App' + type: secret +# `code` rather than `secret`: the PEM is multi-line and a password input drops its newlines. +- name: GITHUB_APP_PRIVATE_KEY + display_title: 'GitHub App Private Key' + value: + locked: false + description: 'Private key (PEM) generated on the GitHub App settings page' + type: code +- name: GITHUB_APP_WEBHOOK_SECRET + display_title: 'GitHub App Webhook Secret' + value: + locked: false + description: 'Webhook secret set on the GitHub App; verifies the installation events GitHub sends' + type: secret +## ------ End of Configs added for the GitHub App ------ ## ## ------ Configs added for Notion ------ ## - name: NOTION_CLIENT_ID diff --git a/config/locales/en.yml b/config/locales/en.yml index 86f87137cf..d571b41942 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -502,7 +502,7 @@ en: github: name: 'GitHub' short_description: 'Open a GitHub issue whenever a ticket is created.' - description: 'Hand cases over to engineering automatically. Whenever a ticket is created, this integration opens an issue in the GitHub repository you configure, carrying the contact, the ticket details and a link back to the conversation.' + description: 'Hand cases over to engineering automatically. Connect the Pathors Inbox GitHub App and pick a repository; whenever a ticket is created, the app opens an issue there carrying the contact, the ticket details and a link back to the conversation.' issue_created_note: 'GitHub issue created for this ticket: %{url}' errors: repository_not_granted: 'The GitHub App is not installed on that repository.' diff --git a/config/locales/zh_TW.yml b/config/locales/zh_TW.yml index 5583da11fe..c347431775 100644 --- a/config/locales/zh_TW.yml +++ b/config/locales/zh_TW.yml @@ -468,7 +468,7 @@ zh_TW: github: name: 'GitHub' short_description: '建立工單時自動開一則 GitHub issue。' - description: '自動把案件交接給工程團隊。每當有人建立工單，此整合就會在您設定的 GitHub 儲存庫開一則 issue，並帶上聯絡人、工單資訊與回到對話的連結。' + description: '自動把案件交接給工程團隊。連接 Pathors Inbox GitHub App 並選好儲存庫後，每當有人建立工單，App 就會在該儲存庫開一則 issue，並帶上聯絡人、工單資訊與回到對話的連結。' issue_created_note: '已為這張工單建立 GitHub issue：%{url}' errors: repository_not_granted: 'GitHub App 沒有安裝在這個儲存庫上。' From d3e71c15b3e662f2a56f8120efbdf34ad92cf5c7 Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 17:19:01 +0800 Subject: [PATCH 09/10] refactor(integrations): dispatch GitHub webhook events from lookup tables Co-Authored-By: Claude Opus 5.5 --- app/controllers/webhooks/github_controller.rb | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/app/controllers/webhooks/github_controller.rb b/app/controllers/webhooks/github_controller.rb index 9e936f852c..3236006a42 100644 --- a/app/controllers/webhooks/github_controller.rb +++ b/app/controllers/webhooks/github_controller.rb @@ -1,13 +1,19 @@ class Webhooks::GithubController < ActionController::API before_action :verify_signature! + EVENT_HANDLERS = { + 'installation' => :handle_installation, + 'installation_repositories' => :handle_repositories_removed + }.freeze + INSTALLATION_ACTIONS = { + 'deleted' => :prompt_reauthorization!, + 'suspend' => :prompt_reauthorization!, + 'unsuspend' => :reauthorized! + }.freeze + def events - case request.headers['X-GitHub-Event'] - when 'installation' - handle_installation - when 'installation_repositories' - handle_repositories_removed - end + handler = EVENT_HANDLERS[request.headers['X-GitHub-Event']] + send(handler) if handler head :ok end @@ -24,12 +30,8 @@ def verify_signature! end def handle_installation - case payload['action'] - when 'deleted', 'suspend' - installation_hooks.find_each(&:prompt_reauthorization!) - when 'unsuspend' - installation_hooks.find_each(&:reauthorized!) - end + hook_action = INSTALLATION_ACTIONS[payload['action']] + installation_hooks.find_each(&hook_action) if hook_action end def handle_repositories_removed From e3ae66c15f4b3aca624ac2b77393321bed95d57f Mon Sep 17 00:00:00 2001 From: YJack0000 Date: Tue, 6 Oct 2026 17:19:02 +0800 Subject: [PATCH 10/10] fix(integrations): send an expired GitHub connect link back to its account The connect state now lives an hour, and a genuine but expired one returns the admin to the GitHub settings page with a notice to press Connect again instead of the app root. A leftover personal-token hook can only be deleted, and a non-positive token TTL is no longer written to the cache. Co-Authored-By: Claude Opus 5.5 --- .../integrations/github_controller.rb | 6 +++++- .../github/callbacks_controller.rb | 17 ++++++++++++++-- app/helpers/github/integration_helper.rb | 20 +++++++++++++++---- .../i18n/locale/en/integrations.json | 3 ++- .../i18n/locale/zh_TW/integrations.json | 3 ++- .../settings/integrations/Github.vue | 12 ++++++++--- .../integrations/integrations.routes.js | 1 + lib/integrations/github/app_client.rb | 2 +- .../integrations/github_controller_spec.rb | 19 ++++++++++++++++++ .../github/callbacks_controller_spec.rb | 4 ++-- spec/models/integrations/app_spec.rb | 2 +- 11 files changed, 73 insertions(+), 16 deletions(-) diff --git a/app/controllers/api/v1/accounts/integrations/github_controller.rb b/app/controllers/api/v1/accounts/integrations/github_controller.rb index eec49475bc..8faa8ad091 100644 --- a/app/controllers/api/v1/accounts/integrations/github_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/github_controller.rb @@ -40,8 +40,12 @@ def destroy private + # A leftover personal-token hook has no installation to list or configure; + # it can only be deleted or replaced by installing the app. def fetch_hook - @hook = Current.account.hooks.find_by!(app_id: 'github') + hooks = Current.account.hooks.where(app_id: 'github') + hooks = hooks.where.not(reference_id: nil) unless action_name == 'destroy' + @hook = hooks.first! end def installation_id diff --git a/app/controllers/github/callbacks_controller.rb b/app/controllers/github/callbacks_controller.rb index 32b4d3a555..ab89f7f106 100644 --- a/app/controllers/github/callbacks_controller.rb +++ b/app/controllers/github/callbacks_controller.rb @@ -8,7 +8,7 @@ class Github::CallbacksController < ApplicationController def show account_id = verify_github_state(params[:state]) - return redirect_to(frontend_url) if account_id.blank? + return redirect_expired_state if account_id.blank? query = if params[:setup_action] == 'request' # An org member without admin rights only *requested* the install; @@ -17,11 +17,24 @@ def show else params.permit(:code, :installation_id, :state).to_h end - redirect_to "#{frontend_url}/app/accounts/#{account_id}/settings/integrations/github?#{query.to_query}" + redirect_to settings_url(account_id, query) end private + # The install itself may already have finished on GitHub, so the admin is + # told to press Connect again rather than dropped on the app root. + def redirect_expired_state + account_id = expired_github_state_account_id(params[:state]) + return redirect_to(frontend_url) if account_id.blank? + + redirect_to settings_url(account_id, { error: 'state_expired' }) + end + + def settings_url(account_id, query) + "#{frontend_url}/app/accounts/#{account_id}/settings/integrations/github?#{query.to_query}" + end + def frontend_url ENV.fetch('FRONTEND_URL', 'http://localhost:3000') end diff --git a/app/helpers/github/integration_helper.rb b/app/helpers/github/integration_helper.rb index 87ddf1c07d..6681012c1f 100644 --- a/app/helpers/github/integration_helper.rb +++ b/app/helpers/github/integration_helper.rb @@ -2,7 +2,9 @@ # hands `state` back on the callback, and it is the only thing that tells us # which account started the install. module Github::IntegrationHelper - STATE_TTL = 15.minutes + # The state only names the account; binding is gated separately by the + # admin session, so a long TTL costs nothing and survives slow installs. + STATE_TTL = 1.hour def generate_github_state(account_id) secret = github_client_secret @@ -13,17 +15,27 @@ def generate_github_state(account_id) end def verify_github_state(token) + decode_github_state(token, verify_expiration: true) + end + + # Signed by us but past its expiry: good enough to send the admin back to + # their own settings page to retry, never to bind anything. + def expired_github_state_account_id(token) + decode_github_state(token, verify_expiration: false) + end + + private + + def decode_github_state(token, verify_expiration:) secret = github_client_secret return if token.blank? || secret.blank? - JWT.decode(token, secret, true, { algorithm: 'HS256', required_claims: %w[exp] }).first['sub'] + JWT.decode(token, secret, true, { algorithm: 'HS256', required_claims: %w[exp], verify_expiration: verify_expiration }).first['sub'] rescue JWT::DecodeError => e Rails.logger.warn("Rejected GitHub install state: #{e.message}") nil end - private - def github_client_secret GlobalConfigService.load('GITHUB_APP_CLIENT_SECRET', nil) end diff --git a/app/javascript/dashboard/i18n/locale/en/integrations.json b/app/javascript/dashboard/i18n/locale/en/integrations.json index 639e2aa82c..f3550bc83f 100644 --- a/app/javascript/dashboard/i18n/locale/en/integrations.json +++ b/app/javascript/dashboard/i18n/locale/en/integrations.json @@ -165,7 +165,8 @@ "AWAITING_APPROVAL": "Waiting for your GitHub organization owner to approve the installation. You'll be able to pick a repository once they approve.", "ERRORS": { "INSTALLATION_NOT_VERIFIED": "The GitHub account you signed in with can't see that installation, so it wasn't connected. Install the app with an account that has access to it, or ask an organization owner to connect it.", - "CONNECTION_FAILED": "Something went wrong while connecting GitHub. Try connecting again." + "CONNECTION_FAILED": "Something went wrong while connecting GitHub. Try connecting again.", + "STATE_EXPIRED": "The connect link expired before GitHub sent you back. The app may already be installed on GitHub; press Connect again to finish." } }, "SHOPIFY": { diff --git a/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json b/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json index 11992bbdfd..3209d03c94 100644 --- a/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json +++ b/app/javascript/dashboard/i18n/locale/zh_TW/integrations.json @@ -165,7 +165,8 @@ "AWAITING_APPROVAL": "正在等待你的 GitHub 組織擁有者核准安裝。核准後，你就能在這裡選擇儲存庫。", "ERRORS": { "INSTALLATION_NOT_VERIFIED": "你登入的 GitHub 帳號看不到這個安裝，因此沒有完成連接。請改用有權限存取它的帳號安裝，或請組織擁有者來連接。", - "CONNECTION_FAILED": "連接 GitHub 時發生問題，請再試一次。" + "CONNECTION_FAILED": "連接 GitHub 時發生問題，請再試一次。", + "STATE_EXPIRED": "連接的連結在回到這裡之前就過期了。App 可能已經裝在 GitHub 上，再按一次「連接」就能完成。" } }, "SHOPIFY": { diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue index 88a0665517..54092c606c 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/Github.vue @@ -15,6 +15,7 @@ import Button from 'dashboard/components-next/button/Button.vue'; const props = defineProps({ setupAction: { type: String, default: '' }, + error: { type: String, default: '' }, code: { type: String, default: '' }, installationId: { type: String, default: '' }, state: { type: String, default: '' }, @@ -48,6 +49,10 @@ const CONNECT_ERROR_NOTICES = { color: 'ruby', message: t('INTEGRATION_SETTINGS.GITHUB.ERRORS.CONNECTION_FAILED'), }, + state_expired: { + color: 'amber', + message: t('INTEGRATION_SETTINGS.GITHUB.ERRORS.STATE_EXPIRED'), + }, }; const integrationLoaded = ref(false); @@ -89,13 +94,14 @@ const completeInstall = async ({ code, installationId, state }) => { onMounted(async () => { // Clearing the install redirect's query also clears these props, so read // them first. - const { setupAction, code, installationId, state } = props; - notice.value = SETUP_ACTION_NOTICES[setupAction] ?? null; + const { setupAction, error, code, installationId, state } = props; + notice.value = + SETUP_ACTION_NOTICES[setupAction] ?? CONNECT_ERROR_NOTICES[error] ?? null; if (code && installationId && state) { await completeInstall({ code, installationId, state }); } // The code is single-use; a reload must not submit it again. - if (setupAction || code || installationId || state) { + if (setupAction || error || code || installationId || state) { router.replace(route.path); } await store.dispatch('integrations/get', 'github'); diff --git a/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js b/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js index 77bdf8ebfe..1a45f6c9a4 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js +++ b/app/javascript/dashboard/routes/dashboard/settings/integrations/integrations.routes.js @@ -110,6 +110,7 @@ export default { }, props: route => ({ setupAction: route.query.setup_action, + error: route.query.error, code: route.query.code, installationId: route.query.installation_id, state: route.query.state, diff --git a/lib/integrations/github/app_client.rb b/lib/integrations/github/app_client.rb index 573c65315f..0bddef9bbf 100644 --- a/lib/integrations/github/app_client.rb +++ b/lib/integrations/github/app_client.rb @@ -25,7 +25,7 @@ def installation_token(installation_id, repository: nil) ensure_success!(response) expires_in = Time.zone.parse(response['expires_at']) - TOKEN_REFRESH_MARGIN - Time.current - Rails.cache.write(cache_key, response['token'], expires_in: expires_in) + Rails.cache.write(cache_key, response['token'], expires_in: expires_in) if expires_in.positive? response['token'] end diff --git a/spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb index 86d13dd7bf..b45929d129 100644 --- a/spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/github_controller_spec.rb @@ -127,6 +127,16 @@ expect(response).to have_http_status(:unprocessable_entity) expect(hook.reauthorization_required?).to be(true) end + + it 'treats a leftover personal-token hook as not connected' do + hook.update!(reference_id: nil) + + get "#{base_url}/repositories", headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:not_found) + expect(hook.reauthorization_required?).to be(false) + expect(WebMock).not_to have_requested(:any, /github\.com/) + end end describe 'PATCH /api/v1/accounts/:account_id/integrations/github' do @@ -161,5 +171,14 @@ expect(account.hooks.where(app_id: 'github')).to be_empty expect(WebMock).not_to have_requested(:any, /github\.com/) end + + it 'removes a leftover personal-token hook' do + hook.update!(reference_id: nil) + + delete base_url, headers: admin.create_new_auth_token, as: :json + + expect(response).to have_http_status(:ok) + expect(account.hooks.where(app_id: 'github')).to be_empty + end end end diff --git a/spec/controllers/github/callbacks_controller_spec.rb b/spec/controllers/github/callbacks_controller_spec.rb index aecb65e7d6..56143fa5e8 100644 --- a/spec/controllers/github/callbacks_controller_spec.rb +++ b/spec/controllers/github/callbacks_controller_spec.rb @@ -38,12 +38,12 @@ expect(response).to redirect_to('http://www.example.com') end - it 'rejects an expired state' do + it 'sends an expired state back to its account to connect again, without the install' do expired = JWT.encode({ sub: account.id, exp: 1.minute.ago.to_i }, client_secret, 'HS256') get github_callback_path, params: { code: 'oauth-code', installation_id: '4242', state: expired } - expect(response).to redirect_to('http://www.example.com') + expect(response).to redirect_to("#{settings_url}?error=state_expired") end it 'rejects a state without an expiry' do diff --git a/spec/models/integrations/app_spec.rb b/spec/models/integrations/app_spec.rb index 6d6e92e24f..5232772ea4 100644 --- a/spec/models/integrations/app_spec.rb +++ b/spec/models/integrations/app_spec.rb @@ -74,7 +74,7 @@ expect("#{uri.scheme}://#{uri.host}#{uri.path}").to eq('https://github.com/apps/pathors-inbox/installations/new') expect(payload['sub']).to eq(account.id) - expect(payload['exp'] - payload['iat']).to eq(15.minutes.to_i) + expect(payload['exp'] - payload['iat']).to eq(1.hour.to_i) end end