Skip to content

Commit

Permalink
Allow slashes in authorize-session when using target name
Browse files Browse the repository at this point in the history
This requires specifying `**` as the selector in the proto, which will
match any segments and simply requires that the verb is the last part of
the URL, which it already is.
  • Loading branch information
jefferai committed May 30, 2023
1 parent 4d08c52 commit 04709f0
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 118 deletions.
3 changes: 2 additions & 1 deletion internal/gen/controller.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -3124,7 +3124,8 @@
"description": "The ID of the target. Required unless some combination of scope_id/scope_name and name are set.",
"in": "path",
"required": true,
"type": "string"
"type": "string",
"pattern": ".+"
},
{
"name": "body",
Expand Down
216 changes: 108 additions & 108 deletions internal/gen/controller/api/services/target_service.pb.go

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions internal/gen/controller/api/services/target_service.pb.gw.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ service TargetService {
// AuthorizeSession creates authorization information from a given Target.
rpc AuthorizeSession(AuthorizeSessionRequest) returns (AuthorizeSessionResponse) {
option (google.api.http) = {
post: "/v1/targets/{id}:authorize-session"
post: "/v1/targets/{id=**}:authorize-session"
body: "*"
response_body: "item"
};
Expand Down
30 changes: 30 additions & 0 deletions internal/tests/api/targets/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"
"strings"
"testing"
"time"

"github.com/hashicorp/boundary/api"
"github.com/hashicorp/boundary/api/credentiallibraries"
Expand All @@ -21,6 +22,7 @@ import (
"github.com/hashicorp/boundary/globals"
"github.com/hashicorp/boundary/internal/credential/vault"
"github.com/hashicorp/boundary/internal/daemon/controller"
"github.com/hashicorp/boundary/internal/daemon/worker"
"github.com/hashicorp/boundary/internal/iam"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -733,3 +735,31 @@ func TestUpdateTarget_WhitespaceInAddress(t *testing.T) {
require.NotNil(updateResult)
require.Equal("10.0.0.1", updateResult.Item.Address)
}

// TestCreateTarget_SlashesInName verifies that we can prop'ly route when using
// a target name for authorizing a session and the name contains slashes
func TestCreateTarget_SlashesInName(t *testing.T) {
require := require.New(t)
tc := controller.NewTestController(t, nil)

tw, _ := worker.NewAuthorizedPkiTestWorker(t, tc.ServersRepo(), "test", tc.ClusterAddrs())
require.NotNil(t, tw)

// Wait for worker to become ready
time.Sleep(10 * time.Second)

client := tc.Client()
token := tc.Token()
client.SetToken(token.Token)
_, proj := iam.TestScopes(t, tc.IamRepo(), iam.WithUserId(token.UserId))

tarClient := targets.NewClient(client)

tar, err := tarClient.Create(tc.Context(), "tcp", proj.GetPublicId(), targets.WithName("foo/bar"), targets.WithTcpTargetDefaultPort(2), targets.WithAddress("127.0.0.1"))
require.NoError(err)
require.NotNil(tar)

authzResult, err := tarClient.AuthorizeSession(tc.Context(), "", targets.WithName("foo/bar"), targets.WithScopeId(proj.PublicId))
require.NoError(err)
require.NotNil(authzResult)
}
9 changes: 4 additions & 5 deletions internal/tests/cluster/session_cleanup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ func testWorkerSessionCleanupSingle(burdenCase timeoutBurdenType) func(t *testin
PublicClusterAddr: pl.Addr().String(),
WorkerStatusGracePeriodDuration: controllerGracePeriod(burdenCase),
})
defer c1.Shutdown()

expectWorkers(t, c1)

Expand All @@ -118,7 +117,6 @@ func testWorkerSessionCleanupSingle(burdenCase timeoutBurdenType) func(t *testin
Logger: logger.Named("w1"),
SuccessfulStatusGracePeriodDuration: workerGracePeriod(burdenCase),
})
defer w1.Shutdown()

err = w1.Worker().WaitForNextSuccessfulStatusUpdate()
require.NoError(err)
Expand Down Expand Up @@ -228,7 +226,6 @@ func testWorkerSessionCleanupMulti(burdenCase timeoutBurdenType) func(t *testing
PublicClusterAddr: pl1.Addr().String(),
WorkerStatusGracePeriodDuration: controllerGracePeriod(burdenCase),
})
defer c1.Shutdown()

// ******************
// ** Controller 2 **
Expand All @@ -240,7 +237,6 @@ func testWorkerSessionCleanupMulti(burdenCase timeoutBurdenType) func(t *testing
PublicClusterAddr: pl2.Addr().String(),
WorkerStatusGracePeriodDuration: controllerGracePeriod(burdenCase),
})
defer c2.Shutdown()
expectWorkers(t, c1)
expectWorkers(t, c2)

Expand Down Expand Up @@ -279,7 +275,10 @@ func testWorkerSessionCleanupMulti(burdenCase timeoutBurdenType) func(t *testing
Logger: logger.Named("w1"),
SuccessfulStatusGracePeriodDuration: workerGracePeriod(burdenCase),
})
defer w1.Shutdown()
// Worker needs some extra time to become ready, otherwise for a
// currently-unknown reason the next successful status update fails
// because it's not sent before the context times out.
time.Sleep(5 * time.Second)

err = w1.Worker().WaitForNextSuccessfulStatusUpdate()
require.NoError(err)
Expand Down

0 comments on commit 04709f0

Please sign in to comment.