Skip to content

Commit

Permalink
Read token endpoint from well-known configuration (#9)
Browse files Browse the repository at this point in the history 
* Add oidc client to read token URL from IAS well known endpoint

* Move oidc client

* Rename functions and adapt tests

* Adapt tests

* Fix test names

* Remove duplicate test

* Add missing secret expects
  • Loading branch information
Tim Riffer authored Apr 25, 2023
1 parent 1a5ccf1 commit fffd795
Show file tree
Hide file tree
Showing 8 changed files with 406 additions and 31 deletions.
6 changes: 6 additions & 0 deletions controllers/eventingauth_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ func verifySecretExistsOnTargetCluster() *corev1.Secret {
s := corev1.Secret{}
Eventually(func(g Gomega) {
g.Expect(targetClusterK8sClient.Get(context.TODO(), appSecretObjectKey, &s)).Should(Succeed())
g.Expect(s.Data).To(HaveKey("client_id"))
g.Expect(s.Data["client_id"]).ToNot(BeEmpty())
g.Expect(s.Data).To(HaveKey("client_secret"))
g.Expect(s.Data["client_secret"]).ToNot(BeEmpty())
g.Expect(s.Data).To(HaveKey("token_url"))
g.Expect(s.Data["token_url"]).To(ContainSubstring("/token"))
}, defaultTimeout).Should(Succeed())

return &s
Expand Down
2 changes: 1 addition & 1 deletion controllers/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ type iasClientStub struct {
}

func (i iasClientStub) CreateApplication(_ context.Context, name string) (ias.Application, error) {
return ias.NewApplication(fmt.Sprintf("id-for-%s", name), fmt.Sprintf("client-id-for-%s", name), "test-client-secret", "https://test-token-url.com"), nil
return ias.NewApplication(fmt.Sprintf("id-for-%s", name), fmt.Sprintf("client-id-for-%s", name), "test-client-secret", "https://test-token-url.com/token"), nil
}

func (i iasClientStub) DeleteApplication(_ context.Context, _ string) error {
Expand Down
61 changes: 46 additions & 15 deletions internal/ias/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"github.com/deepmap/oapi-codegen/pkg/securityprovider"
"github.com/google/uuid"
"github.com/kyma-project/eventing-auth-manager/internal/ias/internal/api"
"github.com/kyma-project/eventing-auth-manager/internal/ias/internal/oidc"
"github.com/pkg/errors"
"net/http"
ctrl "sigs.k8s.io/controller-runtime"
"strings"
"time"
)

type Client interface {
Expand All @@ -24,25 +26,33 @@ func NewIasClient(iasTenantUrl, user, password string) (Client, error) {
return nil, err
}

applicationsEndpointUrl := fmt.Sprintf("%s/%s", iasTenantUrl, "Applications/v1/")
applicationsEndpointUrl := fmt.Sprintf("%s/Applications/v1/", iasTenantUrl)
apiClient, err := api.NewClientWithResponses(applicationsEndpointUrl, api.WithRequestEditorFn(basicAuthProvider.Intercept))
if err != nil {
return nil, err
}
return client{
api: apiClient,
tenantUrl: iasTenantUrl,

oidcHttpClient := &http.Client{
Timeout: time.Second * 5,
}

return &client{
api: apiClient,
oidcClient: oidc.NewOidcClient(oidcHttpClient, iasTenantUrl),
}, nil
}

type client struct {
api api.ClientWithResponsesInterface
tenantUrl string
api api.ClientWithResponsesInterface
oidcClient oidc.Client
// The token URL of the IAS client. Since this URL should only change when the tenant changes and this will lead to the initialization of
// a new client, we can cache the URL to avoid an additional request at each application creation.
tokenUrl *string
}

// CreateApplication creates an application in IAS. This function is not idempotent, because if an application with the specified
// name already exists, it will be deleted and recreated.
func (c client) CreateApplication(ctx context.Context, name string) (Application, error) {
func (c *client) CreateApplication(ctx context.Context, name string) (Application, error) {

existingApp, err := c.getApplicationByName(ctx, name)
if err != nil {
Expand Down Expand Up @@ -78,12 +88,33 @@ func (c client) CreateApplication(ctx context.Context, name string) (Application
return Application{}, err
}

// TODO: Instead of providing tenant URL and making assumption about token URL, we should fetch the token URL from the well-known endpoint.
return NewApplication(appId.String(), *clientId, *clientSecret, c.tenantUrl), nil
// Since the token url is not part of the application response, we have to fetch it from the OIDC configuration.
tokenUrl, err := c.GetTokenUrl(ctx)
if err != nil {
return Application{}, err
}

return NewApplication(appId.String(), *clientId, *clientSecret, *tokenUrl), nil
}

func (c *client) GetTokenUrl(ctx context.Context) (*string, error) {
if c.tokenUrl == nil {
tokenEndpoint, err := c.oidcClient.GetTokenEndpoint(ctx)
if err != nil {
return nil, err
}
if tokenEndpoint == nil {
return nil, errors.New("failed to fetch token url")
}

c.tokenUrl = tokenEndpoint
}

return c.tokenUrl, nil
}

// DeleteApplication deletes an application in IAS. If the application does not exist, this function does nothing.
func (c client) DeleteApplication(ctx context.Context, name string) error {
func (c *client) DeleteApplication(ctx context.Context, name string) error {
existingApp, err := c.getApplicationByName(ctx, name)
if err != nil {
return err
Expand All @@ -95,7 +126,7 @@ func (c client) DeleteApplication(ctx context.Context, name string) error {
return c.deleteApplication(ctx, *existingApp.Id)
}

func (c client) getApplicationByName(ctx context.Context, name string) (*api.ApplicationResponse, error) {
func (c *client) getApplicationByName(ctx context.Context, name string) (*api.ApplicationResponse, error) {
appsFilter := fmt.Sprintf("name eq %s", name)
res, err := c.api.GetAllApplicationsWithResponse(ctx, &api.GetAllApplicationsParams{Filter: &appsFilter})
if err != nil {
Expand Down Expand Up @@ -127,7 +158,7 @@ func (c client) getApplicationByName(ctx context.Context, name string) (*api.App
return nil, nil
}

func (c client) createNewApplication(ctx context.Context, name string) (uuid.UUID, error) {
func (c *client) createNewApplication(ctx context.Context, name string) (uuid.UUID, error) {
newApplication := newIasApplication(name)
res, err := c.api.CreateApplicationWithResponse(ctx, &api.CreateApplicationParams{}, newApplication)
if err != nil {
Expand All @@ -143,7 +174,7 @@ func (c client) createNewApplication(ctx context.Context, name string) (uuid.UUI
return extractApplicationId(res)
}

func (c client) createSecret(ctx context.Context, appId uuid.UUID) (*string, error) {
func (c *client) createSecret(ctx context.Context, appId uuid.UUID) (*string, error) {
res, err := c.api.CreateApiSecretWithResponse(ctx, appId, newSecretRequest())
if err != nil {
return nil, err
Expand All @@ -157,7 +188,7 @@ func (c client) createSecret(ctx context.Context, appId uuid.UUID) (*string, err
return res.JSON201.Secret, nil
}

func (c client) getClientId(ctx context.Context, appId uuid.UUID) (*string, error) {
func (c *client) getClientId(ctx context.Context, appId uuid.UUID) (*string, error) {
// The client ID is generated only after an API secret is created, so we need to retrieve the application again to get the client ID.
applicationResponse, err := c.api.GetApplicationWithResponse(ctx, appId, &api.GetApplicationParams{})
if err != nil {
Expand All @@ -171,7 +202,7 @@ func (c client) getClientId(ctx context.Context, appId uuid.UUID) (*string, erro
return applicationResponse.JSON200.UrnSapIdentityApplicationSchemasExtensionSci10Authentication.ClientId, nil
}

func (c client) deleteApplication(ctx context.Context, id uuid.UUID) error {
func (c *client) deleteApplication(ctx context.Context, id uuid.UUID) error {
res, err := c.api.DeleteApplicationWithResponse(ctx, id)
if err != nil {
return err
Expand Down
75 changes: 63 additions & 12 deletions internal/ias/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,24 @@ import (
"github.com/google/uuid"
"github.com/kyma-project/eventing-auth-manager/internal/ias/internal/api"
"github.com/kyma-project/eventing-auth-manager/internal/ias/internal/mocks"
oidcmocks "github.com/kyma-project/eventing-auth-manager/internal/ias/internal/oidc/mocks"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"k8s.io/utils/pointer"
"net/http"
"testing"
)

func Test_CreateApplication(t *testing.T) {
appId := uuid.MustParse("90764f89-f041-4ccf-8da9-7a7c2d60d7fc")
tests := []struct {
name string
givenApiMock func() *mocks.ClientWithResponsesInterface
assertCalls func(*testing.T, *mocks.ClientWithResponsesInterface)
wantApp Application
wantError error
name string
givenApiMock func() *mocks.ClientWithResponsesInterface
oidcClientMock *oidcmocks.Client
clientTokenUrlMock *string
assertCalls func(*testing.T, *mocks.ClientWithResponsesInterface)
wantApp Application
wantError error
}{
{
name: "should create new application when fetching existing applications returns status 200 and no applications",
Expand All @@ -34,7 +38,8 @@ func Test_CreateApplication(t *testing.T) {

return &clientMock
},
wantApp: NewApplication(appId.String(), "clientIdMock", "clientSecretMock", "https://test.com"),
oidcClientMock: mockGetTokenUrl(t, pointer.String("https://test.com/token")),
wantApp: NewApplication(appId.String(), "clientIdMock", "clientSecretMock", "https://test.com/token"),
},
{
name: "should create new application when fetching existing applications returns status 404",
Expand All @@ -49,7 +54,8 @@ func Test_CreateApplication(t *testing.T) {

return &clientMock
},
wantApp: NewApplication(appId.String(), "clientIdMock", "clientSecretMock", "https://test.com"),
oidcClientMock: mockGetTokenUrl(t, pointer.String("https://test.com/token")),
wantApp: NewApplication(appId.String(), "clientIdMock", "clientSecretMock", "https://test.com/token"),
},
{
name: "should recreate application when application already exists",
Expand All @@ -67,7 +73,8 @@ func Test_CreateApplication(t *testing.T) {

return &clientMock
},
wantApp: NewApplication(appId.String(), "clientIdMock", "clientSecretMock", "https://test.com"),
oidcClientMock: mockGetTokenUrl(t, pointer.String("https://test.com/token")),
wantApp: NewApplication(appId.String(), "clientIdMock", "clientSecretMock", "https://test.com/token"),
},
{
name: "should return an error when multiple applications exist for the given name",
Expand Down Expand Up @@ -165,15 +172,48 @@ func Test_CreateApplication(t *testing.T) {
wantApp: Application{},
wantError: errors.New("failed to retrieve client ID"),
},
{
name: "should return an error when token URL wasn't fetched",
givenApiMock: func() *mocks.ClientWithResponsesInterface {
clientMock := mocks.ClientWithResponsesInterface{}

mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock)
mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String())
mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock")
mockGetApplicationWithResponseStatusOK(&clientMock, appId, "clientIdMock")

return &clientMock
},
oidcClientMock: mockGetTokenUrl(t, nil),
wantApp: Application{},
wantError: errors.New("failed to fetch token url"),
},
{
name: "should create new application without fetching token URL when it is already cached in the client",
givenApiMock: func() *mocks.ClientWithResponsesInterface {
clientMock := mocks.ClientWithResponsesInterface{}

mockGetAllApplicationsWithResponseStatusOkEmptyResponse(&clientMock)
mockCreateApplicationWithResponseStatusCreated(&clientMock, appId.String())
mockCreateApiSecretWithResponseStatusCreated(&clientMock, appId, "clientSecretMock")
mockGetApplicationWithResponseStatusOK(&clientMock, appId, "clientIdMock")

return &clientMock
},
clientTokenUrlMock: pointer.String("https://from-cache.com/token"),
wantApp: NewApplication(appId.String(), "clientIdMock", "clientSecretMock", "https://from-cache.com/token"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// given
apiMock := tt.givenApiMock()
oidcMock := tt.oidcClientMock

client := client{
api: apiMock,
tenantUrl: "https://test.com",
api: apiMock,
oidcClient: oidcMock,
tokenUrl: tt.clientTokenUrlMock,
}

// when
Expand All @@ -195,6 +235,10 @@ func Test_CreateApplication(t *testing.T) {
apiMock.AssertExpectations(t)
}

if oidcMock != nil {
oidcMock.AssertExpectations(t)
}

})
}
}
Expand Down Expand Up @@ -260,8 +304,7 @@ func Test_DeleteApplication(t *testing.T) {
apiMock := tt.givenApiMock()

client := client{
api: apiMock,
tenantUrl: "https://test.com",
api: apiMock,
}

// when
Expand Down Expand Up @@ -427,3 +470,11 @@ func mockDeleteApplicationWithResponseStatusNotFound(clientMock *mocks.ClientWit
},
}, nil)
}

func mockGetTokenUrl(t *testing.T, tokenUrl *string) *oidcmocks.Client {
clientMock := oidcmocks.NewClient(t)
clientMock.On("GetTokenEndpoint", mock.Anything).
Return(tokenUrl, nil)

return clientMock
}
91 changes: 91 additions & 0 deletions internal/ias/internal/oidc/mocks/client.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit fffd795

Please sign in to comment.