Anthony/bitbucket permission fix 2 (#20)

* Added the check to the validate function

Author: notanthony

pkg/bitbucket/client.go

@@ -5,8 +5,11 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"strings"
 
 	"github.com/conductorone/baton-sdk/pkg/uhttp"
+	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
+	"go.uber.org/zap"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -41,8 +44,9 @@ const (
 )
 
 type Client struct {
-	wrapper *uhttp.BaseHttpClient
-	scope   Scope
+	wrapper      *uhttp.BaseHttpClient
+	scope        Scope
+	workspaceIDs map[string]bool
 }
 
 func NewClient(httpClient *http.Client) *Client {
@@ -108,29 +112,112 @@ func (c *Client) WorkspaceId() (string, error) {
 	}
 }
 
-// If client have access to multiple workspaces, method `WorkspaceIds`
-// returns list of workspace ids otherwise it returns error.
-func (c *Client) WorkspaceIDs(ctx context.Context) ([]string, error) {
-	workspaceIDs := make([]string, 0)
+func isPermissionDeniedErr(err error) bool {
+	e, ok := status.FromError(err)
+	if ok && e.Code() == codes.PermissionDenied {
+		return true
+	}
+	// In most cases the error code is unknown and the error message contains "status 403".
+	if (!ok || e.Code() == codes.Unknown) && strings.Contains(err.Error(), "status 403") {
+		return true
+	}
+	return false
+}
 
-	if c.IsUserScoped() {
-		workspaces, err := c.GetAllWorkspaces(ctx)
-		if err != nil {
-			return nil, err
+func (c *Client) checkPermissions(ctx context.Context, workspace *Workspace) (bool, error) {
+	l := ctxzap.Extract(ctx)
+	logMissingPermission := func(obj string, err error) {
+		l.Error(
+			"missing permission to list object in workspace",
+			zap.String("workspace", workspace.Slug),
+			zap.String("workspace id", workspace.Id),
+			zap.String("object", obj),
+			zap.Error(err),
+		)
+	}
+	paginationVars := PaginationVars{
+		Limit: 1,
+		Page:  "",
+	}
+	_, err := c.GetWorkspaceUserGroups(ctx, workspace.Id)
+	if err != nil {
+		if isPermissionDeniedErr(err) {
+			logMissingPermission("userGroups", err)
+			return false, nil
 		}
-
-		for _, workspace := range workspaces {
-			workspaceIDs = append(workspaceIDs, workspace.Id)
+		return false, err
+	}
+	_, _, err = c.GetWorkspaceMembers(ctx, workspace.Id, paginationVars)
+	if err != nil {
+		if isPermissionDeniedErr(err) {
+			logMissingPermission("users", err)
+			return false, nil
 		}
+		return false, err
+	}
+	_, _, err = c.GetWorkspaceProjects(ctx, workspace.Id, paginationVars)
+	if err != nil {
+		if isPermissionDeniedErr(err) {
+			logMissingPermission("projects", err)
+			return false, nil
+		}
+		return false, err
+	}
+	return true, nil
+}
+
+func (c *Client) filterWorkspaces(ctx context.Context, workspaces []Workspace) ([]Workspace, error) {
+	filteredWorkspaces := make([]Workspace, 0)
 
-		if len(workspaceIDs) == 0 {
-			return nil, status.Error(codes.NotFound, "no workspaces found")
+	for _, workspace := range workspaces {
+		// We call this function in order to initialize the workspaceID's map. In that case we need to return all workspaces,
+		// so they can be filtered and only the valid ones are set in the workspaceIds map.
+		_, ok := c.workspaceIDs[workspace.Id]
+		if c.workspaceIDs != nil && !ok {
+			continue
 		}
-	} else {
-		return nil, status.Error(codes.InvalidArgument, "client is not user scoped")
+
+		filteredWorkspaces = append(filteredWorkspaces, workspace)
+	}
+
+	return filteredWorkspaces, nil
+}
+
+// If client have access to multiple workspaces, method `WorkspaceIDs`
+// returns list of workspace ids otherwise it returns error.
+func (c *Client) SetWorkspaceIDs(ctx context.Context, workspaceIDs []string) error {
+	if !c.IsUserScoped() {
+		return status.Error(codes.InvalidArgument, "client is not user scoped")
+	}
+	c.workspaceIDs = make(map[string]bool)
+	givenWorkspaceIDs := make(map[string]bool)
+	for _, workspaceId := range workspaceIDs {
+		givenWorkspaceIDs[workspaceId] = true
 	}
 
-	return workspaceIDs, nil
+	workspaces, err := c.GetAllWorkspaces(ctx)
+	if err != nil {
+		return err
+	}
+
+	for _, workspace := range workspaces {
+		workspace := workspace
+		if _, ok := givenWorkspaceIDs[workspace.Id]; !ok && len(givenWorkspaceIDs) > 0 {
+			continue
+		}
+		ok, err := c.checkPermissions(ctx, &workspace)
+		if err != nil {
+			return err
+		}
+		if !ok {
+			continue
+		}
+		c.workspaceIDs[workspace.Id] = true
+	}
+	if len(c.workspaceIDs) == 0 {
+		return status.Error(codes.Unauthenticated, "no authenticated workspaces found")
+	}
+	return nil
 }
 
 // GetWorkspaces lists all workspaces current user belongs to.
@@ -150,7 +237,10 @@ func (c *Client) GetWorkspaces(ctx context.Context, getWorkspacesVars Pagination
 			prepareFilters(""),
 		},
 	)
-
+	if err != nil {
+		return nil, "", err
+	}
+	workspacesResponse.Values, err = c.filterWorkspaces(ctx, workspacesResponse.Values)
 	if err != nil {
 		return nil, "", err
 	}
@@ -202,8 +292,10 @@ func (c *Client) GetWorkspace(ctx context.Context, workspaceId string) (*Workspa
 			prepareFilters(""),
 		},
 	)
-
 	if err != nil {
+		if isPermissionDeniedErr(err) {
+			return nil, status.Error(codes.PermissionDenied, "missing permission to get workspace")
+		}
 		return nil, err
 	}
 
@@ -228,7 +320,6 @@ func (c *Client) GetWorkspaceMembers(ctx context.Context, workspaceId string, ge
 			prepareFilters("", "-*.workspace"),
 		},
 	)
-
 	if err != nil {
 		return nil, "", err
 	}

pkg/connector/connector.go

@@ -8,10 +8,7 @@ import (
 	v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2"
 	"github.com/conductorone/baton-sdk/pkg/annotations"
 	"github.com/conductorone/baton-sdk/pkg/connectorbuilder"
-	"github.com/conductorone/baton-sdk/pkg/pagination"
 	"github.com/conductorone/baton-sdk/pkg/uhttp"
-	"github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap"
-	"go.uber.org/zap"
 )
 
 var (
@@ -68,78 +65,6 @@ func (bb *Bitbucket) Metadata(ctx context.Context) (*v2.ConnectorMetadata, error
 	}, nil
 }
 
-// Get all the valid workspaces, ie. the workspaces that the user has access to.
-func (bb *Bitbucket) getValidWorkspaces(ctx context.Context) ([]string, error) {
-	workspaceObject := workspaceBuilder(bb.client, bb.workspaces)
-	pageToken := ""
-	workspaceResources := make([]*v2.Resource, 0)
-	for {
-		temp, pageToken, _, err := workspaceObject.List(ctx, nil, &pagination.Token{Token: pageToken})
-		if err != nil {
-			return nil, err
-		}
-		workspaceResources = append(workspaceResources, temp...)
-		if pageToken == "" {
-			break
-		}
-	}
-
-	validWorkspaces := make([]string, 0)
-	for _, workspace := range workspaceResources {
-		ok, err := bb.checkPermissions(ctx, workspace)
-		if err != nil {
-			return nil, fmt.Errorf("bitbucket-connector: failed to verify permissions: %w", err)
-		}
-		if !ok {
-			continue
-		}
-		validWorkspaces = append(validWorkspaces, workspace.DisplayName)
-	}
-	return validWorkspaces, nil
-}
-
-func (bb *Bitbucket) checkPermissions(ctx context.Context, workspace *v2.Resource) (bool, error) {
-	l := ctxzap.Extract(ctx)
-	logMissingPermission := func(obj string, err error) {
-		l.Error(
-			"missing permission to list object in workspace",
-			zap.String("workspace", workspace.DisplayName),
-			zap.String("workspace id", workspace.Id.Resource),
-			zap.String("object", obj),
-			zap.Error(err),
-		)
-	}
-	paginationVars := bitbucket.PaginationVars{
-		Limit: 1,
-		Page:  "",
-	}
-	_, err := bb.client.GetWorkspaceUserGroups(ctx, workspace.Id.Resource)
-	if err != nil {
-		if isPermissionDeniedErr(err) {
-			logMissingPermission("userGroups", err)
-			return false, nil
-		}
-		return false, err
-	}
-	_, _, err = bb.client.GetWorkspaceMembers(ctx, workspace.Id.Resource, paginationVars)
-	if err != nil {
-		if isPermissionDeniedErr(err) {
-			logMissingPermission("users", err)
-			return false, nil
-		}
-		return false, err
-	}
-	_, _, err = bb.client.GetWorkspaceProjects(ctx, workspace.Id.Resource, paginationVars)
-	if err != nil {
-		if isPermissionDeniedErr(err) {
-			logMissingPermission("projects", err)
-			return false, nil
-		}
-		return false, err
-	}
-	return true, nil
-}
-
 // Validate hits the Bitbucket API to validate that the configured credentials are valid and compatible.
 func (bb *Bitbucket) Validate(ctx context.Context) (annotations.Annotations, error) {
 	// get the scope of used credentials
@@ -151,12 +76,12 @@ func (bb *Bitbucket) Validate(ctx context.Context) (annotations.Annotations, err
 	if err != nil {
 		return nil, err
 	}
-	bb.workspaces, err = bb.getValidWorkspaces(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("bitbucket-connector: failed to get valid workspaces: %w", err)
-	}
-	if len(bb.workspaces) == 0 {
-		return nil, fmt.Errorf("bitbucket-connector: no authorized workspaces found")
+
+	if bb.client.IsUserScoped() {
+		err = bb.client.SetWorkspaceIDs(ctx, bb.workspaces)
+		if err != nil {
+			return nil, fmt.Errorf("bitbucket-connector: failed to get workspace ids: %w", err)
+		}
 	}
 	return nil, nil
 }

pkg/connector/helpers.go

@@ -9,8 +9,6 @@ import (
 	"github.com/conductorone/baton-sdk/pkg/pagination"
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 )
 
 var ResourcesPageSize = 50
@@ -93,14 +91,3 @@ func ParseEntitlementID(id string) (*v2.ResourceId, string, error) {
 	}
 	return resourceId, parts[len(parts)-1], nil
 }
-func isPermissionDeniedErr(err error) bool {
-	e, ok := status.FromError(err)
-	if ok && e.Code() == codes.PermissionDenied {
-		return true
-	}
-	// In most cases the error code is unknown and the error message contains "status 403".
-	if (!ok || e.Code() == codes.Unknown) && strings.Contains(err.Error(), "status 403") {
-		return true
-	}
-	return false
-}