Repository: nats-io/nsc (go)

Issue:
Allow revoking users that are not in the nkey store
Currently `nsc` prevents revoking a user that is not present in the filesystem

```sh
nsc revocations delete_user -a KUBECON -n UBS7HNAO2NPNLT...
Error: user UBS7HNAO2NPNLT3DFXH2ABAJT55N2AF67UANPL7S7AFNKYMNSMBTZDMX does not exist in the current account
```



Pull request:
[Adding] ability to revoke user by nkey only
Fixes #302

Signed-off-by: Matthias Hanel <mh@synadia.com>

Patch:
diff --git a/cmd/revoke_clearactivation.go b/cmd/revoke_clearactivation.go
index 8fe9202..b2b9176 100644
--- a/cmd/revoke_clearactivation.go
+++ b/cmd/revoke_clearactivation.go
@@ -29,6 +29,7 @@ func createClearRevokeActivationCmd() *cobra.Command {
 	var params RevokeClearActivationParams
 	cmd := &cobra.Command{
 		Use:          "delete_activation",
+		Aliases:      []string{"delete-activation"},
 		Short:        "Remove an account revocation from an export",
 		Args:         MaxArgs(0),
 		SilenceUsage: true,
diff --git a/cmd/revoke_listactivation.go b/cmd/revoke_listactivation.go
index 2d51cf9..789f912 100644
--- a/cmd/revoke_listactivation.go
+++ b/cmd/revoke_listactivation.go
@@ -31,6 +31,7 @@ func createRevokeListActivationCmd() *cobra.Command {
 	var params RevokeListActivationParams
 	cmd := &cobra.Command{
 		Use:          "list_activations",
+		Aliases:      []string{"list-activations"},
 		Short:        "List account revocations for an export",
 		Args:         MaxArgs(0),
 		SilenceUsage: true,
diff --git a/cmd/revoke_listusers.go b/cmd/revoke_listusers.go
index fed5923..2518ee0 100644
--- a/cmd/revoke_listusers.go
+++ b/cmd/revoke_listusers.go
@@ -29,6 +29,7 @@ func createRevokeListUsersCmd() *cobra.Command {
 	var params RevokeListUserParams
 	cmd := &cobra.Command{
 		Use:          "list_users",
+		Aliases:      []string{"list-users"},
 		Short:        "List users revoked in an account",
 		Args:         MaxArgs(0),
 		SilenceUsage: true,
diff --git a/cmd/revokeactivation.go b/cmd/revokeactivation.go
index 4bc5149..c1a75be 100644
--- a/cmd/revokeactivation.go
+++ b/cmd/revokeactivation.go
@@ -31,6 +31,7 @@ func createRevokeActivationCmd() *cobra.Command {
 	var params RevokeActivationParams
 	cmd := &cobra.Command{
 		Use:          "add_activation",
+		Aliases:      []string{"add-activation"},
 		Short:        "Revoke an accounts access to an export",
 		Args:         MaxArgs(0),
 		SilenceUsage: true,
diff --git a/cmd/revokeclearuser.go b/cmd/revokeclearuser.go
index 79e954d..40f34ab 100644
--- a/cmd/revokeclearuser.go
+++ b/cmd/revokeclearuser.go
@@ -17,8 +17,11 @@ package cmd
 
 import (
 	"fmt"
+	"strings"
 	"time"
 
+	cli "github.com/nats-io/cliprompts/v2"
+
 	"github.com/nats-io/jwt"
 	"github.com/nats-io/nkeys"
 	"github.com/nats-io/nsc/cmd/store"
@@ -29,6 +32,7 @@ func createClearRevokeUserCmd() *cobra.Command {
 	var params ClearRevokeUserParams
 	cmd := &cobra.Command{
 		Use:          "delete_user",
+		Aliases:      []string{"delete-user"},
 		Short:        "Remove a user revocation",
 		Args:         MaxArgs(0),
 		SilenceUsage: true,
@@ -44,6 +48,7 @@ func createClearRevokeUserCmd() *cobra.Command {
 		},
 	}
 	cmd.Flags().StringVarP(&params.user, "name", "n", "", "user name")
+	cmd.Flags().StringVarP(&params.userPubKey, "user-public-key", "u", "", "public user key (use when user was generated outside of nsc)")
 	params.AccountContextParams.BindFlags(cmd)
 
 	return cmd
@@ -74,12 +79,43 @@ func (p *ClearRevokeUserParams) PreInteractive(ctx ActionCtx) error {
 	if err = p.AccountContextParams.Edit(ctx); err != nil {
 		return err
 	}
-	if p.user == "" {
-		p.user, err = ctx.StoreCtx().PickUser(p.AccountContextParams.Name)
-		if err != nil {
-			return err
+
+	st := ctx.StoreCtx().Store
+	accClaim, err := st.ReadAccountClaim(p.AccountContextParams.Name)
+	if err != nil || len(accClaim.Revocations) == 0 {
+		return err
+	}
+	keyToUserName := map[string]string{}
+	users, err := st.ListEntries(store.Accounts, p.AccountContextParams.Name, store.Users)
+	if err != nil {
+		return err
+	}
+	for _, user := range users {
+		if uc, err := st.ReadUserClaim(p.AccountContextParams.Name, user); err == nil {
+			if _, ok := accClaim.Revocations[uc.Subject]; ok {
+				keyToUserName[uc.Subject] = uc.Name
+			}
 		}
 	}
+
+	choices := make([]string, 1, len(accClaim.Revocations)+1)
+	choices[0] = "none"
+	for userNkey := range accClaim.Revocations {
+		choice := ""
+		if name, ok := keyToUserName[userNkey]; ok {
+			choice = fmt.Sprintf("%s: %s", userNkey, name)
+		} else {
+			choice = userNkey
+		}
+		choices = append(choices, choice)
+	}
+	if choice, err := cli.Select("Revoked users to remove", "", choices); err != nil {
+		return err
+	} else if choice == 0 {
+		return fmt.Errorf("no Nkey selected")
+	} else {
+		p.userPubKey = strings.Split(choices[choice], ":")[0]
+	}
 	if err := p.SignerParams.Edit(ctx); err != nil {
 		return err
 	}
@@ -93,15 +129,17 @@ func (p *ClearRevokeUserParams) Load(ctx ActionCtx) error {
 		return err
 	}
 
-	if p.user == "" {
+	if p.user == "" && p.userPubKey == "" {
 		n := ctx.StoreCtx().DefaultUser(p.AccountContextParams.Name)
 		if n != nil {
 			p.user = *n
 		}
 	}
-
-	if p.user == "" {
-		return fmt.Errorf("user is required")
+	if p.userPubKey == "" && p.user == "" {
+		return fmt.Errorf("user or user-public-key are required")
+	}
+	if p.userPubKey != "" && p.user != "" {
+		return fmt.Errorf("user and user-public-key are mutually exclusive")
 	}
 
 	p.claim, err = ctx.StoreCtx().Store.ReadAccountClaim(p.AccountContextParams.Name)
@@ -109,6 +147,9 @@ func (p *ClearRevokeUserParams) Load(ctx ActionCtx) error {
 		return err
 	}
 
+	if p.userPubKey != "" {
+		return nil
+	}
 	userClaim, err := ctx.StoreCtx().Store.ReadUserClaim(p.AccountContextParams.Name, p.user)
 	if err != nil {
 		return err
@@ -124,7 +165,9 @@ func (p *ClearRevokeUserParams) Load(ctx ActionCtx) error {
 }
 
 func (p *ClearRevokeUserParams) Validate(ctx ActionCtx) error {
-
+	if !nkeys.IsValidPublicUserKey(p.userPubKey) {
+		return fmt.Errorf("user-public-key %q is not a valid public user nkey", p.userPubKey)
+	}
 	if err := p.SignerParams.Resolve(ctx); err != nil {
 		return err
 	}
diff --git a/cmd/revokeuser.go b/cmd/revokeuser.go
index dfcf6a0..ef9009a 100644
--- a/cmd/revokeuser.go
+++ b/cmd/revokeuser.go
@@ -31,6 +31,7 @@ func createRevokeUserCmd() *cobra.Command {
 	var params RevokeUserParams
 	cmd := &cobra.Command{
 		Use:          "add_user",
+		Aliases:      []string{"add-user"},
 		Short:        "Revoke a user",
 		Args:         MaxArgs(0),
 		SilenceUsage: true,
@@ -40,6 +41,7 @@ func createRevokeUserCmd() *cobra.Command {
 	}
 	cmd.Flags().StringVarP(&params.user, "name", "n", "", "user name")
 	cmd.Flags().IntVarP(&params.at, "at", "", 0, "revokes all user credentials created before a Unix timestamp ('0' is treated as now)")
+	cmd.Flags().StringVarP(&params.userPubKey, "user-public-key", "u", "", "public user key (use when user was generated outside of nsc)")
 
 	params.AccountContextParams.BindFlags(cmd)
 
@@ -75,20 +77,26 @@ func (p *RevokeUserParams) canParse(s string) error {
 }
 
 func (p *RevokeUserParams) PreInteractive(ctx ActionCtx) error {
-	var err error
-
-	if err = p.AccountContextParams.Edit(ctx); err != nil {
+	if err := p.AccountContextParams.Edit(ctx); err != nil {
 		return err
 	}
-	if p.user == "" {
-		p.user, err = ctx.StoreCtx().PickUser(p.AccountContextParams.Name)
-		if err != nil {
-			return err
+	byName, err := cli.Confirm("Do you want to revoke a user (known to nsc) by name?", true)
+	if err != nil {
+		return err
+	}
+	if byName {
+		if p.user == "" {
+			p.user, err = ctx.StoreCtx().PickUser(p.AccountContextParams.Name)
+			if err != nil {
+				return err
+			}
 		}
+	} else if p.userPubKey, err = cli.Prompt("Enter user public nkey", ""); err != nil {
+		return err
 	}
 	if p.at == 0 {
 		at := fmt.Sprintf("%d", p.at)
-		at, err = cli.Prompt("revoke all credentials created before (0 is now)", at, cli.Val(p.canParse))
+		at, err := cli.Prompt("revoke all credentials created before (0 is now)", at, cli.Val(p.canParse))
 		if err != nil {
 			return err
 		}
@@ -100,6 +108,7 @@ func (p *RevokeUserParams) PreInteractive(ctx ActionCtx) error {
 	if err := p.SignerParams.Edit(ctx); err != nil {
 		return err
 	}
+
 	return nil
 }
 
@@ -110,22 +119,25 @@ func (p *RevokeUserParams) Load(ctx ActionCtx) error {
 		return err
 	}
 
-	if p.user == "" {
+	if p.user == "" && p.userPubKey == "" {
 		n := ctx.StoreCtx().DefaultUser(p.AccountContextParams.Name)
 		if n != nil {
 			p.user = *n
 		}
 	}
-
-	if p.user == "" {
-		return fmt.Errorf("user is required")
+	if p.userPubKey == "" && p.user == "" {
+		return fmt.Errorf("user or user-public-key are required")
+	}
+	if p.userPubKey != "" && p.user != "" {
+		return fmt.Errorf("user and user-public-key are mutually exclusive")
 	}
-
 	p.claim, err = ctx.StoreCtx().Store.ReadAccountClaim(p.AccountContextParams.Name)
 	if err != nil {
 		return err
 	}
-
+	if p.userPubKey != "" {
+		return nil
+	}
 	userClaim, err := ctx.StoreCtx().Store.ReadUserClaim(p.AccountContextParams.Name, p.user)
 	if err != nil {
 		return err
@@ -141,7 +153,9 @@ func (p *RevokeUserParams) Load(ctx ActionCtx) error {
 }
 
 func (p *RevokeUserParams) Validate(ctx ActionCtx) error {
-
+	if !nkeys.IsValidPublicUserKey(p.userPubKey) {
+		return fmt.Errorf("user-public-key %q is not a valid public user nkey", p.userPubKey)
+	}
 	if err := p.SignerParams.Resolve(ctx); err != nil {
 		return err
 	}


Test patch:
diff --git a/cmd/revokeclearuser_test.go b/cmd/revokeclearuser_test.go
index 713d605..cb3bbe4 100644
--- a/cmd/revokeclearuser_test.go
+++ b/cmd/revokeclearuser_test.go
@@ -77,9 +77,13 @@ func TestRevokeClearUserInteractive(t *testing.T) {
 	require.True(t, ac.IsRevokedAt(u.Subject, time.Unix(0, 0)))
 	require.False(t, ac.IsRevokedAt(u.Subject, time.Now().Add(1*time.Hour)))
 
-	input := []interface{}{0, 0}
+	input := []interface{}{0, 1}
 	cmd := createClearRevokeUserCmd()
 	HoistRootFlags(cmd)
 	_, _, err = ExecuteInteractiveCmd(cmd, input, "-i")
 	require.NoError(t, err)
+
+	ac, err = ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.Len(t, ac.Revocations, 0)
 }
diff --git a/cmd/revokeuser_test.go b/cmd/revokeuser_test.go
index 0223c21..de07c38 100644
--- a/cmd/revokeuser_test.go
+++ b/cmd/revokeuser_test.go
@@ -118,6 +118,8 @@ func Test_RevokeUserAccountNameRequired(t *testing.T) {
 	require.Len(t, ac.Revocations, 0)
 }
 
+const keyToRevoke = "UCLMBZ5CBDRDG2TAYOJK23U7IGKPTTW7DTWNOP4TUW4PAB3GRUSKXG3N"
+
 func TestRevokeUserInteractive(t *testing.T) {
 	ts := NewTestStore(t, "test")
 	defer ts.Done(t)
@@ -129,7 +131,7 @@ func TestRevokeUserInteractive(t *testing.T) {
 	ts.AddUser(t, "B", "one")
 	ts.AddUser(t, "B", "two")
 
-	input := []interface{}{0, 0, "0"}
+	input := []interface{}{0, true, 0, "0"}
 	cmd := createRevokeUserCmd()
 	HoistRootFlags(cmd)
 	_, _, err := ExecuteInteractiveCmd(cmd, input, "-i")
@@ -141,6 +143,55 @@ func TestRevokeUserInteractive(t *testing.T) {
 
 	u, err := ts.Store.ReadUserClaim("A", "one")
 	require.NoError(t, err)
+	uPub := u.Subject
+	require.True(t, ac.IsRevokedAt(uPub, time.Unix(0, 0)))
+	require.False(t, ac.IsRevokedAt(uPub, time.Now().Add(1*time.Hour)))
+
+	cmd = createRevokeUserCmd()
+	HoistRootFlags(cmd)
+	input = []interface{}{0, false, keyToRevoke, "0"}
+	_, _, err = ExecuteInteractiveCmd(cmd, input, "-i")
+	require.NoError(t, err)
+
+	ac, err = ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.True(t, ac.Revocations.IsRevoked(keyToRevoke, time.Unix(0, 0)))
+	require.False(t, ac.Revocations.IsRevoked(keyToRevoke, time.Now().Add(1*time.Hour)))
+	require.Len(t, ac.Revocations, 2)
+}
+
+func TestRevokeUserByNkey(t *testing.T) {
+	ts := NewTestStore(t, "test")
+	defer ts.Done(t)
+
+	ts.AddAccount(t, "A")
+	ts.AddUser(t, "A", "one")
+	ts.AddUser(t, "A", "two")
+
+	u, err := ts.Store.ReadUserClaim("A", "one")
+	require.NoError(t, err)
+
+	cmd := createRevokeUserCmd()
+	HoistRootFlags(cmd)
+	_, _, err = ExecuteCmd(cmd, "-u", u.Subject)
+	require.NoError(t, err)
+	_, _, err = ExecuteCmd(cmd, "-u", keyToRevoke)
+	require.NoError(t, err)
+
+	ac, err := ts.Store.ReadAccountClaim("A")
+	require.NoError(t, err)
+	require.Len(t, ac.Revocations, 2)
+	require.True(t, ac.Revocations.IsRevoked(u.Subject, time.Unix(0, 0)))
+	require.True(t, ac.Revocations.IsRevoked(keyToRevoke, time.Unix(0, 0)))
+
+	// make sure one is expired
+	u, err = ts.Store.ReadUserClaim("A", "one")
+	require.NoError(t, err)
 	require.True(t, ac.IsRevokedAt(u.Subject, time.Unix(0, 0)))
 	require.False(t, ac.IsRevokedAt(u.Subject, time.Now().Add(1*time.Hour)))
+	// make sure two is not expired
+	u, err = ts.Store.ReadUserClaim("A", "two")
+	require.NoError(t, err)
+	require.False(t, ac.IsRevokedAt(u.Subject, time.Unix(0, 0)))
+	require.False(t, ac.IsRevokedAt(u.Subject, time.Now().Add(1*time.Hour)))
 }