Skip to content

Commit

Permalink
GitHub proxy: web apis for enrolment (#51423)
Browse files Browse the repository at this point in the history
* GitHub proxy: web apis for enrolment

* github integration update

* updates

* add ut

* add overwrite test

* fix lint

* add generic field size check

* fix lint

* update log for using existing oauth id
  • Loading branch information
greedy52 committed Feb 7, 2025
1 parent 580e743 commit 596b143
Show file tree
Hide file tree
Showing 16 changed files with 918 additions and 19 deletions.
10 changes: 9 additions & 1 deletion api/types/integration_github.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ limitations under the License.
*/
package types

import "regexp"
import (
"regexp"

"github.com/gravitational/trace"
)

// validGitHubOrganizationName filters the allowed characters in GitHub
// organization name.
Expand All @@ -28,5 +32,9 @@ var validGitHubOrganizationName = regexp.MustCompile(`^[a-zA-Z0-9]([-a-zA-Z0-9]*
// ValidateGitHubOrganizationName returns an error if a given string is not a
// valid GitHub organization name.
func ValidateGitHubOrganizationName(name string) error {
const maxGitHubOrgNameLength = 39
if len(name) > maxGitHubOrgNameLength {
return trace.BadParameter("GitHub organization name cannot exceed %d characters", maxGitHubOrgNameLength)
}
return ValidateResourceName(validGitHubOrganizationName, name)
}
4 changes: 4 additions & 0 deletions api/types/integration_github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ func TestValidateGitHubOrganizationName(t *testing.T) {
name: "invalid charactersc",
checkError: require.Error,
},
{
name: "org-name-is-too-looooooooooooooooooooooong",
checkError: require.Error,
},
}

for _, test := range tests {
Expand Down
9 changes: 9 additions & 0 deletions api/types/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,18 @@ func NewEICENode(spec ServerSpecV2, labels map[string]string) (Server, error) {

// NewGitHubServer creates a new Git server for GitHub.
func NewGitHubServer(githubSpec GitHubServerMetadata) (Server, error) {
return NewGitHubServerWithName(uuid.NewString(), githubSpec)
}

// NewGitHubServerWithName creates a new Git server for GitHub with provided
// name.
func NewGitHubServerWithName(name string, githubSpec GitHubServerMetadata) (Server, error) {
server := &ServerV2{
Kind: KindGitServer,
SubKind: SubKindGitHub,
Metadata: Metadata{
Name: name,
},
Spec: ServerSpecV2{
GitHub: &githubSpec,
},
Expand Down
3 changes: 2 additions & 1 deletion lib/auth/integration/credentials/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ func GetByPurpose(ctx context.Context, ref *types.PluginStaticCredentialsRef, pu
if ref == nil {
return nil, trace.BadParameter("missing credentials ref")
}
labels := ref.Labels
// Clone the labels to avoid setting the source.
labels := maps.Clone(ref.Labels)
if len(labels) == 0 {
return nil, trace.BadParameter("missing labels from credentials ref")
}
Expand Down
47 changes: 32 additions & 15 deletions lib/auth/integration/integrationv1/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,7 @@ func (s *Service) ExportIntegrationCertAuthorities(ctx context.Context, in *inte
}
}

func buildGitHubOAuthCredentials(ig types.Integration) (*types.PluginStaticCredentialsV1, error) {
if ig.GetCredentials() == nil || ig.GetCredentials().GetIdSecret() == nil {
return nil, trace.BadParameter("GitHub integration requires OAuth ID and secret for credentials")
}

func buildGitHubOAuthCredentials(idSecret *types.PluginIdSecretCredential) (*types.PluginStaticCredentialsV1, error) {
return &types.PluginStaticCredentialsV1{
ResourceHeader: types.ResourceHeader{
Metadata: types.Metadata{
Expand All @@ -75,8 +71,8 @@ func buildGitHubOAuthCredentials(ig types.Integration) (*types.PluginStaticCrede
Spec: &types.PluginStaticCredentialsSpecV1{
Credentials: &types.PluginStaticCredentialsSpecV1_OAuthClientSecret{
OAuthClientSecret: &types.PluginStaticCredentialsOAuthClientSecret{
ClientId: ig.GetCredentials().GetIdSecret().Id,
ClientSecret: ig.GetCredentials().GetIdSecret().Secret,
ClientId: idSecret.Id,
ClientSecret: idSecret.Secret,
},
},
},
Expand Down Expand Up @@ -110,7 +106,10 @@ func (s *Service) newGitHubSSHCA(ctx context.Context) (*types.PluginStaticCreden
func (s *Service) createGitHubCredentials(ctx context.Context, ig types.Integration) error {
var creds []types.PluginStaticCredentials

if oauthCred, err := buildGitHubOAuthCredentials(ig); err != nil {
if ig.GetCredentials() == nil || ig.GetCredentials().GetIdSecret() == nil {
return trace.BadParameter("GitHub integration requires OAuth ID and secret for credentials")
}
if oauthCred, err := buildGitHubOAuthCredentials(ig.GetCredentials().GetIdSecret()); err != nil {
return trace.Wrap(err)
} else {
creds = append(creds, oauthCred)
Expand Down Expand Up @@ -153,19 +152,37 @@ func (s *Service) maybeUpdateStaticCredentials(ctx context.Context, newIg types.

// Preserve existing credentials.
if newIg.GetCredentials() == nil {
newIg.SetCredentials(oldIg.GetCredentials())
return nil
return trace.Wrap(newIg.SetCredentials(oldIg.GetCredentials()))
}

switch newIg.GetSubKind() {
case types.IntegrationSubKindGitHub:
if oauthCred, err := buildGitHubOAuthCredentials(newIg); err != nil {
oauthIdSecret := newIg.GetCredentials().GetIdSecret()
switch {
case oauthIdSecret == nil || (oauthIdSecret.Id == "" && oauthIdSecret.Secret == ""):
return trace.BadParameter("GitHub integration requires OAuth credentials")
case oauthIdSecret.Id != "" && oauthIdSecret.Secret == "":
return trace.BadParameter("missing OAuth secret for OAuth credentials")
case oauthIdSecret.Id == "" && oauthIdSecret.Secret != "":
// Special case where only secret is getting updated.
oldIdSecret, err := s.getStaticCredentialsWithPurpose(ctx, oldIg, credentials.PurposeGitHubOAuth)
if err != nil {
return trace.Wrap(err)
}
oldId, _ := oldIdSecret.GetOAuthClientSecret()
oauthIdSecret.Id = oldId
s.logger.DebugContext(ctx, "Updating integration with existing OAuth client ID", "integration", newIg.GetName())
}

oauthCred, err := buildGitHubOAuthCredentials(oauthIdSecret)
if err != nil {
return trace.Wrap(err)
}
// Copy ref from old integration and overwrite the OAuth settings.
if err := newIg.SetCredentials(oldIg.GetCredentials()); err != nil {
return trace.Wrap(err)
} else {
// Copy ref.
newIg.SetCredentials(oldIg.GetCredentials())
return trace.Wrap(s.updateStaticCredentials(ctx, newIg, oauthCred))
}
return trace.Wrap(s.updateStaticCredentials(ctx, newIg, oauthCred))
}
return nil
}
Expand Down
59 changes: 58 additions & 1 deletion lib/auth/integration/integrationv1/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,63 @@ func TestIntegrationCRUD(t *testing.T) {
},
ErrAssertion: noError,
},
{
Name: "OAuth secret only update",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindIntegration},
Verbs: []string{types.VerbUpdate, types.VerbCreate},
}}},
},
Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
oldIg, err := newGitHubIntegration(igName, "id", "secret")
if err != nil {
return trace.Wrap(err)
}
_, err = resourceSvc.CreateIntegration(ctx, &integrationpb.CreateIntegrationRequest{Integration: oldIg})
if err != nil {
return trace.Wrap(err)
}

newIg, err := newGitHubIntegration(igName, "", "new-secret")
if err != nil {
return trace.Wrap(err)
}
_, err = resourceSvc.UpdateIntegration(ctx, &integrationpb.UpdateIntegrationRequest{Integration: newIg})
return err
},
Validate: func(t *testing.T, igName string) {
mustFindGitHubCredentials(t, localClient, igName, "id", "new-secret")
},
ErrAssertion: noError,
},
{
Name: "OAuth update fails when secret is missing",
Role: types.RoleSpecV6{
Allow: types.RoleConditions{Rules: []types.Rule{{
Resources: []string{types.KindIntegration},
Verbs: []string{types.VerbUpdate, types.VerbCreate},
}}},
},
Test: func(ctx context.Context, resourceSvc *Service, igName string) error {
oldIg, err := newGitHubIntegration(igName, "id", "secret")
if err != nil {
return trace.Wrap(err)
}
_, err = resourceSvc.CreateIntegration(ctx, &integrationpb.CreateIntegrationRequest{Integration: oldIg})
if err != nil {
return trace.Wrap(err)
}

newIg, err := newGitHubIntegration(igName, "new-id", "")
if err != nil {
return trace.Wrap(err)
}
_, err = resourceSvc.UpdateIntegration(ctx, &integrationpb.UpdateIntegrationRequest{Integration: newIg})
return err
},
ErrAssertion: trace.IsBadParameter,
},

// Delete
{
Expand Down Expand Up @@ -739,7 +796,7 @@ func newGitHubIntegration(name, id, secret string) (*types.IntegrationV1, error)
return nil, trace.Wrap(err)
}

if secret != "" {
if id != "" || secret != "" {
ig.SetCredentials(&types.PluginCredentialsV1{
Credentials: &types.PluginCredentialsV1_IdSecret{
IdSecret: &types.PluginIdSecretCredential{
Expand Down
6 changes: 6 additions & 0 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1015,6 +1015,7 @@ func (h *Handler) bindDefaultEndpoints() {
h.PUT("/webapi/sites/:site/integrations/:name", h.WithClusterAuth(h.integrationsUpdate))
h.GET("/webapi/sites/:site/integrations/:name/stats", h.WithClusterAuth(h.integrationStats))
h.GET("/webapi/sites/:site/integrations/:name/discoveryrules", h.WithClusterAuth(h.integrationDiscoveryRules))
h.GET("/webapi/sites/:site/integrations/:name/ca", h.WithClusterAuth(h.integrationsExportCA))
h.DELETE("/webapi/sites/:site/integrations/:name_or_subkind", h.WithClusterAuth(h.integrationsDelete))

// GET the Microsoft Teams plugin app.zip file.
Expand Down Expand Up @@ -1142,6 +1143,11 @@ func (h *Handler) bindDefaultEndpoints() {
h.PUT("/webapi/sites/:site/lastseennotification", h.WithClusterAuth(h.notificationsUpsertLastSeenTimestamp))
// Upsert a notification state when to mark a notification as read or hide it.
h.PUT("/webapi/sites/:site/notificationstate", h.WithClusterAuth(h.notificationsUpsertNotificationState))

// Git servers
h.PUT("/webapi/sites/:site/gitservers", h.WithClusterAuth(h.gitServerCreateOrUpsert))
h.GET("/webapi/sites/:site/gitservers/:name", h.WithClusterAuth(h.gitServerGet))
h.DELETE("/webapi/sites/:site/gitservers/:name", h.WithClusterAuth(h.gitServerDelete))
}

// GetProxyClient returns authenticated auth server client
Expand Down
97 changes: 97 additions & 0 deletions lib/web/git_servers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*
* Teleport
* Copyright (C) 2025 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 <http://www.gnu.org/licenses/>.
*/

package web

import (
"net/http"

"github.com/gravitational/trace"
"github.com/julienschmidt/httprouter"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/httplib"
"github.com/gravitational/teleport/lib/reversetunnelclient"
"github.com/gravitational/teleport/lib/web/ui"
)

func (h *Handler) gitServerCreateOrUpsert(_ http.ResponseWriter, r *http.Request, _ httprouter.Params, sctx *SessionContext, site reversetunnelclient.RemoteSite) (interface{}, error) {
var req *ui.CreateGitServerRequest
if err := httplib.ReadResourceJSON(r, &req); err != nil {
return nil, trace.Wrap(err)
}
if err := req.Check(); err != nil {
return nil, trace.Wrap(err)
}

// Only GitHub server is supported. Above req.Check() performs necessary
// checks to ensure all the fields are set.
gitServer, err := types.NewGitHubServerWithName(req.Name, types.GitHubServerMetadata{
Organization: req.GitHub.Organization,
Integration: req.GitHub.Integration,
})
if err != nil {
return nil, trace.Wrap(err)
}

userClient, err := sctx.GetUserClient(r.Context(), site)
if err != nil {
return nil, trace.Wrap(err)
}
gitServiceClient := userClient.GitServerClient()

if req.Overwrite {
upserted, err := gitServiceClient.UpsertGitServer(r.Context(), gitServer)
return upserted, trace.Wrap(err)
}

created, err := gitServiceClient.CreateGitServer(r.Context(), gitServer)
return created, trace.Wrap(err)
}

func (h *Handler) gitServerGet(_ http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnelclient.RemoteSite) (interface{}, error) {
name := p.ByName("name")
if name == "" {
return nil, trace.BadParameter("git server name is required")
}

clt, err := sctx.GetUserClient(r.Context(), site)
if err != nil {
return nil, trace.Wrap(err)
}

gitServer, err := clt.GitServerClient().GetGitServer(r.Context(), name)
if err != nil {
return nil, trace.Wrap(err)
}
return ui.MakeGitServer(site.GetName(), gitServer, false), nil
}

func (h *Handler) gitServerDelete(_ http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnelclient.RemoteSite) (interface{}, error) {
name := p.ByName("name")
if name == "" {
return nil, trace.BadParameter("git server name is required")
}

clt, err := sctx.GetUserClient(r.Context(), site)
if err != nil {
return nil, trace.Wrap(err)
}

return nil, clt.GitServerClient().DeleteGitServer(r.Context(), name)
}
Loading

0 comments on commit 596b143

Please sign in to comment.