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

fix: update to check for supported scopes for OAuth destinations #4585

Merged
merged 9 commits into from
Apr 22, 2024
2 changes: 1 addition & 1 deletion regulation-worker/internal/delete/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
85 changes: 26 additions & 59 deletions regulation-worker/internal/delete/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{{}},
Expand Down Expand Up @@ -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{{}},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down
3 changes: 2 additions & 1 deletion router/transformer/transformer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
},
},
Expand Down
48 changes: 41 additions & 7 deletions services/oauth/v2/destination_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
Expand All @@ -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")
}
Expand Down
133 changes: 133 additions & 0 deletions services/oauth/v2/destination_info_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
})
}
})
})
Loading
Loading