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

GitHub proxy part 4.5: git server in UI #49644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/services/useracl.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ type UserACL struct {
Contact ResourceAccess `json:"contact"`
// FileTransferAccess defines the ability to perform remote file operations via SCP or SFTP
FileTransferAccess bool `json:"fileTransferAccess"`
// GitServers defines access to Git servers.
GitServers ResourceAccess `json:"gitServers"`
}

func hasAccess(roleSet RoleSet, ctx *Context, kind string, verbs ...string) bool {
Expand Down Expand Up @@ -164,6 +166,7 @@ func NewUserACL(user types.User, userRoles RoleSet, features proto.Features, des
desktopAccess := newAccess(userRoles, ctx, types.KindWindowsDesktop)
cnDiagnosticAccess := newAccess(userRoles, ctx, types.KindConnectionDiagnostic)
samlIdpServiceProviderAccess := newAccess(userRoles, ctx, types.KindSAMLIdPServiceProvider)
gitServersAccess := newAccess(userRoles, ctx, types.KindGitServer)

// active sessions are a special case - if a user's role set has any join_sessions
// policies then the ACL must permit showing active sessions
Expand Down Expand Up @@ -266,5 +269,6 @@ func NewUserACL(user types.User, userRoles RoleSet, features proto.Features, des
AccessGraphSettings: accessGraphSettings,
Contact: contact,
FileTransferAccess: fileTransferAccess,
GitServers: gitServersAccess,
}
}
1 change: 1 addition & 0 deletions lib/services/useracl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func TestNewUserACL(t *testing.T) {
require.Empty(t, cmp.Diff(userContext.License, denied))
require.Empty(t, cmp.Diff(userContext.Download, denied))
require.Empty(t, cmp.Diff(userContext.Contact, allowedRW))
require.Empty(t, cmp.Diff(userContext.GitServers, denied))

// test enabling of the 'Use' verb
require.Empty(t, cmp.Diff(userContext.Integrations, ResourceAccess{true, true, true, true, true, true}))
Expand Down
14 changes: 10 additions & 4 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -2948,6 +2948,7 @@ func makeUnifiedResourceRequest(r *http.Request) (*proto.ListUnifiedResourcesReq
types.KindWindowsDesktop,
types.KindKubernetesCluster,
types.KindSAMLIdPServiceProvider,
types.KindGitServer,
}
}

Expand Down Expand Up @@ -3077,11 +3078,16 @@ func (h *Handler) clusterUnifiedResourcesGet(w http.ResponseWriter, request *htt
for _, enriched := range page {
switch r := enriched.ResourceWithLabels.(type) {
case types.Server:
logins, err := calculateSSHLogins(identity, enriched.Logins)
if err != nil {
return nil, trace.Wrap(err)
var logins []string
switch enriched.GetKind() {
case types.KindNode:
logins, err = calculateSSHLogins(identity, enriched.Logins)
if err != nil {
return nil, trace.Wrap(err)
}
case types.KindGitServer:
break
}

unifiedResources = append(unifiedResources, ui.MakeServer(site.GetName(), r, logins, enriched.RequiresRequest))
case types.DatabaseServer:
if !hasFetchedDBUsersAndNames {
Expand Down
12 changes: 11 additions & 1 deletion lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,7 @@ func TestUnifiedResourcesGet(t *testing.T) {
role := defaultRoleForNewUser(&types.UserV2{Metadata: types.Metadata{Name: username}}, loginUser)
role.SetAWSRoleARNs(types.Allow, []string{"arn:aws:iam::999999999999:role/ProdInstance"})
role.SetAppLabels(types.Allow, types.Labels{"env": []string{"prod"}})
role.SetGitHubPermissions(types.Allow, []types.GitHubPermission{{Organizations: []string{types.Wildcard}}})

// This role is used to test that DevInstance AWS Role is only available to AppServices that have env:dev label.
roleForDev, err := types.NewRole("dev-access", types.RoleSpecV6{
Expand Down Expand Up @@ -1377,6 +1378,15 @@ func TestUnifiedResourcesGet(t *testing.T) {
err = env.server.Auth().UpsertWindowsDesktop(context.Background(), win)
require.NoError(t, err)

// add git server
gitServer, err := types.NewGitHubServer(types.GitHubServerMetadata{
Organization: "org1",
Integration: "org1",
})
require.NoError(t, err)
_, err = env.server.Auth().GitServers.UpsertGitServer(context.Background(), gitServer)
require.NoError(t, err)

clusterName := env.server.ClusterName()
endpoint := pack.clt.Endpoint("webapi", "sites", clusterName, "resources")

Expand Down Expand Up @@ -1427,7 +1437,7 @@ func TestUnifiedResourcesGet(t *testing.T) {
require.NoError(t, err)
res = clusterNodesGetResponse{}
require.NoError(t, json.Unmarshal(re.Bytes(), &res))
require.Len(t, res.Items, 10)
require.Len(t, res.Items, 11)
require.Equal(t, "", res.StartKey)

// Only list valid AWS Roles for AWS Apps
Expand Down
17 changes: 17 additions & 0 deletions lib/web/ui/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ type Server struct {
AWS *AWSMetadata `json:"aws,omitempty"`
// RequireRequest indicates if a returned resource is only accessible after an access request
RequiresRequest bool `json:"requiresRequest,omitempty"`
// GitHub contains metadata for GitHub proxy severs.
GitHub *GitHubServerMetadata `json:"github,omitempty"`
}

// AWSMetadata describes the AWS metadata for instances hosted in AWS.
Expand All @@ -67,6 +69,12 @@ type AWSMetadata struct {
SubnetID string `json:"subnetId"`
}

// GitHubServerMetadata contains metadata for GitHub proxy severs.
type GitHubServerMetadata struct {
Integration string `json:"integration"`
Organization string `json:"organization"`
}

// MakeServer creates a server object for the web ui
func MakeServer(clusterName string, server types.Server, logins []string, requiresRequest bool) Server {
serverLabels := server.GetStaticLabels()
Expand Down Expand Up @@ -98,6 +106,15 @@ func MakeServer(clusterName string, server types.Server, logins []string, requir
}
}

if server.GetKind() == types.KindGitServer &&
server.GetSubKind() == types.SubKindGitHub {
if github := server.GetGitHub(); github != nil {
uiServer.GitHub = &GitHubServerMetadata{
Integration: github.Integration,
Organization: github.Organization,
}
}
}
Comment on lines +109 to +117
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this should be in its own method, something like MakeGitServer. If its inside here, it gives the impression that this is a "server type" instead of its own thing.

return uiServer
}

Expand Down
59 changes: 59 additions & 0 deletions lib/web/ui/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -590,3 +590,62 @@ func TestSortedLabels(t *testing.T) {
})
}
}

func TestMakeServer(t *testing.T) {
tests := []struct {
name string
inputServer types.Server
inputLogins []string
output Server
}{
{
name: "git server",
inputServer: makeGitServer(t, "org1"),
output: Server{
ClusterName: "cluster",
Kind: "git_server",
SubKind: "github",
Name: "org1",
Hostname: "org1.github-org",
GitHub: &GitHubServerMetadata{
Integration: "org1",
Organization: "org1",
},
// Internal labels get filtered.
Labels: []ui.Label{},
},
},
{
name: "node",
inputServer: makeTestServer(t, "server1", map[string]string{"env": "dev"}),
inputLogins: []string{"alice"},
output: Server{
ClusterName: "cluster",
Kind: "node",
SubKind: "teleport",
Name: "server1",
SSHLogins: []string{"alice"},
Labels: []ui.Label{{
Name: "env",
Value: "dev",
}},
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require.Equal(t, test.output, MakeServer("cluster", test.inputServer, test.inputLogins, false))
})
}
}

func makeGitServer(t *testing.T, org string) types.Server {
t.Helper()
server, err := types.NewGitHubServer(types.GitHubServerMetadata{
Integration: org,
Organization: org,
})
require.NoError(t, err)
server.SetName(org)
return server
}
1 change: 1 addition & 0 deletions web/packages/design/src/ResourceIcon/assets/git-dark.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions web/packages/design/src/ResourceIcon/assets/git-light.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions web/packages/design/src/ResourceIcon/icons.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ import g2 from './assets/g2.svg';
import gable from './assets/gable.svg';
import gemDark from './assets/gem-dark.svg';
import gemLight from './assets/gem-light.svg';
import gitDark from './assets/git-dark.svg';
import gitLight from './assets/git-light.svg';
import githubDark from './assets/github-dark.svg';
import githubLight from './assets/github-light.svg';
import gitlab from './assets/gitlab.svg';
Expand Down Expand Up @@ -408,6 +410,8 @@ export {
gable,
gemDark,
gemLight,
gitDark,
gitLight,
githubDark,
githubLight,
gitlab,
Expand Down
1 change: 1 addition & 0 deletions web/packages/design/src/ResourceIcon/resourceIconSpecs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export const resourceIconSpecs = {
g2: forAllThemes(i.g2),
gable: forAllThemes(i.gable),
gem: { dark: i.gemDark, light: i.gemLight },
git: { dark: i.gitDark, light: i.gitLight },
github: { dark: i.githubDark, light: i.githubLight },
gitlab: forAllThemes(i.gitlab),
gmail: forAllThemes(i.gmail),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,8 @@ function getPrettyResourceKind(kind: RequestableResourceKind): string {
return 'SAML Application';
case 'namespace':
return 'Namespace';
case 'git_server':
return 'Git';
default:
kind satisfies never;
return kind;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,6 @@ export function getEmptyResourceState(): ResourceMap {
role: {},
saml_idp_service_provider: {},
namespace: {},
git_server: {},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const kindToLabel: Record<SharedUnifiedResource['resource']['kind'], string> = {
kube_cluster: 'Kubernetes',
node: 'Server',
user_group: 'User group',
git_server: 'Git',
};

const sortFieldOptions = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { databases, moreDatabases } from 'teleport/Databases/fixtures';
import { kubes, moreKubes } from 'teleport/Kubes/fixtures';
import { desktops, moreDesktops } from 'teleport/Desktops/fixtures';
import { moreNodes, nodes } from 'teleport/Nodes/fixtures';
import { gitServers } from 'teleport/GitServers/fixtures';

import { UrlResourcesParams } from 'teleport/config';
import { ResourcesResponse } from 'teleport/services/agents';
Expand Down Expand Up @@ -70,6 +71,7 @@ const allResources = [
...moreKubes,
...moreDesktops,
...moreNodes,
...gitServers,
];

const story = ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,8 @@ function getResourcePinningSupport(
function generateUnifiedResourceKey(
resource: SharedUnifiedResource['resource']
): string {
if (resource.kind === 'node') {
return `${resource.hostname}/${resource.id}/node`.toLowerCase();
if (resource.kind === 'node' || resource.kind == 'git_server') {
return `${resource.hostname}/${resource.id}/${resource.kind}`.toLowerCase();
}
return `${resource.name}/${resource.kind}`.toLowerCase();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
Kubernetes as KubernetesIcon,
Server as ServerIcon,
Desktop as DesktopIcon,
GitHub as GitHubIcon,
} from 'design/Icon';
import { ResourceIconName } from 'design/ResourceIcon';

Expand All @@ -37,6 +38,7 @@ import {
UnifiedResourceDesktop,
UnifiedResourceKube,
UnifiedResourceUserGroup,
UnifiedResourceGitServer,
SharedUnifiedResource,
} from '../types';

Expand Down Expand Up @@ -172,6 +174,26 @@ export function makeUnifiedResourceViewItemUserGroup(
};
}

export function makeUnifiedResourceViewItemGitServer(
resource: UnifiedResourceGitServer,
ui: UnifiedResourceUi
): UnifiedResourceViewItem {
return {
name: resource.github ? resource.github.organization : resource.hostname,
SecondaryIcon: GitHubIcon,
primaryIconName: 'git',
ActionButton: ui.ActionButton,
labels: resource.labels,
cardViewProps: {
primaryDesc: 'GitHub Organization',
},
listViewProps: {
resourceType: 'GitHub Organization',
},
requiresRequest: resource.requiresRequest,
};
}

function formatNodeSubKind(subKind: NodeSubKind): string {
switch (subKind) {
case 'openssh-ec2-ice':
Expand Down Expand Up @@ -216,5 +238,7 @@ export function mapResourceToViewItem({ resource, ui }: SharedUnifiedResource) {
return makeUnifiedResourceViewItemDesktop(resource, ui);
case 'user_group':
return makeUnifiedResourceViewItemUserGroup(resource, ui);
case 'git_server':
return makeUnifiedResourceViewItemGitServer(resource, ui);
}
}
16 changes: 15 additions & 1 deletion web/packages/shared/components/UnifiedResources/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,19 @@ export type UnifiedResourceUserGroup = {
requiresRequest?: boolean;
};

export interface UnifiedResourceGitServer {
kind: 'git_server';
id: string;
hostname: string;
labels: ResourceLabel[];
subKind: 'github';
github?: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is resource.github nil?

organization: string;
integration: string;
};
requiresRequest?: boolean;
}

export type UnifiedResourceUi = {
ActionButton: React.ReactElement;
};
Expand All @@ -96,7 +109,8 @@ export type SharedUnifiedResource = {
| UnifiedResourceNode
| UnifiedResourceKube
| UnifiedResourceDesktop
| UnifiedResourceUserGroup;
| UnifiedResourceUserGroup
| UnifiedResourceGitServer;
ui: UnifiedResourceUi;
};

Expand Down
Loading
Loading