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

Add webapi v2 endpoints for creating discovery token and enrolling eks with labels #50472

Merged
merged 1 commit into from
Jan 9, 2025
Merged
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
3 changes: 0 additions & 3 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import (
"github.com/gravitational/trace"
)

// WebAPIVersion is a current webapi version
const WebAPIVersion = "v1"

const (
// SSHAuthSock is the environment variable pointing to the
// Unix socket the SSH agent is running on.
Expand Down
4 changes: 3 additions & 1 deletion lib/auth/trustedcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,9 @@ func (a *Server) sendValidateRequestToProxy(ctx context.Context, host string, va
opts = append(opts, roundtrip.HTTPClient(insecureWebClient))
}

clt, err := roundtrip.NewClient(proxyAddr.String(), teleport.WebAPIVersion, opts...)
// We do not add the version prefix since web api endpoints will
// contain differing version prefixes.
clt, err := roundtrip.NewClient(proxyAddr.String(), "" /* version prefix */, opts...)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
5 changes: 3 additions & 2 deletions lib/client/https_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (
"github.com/gravitational/trace"
"golang.org/x/net/http/httpproxy"

"github.com/gravitational/teleport"
tracehttp "github.com/gravitational/teleport/api/observability/tracing/http"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/httplib"
Expand Down Expand Up @@ -62,7 +61,9 @@ func httpTransport(insecure bool, pool *x509.CertPool) *http.Transport {

func NewWebClient(url string, opts ...roundtrip.ClientParam) (*WebClient, error) {
opts = append(opts, roundtrip.SanitizerEnabled(true))
clt, err := roundtrip.NewClient(url, teleport.WebAPIVersion, opts...)
// We do not add the version prefix since web api endpoints will contain
// differing version prefixes.
clt, err := roundtrip.NewClient(url, "" /* version prefix */, opts...)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/client/weblogin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestHostCredentialsHttpFallback(t *testing.T) {
// Start an http server (not https) so that the request only succeeds
// if the fallback occurs.
var handler http.HandlerFunc = func(w http.ResponseWriter, r *http.Request) {
if r.RequestURI != "/v1/webapi/host/credentials" {
if r.RequestURI != "/webapi/host/credentials" {
w.WriteHeader(http.StatusNotFound)
return
}
Expand Down
47 changes: 47 additions & 0 deletions lib/httplib/httplib.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package httplib

import (
"bufio"
"context"
"encoding/json"
"errors"
"log/slog"
Expand All @@ -33,6 +34,7 @@ import (
"strconv"
"strings"

"github.com/coreos/go-semver/semver"
"github.com/gravitational/roundtrip"
"github.com/gravitational/trace"
"github.com/julienschmidt/httprouter"
Expand Down Expand Up @@ -211,6 +213,51 @@ func ConvertResponse(re *roundtrip.Response, err error) (*roundtrip.Response, er
return re, trace.ReadError(re.Code(), re.Bytes())
}

// ProxyVersion describes the parts of a Proxy semver
// version in the format: major.minor.patch-preRelease
type ProxyVersion struct {
// Major is the first part of version.
Major int64 `json:"major"`
// Minor is the second part of version.
Minor int64 `json:"minor"`
// Patch is the third part of version.
Patch int64 `json:"patch"`
// PreRelease is only defined if there was a hyphen
// and a word at the end of version eg: the prerelease
// value of version 18.0.0-dev is "dev".
PreRelease string `json:"preRelease"`
// String contains the whole version.
String string `json:"string"`
}

// RouteNotFoundResponse writes a JSON error reply containing
// a not found error, a Version object, and a not found HTTP status code.
func RouteNotFoundResponse(ctx context.Context, w http.ResponseWriter, proxyVersion string) {
SetDefaultSecurityHeaders(w.Header())

errObj := &trace.TraceErr{
Err: trace.NotFound("path not found"),
}

ver, err := semver.NewVersion(proxyVersion)
if err != nil {
slog.DebugContext(ctx, "Error parsing Teleport proxy semver version", "err", err)
} else {
verObj := ProxyVersion{
Major: ver.Major,
Minor: ver.Minor,
Patch: ver.Patch,
String: proxyVersion,
PreRelease: string(ver.PreRelease),
}
fields := make(map[string]interface{})
fields["proxyVersion"] = verObj
errObj.Fields = fields
}

roundtrip.ReplyJSON(w, http.StatusNotFound, errObj)
}

// ParseBool will parse boolean variable from url query
// returns value, ok, error
func ParseBool(q url.Values, name string) (bool, bool, error) {
Expand Down
49 changes: 37 additions & 12 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,6 @@ func (h *APIHandler) Close() error {

// NewHandler returns a new instance of web proxy handler
func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {
const apiPrefix = "/" + teleport.WebAPIVersion

cfg.SetDefaults()

h := &Handler{
Expand Down Expand Up @@ -612,13 +610,31 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {
h.nodeWatcher = cfg.NodeWatcher
}

routingHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// ensure security headers are set for all responses
httplib.SetDefaultSecurityHeaders(w.Header())

// request is going to the API?
if strings.HasPrefix(r.URL.Path, apiPrefix) {
http.StripPrefix(apiPrefix, h).ServeHTTP(w, r)
const v1Prefix = "/v1"
notFoundRoutingHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Request is going to the API?
// If no routes were matched, it could be because it's a path with `v1` prefix
// (eg: the Teleport web app will call "most" endpoints with v1 prefixed).
//
// `v1` paths are not defined with `v1` prefix. If the path turns out to be prefixed
// with `v1`, it will be stripped and served again. Historically, that's how it started
// and should be kept that way to prevent breakage.
//
// v2+ prefixes will be expected by both caller and definition and will not be stripped.
if strings.HasPrefix(r.URL.Path, v1Prefix) {
pathParts := strings.Split(r.URL.Path, "/")
if len(pathParts) > 2 {
// check against known second part of path to ensure we
// aren't allowing paths like /v1/v2/webapi
// part[0] is empty space from leading slash "/"
// part[1] is the prefix "v1"
switch pathParts[2] {
case "webapi", "enterprise", "scripts", ".well-known", "workload-identity":
http.StripPrefix(v1Prefix, h).ServeHTTP(w, r)
return
}
}
httplib.RouteNotFoundResponse(r.Context(), w, teleport.Version)
return
}

Expand Down Expand Up @@ -670,11 +686,12 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) {
h.logger.ErrorContext(r.Context(), "Failed to execute index page template", "error", err)
}
} else {
http.NotFound(w, r)
httplib.RouteNotFoundResponse(r.Context(), w, teleport.Version)
return
}
})

h.NotFound = routingHandler
h.NotFound = notFoundRoutingHandler

if cfg.PluginRegistry != nil {
if err := cfg.PluginRegistry.RegisterProxyWebHandlers(h); err != nil {
Expand Down Expand Up @@ -867,8 +884,12 @@ func (h *Handler) bindDefaultEndpoints() {
h.POST("/webapi/tokens", h.WithAuth(h.upsertTokenHandle))
// used for updating a token
h.PUT("/webapi/tokens", h.WithAuth(h.upsertTokenHandle))
// used for creating tokens used during guided discover flows
// TODO(kimlisa): DELETE IN 19.0 - Replaced by /v2/webapi/token endpoint
// MUST delete with related code found in web/packages/teleport/src/services/joinToken/joinToken.ts(fetchJoinToken)
h.POST("/webapi/token", h.WithAuth(h.createTokenForDiscoveryHandle))
// used for creating tokens used during guided discover flows
// v2 endpoint processes "suggestedLabels" field
h.POST("/v2/webapi/token", h.WithAuth(h.createTokenForDiscoveryHandle))
kimlisa marked this conversation as resolved.
Show resolved Hide resolved
h.GET("/webapi/tokens", h.WithAuth(h.getTokens))
h.DELETE("/webapi/tokens", h.WithAuth(h.deleteToken))

Expand Down Expand Up @@ -1000,7 +1021,11 @@ func (h *Handler) bindDefaultEndpoints() {
h.GET("/webapi/scripts/integrations/configure/deployservice-iam.sh", h.WithLimiter(h.awsOIDCConfigureDeployServiceIAM))
h.POST("/webapi/sites/:site/integrations/aws-oidc/:name/ec2", h.WithClusterAuth(h.awsOIDCListEC2))
h.POST("/webapi/sites/:site/integrations/aws-oidc/:name/eksclusters", h.WithClusterAuth(h.awsOIDCListEKSClusters))
// TODO(kimlisa): DELETE IN 19.0 - replaced by /v2/webapi/sites/:site/integrations/aws-oidc/:name/enrolleksclusters
// MUST delete with related code found in web/packages/teleport/src/services/integrations/integrations.ts(enrollEksClusters)
h.POST("/webapi/sites/:site/integrations/aws-oidc/:name/enrolleksclusters", h.WithClusterAuth(h.awsOIDCEnrollEKSClusters))
// v2 endpoint introduces "extraLabels" field.
h.POST("/v2/webapi/sites/:site/integrations/aws-oidc/:name/enrolleksclusters", h.WithClusterAuth(h.awsOIDCEnrollEKSClusters))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: from endpoint organization perspective - Instead of mixing v1 and v2 endpoints here, what do you think of creating v2 endpoint method func (h *Handler) bindV2Endpoints() {}? See https://github.com/gravitational/teleport/blob/master/lib/web/apiserver.go#L758 for current bindDefaultEndpoints method for reference.

I do not have a strong preference either way but feel like separation will make it more cleaner as number of v2 grows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'll leave it as is, b/c we tend to group similar endpoints together and i think the binding method will separate them

flyinghermit marked this conversation as resolved.
Show resolved Hide resolved
h.POST("/webapi/sites/:site/integrations/aws-oidc/:name/ec2ice", h.WithClusterAuth(h.awsOIDCListEC2ICE))
h.POST("/webapi/sites/:site/integrations/aws-oidc/:name/deployec2ice", h.WithClusterAuth(h.awsOIDCDeployEC2ICE))
h.POST("/webapi/sites/:site/integrations/aws-oidc/:name/securitygroups", h.WithClusterAuth(h.awsOIDCListSecurityGroups))
Expand Down
116 changes: 113 additions & 3 deletions lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import (
"testing"
"time"

"github.com/coreos/go-semver/semver"
"github.com/gogo/protobuf/proto"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
Expand Down Expand Up @@ -464,7 +465,7 @@ func newWebSuiteWithConfig(t *testing.T, cfg webSuiteConfig) *WebSuite {

// Expired sessions are purged immediately
var sessionLingeringThreshold time.Duration
fs, err := newDebugFileSystem()
fs, err := NewDebugFileSystem(false)
require.NoError(t, err)

features := *modules.GetModules().Features().ToProto() // safe to dereference because ToProto creates a struct and return a pointer to it
Expand Down Expand Up @@ -3433,6 +3434,115 @@ func TestTokenGeneration(t *testing.T) {
}
}

func TestEndpointNotFoundHandling(t *testing.T) {
t.Parallel()
const username = "test-user@example.com"
// Allow user to create tokens.
roleTokenCRD, err := types.NewRole(services.RoleNameForUser(username), types.RoleSpecV6{
Allow: types.RoleConditions{
Rules: []types.Rule{
types.NewRule(types.KindToken,
[]string{types.VerbCreate}),
},
},
})
require.NoError(t, err)

env := newWebPack(t, 1)
proxy := env.proxies[0]
pack := proxy.authPack(t, username, []types.Role{roleTokenCRD})

tt := []struct {
name string
endpoint string
shouldErr bool
}{
{
name: "valid endpoint without v1 prefix",
endpoint: "webapi/token",
},
{
name: "valid endpoint with v1 prefix",
endpoint: "v1/webapi/token",
},
{
name: "valid endpoint with v2 prefix",
endpoint: "v2/webapi/token",
},
{
name: "invalid double version prefixes",
endpoint: "v1/v2/webapi/token",
shouldErr: true,
},
{
name: "route not matched version prefix",
endpoint: "v9999999/webapi/token",
shouldErr: true,
},
{
name: "non api route with prefix",
endpoint: "v1/something/else",
shouldErr: true,
},
{
name: "invalid triple version prefixes",
endpoint: "v1/v1/v1/webapi/token",
shouldErr: true,
},
{
name: "invalid just prefix",
endpoint: "v1",
shouldErr: true,
},
{
name: "invalid prefix",
endpoint: "v1s/webapi/token",
shouldErr: true,
},
}

for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
re, err := pack.clt.PostJSON(context.Background(), fmt.Sprintf("%s/%s", proxy.web.URL, tc.endpoint), types.ProvisionTokenSpecV2{
Roles: []types.SystemRole{types.RoleNode},
JoinMethod: types.JoinMethodToken,
})

if tc.shouldErr {
require.True(t, trace.IsNotFound(err))

jsonResp := struct {
Error struct {
Message string
}
Fields struct {
ProxyVersion httplib.ProxyVersion
}
}{}

require.NoError(t, json.Unmarshal(re.Bytes(), &jsonResp))
require.Equal(t, "path not found", jsonResp.Error.Message)
require.Equal(t, teleport.Version, jsonResp.Fields.ProxyVersion.String)

ver, err := semver.NewVersion(teleport.Version)
require.NoError(t, err)
require.Equal(t, ver.Major, jsonResp.Fields.ProxyVersion.Major)
require.Equal(t, ver.Minor, jsonResp.Fields.ProxyVersion.Minor)
require.Equal(t, ver.Patch, jsonResp.Fields.ProxyVersion.Patch)
require.Equal(t, string(ver.PreRelease), jsonResp.Fields.ProxyVersion.PreRelease)

} else {
require.NoError(t, err)

var responseToken nodeJoinToken
err = json.Unmarshal(re.Bytes(), &responseToken)
require.NoError(t, err)
require.Equal(t, types.JoinMethodToken, responseToken.Method)
}
})
}
}

func TestInstallDatabaseScriptGeneration(t *testing.T) {
const username = "test-user@example.com"

Expand Down Expand Up @@ -5015,7 +5125,7 @@ func TestDeleteMFA(t *testing.T) {
jar, err := cookiejar.New(nil)
require.NoError(t, err)
opts := []roundtrip.ClientParam{roundtrip.BearerAuth(pack.session.Token), roundtrip.CookieJar(jar), roundtrip.HTTPClient(client.NewInsecureWebClient())}
rclt, err := roundtrip.NewClient(proxy.webURL.String(), teleport.WebAPIVersion, opts...)
rclt, err := roundtrip.NewClient(proxy.webURL.String(), "", opts...)
require.NoError(t, err)
clt := client.WebClient{Client: rclt}
jar.SetCookies(&proxy.webURL, pack.cookies)
Expand Down Expand Up @@ -8319,7 +8429,7 @@ func createProxy(ctx context.Context, t *testing.T, proxyID string, node *regula
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, proxyServer.Close()) })

fs, err := newDebugFileSystem()
fs, err := NewDebugFileSystem(false)
require.NoError(t, err)

authID := state.IdentityID{
Expand Down
6 changes: 5 additions & 1 deletion lib/web/apiserver_test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@ import (
)

// NewDebugFileSystem returns the HTTP file system implementation
func newDebugFileSystem() (http.FileSystem, error) {
func NewDebugFileSystem(isEnterprise bool) (http.FileSystem, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need to be exported?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i forgot to push the branch that required this change in enterprise: https://github.com/gravitational/teleport.e/pull/5818, i added a enterprise not found handler test there (just in case...)

// If the location of the UI changes on disk then this will need to be updated.
assetsPath := "../../webassets/teleport"

if isEnterprise {
assetsPath = "../../../webassets/teleport"
}

// Ensure we have the built assets available before continuing.
for _, af := range []string{"index.html", "/app"} {
_, err := os.Stat(filepath.Join(assetsPath, af))
Expand Down
1 change: 1 addition & 0 deletions lib/web/integrations_awsoidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,7 @@ func (h *Handler) awsOIDCConfigureEKSIAM(w http.ResponseWriter, r *http.Request,
}

// awsOIDCEnrollEKSClusters enroll EKS clusters by installing teleport-kube-agent Helm chart on them.
// v2 endpoint introduces "extraLabels" field.
func (h *Handler) awsOIDCEnrollEKSClusters(w http.ResponseWriter, r *http.Request, p httprouter.Params, sctx *SessionContext, site reversetunnelclient.RemoteSite) (any, error) {
ctx := r.Context()

Expand Down
Loading
Loading