diff --git a/server/plugin/api.go b/server/plugin/api.go index 05e32da5f..52ae04105 100644 --- a/server/plugin/api.go +++ b/server/plugin/api.go @@ -309,6 +309,10 @@ func (p *Plugin) connectUserToGitHub(c *Context, w http.ResponseWriter, r *http. privateAllowed := false pValBool, _ := strconv.ParseBool(r.URL.Query().Get("private")) if pValBool { + if !p.getConfiguration().EnablePrivateRepo { + http.Error(w, "private repositories are disabled", http.StatusForbidden) + return + } privateAllowed = true } @@ -425,13 +429,20 @@ func (p *Plugin) completeConnectUserToGitHub(c *Context, w http.ResponseWriter, } githubClient := p.githubConnectToken(*tok) - gitUser, _, err := githubClient.Users.Get(ctx, "") + gitUser, resp, err := githubClient.Users.Get(ctx, "") if err != nil { c.Log.WithError(err).Errorf("Failed to get authenticated GitHub user") p.writeAPIError(w, &APIErrorResponse{Message: "failed to get authenticated GitHub user", StatusCode: http.StatusInternalServerError}) return } + if scopeErr := p.validateOAuthScopes(resp, state.PrivateAllowed); scopeErr != nil { + c.Log.WithError(scopeErr).Warnf("Mismatching OAuth scopes, rejecting connection") + rErr = errors.New("OAuth token scope does not match the requested permissions") + p.writeAPIError(w, &APIErrorResponse{Message: "OAuth token scope does not match the requested permissions. Please reconnect your GitHub account.", StatusCode: http.StatusForbidden}) + return + } + userInfo := &GitHubUserInfo{ UserID: state.UserID, Token: tok, diff --git a/server/plugin/api_test.go b/server/plugin/api_test.go index 5ac79e652..4b662c4cc 100644 --- a/server/plugin/api_test.go +++ b/server/plugin/api_test.go @@ -63,6 +63,31 @@ func TestWithRecovery(t *testing.T) { } } +func TestConnectUserToGitHub_PrivateRepoDisabled(t *testing.T) { + p := NewPlugin() + p.setConfiguration( + &Configuration{ + GitHubOrg: "mockOrg", + GitHubOAuthClientID: "mockID", + GitHubOAuthClientSecret: "mockSecret", + EncryptionKey: "mockKey123456789", + EnablePrivateRepo: false, + }) + p.initializeAPI() + api := &plugintest.API{} + p.SetAPI(api) + p.client = pluginapi.NewClient(api, p.Driver) + + req := httptest.NewRequest(http.MethodGet, "/oauth/connect?private=true", nil) + req.Header.Set("Mattermost-User-ID", "testuser") + rec := httptest.NewRecorder() + + p.ServeHTTP(&plugin.Context{}, rec, req) + + assert.Equal(t, http.StatusForbidden, rec.Code) + assert.Contains(t, rec.Body.String(), "private repositories are disabled") +} + func TestPlugin_ServeHTTP(t *testing.T) { httpTestJSON := testutils.HTTPTest{ T: t, diff --git a/server/plugin/command.go b/server/plugin/command.go index 9c4d273a0..e8f03bcc6 100644 --- a/server/plugin/command.go +++ b/server/plugin/command.go @@ -1061,7 +1061,7 @@ func (p *Plugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*mo qparams := "" if privateAllowed { if !p.getConfiguration().EnablePrivateRepo { - p.postCommandResponse(args, "Private repositories are disabled. Please ask a System Admin to enabled them.") + p.postCommandResponse(args, "Private repositories are disabled. Please ask a System Admin to enable them.") return &model.CommandResponse{}, nil } qparams = "?private=true" diff --git a/server/plugin/plugin.go b/server/plugin/plugin.go index fcab70252..3fd21f43c 100644 --- a/server/plugin/plugin.go +++ b/server/plugin/plugin.go @@ -595,6 +595,38 @@ func (p *Plugin) getOAuthConfigForChimeraApp(scopes []string) (*oauth2.Config, e }, nil } +const oauthScopesHeader = "X-OAuth-Scopes" + +func (p *Plugin) validateOAuthScopes(resp *github.Response, privateAllowed bool) error { + if privateAllowed && p.getConfiguration().EnablePrivateRepo { + return nil + } + + if resp == nil || resp.Response == nil { + return errors.New("missing GitHub API response for scope validation") + } + + scopeHeader := resp.Header.Get(oauthScopesHeader) + granted := parseScopes(scopeHeader) + + if slices.Contains(granted, string(github.ScopeRepo)) { + return fmt.Errorf("token was granted %q scope but only %q was requested", github.ScopeRepo, github.ScopePublicRepo) + } + + return nil +} + +func parseScopes(header string) []string { + var scopes []string + for s := range strings.SplitSeq(header, ",") { + s = strings.TrimSpace(s) + if s != "" { + scopes = append(scopes, s) + } + } + return scopes +} + type GitHubUserInfo struct { UserID string Token *oauth2.Token diff --git a/server/plugin/utils_test.go b/server/plugin/utils_test.go index c4d629a1a..a6ea39d9b 100644 --- a/server/plugin/utils_test.go +++ b/server/plugin/utils_test.go @@ -5,6 +5,7 @@ package plugin import ( "fmt" + "net/http" "testing" "github.com/google/go-github/v54/github" @@ -285,3 +286,112 @@ func TestLastN(t *testing.T) { assert.Equal(t, tc.Expected, lastN(tc.Text, tc.N)) } } + +func TestParseScopes(t *testing.T) { + tests := []struct { + name string + header string + expected []string + }{ + {name: "empty header", header: "", expected: nil}, + {name: "single scope", header: "repo", expected: []string{"repo"}}, + {name: "multiple scopes", header: "repo, notifications, read:org", expected: []string{"repo", "notifications", "read:org"}}, + {name: "public_repo only", header: "public_repo, notifications", expected: []string{"public_repo", "notifications"}}, + {name: "extra whitespace", header: " repo , notifications , read:org ", expected: []string{"repo", "notifications", "read:org"}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := parseScopes(tc.header) + assert.Equal(t, tc.expected, result) + }) + } +} + +func TestValidateOAuthScopes(t *testing.T) { + makeResponse := func(scopes string) *github.Response { + h := make(http.Header) + h.Set(oauthScopesHeader, scopes) + return &github.Response{ + Response: &http.Response{ + Header: h, + }, + } + } + + tests := []struct { + name string + resp *github.Response + privateAllowed bool + enablePrivateRepo bool + expectError bool + }{ + { + name: "private allowed and enabled — skip validation", + resp: makeResponse("repo, notifications, read:org"), + privateAllowed: true, + enablePrivateRepo: true, + expectError: false, + }, + { + name: "private not allowed — public_repo is fine", + resp: makeResponse("public_repo, notifications, read:org"), + privateAllowed: false, + enablePrivateRepo: false, + expectError: false, + }, + { + name: "private not allowed — repo scope is rejected", + resp: makeResponse("repo, notifications, read:org"), + privateAllowed: false, + enablePrivateRepo: false, + expectError: true, + }, + { + name: "private allowed but admin disabled — repo scope is rejected", + resp: makeResponse("repo, notifications, read:org"), + privateAllowed: true, + enablePrivateRepo: false, + expectError: true, + }, + { + name: "nil response — returns error", + resp: nil, + privateAllowed: false, + enablePrivateRepo: false, + expectError: true, + }, + { + name: "nil inner response — returns error", + resp: &github.Response{ + Response: nil, + }, + privateAllowed: false, + enablePrivateRepo: false, + expectError: true, + }, + { + name: "empty scope header — no error", + resp: makeResponse(""), + privateAllowed: false, + enablePrivateRepo: false, + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + p := NewPlugin() + p.setConfiguration(&Configuration{ + EnablePrivateRepo: tc.enablePrivateRepo, + }) + + err := p.validateOAuthScopes(tc.resp, tc.privateAllowed) + if tc.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +}