MM-67866 Improving connection logic

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,

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,

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"

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

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)
+			}
+		})
+	}
+}