From e89a7cd9fb27dbbfe830c81a27677094e420d535 Mon Sep 17 00:00:00 2001 From: Yassine Bounekhla Date: Wed, 15 Jan 2025 03:44:30 -0500 Subject: [PATCH] CR --- lib/web/apiserver.go | 3 - lib/web/resources_test.go | 173 ++++++++++++++++-- lib/web/ui/auth_connectors.go | 18 +- .../AuthConnectors/AuthConnectorTile.test.tsx | 110 +++++++++++ 4 files changed, 277 insertions(+), 27 deletions(-) create mode 100644 web/packages/teleport/src/AuthConnectors/AuthConnectorTile.test.tsx diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 786d9c6ee36ab..4c30bcc83ae78 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -3696,9 +3696,6 @@ func (h *Handler) setDefaultConnectorHandle(w http.ResponseWriter, r *http.Reque if err := httplib.ReadJSON(r, &req); err != nil { return nil, trace.Wrap(err) } - if err := req.CheckAndSetDefaults(); err != nil { - return nil, trace.Wrap(err) - } clt, err := ctx.GetClient() if err != nil { diff --git a/lib/web/resources_test.go b/lib/web/resources_test.go index 3a593cc0df1d7..123a6ebd895e9 100644 --- a/lib/web/resources_test.go +++ b/lib/web/resources_test.go @@ -36,6 +36,7 @@ import ( "k8s.io/apimachinery/pkg/util/yaml" "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/constants" kubeproto "github.com/gravitational/teleport/api/gen/proto/go/teleport/kube/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/defaults" @@ -417,30 +418,124 @@ func TestRoleCRUD(t *testing.T) { } } -func TestGetGithubConnectors(t *testing.T) { +func TestGithubConnectorsCRUD(t *testing.T) { ctx := context.Background() - m := &mockedResourceAPIGetter{} + env := newWebPack(t, 1) + proxy := env.proxies[0] - m.mockGetGithubConnectors = func(ctx context.Context, withSecrets bool) ([]types.GithubConnector, error) { - connector, err := types.NewGithubConnector("test", types.GithubConnectorSpecV3{ - TeamsToLogins: []types.TeamMapping{ - { - Organization: "octocats", - Team: "dummy", - Logins: []string{"dummy"}, - }, - }, - }) - require.NoError(t, err) + pack := proxy.authPack(t, "test-user@example.com", nil) - return []types.GithubConnector{connector}, nil + tests := []struct { + name string + connectors []types.GithubConnector + setDefaultReq *ui.SetDefaultAuthConnectorRequest + wantConnectorName string + wantConnectorType string + }{ + { + name: "no connectors defaults to local auth", + connectors: []types.GithubConnector{}, + wantConnectorName: "", + wantConnectorType: constants.Local, + }, + { + name: "default connector exists in list", + connectors: []types.GithubConnector{ + makeGithubConnector(t, "github-1"), + }, + setDefaultReq: &ui.SetDefaultAuthConnectorRequest{ + Name: "github-1", + Type: constants.Github, + }, + wantConnectorName: "github-1", + wantConnectorType: constants.Github, + }, + { + name: "default connector missing defaults to last in list", + connectors: []types.GithubConnector{ + makeGithubConnector(t, "github-1"), + makeGithubConnector(t, "github-2"), + }, + setDefaultReq: &ui.SetDefaultAuthConnectorRequest{ + Name: "missing", + Type: constants.Github, + }, + wantConnectorName: "github-2", + wantConnectorType: constants.Github, + }, + { + name: "local auth type always defaults to local", + connectors: []types.GithubConnector{ + makeGithubConnector(t, "github-1"), + }, + setDefaultReq: &ui.SetDefaultAuthConnectorRequest{ + Name: "local", + Type: constants.Local, + }, + wantConnectorName: "", + wantConnectorType: constants.Local, + }, + { + name: "missing default with no connectors defaults to local", + connectors: []types.GithubConnector{}, + setDefaultReq: &ui.SetDefaultAuthConnectorRequest{ + Name: "missing", + Type: constants.Github, + }, + wantConnectorName: "", + wantConnectorType: constants.Local, + }, } - // Test response is converted to ui objects. - connectors, err := getGithubConnectors(ctx, m) - require.NoError(t, err) - require.Len(t, connectors, 1) - require.Contains(t, connectors[0].Content, "name: test") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup initial connectors + for _, conn := range tt.connectors { + raw, err := services.MarshalGithubConnector(conn) + resp, err := pack.clt.PostJSON(ctx, pack.clt.Endpoint("webapi", "github"), ui.ResourceItem{ + Kind: types.KindGithubConnector, + Name: conn.GetName(), + Content: string(raw), + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + } + + // Set default connector if specified + if tt.setDefaultReq != nil { + resp, err := pack.clt.PutJSON(ctx, pack.clt.Endpoint("webapi", "defaultconnector"), tt.setDefaultReq) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + } + + // Get connectors and verify response + resp, err := pack.clt.Get(ctx, pack.clt.Endpoint("webapi", "github"), url.Values{}) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.Code()) + + var connResponse ui.ListAuthConnectorsResponse + err = json.Unmarshal(resp.Bytes(), &connResponse) + require.NoError(t, err) + + // Verify connector name and type + assert.Equal(t, tt.wantConnectorName, connResponse.DefaultConnectorName) + assert.Equal(t, tt.wantConnectorType, connResponse.DefaultConnectorType) + + // Verify connectors list + require.Equal(t, len(tt.connectors), len(connResponse.Connectors)) + for i, conn := range tt.connectors { + expectedItem, err := ui.NewResourceItem(conn) + require.NoError(t, err) + require.Equal(t, expectedItem.Name, connResponse.Connectors[i].Name) + } + + // Cleanup connectors + for _, conn := range tt.connectors { + _, err := pack.clt.Delete(ctx, pack.clt.Endpoint("webapi", "github", conn.GetName())) + require.NoError(t, err) + } + }) + } } func TestGetTrustedClusters(t *testing.T) { @@ -622,6 +717,8 @@ type mockedResourceAPIGetter struct { mockGetTrustedClusters func(ctx context.Context) ([]types.TrustedCluster, error) mockDeleteTrustedCluster func(ctx context.Context, name string) error mockListResources func(ctx context.Context, req proto.ListResourcesRequest) (*types.ListResourcesResponse, error) + mockGetAuthPreference func(ctx context.Context) (types.AuthPreference, error) + mockUpsertAuthPreference func(ctx context.Context, pref types.AuthPreference) (types.AuthPreference, error) } func (m *mockedResourceAPIGetter) GetRole(ctx context.Context, name string) (types.Role, error) { @@ -717,6 +814,21 @@ func (m *mockedResourceAPIGetter) ListResources(ctx context.Context, req proto.L return nil, trace.NotImplemented("mockListResources not implemented") } +// Add new mock methods +func (m *mockedResourceAPIGetter) GetAuthPreference(ctx context.Context) (types.AuthPreference, error) { + if m.mockGetAuthPreference != nil { + return m.mockGetAuthPreference(ctx) + } + return nil, trace.NotImplemented("mockGetAuthPreference not implemented") +} + +func (m *mockedResourceAPIGetter) UpsertAuthPreference(ctx context.Context, pref types.AuthPreference) (types.AuthPreference, error) { + if m.mockUpsertAuthPreference != nil { + return m.mockUpsertAuthPreference(ctx, pref) + } + return nil, trace.NotImplemented("mockUpsertAuthPreference not implemented") +} + func Test_newKubeListRequest(t *testing.T) { type args struct { query string @@ -793,3 +905,26 @@ func Test_newKubeListRequest(t *testing.T) { }) } } + +func makeAuthPreference(t *testing.T, connectorName string, authType string) types.AuthPreference { + pref, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{ + ConnectorName: connectorName, + Type: authType, + }) + require.NoError(t, err) + return pref +} + +func makeGithubConnector(t *testing.T, name string) types.GithubConnector { + connector, err := types.NewGithubConnector(name, types.GithubConnectorSpecV3{ + TeamsToRoles: []types.TeamRolesMapping{ + { + Organization: "octocats", + Team: "dummy", + Roles: []string{"dummy"}, + }, + }, + }) + require.NoError(t, err) + return connector +} diff --git a/lib/web/ui/auth_connectors.go b/lib/web/ui/auth_connectors.go index 05ed7964d3bfa..b28aa2f2144d2 100644 --- a/lib/web/ui/auth_connectors.go +++ b/lib/web/ui/auth_connectors.go @@ -19,6 +19,8 @@ package ui import ( + "slices" + "github.com/gravitational/trace" "github.com/gravitational/teleport/api/constants" @@ -42,6 +44,14 @@ type SetDefaultAuthConnectorRequest struct { Type string `json:"type"` } +// ValidConnectorTypes defines the allowed auth connector types +var ValidConnectorTypes = []string{ + constants.SAML, + constants.OIDC, + constants.Github, + constants.LocalConnector, +} + // CheckAndSetDefaults checks if the provided values are valid. func (r *SetDefaultAuthConnectorRequest) CheckAndSetDefaults() error { if r.Name == "" && r.Type != "local" { @@ -50,11 +60,9 @@ func (r *SetDefaultAuthConnectorRequest) CheckAndSetDefaults() error { if r.Type == "" { return trace.BadParameter("missing connector type") } - if r.Type != constants.SAML && - r.Type != constants.OIDC && - r.Type != constants.Github && - r.Type != constants.LocalConnector { - return trace.BadParameter("invalid connector type") + + if !slices.Contains(ValidConnectorTypes, r.Type) { + return trace.BadParameter("unsupported connector type: %q", r.Type) } return nil diff --git a/web/packages/teleport/src/AuthConnectors/AuthConnectorTile.test.tsx b/web/packages/teleport/src/AuthConnectors/AuthConnectorTile.test.tsx new file mode 100644 index 0000000000000..89dd86030a487 --- /dev/null +++ b/web/packages/teleport/src/AuthConnectors/AuthConnectorTile.test.tsx @@ -0,0 +1,110 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { useState } from 'react'; + +import { fireEvent, render } from 'design/utils/testing'; +import { AuthType } from 'shared/services'; + +import { AuthConnectorTile } from './AuthConnectorTile'; +import getSsoIcon from './ssoIcons/getSsoIcon'; + +test('default, real connector, renders properly', () => { + render(); + + expect(screen.getByText('Okta')).toBeInTheDocument(); + expect(screen.queryByText('Default')).not.toBeInTheDocument(); + + const optionsButton = screen.getByTestId('button'); + fireEvent.click(optionsButton); + + expect(screen.getByText('Set as default')).toBeInTheDocument(); + expect(screen.getByText('Edit')).toBeInTheDocument(); + expect(screen.getByText('Delete')).toBeInTheDocument(); +}); + +test('non-default, real connector, renders properly', () => { + render(); + + expect(screen.getByText('Okta')).toBeInTheDocument(); + expect(screen.getByText('Default')).toBeInTheDocument(); + + const optionsButton = screen.getByTestId('button'); + fireEvent.click(optionsButton); + + expect(screen.queryByText('Set as default')).not.toBeInTheDocument(); + expect(screen.getByText('Edit')).toBeInTheDocument(); + expect(screen.getByText('Delete')).toBeInTheDocument(); +}); + +// "local" connector for has no edit/delete functionality, only set as default +test('non-default, real connector, with no edit/delete functionality renders properly', () => { + render(); + + expect(screen.getByText('Okta')).toBeInTheDocument(); + expect(screen.queryByText('Default')).not.toBeInTheDocument(); + + const optionsButton = screen.getByTestId('button'); + fireEvent.click(optionsButton); + + expect(screen.getByText('Set as default')).toBeInTheDocument(); + expect(screen.queryByText('Edit')).not.toBeInTheDocument(); + expect(screen.queryByText('Delete')).not.toBeInTheDocument(); +}); + +test('default, real connector, with no edit/delete functionality renders properly', () => { + render( + + ); + + expect(screen.getByText('Okta')).toBeInTheDocument(); + expect(screen.getByText('Default')).toBeInTheDocument(); + + expect(screen.queryByTestId('button')).not.toBeInTheDocument(); +}); + +test('placeholder renders properly', () => { + render(); + + expect(screen.getByText('Okta')).toBeInTheDocument(); + expect(screen.queryByText('Default')).not.toBeInTheDocument(); + + expect(screen.getByText('Set Up')).toBeInTheDocument(); + + expect(screen.queryByTestId('button')).not.toBeInTheDocument(); +}); + +const props = { + name: 'Okta', + id: 'okta-connector', + kind: 'saml' as AuthType, + Icon: getSsoIcon('saml', 'okta'), + isDefault: false, + isPlaceholder: false, + onSetup: () => null, + onEdit: () => null, + onDelete: () => null, + onSetAsDefault: () => null, +};