wrap errors when errors with 200 status code (#67)

* add grpc code mapping when 200 code and "ok": false in the body

* refactor request some more

* include slack-go library errors

* wrap slack-go errors

* remove old scim functions

* remove repeated code

* add better error detection

* make more readable

Author: aldevv

pkg/connector/client/helpers.go

@@ -2,10 +2,15 @@ package client
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
 	"net/http"
+	"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"
 )
 
 func logBody(ctx context.Context, response *http.Response) {
@@ -29,3 +34,103 @@ func logBody(ctx context.Context, response *http.Response) {
 	}
 	l.Info("response body", zap.String("body", string(body)))
 }
+
+// Slack API may return errors in the response body even when the HTTP status code is 200.
+//
+//	extracts a Slack error from a Go error and wraps it with the appropriate gRPC code.
+//	This is useful when working with the slack-go library which returns plain Go errors.
+func WrapError(err error, contextMsg string) error {
+	if err == nil {
+		return nil
+	}
+
+	grpcCode := MapSlackErrorToGRPCCode(err.Error())
+	return uhttp.WrapErrors(grpcCode, contextMsg, err)
+}
+
+func containsAny(s string, substrs ...string) bool {
+	for _, substr := range substrs {
+		if strings.Contains(s, substr) {
+			return true
+		}
+	}
+	return false
+}
+
+// maps Slack error strings to gRPC codes.
+func MapSlackErrorToGRPCCode(slackError string) codes.Code {
+	err := strings.ToLower(slackError)
+
+	if containsAny(err, "invalid_auth", "token_revoked", "token_expired", "not_authed", "account_inactive") {
+		return codes.Unauthenticated
+	}
+
+	if containsAny(err, "missing_scope", "access_denied", "not_allowed_token_type",
+		"team_access_not_granted", "no_permission", "ekm_access_denied") {
+		return codes.PermissionDenied
+	}
+
+	if containsAny(err, "ratelimited") {
+		return codes.Unavailable
+	}
+
+	if containsAny(err, "user_not_found") {
+		return codes.NotFound
+	}
+
+	if containsAny(err, "user_already_team_member") {
+		return codes.AlreadyExists
+	}
+
+	if containsAny(err, "invalid_arguments", "missing_argument", "invalid_arg_name",
+		"invalid_array_arg", "invalid_charset", "invalid_form_data", "invalid_post_type",
+		"missing_post_type", "limit_required") {
+		return codes.InvalidArgument
+	}
+
+	if containsAny(err, "user_already_deleted", "two_factor_setup_required") {
+		return codes.FailedPrecondition
+	}
+
+	if containsAny(err, "internal_error", "service_unavailable", "request_timeout") {
+		return codes.Unavailable
+	}
+
+	if containsAny(err, "fatal_error") {
+		return codes.Internal
+	}
+
+	if containsAny(err, "method_deprecated", "deprecated_endpoint") {
+		return codes.Unimplemented
+	}
+
+	return codes.Unknown
+}
+
+// Slack API may return errors in the response body even when the HTTP status code is 200.
+//
+//	examples:
+//
+//	{"ok":false,"error":"invalid_auth"}
+//	{"ok":false,"error": "user_not_found"}
+func ErrorWithGrpcCodeFromBytes(bodyBytes []byte) error {
+	var baseCheck struct {
+		Ok    bool   `json:"ok"`
+		Error string `json:"error"`
+	}
+
+	if err := json.Unmarshal(bodyBytes, &baseCheck); err != nil {
+		return fmt.Errorf("error parsing Slack API response: %w", err)
+	}
+
+	if !baseCheck.Ok && baseCheck.Error != "" {
+		grpcCode := MapSlackErrorToGRPCCode(baseCheck.Error)
+		return uhttp.WrapErrors(
+			grpcCode,
+			fmt.Sprintf("Slack API error: %s", baseCheck.Error),
+			fmt.Errorf("slack error: %s", baseCheck.Error),
+		)
+	}
+
+	return nil
+}

pkg/connector/client/request.go

@@ -83,43 +83,6 @@ func (c *Client) post(
 	)
 }
 
