diff --git a/cmd/kas-registry.go b/cmd/kas-registry.go index d953b3f3..ecbc71ed 100644 --- a/cmd/kas-registry.go +++ b/cmd/kas-registry.go @@ -11,11 +11,6 @@ import ( "google.golang.org/protobuf/encoding/protojson" ) -const ( - keyRemote = "Remote" - keyCached = "Cached" -) - var policy_kasRegistryCmd *cobra.Command func policy_getKeyAccessRegistry(cmd *cobra.Command, args []string) { @@ -31,19 +26,20 @@ func policy_getKeyAccessRegistry(cmd *cobra.Command, args []string) { cli.ExitWithError(errMsg, err) } - keyType := keyCached key := &policy.PublicKey{} key.PublicKey = &policy.PublicKey_Cached{Cached: kas.GetPublicKey().GetCached()} if kas.GetPublicKey().GetRemote() != "" { - keyType = keyRemote key.PublicKey = &policy.PublicKey_Remote{Remote: kas.GetPublicKey().GetRemote()} } rows := [][]string{ {"Id", kas.GetId()}, {"URI", kas.GetUri()}, - {"PublicKey Type", keyType}, {"PublicKey", kas.GetPublicKey().String()}, } + name := kas.GetName() + if name != "" { + rows = append(rows, []string{"Name", name}) + } if mdRows := getMetadataRows(kas.GetMetadata()); mdRows != nil { rows = append(rows, mdRows...) @@ -66,24 +62,22 @@ func policy_listKeyAccessRegistries(cmd *cobra.Command, args []string) { t := cli.NewTable( cli.NewUUIDColumn(), table.NewFlexColumn("uri", "URI", cli.FlexColumnWidthFour), - table.NewFlexColumn("pk_loc", "PublicKey Location", cli.FlexColumnWidthThree), - table.NewFlexColumn("pk", "PublicKey", cli.FlexColumnWidthThree), + table.NewFlexColumn("name", "Name", cli.FlexColumnWidthThree), + table.NewFlexColumn("pk", "PublicKey", cli.FlexColumnWidthFour), ) rows := []table.Row{} for _, kas := range list { - keyType := keyCached key := policy.PublicKey{} key.PublicKey = &policy.PublicKey_Cached{Cached: kas.GetPublicKey().GetCached()} if kas.GetPublicKey().GetRemote() != "" { - keyType = keyRemote key.PublicKey = &policy.PublicKey_Remote{Remote: kas.GetPublicKey().GetRemote()} } rows = append(rows, table.NewRow(table.RowData{ - "id": kas.GetId(), - "uri": kas.GetUri(), - "pk_loc": keyType, - "pk": kas.GetPublicKey().String(), + "id": kas.GetId(), + "uri": kas.GetUri(), + "name": kas.GetName(), + "pk": kas.GetPublicKey().String(), })) } t = t.WithRows(rows) @@ -98,6 +92,7 @@ func policy_createKeyAccessRegistry(cmd *cobra.Command, args []string) { uri := c.Flags.GetRequiredString("uri") cachedJSON := c.Flags.GetOptionalString("public-keys") remote := c.Flags.GetOptionalString("public-key-remote") + name := c.Flags.GetOptionalString("name") metadataLabels = c.Flags.GetStringSlice("label", metadataLabels, cli.FlagsStringSliceOptions{Min: 0}) if cachedJSON == "" && remote == "" { @@ -106,7 +101,6 @@ func policy_createKeyAccessRegistry(cmd *cobra.Command, args []string) { } key := &policy.PublicKey{} - keyType := keyCached if cachedJSON != "" { if remote != "" { e := fmt.Errorf("only one public key is allowed. Please pass either a cached or remote public key but not both") @@ -119,29 +113,26 @@ func policy_createKeyAccessRegistry(cmd *cobra.Command, args []string) { } key = cached } else { - keyType = keyRemote key.PublicKey = &policy.PublicKey_Remote{Remote: remote} } created, err := h.CreateKasRegistryEntry( uri, key, + name, getMetadataMutable(metadataLabels), ) if err != nil { cli.ExitWithError("Failed to create Registered KAS entry", err) } - keyJSON, err := protojson.Marshal(key) - if err != nil { - cli.ExitWithError("Failed to marshal public key to JSON", err) - } - rows := [][]string{ {"Id", created.GetId()}, {"URI", created.GetUri()}, - {"PublicKey Type", keyType}, - {"PublicKey", string(keyJSON)}, + {"PublicKey", created.GetPublicKey().String()}, + } + if name != "" { + rows = append(rows, []string{"Name", name}) } if mdRows := getMetadataRows(created.GetMetadata()); mdRows != nil { rows = append(rows, mdRows...) @@ -158,19 +149,21 @@ func policy_updateKeyAccessRegistry(cmd *cobra.Command, args []string) { id := c.Flags.GetRequiredID("id") uri := c.Flags.GetOptionalString("uri") + name := c.Flags.GetOptionalString("name") cachedJSON := c.Flags.GetOptionalString("public-keys") remote := c.Flags.GetOptionalString("public-key-remote") metadataLabels = c.Flags.GetStringSlice("label", metadataLabels, cli.FlagsStringSliceOptions{Min: 0}) - if cachedJSON == "" && remote == "" && len(metadataLabels) == 0 && uri == "" { - cli.ExitWithError("No values were passed to update. Please pass at least one value to update (E.G. 'uri', 'public-keys', 'public-key-remote', 'label')", nil) + allEmpty := cachedJSON == "" && remote == "" && len(metadataLabels) == 0 && uri == "" && name == "" + if allEmpty { + cli.ExitWithError("No values were passed to update. Please pass at least one value to update (E.G. 'uri', 'name', 'public-keys', 'public-key-remote', 'label')", nil) } var pubKey *policy.PublicKey //nolint:gocritic // this is more readable than a switch statement if cachedJSON != "" && remote != "" { e := fmt.Errorf("only one public key is allowed. Please pass either a cached or remote public key but not both") - cli.ExitWithError("Issue with update flags 'public-keys' and 'public-key-remote': ", e) + cli.ExitWithError("Issue with update flags 'public-keys' and 'public-key-remote'", e) } else if cachedJSON != "" { cached := new(policy.PublicKey) err := protojson.Unmarshal([]byte(cachedJSON), cached) @@ -185,6 +178,7 @@ func policy_updateKeyAccessRegistry(cmd *cobra.Command, args []string) { updated, err := h.UpdateKasRegistryEntry( id, uri, + name, pubKey, getMetadataMutable(metadataLabels), getMetadataUpdateBehavior(), @@ -195,7 +189,12 @@ func policy_updateKeyAccessRegistry(cmd *cobra.Command, args []string) { rows := [][]string{ {"Id", id}, {"URI", updated.GetUri()}, + {"PublicKey", updated.GetPublicKey().String()}, } + if updated.GetName() != "" { + rows = append(rows, []string{"Name", updated.GetName()}) + } + if mdRows := getMetadataRows(updated.GetMetadata()); mdRows != nil { rows = append(rows, mdRows...) } @@ -248,7 +247,6 @@ func init() { listDoc := man.Docs.GetCommand("policy/kas-registry/list", man.WithRun(policy_listKeyAccessRegistries), ) - // TODO: active, inactive, any state querying [https://github.com/opentdf/otdfctl/issues/68] createDoc := man.Docs.GetCommand("policy/kas-registry/create", man.WithRun(policy_createKeyAccessRegistry), @@ -271,6 +269,12 @@ func init() { createDoc.GetDocFlag("public-key-remote").Default, createDoc.GetDocFlag("public-key-remote").Description, ) + createDoc.Flags().StringP( + createDoc.GetDocFlag("name").Name, + createDoc.GetDocFlag("name").Shorthand, + createDoc.GetDocFlag("name").Default, + createDoc.GetDocFlag("name").Description, + ) injectLabelFlags(&createDoc.Command, false) updateDoc := man.Docs.GetCommand("policy/kas-registry/update", @@ -300,6 +304,12 @@ func init() { updateDoc.GetDocFlag("public-key-remote").Default, updateDoc.GetDocFlag("public-key-remote").Description, ) + updateDoc.Flags().StringP( + updateDoc.GetDocFlag("name").Name, + updateDoc.GetDocFlag("name").Shorthand, + updateDoc.GetDocFlag("name").Default, + updateDoc.GetDocFlag("name").Description, + ) injectLabelFlags(&updateDoc.Command, true) deleteDoc := man.Docs.GetCommand("policy/kas-registry/delete", diff --git a/docs/man/policy/kas-registry/create.md b/docs/man/policy/kas-registry/create.md index 78a4c80d..f1285f94 100644 --- a/docs/man/policy/kas-registry/create.md +++ b/docs/man/policy/kas-registry/create.md @@ -17,6 +17,10 @@ command: - name: public-key-remote shorthand: r description: Remote URI where the public key can be retrieved for the KAS + - name: label + - name: name + shorthand: n + description: Optional name of the registered KAS (must be unique within policy) - name: label description: "Optional metadata 'labels' in the format: key=value" shorthand: l diff --git a/docs/man/policy/kas-registry/update.md b/docs/man/policy/kas-registry/update.md index 2fbf295f..39ab6e80 100644 --- a/docs/man/policy/kas-registry/update.md +++ b/docs/man/policy/kas-registry/update.md @@ -18,6 +18,9 @@ command: - name: public-key-remote shorthand: r description: URI of the public key of the Key Access Server + - name: name + shorthand: n + description: Optional name of the registered KAS (must be unique within policy) - name: label description: "Optional metadata 'labels' in the format: key=value" shorthand: l diff --git a/e2e/kas-registry.bats b/e2e/kas-registry.bats index ec9ceafe..394dc96f 100755 --- a/e2e/kas-registry.bats +++ b/e2e/kas-registry.bats @@ -32,12 +32,70 @@ teardown() { @test "create registration of a KAS with remote key" { URI="https://testing-create-remote.co" - run_otdfctl_kasr create --uri "$URI" -r "$REMOTE_KEY" --json + NAME="my_kas_name" + run_otdfctl_kasr create --uri "$URI" -r "$REMOTE_KEY" -n "$NAME" --json assert_output --partial "$REMOTE_KEY" assert_output --partial "$URI" + assert_output --partial "$NAME" export CREATED="$output" } +@test "create KAS registration with invalid URI - fails" { + BAD_URIS=( + "no-scheme.co" + "localhost" + "http://example.com:abc" + "https ://example.com" + ) + + for URI in "${BAD_URIS[@]}"; do + run_otdfctl_kasr create --uri "$URI" -r "$REMOTE_KEY" + assert_failure + assert_output --partial "Failed to create Registered KAS" + assert_output --partial "uri: " + done +} + +@test "create KAS registration with duplicate URI - fails" { + URI="https://testing-duplication.io" + run_otdfctl_kasr create --uri "$URI" -r "$REMOTE_KEY" + assert_success + run_otdfctl_kasr create --uri "$URI" -r "$REMOTE_KEY" + assert_failure + assert_output --partial "Failed to create Registered KAS entry" + assert_output --partial "AlreadyExists" +} + +@test "create KAS registration with duplicate name - fails" { + NAME="duplicate_name_kas" + run_otdfctl_kasr create --uri "https://testing-duplication.name.io" -r "$REMOTE_KEY" -n "$NAME" + assert_success + run_otdfctl_kasr create --uri "https://testing-duplication.name.net" -r "$REMOTE_KEY" -n "$NAME" + assert_failure + assert_output --partial "Failed to create Registered KAS entry" + assert_output --partial "AlreadyExists" +} + +@test "create KAS registration with invalid name - fails" { + URI="http://creating.kas.invalid.name/kas" + BAD_NAMES=( + "-bad-name" + "bad-name-" + "_bad_name" + "bad_name_" + "name@with!special#chars" + "$(printf 'a%.0s' {1..254})" # Generates a string of 254 'a' characters + ) + + for NAME in "${BAD_NAMES[@]}"; do + echo "testing $NAME" + run_otdfctl_kasr create --uri "$URI" -r "$REMOTE_KEY" -n "$NAME" + assert_failure + assert_output --partial "Failed to create Registered KAS" + assert_output --partial "name: " + done +} + @test "create KAS with cached key then get it" { URI="https://testing-get.gov" export CREATED=$(./otdfctl $HOST $DEBUG_LEVEL $WITH_CREDS policy kas-registry create --uri "$URI" -c "$CACHED_KEY" --json) @@ -60,23 +118,68 @@ teardown() { @test "update registered KAS" { URI="https://testing-update.net" - export CREATED=$(./otdfctl $HOST $DEBUG_LEVEL $WITH_CREDS policy kas-registry create --uri "$URI" -c "$CACHED_KEY" --json) + NAME="new-kas-testing-update" + export CREATED=$(./otdfctl $HOST $DEBUG_LEVEL $WITH_CREDS policy kas-registry create --uri "$URI" -c "$CACHED_KEY" -n "$NAME" --json) ID=$(echo "$CREATED" | jq -r '.id') - run_otdfctl_kasr update --id "$ID" -u "https://newuri.com" --public-key-remote "$REMOTE_KEY" --json + run_otdfctl_kasr update --id "$ID" -u "https://newuri.com" -n "newer-name" --public-key-remote "$REMOTE_KEY" --json assert_output --partial "$ID" assert_output --partial "https://newuri.com" assert_output --partial "$REMOTE_KEY" + assert_output --partial "newer-name" assert_output --partial "uri" refute_output --partial "pem" + refute_output --partial "$NAME" refute_output --partial "cached" } +@test "update registered KAS with invalid URI - fails" { + export CREATED=$(./otdfctl $HOST $DEBUG_LEVEL $WITH_CREDS policy kas-registry create --uri "https://bad-update.uri.kas" -c "$CACHED_KEY" --json) + ID=$(echo "$CREATED" | jq -r '.id') + BAD_URIS=( + "no-scheme.co" + "localhost" + "http://example.com:abc" + "https ://example.com" + ) + + for URI in "${BAD_URIS[@]}"; do + run_otdfctl_kasr update -i "$ID" --uri "$URI" + assert_failure + assert_output --partial "$ID" + assert_output --partial "Failed to update Registered KAS entry" + assert_output --partial "uri: " + done +} + +@test "update registered KAS with invalid name - fails" { + export CREATED=$(./otdfctl $HOST $DEBUG_LEVEL $WITH_CREDS policy kas-registry create --uri "https://bad-update.name.kas" -r "$REMOTE_KEY" --json) + ID=$(echo "$CREATED" | jq -r '.id') + BAD_NAMES=( + "-bad-name" + "bad-name-" + "_bad_name" + "bad_name_" + "name@with!special#chars" + "$(printf 'a%.0s' {1..254})" # Generates a string of 254 'a' characters + ) + + for NAME in "${BAD_NAMES[@]}"; do + run_otdfctl_kasr update --name "$NAME" -r "$REMOTE_KEY" --id "$ID" + assert_failure + assert_output --partial "Failed to update Registered KAS" + assert_output --partial "name: " + done +} + @test "list registered KASes" { URI="https://testing-list.io" - export CREATED=$(./otdfctl $HOST $DEBUG_LEVEL $WITH_CREDS policy kas-registry create --uri "$URI" -c "$CACHED_KEY" --json) + NAME="listed-kas" + export CREATED=$(./otdfctl $HOST $DEBUG_LEVEL $WITH_CREDS policy kas-registry create --uri "$URI" -c "$CACHED_KEY" -n "$NAME" --json) ID=$(echo "$CREATED" | jq -r '.id') run_otdfctl_kasr list --json assert_output --partial "$ID" assert_output --partial "uri" assert_output --partial "$URI" + assert_output --partial "name" + assert_output --partial "$NAME" } diff --git a/pkg/handlers/kas-registry.go b/pkg/handlers/kas-registry.go index 6ec2d20f..a0e903a5 100644 --- a/pkg/handlers/kas-registry.go +++ b/pkg/handlers/kas-registry.go @@ -27,10 +27,11 @@ func (h Handler) ListKasRegistryEntries() ([]*policy.KeyAccessServer, error) { } // Creates the KAS registry and then returns the KAS -func (h Handler) CreateKasRegistryEntry(uri string, publicKey *policy.PublicKey, metadata *common.MetadataMutable) (*policy.KeyAccessServer, error) { +func (h Handler) CreateKasRegistryEntry(uri string, publicKey *policy.PublicKey, name string, metadata *common.MetadataMutable) (*policy.KeyAccessServer, error) { req := &kasregistry.CreateKeyAccessServerRequest{ Uri: uri, PublicKey: publicKey, + Name: name, Metadata: metadata, } @@ -43,10 +44,11 @@ func (h Handler) CreateKasRegistryEntry(uri string, publicKey *policy.PublicKey, } // Updates the KAS registry and then returns the KAS -func (h Handler) UpdateKasRegistryEntry(id string, uri string, publickey *policy.PublicKey, metadata *common.MetadataMutable, behavior common.MetadataUpdateEnum) (*policy.KeyAccessServer, error) { +func (h Handler) UpdateKasRegistryEntry(id, uri, name string, publickey *policy.PublicKey, metadata *common.MetadataMutable, behavior common.MetadataUpdateEnum) (*policy.KeyAccessServer, error) { _, err := h.sdk.KeyAccessServerRegistry.UpdateKeyAccessServer(h.ctx, &kasregistry.UpdateKeyAccessServerRequest{ Id: id, Uri: uri, + Name: name, PublicKey: publickey, Metadata: metadata, MetadataUpdateBehavior: behavior,