Skip to content

Commit

Permalink
Fix AWS App Access creation for AWS OIDC Integration when using the a…
Browse files Browse the repository at this point in the history
…ccount number as name (#44480)

* Fix AWS App Access creation for AWS OIDC Integration

If the integration name is digits only, the resulting address for the
application will look like this:
`123456789012.proxy.example.com:443`
This fails to parse with go's `url.Parse`.

This PR keeps the existing logic for creating the AWS App Access but
does a best effort to fix this issue by removing the `:443` from the
public proxy addr.
If another port is used, we would still get the error.

* prepend the protocol so that url.parse accepts any port number

* move change to types/app
  • Loading branch information
marcoandredinis authored Aug 26, 2024
1 parent cca9525 commit 50d50ed
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 1 deletion.
10 changes: 9 additions & 1 deletion api/types/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,15 @@ func (a *AppV3) CheckAndSetDefaults() error {
default:
return trace.BadParameter("app %q has unexpected Cloud value %q", a.GetName(), a.Spec.Cloud)
}
url, err := url.Parse(a.Spec.PublicAddr)
publicAddr := a.Spec.PublicAddr
// If the public addr has digits in a sub-host and a port, it might cause url.Parse to fail.
// Eg of a failing url: 123.teleport.example.com:3080
// This is not a valid URL, but we have been using it as such.
// To prevent this from failing, we add the `//`.
if !strings.Contains(publicAddr, "//") && strings.Contains(publicAddr, ":") {
publicAddr = "//" + publicAddr
}
url, err := url.Parse(publicAddr)
if err != nil {
return trace.BadParameter("invalid PublicAddr format: %v", err)
}
Expand Down
5 changes: 5 additions & 0 deletions api/types/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ func TestAppPublicAddrValidation(t *testing.T) {
publicAddr: "https://" + constants.KubeTeleportProxyALPNPrefix + "example.com:3080",
check: hasErrTypeBadParameter(),
},
{
name: "addr with numbers in the host",
publicAddr: "123456789012.teleport.example.com:3080",
check: hasNoErr(),
},
}

for _, tc := range tests {
Expand Down
18 changes: 18 additions & 0 deletions lib/web/integrations_awsoidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"encoding/base64"
"fmt"
"net/url"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -974,6 +975,7 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) {
require.NoError(t, err)

proxy := env.proxies[0]
proxy.handler.handler.cfg.PublicProxyAddr = strings.TrimPrefix(proxy.handler.handler.cfg.PublicProxyAddr, "https://")
proxyPublicAddr := proxy.handler.handler.cfg.PublicProxyAddr
pack := proxy.authPack(t, "[email protected]", []types.Role{roleTokenCRD})

Expand Down Expand Up @@ -1040,4 +1042,20 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) {
appServers, err = env.server.Auth().GetApplicationServers(ctx, "default")
require.NoError(t, err)
require.Empty(t, appServers)

t.Run("using the account id as name works as expected", func(t *testing.T) {
// Creating an Integration using the account id as name should not return an error if the proxy is listening at the default HTTPS port
myIntegrationWithAccountID, err := types.NewIntegrationAWSOIDC(types.Metadata{
Name: "123456789012",
}, &types.AWSOIDCIntegrationSpecV1{
RoleARN: "some-arn-role",
})
require.NoError(t, err)

_, err = env.server.Auth().CreateIntegration(ctx, myIntegrationWithAccountID)
require.NoError(t, err)
endpoint = pack.clt.Endpoint("webapi", "sites", "localhost", "integrations", "aws-oidc", "123456789012", "aws-app-access")
_, err = pack.clt.PostJSON(ctx, endpoint, nil)
require.NoError(t, err)
})
}

0 comments on commit 50d50ed

Please sign in to comment.