-func (c *Client) getScim(
-	ctx context.Context,
-	path string,
-	target interface{},
-	queryParameters map[string]interface{},
-) (
-	*v2.RateLimitDescription,
-	error,
-) {
-	return c.doRequest(
-		ctx,
-		http.MethodGet,
-		c.getUrl(path, queryParameters, true),
-		&target,
-		WithBearerToken(c.token),
-	)
-}
-
-func (c *Client) patchScim(
-	ctx context.Context,
-	path string,
-	target interface{},
-	payload map[string]any,
-) (
-	*v2.RateLimitDescription,
-	error,
-) {
-	return c.doRequest(
-		ctx,
-		http.MethodPatch,
-		c.getUrl(path, nil, true),
-		&target,
-		WithBearerToken(c.token),
-		uhttp.WithJSONBody(payload),
-	)
-}
-
 func (c *Client) doRequest(
 	ctx context.Context,
 	method string,
@@ -168,96 +131,18 @@ func (c *Client) doRequest(
 		return &ratelimitData, fmt.Errorf("reading response body: %w", err)
 	}
 
-	if response.StatusCode != http.StatusNoContent && len(bodyBytes) > 0 {
-		if err := json.Unmarshal(bodyBytes, &target); err != nil {
-			logBody(ctx, response)
-			return nil, fmt.Errorf("unmarshaling response: %w", err)
-		}
-	}
-
-	return &ratelimitData, nil
-}
-
-func (c *Client) doScimRequest(
-	ctx context.Context,
-	method string,
-	path string,
-	target interface{},
-	payload interface{},
-) (
-	*v2.RateLimitDescription,
-	error,
-) {
-	options := []uhttp.RequestOption{
-		WithBearerToken(c.token),
-	}
-
-	if payload != nil {
-		options = append(options, uhttp.WithJSONBody(payload))
-	}
-
-	return c.doRequest(
-		ctx,
-		method,
-		c.getUrl(path, nil, true),
-		target,
-		options...,
-	)
-}
-
-func (c *Client) deleteScim(
-	ctx context.Context,
-	path string,
-) (
-	*v2.RateLimitDescription,
-	error,
-) {
-	logger := ctxzap.Extract(ctx)
-	logger.Debug(
-		"making request",
-		zap.String("method", http.MethodDelete),
-		zap.String("url", c.getUrl(path, nil, true).String()),
-	)
-
-	request, err := c.wrapper.NewRequest(
-		ctx,
-		http.MethodDelete,
-		c.getUrl(path, nil, true),
-		WithBearerToken(c.token),
-		uhttp.WithAcceptJSONHeader(),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("creating SCIM delete request: %w", err)
-	}
-
-	var ratelimitData v2.RateLimitDescription
-	response, err := c.wrapper.Do(
-		request,
-		uhttp.WithRatelimitData(&ratelimitData),
-	)
-	if err != nil {
-		logBody(ctx, response)
-		return &ratelimitData, err
-	}
-	defer response.Body.Close()
-
-	if response.StatusCode == http.StatusNoContent {
+	if response.StatusCode == http.StatusNoContent || len(bodyBytes) == 0 {
 		return &ratelimitData, nil
 	}
 
-	bodyBytes, err := io.ReadAll(response.Body)
-	if err != nil {
+	if err := json.Unmarshal(bodyBytes, &target); err != nil {
 		logBody(ctx, response)
-		return &ratelimitData, fmt.Errorf("reading SCIM error response body: %w", err)
+		return nil, fmt.Errorf("unmarshaling response: %w", err)
 	}
 
-	if len(bodyBytes) > 0 {
-		var errorResponse map[string]interface{}
-		if err := json.Unmarshal(bodyBytes, &errorResponse); err != nil {
-			return &ratelimitData, fmt.Errorf("parsing SCIM error response: %w", err)
-		}
-		if detail, ok := errorResponse["detail"].(string); ok {
-			return &ratelimitData, fmt.Errorf("SCIM API error: %s", detail)
+	if response.StatusCode == http.StatusOK {
+		if err := ErrorWithGrpcCodeFromBytes(bodyBytes); err != nil {
+			return &ratelimitData, err
 		}
 	}
 

pkg/connector/client/slack.go

@@ -258,14 +258,15 @@ func (c *Client) ListIDPGroups(
 ) {
 	var response SCIMResponse[GroupResource]
 	urlPathIDPGroups := fmt.Sprintf(UrlPathIDPGroups, c.scimVersion)
-	ratelimitData, err := c.getScim(
+	ratelimitData, err := c.doRequest(
 		ctx,
-		urlPathIDPGroups,
-		&response,
-		map[string]interface{}{
+		http.MethodGet,
+		c.getUrl(urlPathIDPGroups, map[string]interface{}{
 			"startIndex": startIndex,
 			"count":      count,
-		},
+		}, true),
+		&response,
+		WithBearerToken(c.token),
 	)
 	if err != nil {
 		return nil, ratelimitData, fmt.Errorf("error fetching IDP groups: %w", err)
@@ -286,14 +287,15 @@ func (c *Client) ListIDPUsers(
 ) {
 	var response SCIMResponse[UserResource]
 	urlPathIDPUsers := fmt.Sprintf(UrlPathIDPUsers, c.scimVersion)
-	ratelimitData, err := c.getScim(
+	ratelimitData, err := c.doRequest(
 		ctx,
-		urlPathIDPUsers,
-		&response,
-		map[string]interface{}{
+		http.MethodGet,
+		c.getUrl(urlPathIDPUsers, map[string]interface{}{
 			"startIndex": startIndex,
 			"count":      count,
-		},
+		}, true),
+		&response,
+		WithBearerToken(c.token),
 	)
 	if err != nil {
 		return nil, ratelimitData, fmt.Errorf("error fetching IDP users: %w", err)
@@ -312,11 +314,12 @@ func (c *Client) GetIDPGroup(
 	error,
 ) {
 	var response GroupResource
-	ratelimitData, err := c.getScim(
+	ratelimitData, err := c.doRequest(
 		ctx,
-		fmt.Sprintf(UrlPathIDPGroup, c.scimVersion, groupID),
+		http.MethodGet,
+		c.getUrl(fmt.Sprintf(UrlPathIDPGroup, c.scimVersion, groupID), nil, true),
 		&response,
-		nil,
+		WithBearerToken(c.token),
 	)
 	if err != nil {
 		return nil, ratelimitData, fmt.Errorf("error fetching IDP group: %w", err)
@@ -429,12 +432,13 @@ func (c *Client) patchGroup(
 	error,
 ) {
 	var response *GroupResource
-	ratelimitData, err := c.doScimRequest(
+	ratelimitData, err := c.doRequest(
 		ctx,
 		http.MethodPatch,
-		fmt.Sprintf(UrlPathIDPGroup, c.scimVersion, groupID),
+		c.getUrl(fmt.Sprintf(UrlPathIDPGroup, c.scimVersion, groupID), nil, true),
 		&response,
-		requestBody,
+		WithBearerToken(c.token),
+		uhttp.WithJSONBody(requestBody),
 	)
 	if err != nil {
 		return ratelimitData, fmt.Errorf("error patching IDP group: %w", err)
@@ -458,9 +462,13 @@ func (c *Client) DisableUser(
 	*v2.RateLimitDescription,
 	error,
 ) {
-	ratelimitData, err := c.deleteScim(
+	var response any
+	ratelimitData, err := c.doRequest(
 		ctx,
-		fmt.Sprintf(UrlPathIDPUser, c.scimVersion, userID),
+		http.MethodDelete,
+		c.getUrl(fmt.Sprintf(UrlPathIDPUser, c.scimVersion, userID), nil, true),
+		&response,
+		WithBearerToken(c.token),
 	)
 	if err != nil {
 		return ratelimitData, fmt.Errorf("error disabling user: %w", err)
@@ -488,12 +496,14 @@ func (c *Client) EnableUser(
 		},
 	}
 
-	var response *UserResource
-	ratelimitData, err := c.patchScim(
+	var response any
+	ratelimitData, err := c.doRequest(
 		ctx,
-		fmt.Sprintf(UrlPathIDPUser, c.scimVersion, userID),
+		http.MethodPatch,
+		c.getUrl(fmt.Sprintf(UrlPathIDPUser, c.scimVersion, userID), nil, true),
 		&response,
-		requestBody,
+		WithBearerToken(c.token),
+		uhttp.WithJSONBody(requestBody),
 	)
 	if err != nil {
 		return ratelimitData, fmt.Errorf("error enabling user: %w", err)

pkg/connector/connector.go

@@ -72,12 +72,12 @@ func (c *Slack) Metadata(ctx context.Context) (*v2.ConnectorMetadata, error) {
 func (s *Slack) Validate(ctx context.Context) (annotations.Annotations, error) {
 	res, err := s.client.AuthTestContext(ctx)
 	if err != nil {
-		return nil, fmt.Errorf("authenticating: %w", err)
+		return nil, client.WrapError(err, "authenticating")
 	}
 
 	user, err := s.client.GetUserInfoContext(ctx, res.UserID)
 	if err != nil {
-		return nil, fmt.Errorf("retrieving authenticated user: %w", err)
+		return nil, client.WrapError(err, "retrieving authenticated user")
 	}
 
 	isValidUser := user.IsAdmin || user.IsOwner || user.IsPrimaryOwner || user.IsBot