From 19fda69267be2c1793c9095b101495e91f099522 Mon Sep 17 00:00:00 2001 From: david <58231312+davseby@users.noreply.github.com> Date: Wed, 5 Aug 2026 21:05:00 +0300 Subject: [PATCH] make github and slack apps optional --- docker/docker-compose.dev.yaml | 5 +- docker/env/core.example.env | 9 +- docker/github/.gitkeep | 0 server/CLAUDE.md | 2 +- server/core/cmd/core/main.go | 25 ++- .../internal/apps/githubapp/installation.go | 8 + .../apps/githubapp/installation_test.go | 34 ++++ .../core/internal/apps/githubapp/manager.go | 28 ++- .../internal/apps/githubapp/manager_test.go | 90 ++++++++++ .../apps/githubapp/testdata/test-key.pem | 27 +++ .../internal/apps/slackapp/installation.go | 12 ++ .../apps/slackapp/installation_test.go | 33 ++++ server/core/internal/apps/slackapp/link.go | 8 + .../core/internal/apps/slackapp/link_test.go | 24 +++ server/core/internal/apps/slackapp/manager.go | 34 +++- .../internal/apps/slackapp/manager_test.go | 117 ++++++++++++ .../internal/document/hook/manager/manager.go | 10 ++ .../hook/processor/github_tracking.go | 6 +- .../hook/processor/github_tracking_test.go | 115 ++++++++++++ .../internal/server/githubhandler/http.go | 47 +++-- .../server/githubhandler/http_test.go | 167 ++++++++++++++++++ server/core/internal/server/router.go | 48 +++-- .../core/internal/server/slackhandler/http.go | 52 ++++-- .../internal/server/slackhandler/http_test.go | 166 +++++++++++++++++ web/app/components/AppSidebar.vue | 16 +- .../editor/hooks/HookMenuContent.vue | 3 + web/app/components/settings/AppsSection.vue | 31 +++- web/app/composables/api/useGitHubAPI.ts | 13 +- web/app/composables/api/useSlackAPI.ts | 17 +- web/app/utils/api/github.ts | 1 + web/app/utils/api/slack.ts | 1 + 31 files changed, 1076 insertions(+), 73 deletions(-) create mode 100644 docker/github/.gitkeep create mode 100644 server/core/internal/apps/githubapp/installation_test.go create mode 100644 server/core/internal/apps/githubapp/manager_test.go create mode 100644 server/core/internal/apps/githubapp/testdata/test-key.pem create mode 100644 server/core/internal/apps/slackapp/installation_test.go create mode 100644 server/core/internal/apps/slackapp/link_test.go create mode 100644 server/core/internal/apps/slackapp/manager_test.go create mode 100644 server/core/internal/document/hook/processor/github_tracking_test.go create mode 100644 server/core/internal/server/githubhandler/http_test.go create mode 100644 server/core/internal/server/slackhandler/http_test.go diff --git a/docker/docker-compose.dev.yaml b/docker/docker-compose.dev.yaml index 3b56a1ae..31f93606 100644 --- a/docker/docker-compose.dev.yaml +++ b/docker/docker-compose.dev.yaml @@ -66,7 +66,10 @@ services: connector: condition: service_started volumes: - - ./github/oxynote.private-key.pem:/etc/oxynote/core/oxynote.private-key.pem + # optional GitHub App private key: drop oxynote.private-key.pem into + # docker/github/ and set OXYNOTE_CORE_GITHUB_APP_ID to enable the + # GitHub App integration. + - ./github:/etc/oxynote/core/github:ro restart: unless-stopped logging: driver: local diff --git a/docker/env/core.example.env b/docker/env/core.example.env index c9b788a6..c9a7b15a 100644 --- a/docker/env/core.example.env +++ b/docker/env/core.example.env @@ -25,14 +25,17 @@ OXYNOTE_CORE_SERVER_PUBLIC_URL=http://localhost:8081/go OXYNOTE_CORE_BASE_APP_URL=http://localhost:8081 OXYNOTE_CORE_ORIGINS=http://localhost:8081,localhost:8081 -# integrations. GITHUB_APP_ID and the private key are currently REQUIRED for -# core to boot; everything else degrades gracefully when empty. +# Integrations. All of them are optional and degrade gracefully when empty. +# To enable the GitHub App, set GITHUB_APP_ID and drop the app's private key +# into docker/github/oxynote.private-key.pem; an empty GITHUB_APP_ID disables +# the integration entirely. To enable Slack, set all SLACK_* values; an empty +# SLACK_CLIENT_ID disables it. OXYNOTE_CORE_GITHUB_APP_ID= OXYNOTE_CORE_GITHUB_APP_SLUG= OXYNOTE_CORE_GITHUB_SIGNATURE_SECRET= OXYNOTE_CORE_GITHUB_CLIENT_SECRET= OXYNOTE_CORE_GITHUB_INSTALLATION_SIGNING_SECRET= -OXYNOTE_CORE_GITHUB_PRIVATE_KEY_PATH=/etc/oxynote/core/oxynote.private-key.pem +OXYNOTE_CORE_GITHUB_PRIVATE_KEY_PATH=/etc/oxynote/core/github/oxynote.private-key.pem OXYNOTE_CORE_SLACK_CLIENT_ID= OXYNOTE_CORE_SLACK_CLIENT_SECRET= OXYNOTE_CORE_SLACK_SIGNATURE_SECRET= diff --git a/docker/github/.gitkeep b/docker/github/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/server/CLAUDE.md b/server/CLAUDE.md index 30dfc430..b16b23f1 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -102,7 +102,7 @@ Env lives in `docker/env/`: committed `*.example.env` templates list **every** v - core vars are prefixed `OXYNOTE_CORE_`; `buildinfo.Getenv("FOO")` reads `OXYNOTE_CORE_FOO`. - auth-realtime vars are prefixed `OXYNOTE_AUTH_REALTIME_`. -- `OXYNOTE_CORE_GITHUB_APP_ID` and the GitHub App private key (mounted from `docker/github/`) are currently **required** for core to boot. The assistant needs `ANTHROPIC_API_KEY`; without it the AI chat can't complete a turn. +- All integrations are optional. The GitHub App is enabled by setting `OXYNOTE_CORE_GITHUB_APP_ID` and dropping the app's private key into `docker/github/` (mounted into the core container); an empty `OXYNOTE_CORE_GITHUB_APP_ID` disables it — core boots, GitHub routes respond `github.not_configured` (the always-200 `GET /api/github` status endpoint reports `configured: false` as the frontend's capability signal), and github-tracking hooks are skipped. A set app ID with a missing/unreadable key is a boot error. Slack works the same way keyed on `OXYNOTE_CORE_SLACK_CLIENT_ID` (`slack.not_configured`, `GET /api/slack` reports `configured`); a set client ID with other `SLACK_*` values missing is a boot error. The assistant needs `ANTHROPIC_API_KEY`; without it the AI chat can't complete a turn. ## House conventions worth knowing diff --git a/server/core/cmd/core/main.go b/server/core/cmd/core/main.go index f8478cf2..f0aede00 100644 --- a/server/core/cmd/core/main.go +++ b/server/core/cmd/core/main.go @@ -93,17 +93,24 @@ func main() { closers = append([]io.Closer{rdb}, closers...) - githubAppID, err := strconv.ParseInt(buildinfo.Getenv("GITHUB_APP_ID"), 10, 64) - if err != nil { - err = ioutil.AppendCloseErr( - ioutil.MultiCloser(true, closers...), - fmt.Errorf("invalid GITHUB_APP_ID: %w", err), - ) + // an empty GITHUB_APP_ID means the GitHub App integration is disabled. + // The zero app ID makes githubapp.NewManager return an unconfigured + // manager instead of failing. + var githubAppID int64 - log.With("error", err). - Error("cannot parse GITHUB_APP_ID") + if v := buildinfo.Getenv("GITHUB_APP_ID"); v != "" { + githubAppID, err = strconv.ParseInt(v, 10, 64) + if err != nil { + err = ioutil.AppendCloseErr( + ioutil.MultiCloser(true, closers...), + fmt.Errorf("invalid GITHUB_APP_ID: %w", err), + ) - return + log.With("error", err). + Error("cannot parse GITHUB_APP_ID") + + return + } } githubMan, err := githubapp.NewManager(dbc, githubapp.Options{ diff --git a/server/core/internal/apps/githubapp/installation.go b/server/core/internal/apps/githubapp/installation.go index 5ab94e39..8fb9d9c8 100644 --- a/server/core/internal/apps/githubapp/installation.go +++ b/server/core/internal/apps/githubapp/installation.go @@ -28,6 +28,10 @@ type InstallationState struct { // CreateInstallationURL generates the GitHub App installation URL for the // specified organization ID. func (m *Manager) CreateInstallationURL(organizationID string) (string, error) { + if !m.Configured() { + return "", ErrNotConfigured + } + u := url.URL{ Scheme: "https", Host: _githubHost, @@ -57,6 +61,10 @@ func (m *Manager) CreateInstallationURL(organizationID string) (string, error) { // VerifyInstallationState verifies and decrypts the installation state // from the given state string. func (m *Manager) VerifyInstallationState(state string) (*InstallationState, error) { + if !m.Configured() { + return nil, ErrNotConfigured + } + decrypted, err := cryptoutil.DecryptText(state, []byte(m.opt.InstallationSigningSecret)) if err != nil { return nil, fmt.Errorf("failed to decrypt installation state: %w", err) diff --git a/server/core/internal/apps/githubapp/installation_test.go b/server/core/internal/apps/githubapp/installation_test.go new file mode 100644 index 00000000..62a805da --- /dev/null +++ b/server/core/internal/apps/githubapp/installation_test.go @@ -0,0 +1,34 @@ +package githubapp + +import ( + "errors" + "testing" +) + +func Test_Manager_CreateInstallationURL(t *testing.T) { + t.Parallel() + + man, err := NewManager(nil, Options{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _, err = man.CreateInstallationURL("org") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} + +func Test_Manager_VerifyInstallationState(t *testing.T) { + t.Parallel() + + man, err := NewManager(nil, Options{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _, err = man.VerifyInstallationState("state") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} diff --git a/server/core/internal/apps/githubapp/manager.go b/server/core/internal/apps/githubapp/manager.go index 663040ad..9ca38d1f 100644 --- a/server/core/internal/apps/githubapp/manager.go +++ b/server/core/internal/apps/githubapp/manager.go @@ -22,6 +22,9 @@ var ( // ErrInstallationNotFound is returned when a GitHub installation resource is not found. ErrInstallationNotFound = errutil.New(http.StatusNotFound, "github.installation_not_found", "installation not found") + + // ErrNotConfigured is returned when the GitHub App integration is not configured on this deployment. + ErrNotConfigured = errutil.New(http.StatusConflict, "github.not_configured", "github app is not configured") ) // Options holds configuration options for the Github handler. @@ -49,8 +52,18 @@ type Manager struct { opt Options } -// NewManager creates a new GitHub App client with the given app ID and private key path. +// NewManager creates a new GitHub App client with the given app ID and private +// key path. A zero AppID means the GitHub App integration is not configured: +// the manager is still created, but Configured reports false and every method +// that talks to GitHub returns ErrNotConfigured. func NewManager(db DB, opt Options) (*Manager, error) { + if opt.AppID == 0 { + return &Manager{ + db: db, + opt: opt, + }, nil + } + appClient, err := createAppClient(opt.AppID, opt.PrivateKeyPath) if err != nil { return nil, err @@ -63,6 +76,11 @@ func NewManager(db DB, opt Options) (*Manager, error) { }, nil } +// Configured reports whether the GitHub App integration is configured. +func (m *Manager) Configured() bool { + return m.appClient != nil +} + // SignatureSecret returns the signature secret for verifying GitHub request signatures. func (m *Manager) SignatureSecret() string { return m.opt.SignatureSecret @@ -70,6 +88,10 @@ func (m *Manager) SignatureSecret() string { // HasInstallationClient checks whether InstallationClient exists for the given organization ID. func (m *Manager) HasInstallationClient(ctx context.Context, organizationID string) (bool, error) { + if !m.Configured() { + return false, ErrNotConfigured + } + installationID, err := m.db.FetchGithubInstallationByOrganizationID(ctx, organizationID) switch { @@ -105,6 +127,10 @@ func (m *Manager) HasInstallationClient(ctx context.Context, organizationID stri // GetInstallationClient fetches an InstallationClient for the given installation ID. func (m *Manager) GetInstallationClient(ctx context.Context, organizationID string) (*InstallationClient, error) { + if !m.Configured() { + return nil, ErrNotConfigured + } + installationID, err := m.db.FetchGithubInstallationByOrganizationID(ctx, organizationID) switch { diff --git a/server/core/internal/apps/githubapp/manager_test.go b/server/core/internal/apps/githubapp/manager_test.go new file mode 100644 index 00000000..82064b42 --- /dev/null +++ b/server/core/internal/apps/githubapp/manager_test.go @@ -0,0 +1,90 @@ +package githubapp + +import ( + "context" + "errors" + "testing" +) + +func Test_NewManager(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + Opt Options + WantErr bool + WantConfigured bool + }{ + "Zero app ID creates an unconfigured manager": { + Opt: Options{}, + WantErr: false, + WantConfigured: false, + }, + "App ID with missing private key fails": { + Opt: Options{ + AppID: 123, + PrivateKeyPath: "testdata/missing.pem", + }, + WantErr: true, + }, + "App ID with valid private key creates a configured manager": { + Opt: Options{ + AppID: 123, + PrivateKeyPath: "testdata/test-key.pem", + }, + WantErr: false, + WantConfigured: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + man, err := NewManager(nil, tc.Opt) + + if tc.WantErr { + if err == nil { + t.Fatal("expected an error, got nil") + } + + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := man.Configured(); got != tc.WantConfigured { + t.Fatalf("Configured() = %v, want %v", got, tc.WantConfigured) + } + }) + } +} + +func Test_Manager_HasInstallationClient(t *testing.T) { + t.Parallel() + + man, err := NewManager(nil, Options{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _, err = man.HasInstallationClient(context.Background(), "org") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} + +func Test_Manager_GetInstallationClient(t *testing.T) { + t.Parallel() + + man, err := NewManager(nil, Options{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + _, err = man.GetInstallationClient(context.Background(), "org") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} diff --git a/server/core/internal/apps/githubapp/testdata/test-key.pem b/server/core/internal/apps/githubapp/testdata/test-key.pem new file mode 100644 index 00000000..8f059ce8 --- /dev/null +++ b/server/core/internal/apps/githubapp/testdata/test-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEogIBAAKCAQEAoqgS0JJHZ75BIJHxq5zQIwT1C67R9nzvLLxrYHtIVBcqV768 +MAUeprFhWi6Nxrgh1tCPxsxuRSUVARsvik+N9eeb/J5yNV9ILK9hIhnQ/PJ1jEQ/ +uhkIkIZOMU1CUxSnJUmS5sPvn4CXWrjib6oIDfDlwapHjEdaPkpU568cjYBgIXxr +b9mhQ2uu8uL+JhuhP4ZTtWVk7w+dcWDvin0EZlW3qMZ0ImiHoAo/e+tcDSULcyqP +vBWRPDlkpALpgVho8VSUnSiZk0VmvTXkG40tSNhUo39tC5petHiOczLeTZkko/H4 +4lDUKY5o/Llk+VeVK3F7dWR3+AVCatGbRi3UTwIDAQABAoIBAASbPe0qN1t4x99k +QrPZfQuA2G+8Q7X3U86wh1n0I9g9z7UscafqfTO7UkZ7iwhVrp+myGuY3oaWxkGB +w5bnKd4M8R32XsWkLScW0Glm0cRYGXwfcq6ShuuQ4iYDSh2QOCuWKdLfOaoKuj6X +fdIrvo6mg9s4N06BtsyMzlrBWs2nCYBLbVediuprVA4JDiP4+fUE8GRxtOk9X5lV +yELxu0TW8AauGp3bmcnyEkbGYn+Hwf8uty5BPv3ZP0nvBGbI4QaXxf8GAX+p423/ ++rJberowkKOWxtBwPun8t9+pXl4ty4ycHE12340uWqy1Z/X/5IjZCqL0NyC5MYD3 +JCXcVAECgYEA1REu8sEYUCa1fCe8+Ue0Gg1eubCO3704aMa768jSOUtxkhNvfx1n +JiqXy5/GvJLXZ92N0iQXvVr7Ie5f9dsMKhHmMi0gXIHAw8aO+mzt2J/mpFQluAak +1Zrb3WgwjVvcFff5e73kUpp6U/Y3fBrPnRJOcV434WnjKvXYnNGPh6cCgYEAw26E ++ENSn3ZVTjEQej6GtdE0GLmDkyITx5p0PiOZYnl7wGULvmd1Sp/y+UKsK+c7xe// +LFs+NRuxaf6yoTG1GW2qcPLKX5jGarP5k0p1zUHeijhCNAYxNgiPMYrWczaueV/h +MMmAx57QyqwDePOETysgWhdVIOiSVg5DUrssYxkCgYAOgngPAgVU1/398XrWAwMJ +zyVHQfXI2Mrxn68GdIuUaUXXq8ko/+iZWccyj7Pp/Zn7scPJrrmLJJn7io8wX/Vp +HATwH9CXXCKzcl8kQKzF1sXPJQfYi2NIRJ56anD7/2hyeoub2Kem1GxGMXs/0VeY +G7oJ0F+/0wUsVMemjkLW+wKBgCxbemzLAwbUmfocYswMQxEjgbqW6MlNc8XbRcUQ +2XCjeDYr+SVnYBTnBwQSYPe6DTQZDAbqUs1FEtinCa5SlPNHk+b9BHf8OhTf8qa+ +wMwsxjEDIeui+Cp2eOHazF/sKl+8JmWnZsZoQqWts0ARrZc2h6pLH3RPtV8474jo +mOOJAoGAadxqubYjtdCRR4fgOqG4r0dqWBMaN3Ylke7BnMnOx5nHYrMIpuCNYjFe +u3m6g/hfQdl3/yPm49v/eb1ToYXVrwmUeckf4XcO4hXPzmylu6xgN8yHZsxei34p +n1qefunyP7WsU6f1T9okWHz6IrxdJAfRVgjzq1Sk85qH1ZZ9h54= +-----END RSA PRIVATE KEY----- diff --git a/server/core/internal/apps/slackapp/installation.go b/server/core/internal/apps/slackapp/installation.go index 105df3f8..ea2faea3 100644 --- a/server/core/internal/apps/slackapp/installation.go +++ b/server/core/internal/apps/slackapp/installation.go @@ -48,6 +48,10 @@ type InstallationState struct { // CreateExternalInstallationURL generates the Slack installation URL for the // specified organization ID. This is used when installing Slack from the app. func (m *Manager) CreateExternalInstallationURL(organizationID string) (string, error) { + if !m.Configured() { + return "", ErrNotConfigured + } + u := url.URL{ Scheme: "https", Host: _slackHost, @@ -81,6 +85,10 @@ func (m *Manager) CreateExternalInstallationURL(organizationID string) (string, // CreateInternalInstallationURL generates the Slack installation URL for the // specified team ID. This is used when connecting from within Slack. func (m *Manager) CreateInternalInstallationURL(teamID string) (string, error) { + if !m.Configured() { + return "", ErrNotConfigured + } + u, err := url.Parse(m.opt.RedirectURL) if err != nil { return "", fmt.Errorf("failed to parse installation URL: %w", err) @@ -109,6 +117,10 @@ func (m *Manager) CreateInternalInstallationURL(teamID string) (string, error) { // VerifyInstallationState verifies and decrypts the installation state // from the given state string. func (m *Manager) VerifyInstallationState(state string) (*InstallationState, error) { + if !m.Configured() { + return nil, ErrNotConfigured + } + decrypted, err := cryptoutil.DecryptText(state, []byte(m.opt.InstallationSigningSecret)) if err != nil { return nil, fmt.Errorf("failed to decrypt installation state: %w", err) diff --git a/server/core/internal/apps/slackapp/installation_test.go b/server/core/internal/apps/slackapp/installation_test.go new file mode 100644 index 00000000..e64387da --- /dev/null +++ b/server/core/internal/apps/slackapp/installation_test.go @@ -0,0 +1,33 @@ +package slackapp + +import ( + "errors" + "testing" +) + +func Test_Manager_CreateExternalInstallationURL(t *testing.T) { + t.Parallel() + + _, err := newDisabledManager(t).CreateExternalInstallationURL("org") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} + +func Test_Manager_CreateInternalInstallationURL(t *testing.T) { + t.Parallel() + + _, err := newDisabledManager(t).CreateInternalInstallationURL("team") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} + +func Test_Manager_VerifyInstallationState(t *testing.T) { + t.Parallel() + + _, err := newDisabledManager(t).VerifyInstallationState("state") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} diff --git a/server/core/internal/apps/slackapp/link.go b/server/core/internal/apps/slackapp/link.go index d29170a4..1a337ff2 100644 --- a/server/core/internal/apps/slackapp/link.go +++ b/server/core/internal/apps/slackapp/link.go @@ -37,6 +37,10 @@ type LinkState struct { // CreateLinkURL generates a URL for linking a Slack user to an internal account. func (m *Manager) CreateLinkURL(slackUserID, slackTeamID, organizationID string) (string, error) { + if !m.Configured() { + return "", ErrNotConfigured + } + data, err := json.Marshal(LinkState{ SlackUserID: slackUserID, TeamID: slackTeamID, @@ -67,6 +71,10 @@ func (m *Manager) CreateLinkURL(slackUserID, slackTeamID, organizationID string) // VerifyLinkState verifies and decrypts the link state from the given state string. func (m *Manager) VerifyLinkState(state string) (*LinkState, error) { + if !m.Configured() { + return nil, ErrNotConfigured + } + decrypted, err := cryptoutil.DecryptText(state, []byte(m.opt.InstallationSigningSecret)) if err != nil { return nil, fmt.Errorf("failed to decrypt link state: %w", err) diff --git a/server/core/internal/apps/slackapp/link_test.go b/server/core/internal/apps/slackapp/link_test.go new file mode 100644 index 00000000..cff4a382 --- /dev/null +++ b/server/core/internal/apps/slackapp/link_test.go @@ -0,0 +1,24 @@ +package slackapp + +import ( + "errors" + "testing" +) + +func Test_Manager_CreateLinkURL(t *testing.T) { + t.Parallel() + + _, err := newDisabledManager(t).CreateLinkURL("user", "team", "org") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} + +func Test_Manager_VerifyLinkState(t *testing.T) { + t.Parallel() + + _, err := newDisabledManager(t).VerifyLinkState("state") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} diff --git a/server/core/internal/apps/slackapp/manager.go b/server/core/internal/apps/slackapp/manager.go index 859f970a..d0ccea11 100644 --- a/server/core/internal/apps/slackapp/manager.go +++ b/server/core/internal/apps/slackapp/manager.go @@ -21,6 +21,9 @@ var ( // ErrAppNotConnected is returned when a Slack app is not connected to an organization. ErrAppNotConnected = errutil.New(http.StatusBadRequest, "slack.app_not_connected", "slack app is not connected to an organization") + + // ErrNotConfigured is returned when the Slack App integration is not configured on this deployment. + ErrNotConfigured = errutil.New(http.StatusConflict, "slack.not_configured", "slack app is not configured") ) // Options holds configuration options for the Slack manager. @@ -75,7 +78,11 @@ type Manager struct { cleanup func() } -// NewManager creates a new Slack App manager with the given options. +// NewManager creates a new Slack App manager with the given options. An empty +// client ID means the Slack App integration is not configured: the manager is +// still created, but Configured reports false, every method that talks to +// Slack returns ErrNotConfigured, and notifications are not forwarded to +// Slack. func NewManager( log *slog.Logger, db DB, @@ -83,10 +90,6 @@ func NewManager( notifs notification.NotificationReceiver, opt Options, ) (*Manager, error) { - if err := opt.Validate(); err != nil { - return nil, err - } - m := &Manager{ log: log.With("component", "slack-app-manager"), db: db, @@ -94,11 +97,24 @@ func NewManager( opt: opt, } + if opt.ClientID == "" { + return m, nil + } + + if err := opt.Validate(); err != nil { + return nil, err + } + m.cleanup = notifs.OnNotification(m.ProcessNotification) return m, nil } +// Configured reports whether the Slack App integration is configured. +func (m *Manager) Configured() bool { + return m.opt.ClientID != "" +} + // Close stops and closes all processes. func (m *Manager) Close() error { if m.cleanup != nil { @@ -194,6 +210,10 @@ func (m *Manager) VerifyMiddleware(r *http.Request) error { // ExchangeCode exchanges an OAuth code for an app access data. func (m *Manager) ExchangeCode(ctx context.Context, code string) (*AppAccess, error) { + if !m.Configured() { + return nil, ErrNotConfigured + } + resp, err := slack.GetOAuthV2ResponseContext( ctx, http.DefaultClient, @@ -215,6 +235,10 @@ func (m *Manager) ExchangeCode(ctx context.Context, code string) (*AppAccess, er // GetClient creates a Slack client for the given team ID. func (m *Manager) GetClient(ctx context.Context, teamID string) (*slack.Client, error) { + if !m.Configured() { + return nil, ErrNotConfigured + } + app, err := m.db.FetchSlackAppByTeamID(ctx, teamID) if err != nil { if errutil.IsNotFound(err) { diff --git a/server/core/internal/apps/slackapp/manager_test.go b/server/core/internal/apps/slackapp/manager_test.go new file mode 100644 index 00000000..3b1916ba --- /dev/null +++ b/server/core/internal/apps/slackapp/manager_test.go @@ -0,0 +1,117 @@ +package slackapp + +import ( + "context" + "errors" + "io" + "log/slog" + "testing" + + "github.com/oxynote/oxynote/server/core/internal/notification" +) + +// fakeReceiver is a no-op notification receiver for tests. +type fakeReceiver struct{} + +func (fakeReceiver) OnNotification(_ func(context.Context, notification.Notification)) notification.Unsubscribe { + return func() {} +} + +// newDisabledManager creates an unconfigured Manager for tests. +func newDisabledManager(t *testing.T) *Manager { + t.Helper() + + man, err := NewManager( + slog.New(slog.NewTextHandler(io.Discard, nil)), + nil, + nil, + nil, + Options{}, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return man +} + +func Test_NewManager(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + Opt Options + WantErr bool + WantConfigured bool + }{ + "Empty client ID creates an unconfigured manager": { + Opt: Options{}, + WantErr: false, + WantConfigured: false, + }, + "Client ID with missing client secret fails": { + Opt: Options{ + ClientID: "id", + }, + WantErr: true, + }, + "Full options create a configured manager": { + Opt: Options{ + ClientID: "id", + ClientSecret: "secret", + SignatureSecret: "sig", + RedirectURL: "http://localhost/slack", + InstallationSigningSecret: "signing", + }, + WantErr: false, + WantConfigured: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + man, err := NewManager( + slog.New(slog.NewTextHandler(io.Discard, nil)), + nil, + nil, + fakeReceiver{}, + tc.Opt, + ) + + if tc.WantErr { + if err == nil { + t.Fatal("expected an error, got nil") + } + + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := man.Configured(); got != tc.WantConfigured { + t.Fatalf("Configured() = %v, want %v", got, tc.WantConfigured) + } + }) + } +} + +func Test_Manager_ExchangeCode(t *testing.T) { + t.Parallel() + + _, err := newDisabledManager(t).ExchangeCode(context.Background(), "code") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} + +func Test_Manager_GetClient(t *testing.T) { + t.Parallel() + + _, err := newDisabledManager(t).GetClient(context.Background(), "team") + if !errors.Is(err, ErrNotConfigured) { + t.Fatalf("expected ErrNotConfigured, got %v", err) + } +} diff --git a/server/core/internal/document/hook/manager/manager.go b/server/core/internal/document/hook/manager/manager.go index 1a0cf1c7..def6606d 100644 --- a/server/core/internal/document/hook/manager/manager.go +++ b/server/core/internal/document/hook/manager/manager.go @@ -123,6 +123,16 @@ func (m *Manager) processHooks(ctx context.Context) error { continue } + // github-tracking hooks cannot make progress without a + // configured GitHub App; skip them and leave their state + // untouched. + if h.Type == hook.TypeGithubTracking && !m.githubMan.Configured() { + m.log.With("hook_id", h.ID). + Warn("skipping github-tracking hook: github app is not configured") + + continue + } + previousScore := h.Score ok := m.ensureHook(ctx, ps, &h) diff --git a/server/core/internal/document/hook/processor/github_tracking.go b/server/core/internal/document/hook/processor/github_tracking.go index 0220019d..fc2d1569 100644 --- a/server/core/internal/document/hook/processor/github_tracking.go +++ b/server/core/internal/document/hook/processor/github_tracking.go @@ -60,7 +60,8 @@ func (gt *GithubTracking) Process(ctx context.Context, inp Input) (decimal.Decim switch { case err == nil: // OK. - case errors.Is(err, githubapp.ErrInstallationNotFound): + case errors.Is(err, githubapp.ErrInstallationNotFound), + errors.Is(err, githubapp.ErrNotConfigured): return gts.EncodeState(decimal.Zero, GithubTrackingStatusMissingInstallation) default: return decimal.Zero, nil, fmt.Errorf("getting github client: %w", err) @@ -109,7 +110,8 @@ func (gt *GithubTracking) Reset(ctx context.Context, inp Input) (decimal.Decimal switch { case err == nil: // OK. - case errors.Is(err, githubapp.ErrInstallationNotFound): + case errors.Is(err, githubapp.ErrInstallationNotFound), + errors.Is(err, githubapp.ErrNotConfigured): return gts.EncodeState(decimal.Zero, GithubTrackingStatusMissingInstallation) default: return decimal.Zero, nil, fmt.Errorf("getting github client: %w", err) diff --git a/server/core/internal/document/hook/processor/github_tracking_test.go b/server/core/internal/document/hook/processor/github_tracking_test.go new file mode 100644 index 00000000..c27c901c --- /dev/null +++ b/server/core/internal/document/hook/processor/github_tracking_test.go @@ -0,0 +1,115 @@ +package processor + +import ( + "context" + "encoding/json" + "testing" + + "github.com/oxynote/oxynote/server/core/internal/apps/githubapp" + "github.com/shopspring/decimal" +) + +// githubErrInput is a test Input whose Github method always fails with the +// configured error. +type githubErrInput struct { + state State + err error +} + +func (i githubErrInput) State() State { + return i.state +} + +func (i githubErrInput) Github(_ context.Context) (Github, error) { + return nil, i.err +} + +func (i githubErrInput) ChangeDetection() ChangeDetection { + return nil +} + +func Test_GithubTracking_Process(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + Err error + WantStatus GithubTrackingStatus + }{ + "Installation not found degrades to missing installation": { + Err: githubapp.ErrInstallationNotFound, + WantStatus: GithubTrackingStatusMissingInstallation, + }, + "Github app not configured degrades to missing installation": { + Err: githubapp.ErrNotConfigured, + WantStatus: GithubTrackingStatusMissingInstallation, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + gt := &GithubTracking{ + Repository: "repo", + Branch: "main", + } + + inp := githubErrInput{ + state: State("{}"), + err: tc.Err, + } + + score, state, err := gt.Process(context.Background(), inp) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !score.Equal(decimal.Zero) { + t.Fatalf("score = %s, want 0", score) + } + + var gts GithubTrackingState + + if err := json.Unmarshal(state, >s); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if gts.Status != tc.WantStatus { + t.Fatalf("status = %q, want %q", gts.Status, tc.WantStatus) + } + }) + } +} + +func Test_GithubTracking_Reset(t *testing.T) { + t.Parallel() + + gt := &GithubTracking{ + Repository: "repo", + Branch: "main", + } + + inp := githubErrInput{ + state: State("{}"), + err: githubapp.ErrNotConfigured, + } + + score, state, err := gt.Reset(context.Background(), inp) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !score.Equal(decimal.Zero) { + t.Fatalf("score = %s, want 0", score) + } + + var gts GithubTrackingState + + if err := json.Unmarshal(state, >s); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if gts.Status != GithubTrackingStatusMissingInstallation { + t.Fatalf("status = %q, want %q", gts.Status, GithubTrackingStatusMissingInstallation) + } +} diff --git a/server/core/internal/server/githubhandler/http.go b/server/core/internal/server/githubhandler/http.go index 46f45c67..b1b95fca 100644 --- a/server/core/internal/server/githubhandler/http.go +++ b/server/core/internal/server/githubhandler/http.go @@ -71,9 +71,30 @@ func NewHandler( } } +// RequireConfigured is a middleware that rejects requests with +// githubapp.ErrNotConfigured when the GitHub App integration is not +// configured on this deployment. +func (h *Handler) RequireConfigured(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !h.man.Configured() { + httpserver.RespondError(h.log, w, githubapp.ErrNotConfigured) + return + } + + next.ServeHTTP(w, r) + }) +} + // VerifySignature is a middleware that verifies the Github request signature. func (h *Handler) VerifySignature(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // respond with a bare 404 when the GitHub App is not configured: + // no GitHub App can be pointing its webhooks at this deployment. + if !h.man.Configured() { + httpserver.RespondError(h.log, w, errutil.NewPlain(http.StatusNotFound)) + return + } + payload, err := github.ValidatePayload(r, []byte(h.man.SignatureSecret())) if err != nil { httpserver.RespondError(h.log, w, err) @@ -133,20 +154,26 @@ func (h *Handler) CheckInstallation(w http.ResponseWriter, r *http.Request) { var connected bool - _, err = h.db.FetchGithubInstallationByOrganizationID(r.Context(), session.ActiveOrganizationID) + // skip the DB lookup when the GitHub App is not configured: this + // endpoint always responds 200 so the frontend can use the configured + // flag as the capability signal. + if h.man.Configured() { + _, err = h.db.FetchGithubInstallationByOrganizationID(r.Context(), session.ActiveOrganizationID) - switch { - case errutil.IsNotFound(err): - // OK. - case err == nil: - connected = true - default: - httpserver.RespondError(h.log, w, err) - return + switch { + case errutil.IsNotFound(err): + // OK. + case err == nil: + connected = true + default: + httpserver.RespondError(h.log, w, err) + return + } } httpserver.Respond(h.log, w, map[string]bool{ - "connected": connected, + "connected": connected, + "configured": h.man.Configured(), }, http.StatusOK) } diff --git a/server/core/internal/server/githubhandler/http_test.go b/server/core/internal/server/githubhandler/http_test.go new file mode 100644 index 00000000..818bb2b5 --- /dev/null +++ b/server/core/internal/server/githubhandler/http_test.go @@ -0,0 +1,167 @@ +package githubhandler + +import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/json" + "encoding/pem" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "testing" + + "github.com/oxynote/oxynote/server/core/internal/apps/githubapp" + "github.com/oxynote/oxynote/server/core/internal/server/auth" +) + +// newTestManager creates a githubapp.Manager for tests. When configured is +// true, a throwaway RSA key is generated so the manager builds a real app +// client. +func newTestManager(t *testing.T, configured bool) *githubapp.Manager { + t.Helper() + + opt := githubapp.Options{} + + if configured { + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + keyPath := filepath.Join(t.TempDir(), "test-key.pem") + + data := pem.EncodeToMemory(&pem.Block{ + Type: "RSA PRIVATE KEY", + Bytes: x509.MarshalPKCS1PrivateKey(key), + }) + + if err := os.WriteFile(keyPath, data, 0o600); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + opt.AppID = 123 + opt.PrivateKeyPath = keyPath + } + + man, err := githubapp.NewManager(nil, opt) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return man +} + +func newTestHandler(t *testing.T, configured bool) *Handler { + t.Helper() + + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + + return NewHandler(log, nil, nil, newTestManager(t, configured)) +} + +func Test_Handler_CheckInstallation(t *testing.T) { + t.Parallel() + + h := newTestHandler(t, false) + + req := httptest.NewRequest(http.MethodGet, "/api/github", nil) + req = req.WithContext(auth.AddSessionToContext(req.Context(), auth.Session{ + UserID: "user", + ActiveOrganizationID: "org", + })) + + rec := httptest.NewRecorder() + h.CheckInstallation(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + var body map[string]bool + + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if body["connected"] || body["configured"] { + t.Fatalf("body = %v, want connected=false configured=false", body) + } +} + +func Test_Handler_RequireConfigured(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + Configured bool + WantStatus int + WantCode string + }{ + "Not configured responds with github.not_configured": { + Configured: false, + WantStatus: http.StatusConflict, + WantCode: "github.not_configured", + }, + "Configured passes through": { + Configured: true, + WantStatus: http.StatusNoContent, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t, tc.Configured) + + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodGet, "/api/github/install", nil) + rec := httptest.NewRecorder() + + h.RequireConfigured(next).ServeHTTP(rec, req) + + if rec.Code != tc.WantStatus { + t.Fatalf("status = %d, want %d", rec.Code, tc.WantStatus) + } + + if tc.WantCode == "" { + return + } + + var body map[string]string + + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if body["code"] != tc.WantCode { + t.Fatalf("code = %q, want %q", body["code"], tc.WantCode) + } + }) + } +} + +func Test_Handler_VerifySignature(t *testing.T) { + t.Parallel() + + h := newTestHandler(t, false) + + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodPost, "/api/x/github/events", nil) + rec := httptest.NewRecorder() + + h.VerifySignature(next).ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound) + } +} diff --git a/server/core/internal/server/router.go b/server/core/internal/server/router.go index 2d73b3ba..ad71f1c3 100644 --- a/server/core/internal/server/router.go +++ b/server/core/internal/server/router.go @@ -163,16 +163,22 @@ func (s *Server) router() chi.Router { })) r.Route("/github", func(sr chi.Router) { + // the status endpoint stays available when the GitHub App is not + // configured: it is the capability signal for the frontend. sr.Get("/", s.handlers.github.CheckInstallation) - sr.Delete("/", s.handlers.github.DisconnectOrganization) - sr.Get("/install", s.handlers.github.FetchInstall) - sr.Get("/connect", s.handlers.github.ConnectOrganization) - sr.Get("/issues", s.handlers.github.FetchIssues) - sr.Route("/repositories", func(ssr chi.Router) { - ssr.Get("/", s.handlers.github.FetchRepositories) - ssr.Route("/{repository}", func(sssr chi.Router) { - sssr.Get("/branches", s.handlers.github.FetchRepositoryBranches) - sssr.Get("/tree", s.handlers.github.FetchRepositoryTree) + + sr.Group(func(ssr chi.Router) { + ssr.Use(s.handlers.github.RequireConfigured) + ssr.Delete("/", s.handlers.github.DisconnectOrganization) + ssr.Get("/install", s.handlers.github.FetchInstall) + ssr.Get("/connect", s.handlers.github.ConnectOrganization) + ssr.Get("/issues", s.handlers.github.FetchIssues) + ssr.Route("/repositories", func(sssr chi.Router) { + sssr.Get("/", s.handlers.github.FetchRepositories) + sssr.Route("/{repository}", func(ssssr chi.Router) { + ssssr.Get("/branches", s.handlers.github.FetchRepositoryBranches) + ssssr.Get("/tree", s.handlers.github.FetchRepositoryTree) + }) }) }) }) @@ -187,16 +193,22 @@ func (s *Server) router() chi.Router { }) r.Route("/slack", func(sr chi.Router) { + // the status endpoint stays available when the Slack App is not + // configured: it is the capability signal for the frontend. sr.Get("/", s.handlers.slack.CheckInstallation) - sr.Delete("/", s.handlers.slack.DisconnectOrganization) - sr.Get("/install", s.handlers.slack.FetchInstall) - sr.Get("/connect", s.handlers.slack.ConnectOrganization) - sr.Get("/messages", s.handlers.slack.FetchMessages) - sr.Route("/users", func(ssr chi.Router) { - ssr.Get("/", s.handlers.slack.FetchUserLink) - ssr.Put("/settings", s.handlers.slack.UpdateUserLink) - ssr.Delete("/", s.handlers.slack.DeleteUserLink) - ssr.Get("/link", s.handlers.slack.LinkUser) + + sr.Group(func(ssr chi.Router) { + ssr.Use(s.handlers.slack.RequireConfigured) + ssr.Delete("/", s.handlers.slack.DisconnectOrganization) + ssr.Get("/install", s.handlers.slack.FetchInstall) + ssr.Get("/connect", s.handlers.slack.ConnectOrganization) + ssr.Get("/messages", s.handlers.slack.FetchMessages) + ssr.Route("/users", func(sssr chi.Router) { + sssr.Get("/", s.handlers.slack.FetchUserLink) + sssr.Put("/settings", s.handlers.slack.UpdateUserLink) + sssr.Delete("/", s.handlers.slack.DeleteUserLink) + sssr.Get("/link", s.handlers.slack.LinkUser) + }) }) }) diff --git a/server/core/internal/server/slackhandler/http.go b/server/core/internal/server/slackhandler/http.go index 341a417a..65a222df 100644 --- a/server/core/internal/server/slackhandler/http.go +++ b/server/core/internal/server/slackhandler/http.go @@ -67,9 +67,30 @@ func NewHandler(log *slog.Logger, db DB, client *http.Client, man *slackapp.Mana } } +// RequireConfigured is a middleware that rejects requests with +// slackapp.ErrNotConfigured when the Slack App integration is not configured +// on this deployment. +func (h *Handler) RequireConfigured(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !h.man.Configured() { + httpserver.RespondError(h.log, w, slackapp.ErrNotConfigured) + return + } + + next.ServeHTTP(w, r) + }) +} + // VerifySignature is a middleware that verifies the Slack request signature. func (h *Handler) VerifySignature(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // respond with a bare 404 when the Slack App is not configured: + // no Slack App can be pointing its callbacks at this deployment. + if !h.man.Configured() { + httpserver.RespondError(h.log, w, errutil.NewPlain(http.StatusNotFound)) + return + } + if err := h.man.VerifyMiddleware(r); err != nil { httpserver.RespondError(h.log, w, err) return @@ -82,6 +103,11 @@ func (h *Handler) VerifySignature(next http.Handler) http.Handler { // InstallApp handles the installation of a Slack app by exchanging the // code for an access token. func (h *Handler) InstallApp(w http.ResponseWriter, r *http.Request) { + if !h.man.Configured() { + httpserver.RespondError(h.log, w, errutil.NewPlain(http.StatusNotFound)) + return + } + code := r.URL.Query().Get("code") if code == "" { httpserver.RespondError(h.log, w, ErrMissingCode) @@ -155,20 +181,26 @@ func (h *Handler) CheckInstallation(w http.ResponseWriter, r *http.Request) { var connected bool - _, err = h.db.FetchSlackAppByOrganizationID(r.Context(), session.ActiveOrganizationID) + // skip the DB lookup when the Slack App is not configured: this + // endpoint always responds 200 so the frontend can use the configured + // flag as the capability signal. + if h.man.Configured() { + _, err = h.db.FetchSlackAppByOrganizationID(r.Context(), session.ActiveOrganizationID) - switch { - case errutil.IsNotFound(err): - // OK. - case err == nil: - connected = true - default: - httpserver.RespondError(h.log, w, err) - return + switch { + case errutil.IsNotFound(err): + // OK. + case err == nil: + connected = true + default: + httpserver.RespondError(h.log, w, err) + return + } } httpserver.Respond(h.log, w, map[string]bool{ - "connected": connected, + "connected": connected, + "configured": h.man.Configured(), }, http.StatusOK) } diff --git a/server/core/internal/server/slackhandler/http_test.go b/server/core/internal/server/slackhandler/http_test.go new file mode 100644 index 00000000..d2acc199 --- /dev/null +++ b/server/core/internal/server/slackhandler/http_test.go @@ -0,0 +1,166 @@ +package slackhandler + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "github.com/oxynote/oxynote/server/core/internal/apps/slackapp" + "github.com/oxynote/oxynote/server/core/internal/notification" + "github.com/oxynote/oxynote/server/core/internal/server/auth" +) + +// fakeReceiver is a no-op notification receiver for tests. +type fakeReceiver struct{} + +func (fakeReceiver) OnNotification(_ func(context.Context, notification.Notification)) notification.Unsubscribe { + return func() {} +} + +// newTestHandler creates a Handler backed by a Slack manager that is either +// configured with dummy credentials or fully unconfigured. +func newTestHandler(t *testing.T, configured bool) *Handler { + t.Helper() + + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + opt := slackapp.Options{} + + if configured { + opt = slackapp.Options{ + ClientID: "id", + ClientSecret: "secret", + SignatureSecret: "sig", + RedirectURL: "http://localhost/slack", + InstallationSigningSecret: "signing", + } + } + + man, err := slackapp.NewManager(log, nil, nil, fakeReceiver{}, opt) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return NewHandler(log, nil, nil, man) +} + +func Test_Handler_CheckInstallation(t *testing.T) { + t.Parallel() + + h := newTestHandler(t, false) + + req := httptest.NewRequest(http.MethodGet, "/api/slack", nil) + req = req.WithContext(auth.AddSessionToContext(req.Context(), auth.Session{ + UserID: "user", + ActiveOrganizationID: "org", + })) + + rec := httptest.NewRecorder() + h.CheckInstallation(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + var body map[string]bool + + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if body["connected"] || body["configured"] { + t.Fatalf("body = %v, want connected=false configured=false", body) + } +} + +func Test_Handler_RequireConfigured(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + Configured bool + WantStatus int + WantCode string + }{ + "Not configured responds with slack.not_configured": { + Configured: false, + WantStatus: http.StatusConflict, + WantCode: "slack.not_configured", + }, + "Configured passes through": { + Configured: true, + WantStatus: http.StatusNoContent, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t, tc.Configured) + + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodGet, "/api/slack/install", nil) + rec := httptest.NewRecorder() + + h.RequireConfigured(next).ServeHTTP(rec, req) + + if rec.Code != tc.WantStatus { + t.Fatalf("status = %d, want %d", rec.Code, tc.WantStatus) + } + + if tc.WantCode == "" { + return + } + + var body map[string]string + + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if body["code"] != tc.WantCode { + t.Fatalf("code = %q, want %q", body["code"], tc.WantCode) + } + }) + } +} + +func Test_Handler_VerifySignature(t *testing.T) { + t.Parallel() + + h := newTestHandler(t, false) + + next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + + req := httptest.NewRequest(http.MethodPost, "/api/x/slack/events", nil) + rec := httptest.NewRecorder() + + h.VerifySignature(next).ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound) + } +} + +func Test_Handler_InstallApp(t *testing.T) { + t.Parallel() + + h := newTestHandler(t, false) + + req := httptest.NewRequest(http.MethodGet, "/api/x/slack/install?code=abc", nil) + rec := httptest.NewRecorder() + + h.InstallApp(rec, req) + + if rec.Code != http.StatusNotFound { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound) + } +} diff --git a/web/app/components/AppSidebar.vue b/web/app/components/AppSidebar.vue index eccdb274..0ca5906b 100644 --- a/web/app/components/AppSidebar.vue +++ b/web/app/components/AppSidebar.vue @@ -27,8 +27,10 @@ const emit = defineEmits<{ const { t } = useI18n({ useScope: "global" }) const { fetchDocumentTree, updateDocumentTree } = useDocumentAPI() const { fetchOrganization, safeSignOut } = useAuthSession() -const { fetchGitHubConnectionStatus, fetchGitHubInstallURL } = useGitHubAPI() -const { fetchSlackConnectionStatus, fetchSlackInstallURL } = useSlackAPI() +const { fetchGitHubConnectionStatus, gitHubConfigured, fetchGitHubInstallURL } = + useGitHubAPI() +const { fetchSlackConnectionStatus, slackConfigured, fetchSlackInstallURL } = + useSlackAPI() const { useFetchNotificationCount } = useNotificationAPI() const fetchNotificationCount = useFetchNotificationCount({ read: false }) const editorStore = useEditorStore() @@ -150,7 +152,10 @@ const sections = computed(() => { }) } - if (!fetchGitHubConnectionStatus.data.value?.connected) { + if ( + gitHubConfigured.value && + !fetchGitHubConnectionStatus.data.value?.connected + ) { nextStepsItems.push({ id: "connect-github", name: t("sidebar.sections.next-steps.items.connect-github"), @@ -167,7 +172,10 @@ const sections = computed(() => { }) } - if (!fetchSlackConnectionStatus.data.value?.connected) { + if ( + slackConfigured.value && + !fetchSlackConnectionStatus.data.value?.connected + ) { nextStepsItems.push({ id: "connect-slack", name: t("sidebar.sections.next-steps.items.connect-slack"), diff --git a/web/app/components/editor/hooks/HookMenuContent.vue b/web/app/components/editor/hooks/HookMenuContent.vue index d966c1f4..57354352 100644 --- a/web/app/components/editor/hooks/HookMenuContent.vue +++ b/web/app/components/editor/hooks/HookMenuContent.vue @@ -13,6 +13,8 @@ const emit = defineEmits<{ (e: "open-settings", target: "github"): void }>() +const { gitHubConfigured } = useGitHubAPI() + const isSubOpen = ref(false) const matchingActiveHooks = computed(() => { @@ -124,6 +126,7 @@ function hookProps(type: string, hook: DocumentHook): any { @force-close="isSubOpen = false" /> import { showToastMessage } from "../toast" -const { fetchGitHubConnectionStatus, fetchGitHubInstallURL, disconnectGitHub } = - useGitHubAPI() -const { fetchSlackConnectionStatus, fetchSlackInstallURL, disconnectSlack } = - useSlackAPI() +const { + fetchGitHubConnectionStatus, + gitHubConfigured, + fetchGitHubInstallURL, + disconnectGitHub, +} = useGitHubAPI() +const { + fetchSlackConnectionStatus, + slackConfigured, + fetchSlackInstallURL, + disconnectSlack, +} = useSlackAPI() const { t } = useI18n({ useScope: "global" }) const { openExternalLink } = useExternalLinks() @@ -62,7 +70,10 @@ async function disconnectSlackApp() {