Skip to content

Commit

Permalink
[v16] Fix AWS App Access creation for AWS OIDC Integration when using…
Browse files Browse the repository at this point in the history
… the account number as name (#45819)

* 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 9be2689 commit f74413a
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, "foo@example.com", []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 f74413a

Please sign in to comment.