Skip to content

Commit

Permalink
Validation of the 3rd party payload (#236)
Browse files Browse the repository at this point in the history
<!-- Thank you for your contribution. Before you submit the pull
request:
1. Follow contributing guidelines, templates, the recommended Git
workflow, and any related documentation.
2. Read and submit the required Contributor Licence Agreements
(https://github.com/kyma-project/community/blob/main/CONTRIBUTING.md#agreements-and-licenses).
3. Test your changes and attach their results to the pull request.
4. Update the relevant documentation.

If the pull request requires a decision, follow the [decision-making
process](https://github.com/kyma-project/community/blob/main/governance.md)
and replace the PR template with the [decision record
template](https://github.com/kyma-project/community/blob/main/.github/ISSUE_TEMPLATE/decision-record.md).
-->

**Description**

Changes proposed in this pull request:

- verify if the runtimeID, that comes from external service Compass is
in the correct UUID format
- validate the URL of the token to the Compass Connector to ensure it
points to the SAP domain
- perform a sanity check on the Token itself

**Related issue(s)**
#54
  • Loading branch information
kyma-bot authored Dec 27, 2024
2 parents a82bf9d + 576c752 commit 1541828
Show file tree
Hide file tree
Showing 7 changed files with 140 additions and 13 deletions.
2 changes: 2 additions & 0 deletions config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ spec:
value: "true"
- name: APP_DIRECTOR_URL
value: https://compass-gateway-auth-oauth.cmp-main.dev.kyma.cloud.sap/director/graphql
- name: APP_CONNECTOR_URL_PATTERN
value: kyma.cloud.sap/connector/graphql
ports:
- containerPort: 8080
name: metrics
Expand Down
23 changes: 18 additions & 5 deletions controllers/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package controllers

import (
"context"
"strings"
"time"

"github.com/kyma-incubator/compass/components/director/pkg/graphql"
"github.com/kyma-project/compass-manager/internal/apperrors"
"github.com/kyma-project/compass-manager/internal/director"
"github.com/kyma-project/compass-manager/internal/util"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
core "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
Expand All @@ -19,17 +21,20 @@ import (
const (
AgentConfigurationSecretName = "compass-agent-configuration"
runtimeAgentComponentNameSpace = "kyma-system"
maxTokenLength = 100
)

type RuntimeAgentConfigurator struct {
Client director.Client
Log *logrus.Logger
Client director.Client
ConnectorURLPattern string
Log *logrus.Logger
}

func NewRuntimeAgentConfigurator(directorClient director.Client, log *logrus.Logger) *RuntimeAgentConfigurator {
func NewRuntimeAgentConfigurator(directorClient director.Client, connectorURLPattern string, log *logrus.Logger) *RuntimeAgentConfigurator {
return &RuntimeAgentConfigurator{
Client: directorClient,
Log: log,
Client: directorClient,
ConnectorURLPattern: connectorURLPattern,
Log: log,
}
}

Expand Down Expand Up @@ -99,5 +104,13 @@ func (r *RuntimeAgentConfigurator) fetchCompassToken(compassID, globalAccount st
return graphql.OneTimeTokenForRuntimeExt{}, err
}

if !strings.HasSuffix(token.ConnectorURL, r.ConnectorURLPattern) {
return graphql.OneTimeTokenForRuntimeExt{}, errors.New("Connector URL does not match the expected pattern")
}

if len(token.Token) > maxTokenLength {
return graphql.OneTimeTokenForRuntimeExt{}, errors.New("OneTimeToken is too long")
}

return token, nil
}
68 changes: 68 additions & 0 deletions controllers/configurator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package controllers

import (
"testing"

"github.com/kyma-incubator/compass/components/director/pkg/graphql"
"github.com/kyma-project/compass-manager/internal/director/mocks"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestAppError(t *testing.T) {
t.Run("should succeed after fetching correct Compass Token", func(t *testing.T) {
mockDirectorClient := mocks.Client{}
mockDirectorClient.On("GetConnectionToken", "compassID", "globalAccount").Return(graphql.OneTimeTokenForRuntimeExt{
OneTimeTokenForRuntime: graphql.OneTimeTokenForRuntime{
TokenWithURL: graphql.TokenWithURL{
Token: "dGVzdFRva2VuQmFzZWQ2NA==",
ConnectorURL: "kyma.cloud.sap/connector/graphql",
},
},
}, nil)

configurator := NewRuntimeAgentConfigurator(&mockDirectorClient, "kyma.cloud.sap/connector/graphql", logrus.New())

token, err := configurator.fetchCompassToken("compassID", "globalAccount")
require.NoError(t, err)
assert.Equal(t, "kyma.cloud.sap/connector/graphql", token.ConnectorURL)
assert.Equal(t, "dGVzdFRva2VuQmFzZWQ2NA==", token.Token)
})
t.Run("should return error after fetching invalid Connector URL", func(t *testing.T) {
mockDirectorClient := mocks.Client{}
mockDirectorClient.On("GetConnectionToken", "compassID", "globalAccount").Return(graphql.OneTimeTokenForRuntimeExt{
OneTimeTokenForRuntime: graphql.OneTimeTokenForRuntime{
TokenWithURL: graphql.TokenWithURL{
Token: "dGVzdFRva2VuQmFzZWQ2NA==",
ConnectorURL: "invalid.domain/connector/graphql",
},
},
}, nil)

configurator := NewRuntimeAgentConfigurator(&mockDirectorClient, "kyma.cloud.sap/connector/graphql", logrus.New())

token, err := configurator.fetchCompassToken("compassID", "globalAccount")
require.Error(t, err)
require.ErrorContains(t, err, "Connector URL does not match the expected pattern")
assert.Equal(t, token, graphql.OneTimeTokenForRuntimeExt{})
})
t.Run("should return error when Runtime Token is too long", func(t *testing.T) {
mockDirectorClient := mocks.Client{}
mockDirectorClient.On("GetConnectionToken", "compassID", "globalAccount").Return(graphql.OneTimeTokenForRuntimeExt{
OneTimeTokenForRuntime: graphql.OneTimeTokenForRuntime{
TokenWithURL: graphql.TokenWithURL{
Token: "bm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVkbm90LWJhc2U2NC1lbmNvZGVk",
ConnectorURL: "kyma.cloud.sap/connector/graphql",
},
},
}, nil)

configurator := NewRuntimeAgentConfigurator(&mockDirectorClient, "kyma.cloud.sap/connector/graphql", logrus.New())

token, err := configurator.fetchCompassToken("compassID", "globalAccount")
require.Error(t, err)
require.ErrorContains(t, err, "OneTimeToken is too long")
assert.Equal(t, token, graphql.OneTimeTokenForRuntimeExt{})
})
}
7 changes: 4 additions & 3 deletions internal/apperrors/apperrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ const (
const (
ErrCompassManagerInternal ErrReason = "err_compass_manager_internal"

ErrDirectorNilResponse ErrReason = "err_director_nil_response"
ErrDirectorRuntimeIDMismatch ErrReason = "err_director_runtime_id_mismatch"
ErrDirectorClientGraphqlizer ErrReason = "err_director_client_graphqlizer"
ErrDirectorNilResponse ErrReason = "err_director_nil_response"
ErrDirectorRuntimeIDMismatch ErrReason = "err_director_runtime_id_mismatch"
ErrDirectorClientGraphqlizer ErrReason = "err_director_client_graphqlizer"
ErrDirectorRuntimeIDInvalidFormat ErrReason = "err_director_runtime_id_invalid_format"
)

type ErrCode int
Expand Down
6 changes: 6 additions & 0 deletions internal/director/directorclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"

"github.com/google/uuid"
directorApperrors "github.com/kyma-incubator/compass/components/director/pkg/apperrors"
"github.com/kyma-incubator/compass/components/director/pkg/graphql"
"github.com/kyma-incubator/compass/components/director/pkg/graphql/graphqlizer"
Expand Down Expand Up @@ -84,6 +85,11 @@ func (cc *directorClient) CreateRuntime(config *gqlschema.RuntimeInput, globalAc
return "", apperrors.Internal("Failed to register runtime in Director: Received nil response.").SetComponent(apperrors.ErrCompassDirector).SetReason(apperrors.ErrDirectorNilResponse)
}

_, err = uuid.Parse(response.Result.ID)
if err != nil {
return "", apperrors.Internal("Failed to register runtime in Director: Received ID is not in UUID format").SetComponent(apperrors.ErrCompassDirector).SetReason(apperrors.ErrDirectorRuntimeIDInvalidFormat)
}

log.Infof("Successfully registered Runtime %s in Director for Global Account %s", config.Name, globalAccount)

return response.Result.ID, nil
Expand Down
44 changes: 40 additions & 4 deletions internal/director/directorclient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

const (
compassTestingID = "test-runtime-ID-12345"
compassTestingID = "4366e452-2ffb-435d-abbd-81cf5d3965c9"
compassTestingName = "Runtime Test name"
validTokenValue = "12345"
globalAccountValue = "3e64ebae-38b5-46a0-b1ed-9ccee153a0ae"
Expand All @@ -34,17 +34,17 @@ const (
}) { id } }`

expectedOneTimeTokenQuery = `mutation {
result: requestOneTimeTokenForRuntime(id: "test-runtime-ID-12345") {
result: requestOneTimeTokenForRuntime(id: "4366e452-2ffb-435d-abbd-81cf5d3965c9") {
token connectorURL
}}`

expectedGetRuntimeQuery = `query {
result: runtime(id: "test-runtime-ID-12345") {
result: runtime(id: "4366e452-2ffb-435d-abbd-81cf5d3965c9") {
id name description labels
}}`

expectedDeleteRuntimeQuery = `mutation {
result: unregisterRuntime(id: "test-runtime-ID-12345") {
result: unregisterRuntime(id: "4366e452-2ffb-435d-abbd-81cf5d3965c9") {
id
}}`
)
Expand Down Expand Up @@ -158,6 +158,42 @@ func TestDirectorClient_RuntimeRegistering(t *testing.T) {
assert.Empty(t, receivedRuntimeID)
})

t.Run("Should not register Runtime and return error when the Runtime ID from Director is not in UUID format", func(t *testing.T) {
// given
responseDescription := "runtime description"
expectedResponse := &graphql.Runtime{
ID: "non-uuid-format",
Name: compassTestingName,
Description: &responseDescription,
}

expectedID := ""

gqlClient := gql.NewQueryAssertClient(t, nil, []*gcli.Request{expectedRequest}, func(t *testing.T, r interface{}) {
cfg, ok := r.(*CreateRuntimeResponse)
require.True(t, ok)
assert.Empty(t, cfg.Result)
cfg.Result = expectedResponse
})

token := oauth.Token{
AccessToken: validTokenValue,
Expiration: futureExpirationTime,
}

mockedOAuthClient := &oauthmocks.Client{}
mockedOAuthClient.On("GetAuthorizationToken").Return(token, nil)

configClient := NewDirectorClient(gqlClient, mockedOAuthClient)

// when
receivedRuntimeID, err := configClient.CreateRuntime(runtimeInput, globalAccountValue)

// then
assert.Error(t, err)
assert.Equal(t, expectedID, receivedRuntimeID)
})

t.Run("Should return error when the result of the call to Director service is nil", func(t *testing.T) {
// given
validToken := oauth.Token{
Expand Down
3 changes: 2 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type config struct {
SkipDirectorCertVerification bool `envconfig:"default=false"`
DirectorURL string `envconfig:"APP_DIRECTOR_URL,default=https://compass-gateway-auth-oauth.cmp-main.dev.kyma.cloud.sap/director/graphql"`
DirectorOAuthPath string `envconfig:"APP_DIRECTOR_OAUTH_PATH,default=./dev/director.yaml"`
ConnectorURLPattern string `envconfig:"APP_CONNECTOR_URL_PATTERN,default=kyma.cloud.sap/connector/graphql"`
EnabledRegistration bool `envconfig:"APP_ENABLED_REGISTRATION,default=false"`
DryRun bool `envconfig:"APP_DRYRUN,default=false"`
}
Expand Down Expand Up @@ -124,7 +125,7 @@ func main() {
runtimeAgentConfigurator = dry
} else {
compassRegistrator = controllers.NewCompassRegistrator(directorClient, log)
runtimeAgentConfigurator = controllers.NewRuntimeAgentConfigurator(directorClient, log)
runtimeAgentConfigurator = controllers.NewRuntimeAgentConfigurator(directorClient, cfg.ConnectorURLPattern, log)
}

requeueTime := time.Second * 5 //nolint:mnd
Expand Down

0 comments on commit 1541828

Please sign in to comment.