Skip to content

Commit

Permalink
prepend the protocol so that url.parse accepts any port number
Browse files Browse the repository at this point in the history
  • Loading branch information
marcoandredinis committed Jul 26, 2024
1 parent e4b5ea9 commit d95593f
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 27 deletions.
9 changes: 2 additions & 7 deletions lib/web/integrations_awsoidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -951,13 +951,8 @@ func (h *Handler) awsOIDCCreateAWSAppAccess(w http.ResponseWriter, r *http.Reque

getUserGroupLookup := h.getUserGroupLookup(r.Context(), clt)

// If the integration is numbers only (eg aws account id)
// Eg 123456789012.proxy.example.com:443
// The public addr fails to parse with "first path segment in URL cannot contain colon"
// See https://datatracker.ietf.org/doc/html/rfc3986#section-3.3
// Removing the port, if it is the default for HTTPS, ensures this will work for Cloud and most self-hosted deployments.
proxyPublicAddr := strings.TrimSuffix(h.PublicProxyAddr(), ":443")
publicAddr := libutils.DefaultAppPublicAddr(integrationName, proxyPublicAddr)
// Prepending the protocol ensures that url.Parse works even if the integration name only contains digits (eg aws account id).
publicAddr := "https://" + libutils.DefaultAppPublicAddr(integrationName, h.PublicProxyAddr())

appServer, err := types.NewAppServerForAWSOIDCIntegration(integrationName, h.cfg.HostUUID, publicAddr)
if err != nil {
Expand Down
25 changes: 5 additions & 20 deletions lib/web/integrations_awsoidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1016,7 +1016,7 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) {
URI: "https://console.aws.amazon.com",
Integration: "my-integration",
Cloud: "AWS",
PublicAddr: "my-integration." + proxyPublicAddr,
PublicAddr: "https://my-integration." + proxyPublicAddr,
},
},
},
Expand All @@ -1035,7 +1035,7 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) {
require.NoError(t, err)
require.Empty(t, appServers)

t.Run("using the account id as name", func(t *testing.T) {
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",
Expand All @@ -1046,23 +1046,8 @@ func TestAWSOIDCAppAccessAppServerCreationDeletion(t *testing.T) {

_, err = env.server.Auth().CreateIntegration(ctx, myIntegrationWithAccountID)
require.NoError(t, err)

t.Run("returns an error if the proxy is not using the default port for HTTPS", func(t *testing.T) {
endpoint = pack.clt.Endpoint("webapi", "sites", "localhost", "integrations", "aws-oidc", "123456789012", "aws-app-access")
_, err = pack.clt.PostJSON(ctx, endpoint, nil)
require.Error(t, err)
})

t.Run("does not return an error, otherwise", func(t *testing.T) {
initialAddr := proxy.handler.handler.cfg.PublicProxyAddr
t.Cleanup(func() {
proxy.handler.handler.cfg.PublicProxyAddr = initialAddr
})

proxy.handler.handler.cfg.PublicProxyAddr = "proxy.example.com:443"
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)
})
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 d95593f

Please sign in to comment.