diff --git a/regulation-worker/internal/delete/api/api.go b/regulation-worker/internal/delete/api/api.go index c3a92544ef..efa331172c 100644 --- a/regulation-worker/internal/delete/api/api.go +++ b/regulation-worker/internal/delete/api/api.go @@ -104,7 +104,7 @@ func (m *APIManager) deleteWithRetry(ctx context.Context, job model.Job, destina Config: destination.Config, DefinitionConfig: destination.DestDefConfig, } - isOAuth, err := dest.IsOAuthDestination() + isOAuth, err := dest.IsOAuthDestination(common.RudderFlowDelete) if err != nil { pkgLogger.Error(err) return model.JobStatus{Status: model.JobStatusFailed, Error: err} diff --git a/regulation-worker/internal/delete/api/api_test.go b/regulation-worker/internal/delete/api/api_test.go index 19ee4812e2..66d8fa7126 100644 --- a/regulation-worker/internal/delete/api/api_test.go +++ b/regulation-worker/internal/delete/api/api_test.go @@ -271,6 +271,13 @@ type oauthTestCases struct { isOAuthV2Enabled bool } +var defaultDestDefConfig = map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + "rudderScopes": []interface{}{"delete"}, + }, +} + var oauthTests = []oauthTestCases{ { name: "test with a valid token and successful response", @@ -307,12 +314,8 @@ var oauthTests = []oauthTestCases{ Config: map[string]interface{}{ "rudderDeleteAccountId": "xyz", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, deleteResponses: []deleteResponseParams{ { @@ -359,12 +362,8 @@ var oauthTests = []oauthTestCases{ Config: map[string]interface{}{ "rudderDeleteAccountId": "xyz", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, deleteResponses: []deleteResponseParams{ { @@ -419,12 +418,8 @@ var oauthTests = []oauthTestCases{ Config: map[string]interface{}{ "rudderDeleteAccountId": "xyz", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, cpResponses: []testutils.CpResponseParams{ { @@ -466,12 +461,8 @@ var oauthTests = []oauthTestCases{ Config: map[string]interface{}{ "rudderDeleteAccountId": "xyz", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, cpResponses: []testutils.CpResponseParams{ { @@ -522,12 +513,8 @@ var oauthTests = []oauthTestCases{ Config: map[string]interface{}{ "rudderDeleteAccountId": "", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, cpResponses: []testutils.CpResponseParams{}, deleteResponses: []deleteResponseParams{{}}, @@ -570,11 +557,7 @@ var oauthTests = []oauthTestCases{ DestinationID: "1234", Config: map[string]interface{}{}, Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + DestDefConfig: defaultDestDefConfig, }, cpResponses: []testutils.CpResponseParams{}, deleteResponses: []deleteResponseParams{{}}, @@ -611,12 +594,8 @@ var oauthTests = []oauthTestCases{ Config: map[string]interface{}{ "rudderDeleteAccountId": "xyz", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, oauthHttpClientTimeout: 1 * time.Second, @@ -664,12 +643,8 @@ var oauthTests = []oauthTestCases{ "rudderDeleteAccountId": "xyz", "authStatus": "active", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, deleteResponses: []deleteResponseParams{ { @@ -715,12 +690,8 @@ var oauthTests = []oauthTestCases{ "rudderDeleteAccountId": "xyz", "authStatus": "active", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, deleteResponses: []deleteResponseParams{ { @@ -767,12 +738,8 @@ var oauthTests = []oauthTestCases{ "rudderDeleteAccountId": "xyz", "authStatus": "active", }, - Name: "GA", - DestDefConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, + Name: "GA", + DestDefConfig: defaultDestDefConfig, }, deleteResponses: []deleteResponseParams{ { diff --git a/router/transformer/transformer_test.go b/router/transformer/transformer_test.go index a418c5c18c..69c32661fb 100644 --- a/router/transformer/transformer_test.go +++ b/router/transformer/transformer_test.go @@ -500,7 +500,8 @@ var oauthDests = []backendconfig.DestinationT{ Name: "SALESFORCE_OAUTH", Config: map[string]interface{}{ "auth": map[string]interface{}{ - "type": "OAuth", + "type": "OAuth", + "rudderScopes": []interface{}{"delivery"}, }, }, }, diff --git a/services/oauth/v2/destination_info.go b/services/oauth/v2/destination_info.go index deb1bf0a1b..7e7d4b5b31 100644 --- a/services/oauth/v2/destination_info.go +++ b/services/oauth/v2/destination_info.go @@ -3,7 +3,10 @@ package v2 import ( "fmt" + "github.com/samber/lo" + "github.com/rudderlabs/rudder-server/services/oauth" + "github.com/rudderlabs/rudder-server/services/oauth/v2/common" "github.com/rudderlabs/rudder-server/utils/misc" ) @@ -15,7 +18,7 @@ type DestinationInfo struct { Config map[string]interface{} } -func (d *DestinationInfo) IsOAuthDestination() (bool, error) { +func (d *DestinationInfo) IsOAuthDestination(flow common.RudderFlow) (bool, error) { authValue, _ := misc.NestedMapLookup(d.DefinitionConfig, "auth", "type") if authValue == nil { // valid use-case for non-OAuth destinations @@ -26,22 +29,53 @@ func (d *DestinationInfo) IsOAuthDestination() (bool, error) { // we should throw error here, as we expect authValue to be a string if present return false, fmt.Errorf("auth type is not a string: %v", authValue) } - return authType == string(oauth.OAuth), nil + isScopeSupported, err := d.IsOAuthSupportedForFlow(string(flow)) + if err != nil { + return false, err + } + return authType == string(oauth.OAuth) && isScopeSupported, nil +} + +func (d *DestinationInfo) IsOAuthSupportedForFlow(flow string) (bool, error) { + rudderScopesValue, _ := misc.NestedMapLookup(d.DefinitionConfig, "auth", "rudderScopes") + if rudderScopesValue == nil { + // valid use-case for non-OAuth destinations + // when the auth.type is OAuth and rudderScopes is not mentioned, we would assume oauth flow is to be used when it is in "delivery" flow + return flow == string(common.RudderFlowDelivery), nil + } + interfaceArr, ok := rudderScopesValue.([]interface{}) + if !ok { + return false, fmt.Errorf("rudderScopes should be a interface[]") + } + var rudderScopes []string + for _, scopeInterface := range interfaceArr { + scope, ok := scopeInterface.(string) + if !ok { + return false, fmt.Errorf("%v in auth.rudderScopes should be string", scopeInterface) + } + rudderScopes = append(rudderScopes, scope) + } + return lo.Contains(rudderScopes, flow), nil } /* GetAccountID Gets AccountId for OAuth destination based on if rudderFlow is `Delivery` or `Delete` Example: -`GetAccountID(destDetail.Config, "rudderDeleteAccountId")` --> To be used when we make use of OAuth during regulation flow -`GetAccountID(destDetail.Config, "rudderAccountId")` --> To be used when we make use of OAuth during normal event delivery +`GetAccountID(common.RudderFlowDelete)` --> To be used when we make use of OAuth during regulation flow +`GetAccountID(common.RudderFlowDelivery)` --> To be used when we make use of OAuth during normal event delivery */ -func (d *DestinationInfo) GetAccountID(idKey string) (string, error) { - rudderAccountIdInterface, found := d.Config[idKey] - oauthDest, err := d.IsOAuthDestination() +func (d *DestinationInfo) GetAccountID(flow common.RudderFlow) (string, error) { + oauthDest, err := d.IsOAuthDestination(flow) if err != nil { return "", fmt.Errorf("failed to check if destination is oauth destination: %v", err) } + + idKey := common.DeliveryAccountIDKey + if flow == common.RudderFlowDelete { + idKey = common.DeleteAccountIDKey + } + rudderAccountIdInterface, found := d.Config[idKey] if !oauthDest || !found || idKey == "" { return "", fmt.Errorf("destination is not an oauth destination or accountId not found") } diff --git a/services/oauth/v2/destination_info_test.go b/services/oauth/v2/destination_info_test.go new file mode 100644 index 0000000000..7c8e3f4664 --- /dev/null +++ b/services/oauth/v2/destination_info_test.go @@ -0,0 +1,133 @@ +package v2_test + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + v2 "github.com/rudderlabs/rudder-server/services/oauth/v2" + "github.com/rudderlabs/rudder-server/services/oauth/v2/common" +) + +type isOAuthResult struct { + isOAuth bool + err error +} + +type destInfoTestCase struct { + description string + flow common.RudderFlow + inputDefConfig map[string]interface{} + expected isOAuthResult +} + +var isOAuthDestTestCases = []destInfoTestCase{ + { + description: "should pass for a destination which contains OAuth and rudderScopes", + flow: common.RudderFlowDelivery, + inputDefConfig: map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + "rudderScopes": []interface{}{"delivery"}, + }, + }, + expected: isOAuthResult{ + isOAuth: true, + }, + }, + { + description: "should pass for a destination which contains OAuth but not rudderScopes", + flow: common.RudderFlowDelivery, + inputDefConfig: map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + }, + }, + expected: isOAuthResult{ + isOAuth: true, + }, + }, + { + description: "should return 'false' without error for a destination which contains OAuth with delete rudderScopes when flow is delivery", + flow: common.RudderFlowDelivery, + inputDefConfig: map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + "rudderScopes": []interface{}{"delete"}, + }, + }, + expected: isOAuthResult{ + isOAuth: false, + }, + }, + { + description: "should return 'true' without error for a destination which contains OAuth withoutrudderScopes when flow is delivery", + flow: common.RudderFlowDelivery, + inputDefConfig: map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + }, + }, + expected: isOAuthResult{ + isOAuth: true, + }, + }, + { + description: "should return 'false' with error for a destination which contains OAuth with one of invalid rudderScopes when flow is delivery", + flow: common.RudderFlowDelivery, + inputDefConfig: map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + "rudderScopes": []interface{}{"delivery", 1}, + }, + }, + expected: isOAuthResult{ + isOAuth: false, + err: fmt.Errorf("1 in auth.rudderScopes should be string"), + }, + }, + { + description: "should return 'false' with error for a destination which contains OAuth with invalid rudderScopes type when flow is delivery", + flow: common.RudderFlowDelivery, + inputDefConfig: map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + "rudderScopes": []interface{}{"a"}[0], + }, + }, + expected: isOAuthResult{ + isOAuth: false, + err: fmt.Errorf("rudderScopes should be a interface[]"), + }, + }, + { + description: "should return 'false' without error for a non-OAuth destination when flow is delivery", + flow: common.RudderFlowDelivery, + inputDefConfig: map[string]interface{}{}, + expected: isOAuthResult{ + isOAuth: false, + }, + }, +} + +var _ = Describe("DestinationInfo tests", func() { + Describe("IsOAuthDestination tests", func() { + for _, tc := range isOAuthDestTestCases { + It(tc.description, func() { + d := &v2.DestinationInfo{ + DefinitionName: "dest_def_name", + } + d.DefinitionConfig = tc.inputDefConfig + isOAuth, err := d.IsOAuthDestination(tc.flow) + + Expect(isOAuth).To(Equal(tc.expected.isOAuth)) + if tc.expected.err != nil { + Expect(err).To(Equal(tc.expected.err)) + } else { + Expect(err).To(BeNil()) + } + }) + } + }) +}) diff --git a/services/oauth/v2/http/client_test.go b/services/oauth/v2/http/client_test.go index 249661ed94..50de9921f7 100644 --- a/services/oauth/v2/http/client_test.go +++ b/services/oauth/v2/http/client_test.go @@ -23,6 +23,13 @@ import ( httpClient "github.com/rudderlabs/rudder-server/services/oauth/v2/http" ) +var oauthDefinitionConfig = map[string]interface{}{ + "auth": map[string]interface{}{ + "type": "OAuth", + "rudderScopes": []interface{}{"delivery"}, + }, +} + var _ = Describe("Http/Client", func() { Describe("OAuthHttpClient", func() { It("should return an http client", func() { @@ -102,13 +109,9 @@ var _ = Describe("Http/Client", func() { req, _ := http.NewRequest("POST", "url", bytes.NewBuffer([]byte(`{"input":[{"message":{"userId":"user 1","event":"event1","type":"audiencelist","properties":{"listData":{"add":[{"email":"test@abc.com","phone":"@09876543210","firstName":"test","lastName":"rudderlabs","country":"US","postalCode":"1245"}]},"enablePartialFailure":true},"context":{"ip":"14.5.67.21","library":{"name":"http"}},"timestamp":"2020-02-02T00:23:09.544Z"},"metadata":{"secret":{"access_token":"dummy-access","refresh_token":"dummy-refresh","developer_token":"dummy-dev-token"}},"destination":{"secretConfig":{},"config":{},"name":"GARL","destinationDefinition":{"config":{"auth":{"role":"google_adwords_remarketing_lists_v1","type":"OAuth","provider":"Google","rudderScopes":["delivery"]}},"responseRules":{},"name":"GOOGLE_ADWORDS_REMARKETING_LISTS","displayName":"Google Ads Remarketing Lists (Customer Match)","category":null},"permissions":{"isLocked":false}}}],"destType":"google_adwords_remarketing_lists"}`))) destination := &v2.DestinationInfo{ - DefinitionName: "GOOGLE_ADWORDS_REMARKETING_LISTS", - DefinitionConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, - ID: "25beoSzcLFmimO8FgiVqTNwBG12", + DefinitionName: "GOOGLE_ADWORDS_REMARKETING_LISTS", + DefinitionConfig: oauthDefinitionConfig, + ID: "25beoSzcLFmimO8FgiVqTNwBG12", Config: map[string]interface{}{ "rudderAccountId": "7693729833", }, @@ -167,13 +170,9 @@ var _ = Describe("Http/Client", func() { httpClient := httpClient.NewOAuthHttpClient(&http.Client{}, common.RudderFlowDelivery, &cache, backendconfig.DefaultBackendConfig, rtTf.GetAuthErrorCategoryFromTransformResponse, &optionalArgs) req, _ := http.NewRequest("POST", "url", bytes.NewBuffer([]byte(`{"input":[{"message":{"userId":"user 1","event":"event1","type":"audiencelist","properties":{"listData":{"add":[{"email":"test@abc.com","phone":"@09876543210","firstName":"test","lastName":"rudderlabs","country":"US","postalCode":"1245"}]},"enablePartialFailure":true},"context":{"ip":"14.5.67.21","library":{"name":"http"}},"timestamp":"2020-02-02T00:23:09.544Z"},"metadata":{"secret":{"access_token":"dummy-access","refresh_token":"dummy-refresh","developer_token":"dummy-dev-token"}},"destination":{"secretConfig":{},"config":{},"name":"GARL","destinationDefinition":{"config":{"auth":{"role":"google_adwords_remarketing_lists_v1","type":"OAuth","provider":"Google","rudderScopes":["delivery"]}},"responseRules":{},"name":"GOOGLE_ADWORDS_REMARKETING_LISTS","displayName":"Google Ads Remarketing Lists (Customer Match)","category":null},"permissions":{"isLocked":false}}}],"destType":"google_adwords_remarketing_lists"}`))) destination := &v2.DestinationInfo{ - DefinitionName: "GOOGLE_ADWORDS_REMARKETING_LISTS", - DefinitionConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, - ID: "25beoSzcLFmimO8FgiVqTNwBG12", + DefinitionName: "GOOGLE_ADWORDS_REMARKETING_LISTS", + DefinitionConfig: oauthDefinitionConfig, + ID: "25beoSzcLFmimO8FgiVqTNwBG12", Config: map[string]interface{}{ "rudderAccountId": "7693729833", }, @@ -220,13 +219,9 @@ var _ = Describe("Http/Client", func() { req, _ := http.NewRequest("POST", "url", bytes.NewBuffer([]byte(`{"input":[{"message":{"userId":"user 1","event":"event1","type":"audiencelist","properties":{"listData":{"add":[{"email":"test@abc.com","phone":"@09876543210","firstName":"test","lastName":"rudderlabs","country":"US","postalCode":"1245"}]},"enablePartialFailure":true},"context":{"ip":"14.5.67.21","library":{"name":"http"}},"timestamp":"2020-02-02T00:23:09.544Z"},"metadata":{"secret":{"access_token":"dummy-access","refresh_token":"dummy-refresh","developer_token":"dummy-dev-token"}},"destination":{"secretConfig":{},"config":{},"name":"GARL","destinationDefinition":{"config":{"auth":{"role":"google_adwords_remarketing_lists_v1","type":"OAuth","provider":"Google","rudderScopes":["delivery"]}},"responseRules":{},"name":"GOOGLE_ADWORDS_REMARKETING_LISTS","displayName":"Google Ads Remarketing Lists (Customer Match)","category":null},"permissions":{"isLocked":false}}}],"destType":"google_adwords_remarketing_lists"}`))) destination := &v2.DestinationInfo{ - DefinitionName: "GOOGLE_ADWORDS_REMARKETING_LISTS", - DefinitionConfig: map[string]interface{}{ - "auth": map[string]interface{}{ - "type": "OAuth", - }, - }, - ID: "25beoSzcLFmimO8FgiVqTNwBG12", + DefinitionName: "GOOGLE_ADWORDS_REMARKETING_LISTS", + DefinitionConfig: oauthDefinitionConfig, + ID: "25beoSzcLFmimO8FgiVqTNwBG12", Config: map[string]interface{}{ "rudderAccountId": "7693729833", }, diff --git a/services/oauth/v2/http/transport.go b/services/oauth/v2/http/transport.go index e7cc65978a..a86bf27858 100644 --- a/services/oauth/v2/http/transport.go +++ b/services/oauth/v2/http/transport.go @@ -195,12 +195,7 @@ func (t *OAuthTransport) postRoundTrip(rts *roundTripState) (*http.Response, err } func (rts *roundTripState) getAccountID(flow common.RudderFlow) (string, error) { - accountIdKey := common.DeliveryAccountIDKey - if flow == common.RudderFlowDelete { - accountIdKey = common.DeleteAccountIDKey - } - - accountId, err := rts.destination.GetAccountID(accountIdKey) + accountId, err := rts.destination.GetAccountID(flow) if err != nil { return "", fmt.Errorf("accountId not found for destination(%s) in %s flow", rts.destination.ID, flow) } @@ -232,7 +227,7 @@ func (t *OAuthTransport) RoundTrip(req *http.Request) (*http.Response, error) { } startTime := time.Now() defer t.fireTimerStats("oauth_v2_http_total_roundtrip_latency", tags, startTime) - isOauthDestination, err := destination.IsOAuthDestination() + isOauthDestination, err := destination.IsOAuthDestination(t.flow) if err != nil { return httpResponseCreator(http.StatusInternalServerError, []byte(fmt.Sprintf("checking if destination is oauth destination: %v", err.Error()))), nil }