Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

search: Create the "my" repo group #15093

Merged
merged 6 commits into from
Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 11 additions & 6 deletions cmd/frontend/graphqlbackend/external_services.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ type addExternalServiceInput struct {
Namespace *graphql.ID
}

func currentUserAllowedExternalServices(ctx context.Context) bool {
allowUserExternalServices := conf.ExternalServiceUserMode()
if !allowUserExternalServices {
// The user may have a tag that opts them in
err := backend.CheckActorHasTag(ctx, backend.TagAllowUserExternalServicePublic)
allowUserExternalServices = err == nil
}
return allowUserExternalServices
}

func (r *schemaResolver) AddExternalService(ctx context.Context, args *addExternalServiceArgs) (*externalServiceResolver, error) {
if os.Getenv("EXTSVC_CONFIG_FILE") != "" && !extsvcConfigAllowEdits {
return nil, errors.New("adding external service not allowed when using EXTSVC_CONFIG_FILE")
Expand All @@ -45,12 +55,7 @@ func (r *schemaResolver) AddExternalService(ctx context.Context, args *addExtern
// 🚨 SECURITY: Only site admins may add external services if user mode is disabled.
namespaceUserID := int32(0)
isSiteAdmin := backend.CheckCurrentUserIsSiteAdmin(ctx) == nil
allowUserExternalServices := conf.ExternalServiceUserMode()
if !allowUserExternalServices {
// The user may have a tag that opts them in
err := backend.CheckActorHasTag(ctx, backend.TagAllowUserExternalServicePublic)
allowUserExternalServices = err == nil
}
allowUserExternalServices := currentUserAllowedExternalServices(ctx)
if args.Input.Namespace != nil {
if !allowUserExternalServices {
return nil, errors.New("allow users to add external services is not enabled")
Expand Down
2 changes: 1 addition & 1 deletion cmd/frontend/graphqlbackend/repo_groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (r *schemaResolver) RepoGroups(ctx context.Context) ([]*repoGroup, error) {
return nil, err
}

groupsByName, err := resolveRepoGroups(settings)
groupsByName, err := resolveRepoGroups(ctx, settings)
if err != nil {
return nil, err
}
Expand Down
27 changes: 22 additions & 5 deletions cmd/frontend/graphqlbackend/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ import (
"time"

"github.com/inconshreveable/log15"
otlog "github.com/opentracing/opentracing-go/log"

"github.com/neelance/parallel"
otlog "github.com/opentracing/opentracing-go/log"
"github.com/pkg/errors"

"github.com/sourcegraph/sourcegraph/cmd/frontend/envvar"
"github.com/sourcegraph/sourcegraph/cmd/frontend/types"
"github.com/sourcegraph/sourcegraph/internal/actor"
"github.com/sourcegraph/sourcegraph/internal/api"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/db"
Expand Down Expand Up @@ -379,7 +379,7 @@ func (r RepoRegexpPattern) String() string {

var mockResolveRepoGroups func() (map[string][]RepoGroupValue, error)

func resolveRepoGroups(settings *schema.Settings) (groups map[string][]RepoGroupValue, err error) {
func resolveRepoGroups(ctx context.Context, settings *schema.Settings) (groups map[string][]RepoGroupValue, err error) {
if mockResolveRepoGroups != nil {
return mockResolveRepoGroups()
}
Expand All @@ -396,14 +396,31 @@ func resolveRepoGroups(settings *schema.Settings) (groups map[string][]RepoGroup
if stringRegex, ok := path["regex"].(string); ok {
repos = append(repos, RepoRegexpPattern(stringRegex))
} else {
log15.Warn("ignoring repo group value because regex not specfied", "regex-string", path["regex"])
log15.Warn("ignoring repo group value because regex not specified", "regex-string", path["regex"])
}
default:
log15.Warn("ignoring repo group value of unrecognized type", "value", value, "type", fmt.Sprintf("%T", value))
}
}
groups[name] = repos
}

if !currentUserAllowedExternalServices(ctx) {
return groups, nil
}

a := actor.FromContext(ctx)
names, err := db.Repos.GetUserAddedRepoNames(ctx, a.UID)
if err != nil {
log15.Warn("getting user added repos", "err", err)
} else {
values := make([]RepoGroupValue, 0, len(names))
for _, name := range names {
values = append(values, RepoPath(name))
}
groups["my"] = values
}

return groups, nil
}

Expand Down Expand Up @@ -822,7 +839,7 @@ func resolveRepositories(ctx context.Context, op resolveRepoOp) (resolvedReposit
// groups and the set of repos specified with repo:. (If none are specified
// with repo:, then include all from the group.)
if groupNames := op.repoGroupFilters; len(groupNames) > 0 {
groups, err := resolveRepoGroups(op.userSettings)
groups, err := resolveRepoGroups(ctx, op.userSettings)
if err != nil {
return resolvedRepositories{}, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/frontend/graphqlbackend/search_filter_suggestions.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func (r *schemaResolver) SearchFilterSuggestions(ctx context.Context) (*searchFi
return nil, err
}

groupsByName, err := resolveRepoGroups(settings)
groupsByName, err := resolveRepoGroups(ctx, settings)
if err != nil {
return nil, err
}
Expand Down
59 changes: 58 additions & 1 deletion internal/db/repos.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,14 +253,18 @@ var getBySQLColumns = []string{
getSourcesByRepoQueryStr,
}

func minimalColumns(columns []string) []string {
return columns[:6]
}

func (s *RepoStore) getBySQL(ctx context.Context, querySuffix *sqlf.Query) ([]*types.Repo, error) {
return s.getReposBySQL(ctx, false, querySuffix)
}

func (s *RepoStore) getReposBySQL(ctx context.Context, minimal bool, querySuffix *sqlf.Query) ([]*types.Repo, error) {
columns := getBySQLColumns
if minimal {
columns = columns[:6]
columns = minimalColumns(columns)
}

q := sqlf.Sprintf(
Expand Down Expand Up @@ -957,6 +961,59 @@ func (*RepoStore) listSQL(opt ReposListOptions) (conds []*sqlf.Query, err error)
return conds, nil
}

// GetUserAddedRepoNames will fetch all repos added by the given user
func (s *RepoStore) GetUserAddedRepoNames(ctx context.Context, userID int32) ([]api.RepoName, error) {
s.ensureStore()

columns := minimalColumns(getBySQLColumns)
copied := make([]string, len(columns))
copy(copied, columns)
for i := range copied {
copied[i] = "repo." + copied[i]
}
fmtString := fmt.Sprintf(`
SELECT %s from repo
JOIN external_service_repos esr ON repo.id = esr.repo_id
WHERE esr.external_service_id IN (
SELECT id from external_services
WHERE namespace_user_id = %%s
AND deleted_at IS NULL
)
AND repo.deleted_at IS NULL
`, strings.Join(copied, ","))
q := sqlf.Sprintf(fmtString, userID)

rows, err := s.Query(ctx, q)

if err != nil {
return nil, errors.Wrap(err, "getting user repos")
}
defer rows.Close()

var repos []*types.Repo
for rows.Next() {
var repo types.Repo
if err := scanRepo(rows, &repo); err != nil {
return nil, err
}
repos = append(repos, &repo)
}
if err = rows.Err(); err != nil {
return nil, err
}

// 🚨 SECURITY: This enforces repository permissions
repos, err = authzFilter(ctx, repos, authz.Read)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq: Is it possible to utilize/modify s.getReposBySQL so we only have one place to call authzFilter which soon will be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered it but the new query need to join over another table and I think it would make s.getReposBySQL

When is authzFilter being removed? What is the new plan?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is authzFilter being removed? What is the new plan?

@ryanslade authzFilter will be removed after #13708 is merged.

I considered it but the new query need to join over another table and I think it would make s.getReposBySQL

I'll see what I can do by the time.

if err != nil {
return nil, errors.Wrap(err, "performing authz filter")
}
names := make([]api.RepoName, 0, len(repos))
for _, r := range repos {
names = append(names, r.Name)
}
return names, nil
}

// parseCursorConds checks whether the query is using cursor-based pagination, and
// if so performs the necessary transformations for it to be successful.
func parseCursorConds(opt ReposListOptions) (conds []*sqlf.Query, err error) {
Expand Down
93 changes: 93 additions & 0 deletions internal/db/repos_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"testing"
"time"

"github.com/google/go-cmp/cmp"

"github.com/sourcegraph/sourcegraph/cmd/frontend/types"
"github.com/sourcegraph/sourcegraph/internal/actor"
"github.com/sourcegraph/sourcegraph/internal/api"
Expand Down Expand Up @@ -356,6 +358,97 @@ func TestRepos_List(t *testing.T) {
}
}

func Test_GetUserAddedRepos(t *testing.T) {
if testing.Short() {
t.Skip()
}

MockAuthzFilter = func(ctx context.Context, repos []*types.Repo, p authz.Perms) ([]*types.Repo, error) {
return repos, nil
}
defer func() { MockAuthzFilter = nil }()

dbtesting.SetupGlobalTestDB(t)

ctx := context.Background()

// Create a user
user, err := Users.Create(ctx, NewUser{
Email: "a1@example.com",
Username: "u1",
Password: "p",
EmailVerificationCode: "c",
})
if err != nil {
t.Fatal(err)
}
ctx = actor.WithActor(ctx, &actor.Actor{
UID: user.ID,
})

now := time.Now()

// Create an external service
service := types.ExternalService{
Kind: extsvc.KindGitHub,
DisplayName: "Github - Test",
Config: `{"url": "https://github.com", "repositoryQuery": ["none"], "token": "abc"}`,
CreatedAt: now,
UpdatedAt: now,
NamespaceUserID: &user.ID,
}
confGet := func() *conf.Unified {
return &conf.Unified{}
}
err = ExternalServices.Create(ctx, confGet, &service)
if err != nil {
t.Fatal(err)
}

repo := &types.Repo{
ExternalRepo: api.ExternalRepoSpec{
ID: "r",
ServiceType: extsvc.TypeGitHub,
ServiceID: "https://github.com",
},
Name: "github.com/sourcegraph/sourcegraph",
Private: true,
RepoFields: &types.RepoFields{
URI: "uri",
Description: "description",
Fork: true,
Archived: true,
Cloned: true,
CreatedAt: now,
UpdatedAt: now,
Metadata: new(github.Repository),
Sources: map[string]*types.SourceInfo{
service.URN(): {
ID: service.URN(),
CloneURL: "git@github.com:foo/bar.git",
},
},
},
}
err = Repos.Create(ctx, repo)
if err != nil {
t.Fatal(err)
}

want := []api.RepoName{
repo.Name,
}

have, err := Repos.GetUserAddedRepoNames(ctx, user.ID)
if err != nil {
t.Fatal(err)
}

if diff := cmp.Diff(have, want); diff != "" {
t.Fatalf(diff)
}
}

func TestRepos_List_fork(t *testing.T) {
if testing.Short() {
t.Skip()
Expand Down