From 167a53e2f0b1484c00d6816885dc92bf2ec1fff2 Mon Sep 17 00:00:00 2001 From: Valery Fouques Date: Sun, 11 Jan 2026 23:24:31 +0100 Subject: [PATCH] use scoped signing keys for users Signed-off-by: Valery Fouques --- api/v1alpha1/account_types.go | 3 + api/v1alpha1/user_types.go | 28 ++ api/v1alpha1/zz_generated.deepcopy.go | 5 + charts/nauth-crds/crds/nauth.io_accounts.yaml | 5 + charts/nauth-crds/crds/nauth.io_users.yaml | 4 + .../resources/crds/nauth.io_accounts.yaml | 5 + .../nauth/resources/crds/nauth.io_users.yaml | 4 + cmd/main.go | 17 +- internal/account/account.go | 25 +- internal/account/mocks_test.go | 2 +- internal/controller/account.go | 12 +- internal/controller/user.go | 8 + internal/k8s/account.go | 2 +- internal/k8s/errors.go | 1 + internal/k8s/labels.go | 2 + internal/k8s/secret/secret.go | 9 +- internal/k8s/secret/secret_test.go | 8 +- internal/k8s/secret_types.go | 1 + internal/nats/client.go | 9 + internal/user/claims.go | 4 + internal/user/constants.go | 5 + internal/user/mocks_test.go | 2 +- internal/user/user.go | 328 +++++++++++++++++- internal/user/user_test.go | 107 +++++- .../00-assert-operator-secrets.yaml | 2 +- 25 files changed, 545 insertions(+), 53 deletions(-) create mode 100644 internal/nats/client.go create mode 100644 internal/user/constants.go diff --git a/api/v1alpha1/account_types.go b/api/v1alpha1/account_types.go index 7dc8e72..34a6f27 100644 --- a/api/v1alpha1/account_types.go +++ b/api/v1alpha1/account_types.go @@ -58,6 +58,9 @@ type AccountClaims struct { type AccountStatus struct { // +optional Claims AccountClaims `json:"claims,omitempty"` + // +listType=set + // +optional + SigningKeys []string `json:"signingKeys,omitempty"` // +listType=map // +listMapKey=type // +patchStrategy=merge diff --git a/api/v1alpha1/user_types.go b/api/v1alpha1/user_types.go index 8cd0c00..d54c294 100644 --- a/api/v1alpha1/user_types.go +++ b/api/v1alpha1/user_types.go @@ -17,7 +17,10 @@ limitations under the License. package v1alpha1 import ( + "crypto/md5" + "encoding/hex" "fmt" + "io" "reflect" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -30,6 +33,9 @@ import ( type UserSpec struct { // AccountName references the account used to create the user. AccountName string `json:"accountName"` + // UseSigningKey generates a scopping signing key for the user. + // +Optional + UseSigningKey bool `json:"useSigningKey,omitempty"` // DisplayName is an optional name for the NATS resource representing the user. May be derived if absent. // +optional DisplayName string `json:"displayName,omitempty"` @@ -96,6 +102,28 @@ func (u *User) GetUserSecretName() string { return fmt.Sprintf("%s-nats-user-creds", u.GetName()) } +const ( + SecretNameUserSignTemplate = "%s-u-sign-%s" // #nosec G101 +) + +func mustGenerateShortHashFromID(ID string) string { + hasher := md5.New() + _, err := io.WriteString(hasher, ID) + if err != nil { + panic(fmt.Sprintf("failed to generate hash from ID: %v", err)) + } + + hash := hex.EncodeToString(hasher.Sum(nil)) + if len(hash) > 6 { + return hash[:6] + } + return hash +} + +func (u *User) GetUserSigningKeySecretName() string { + return fmt.Sprintf(SecretNameUserSignTemplate, u.GetName(), mustGenerateShortHashFromID(u.GetName())) +} + // +kubebuilder:object:root=true // UserList contains a list of User. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 30b0f59..84fe11e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -247,6 +247,11 @@ func (in *AccountSpec) DeepCopy() *AccountSpec { func (in *AccountStatus) DeepCopyInto(out *AccountStatus) { *out = *in in.Claims.DeepCopyInto(&out.Claims) + if in.SigningKeys != nil { + in, out := &in.SigningKeys, &out.SigningKeys + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) diff --git a/charts/nauth-crds/crds/nauth.io_accounts.yaml b/charts/nauth-crds/crds/nauth.io_accounts.yaml index 9683fe1..ffd0eae 100644 --- a/charts/nauth-crds/crds/nauth.io_accounts.yaml +++ b/charts/nauth-crds/crds/nauth.io_accounts.yaml @@ -488,6 +488,11 @@ spec: name: type: string type: object + signingKeys: + items: + type: string + type: array + x-kubernetes-list-type: set type: object type: object served: true diff --git a/charts/nauth-crds/crds/nauth.io_users.yaml b/charts/nauth-crds/crds/nauth.io_users.yaml index d64e7dd..71ad443 100644 --- a/charts/nauth-crds/crds/nauth.io_users.yaml +++ b/charts/nauth-crds/crds/nauth.io_users.yaml @@ -117,6 +117,10 @@ spec: type: array type: object type: object + useSigningKey: + description: UseSigningKey generates a scopping signing key for the + user. + type: boolean userLimits: properties: src: diff --git a/charts/nauth/resources/crds/nauth.io_accounts.yaml b/charts/nauth/resources/crds/nauth.io_accounts.yaml index 9683fe1..ffd0eae 100644 --- a/charts/nauth/resources/crds/nauth.io_accounts.yaml +++ b/charts/nauth/resources/crds/nauth.io_accounts.yaml @@ -488,6 +488,11 @@ spec: name: type: string type: object + signingKeys: + items: + type: string + type: array + x-kubernetes-list-type: set type: object type: object served: true diff --git a/charts/nauth/resources/crds/nauth.io_users.yaml b/charts/nauth/resources/crds/nauth.io_users.yaml index d64e7dd..71ad443 100644 --- a/charts/nauth/resources/crds/nauth.io_users.yaml +++ b/charts/nauth/resources/crds/nauth.io_users.yaml @@ -117,6 +117,10 @@ spec: type: array type: object type: object + useSigningKey: + description: UseSigningKey generates a scopping signing key for the + user. + type: boolean userLimits: properties: src: diff --git a/cmd/main.go b/cmd/main.go index fcd9962..4fe26b3 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -19,6 +19,7 @@ package main import ( "crypto/tls" "flag" + "fmt" "os" "path/filepath" @@ -71,6 +72,7 @@ func main() { var probeAddr string var secureMetrics bool var enableHTTP2 bool + var devMode bool var tlsOpts []func(*tls.Config) flag.StringVar(&namespace, "namespace", "", "Limits the scope of nauth to a single namespace. "+ "If not specified, all namespaces will be watched.") @@ -91,9 +93,16 @@ func main() { flag.StringVar(&metricsCertKey, "metrics-cert-key", "tls.key", "The name of the metrics server key file.") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + flag.BoolVar(&devMode, "dev", false, "Run in development mode") + + // Detect if we run inside the cluster + _, err := os.Stat("/var/run/secrets/kubernetes.io/serviceaccount/token") + inCluster := err == nil opts := zap.Options{ - Development: true, + Development: devMode || !inCluster, } + + fmt.Println(opts.Development) opts.BindFlags(flag.CommandLine) flag.Parse() @@ -244,7 +253,11 @@ func main() { os.Exit(1) } - userManager := user.NewManager(accountClient, secretClient) + userManager := user.NewManager( + accountClient, + natsClient, + secretClient, + ) userReconciler := controller.NewUserReconciler( mgr.GetClient(), mgr.GetScheme(), diff --git a/internal/account/account.go b/internal/account/account.go index 77a1dad..e5874cb 100644 --- a/internal/account/account.go +++ b/internal/account/account.go @@ -9,12 +9,14 @@ import ( "io" "log" "os" + "strings" "sync" natsv1alpha1 "github.com/WirelessCar/nauth/api/v1alpha1" "github.com/WirelessCar/nauth/internal/controller" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/k8s/secret" + "github.com/WirelessCar/nauth/internal/nats" "github.com/nats-io/jwt/v2" "github.com/nats-io/nkeys" v1 "k8s.io/api/core/v1" @@ -24,7 +26,7 @@ import ( type SecretClient interface { // TODO: Keys created should be immutable - Apply(ctx context.Context, owner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string) error + Apply(ctx context.Context, owner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string, update bool) error Get(ctx context.Context, namespace string, name string) (map[string]string, error) GetByLabels(ctx context.Context, namespace string, labels map[string]string) (*v1.SecretList, error) Delete(ctx context.Context, namespace string, name string) error @@ -32,26 +34,18 @@ type SecretClient interface { Label(ctx context.Context, namespace, name string, labels map[string]string) error } -type NatsClient interface { - EnsureConnected(namespace string) error - Disconnect() - LookupAccountJWT(string) (string, error) - UploadAccountJWT(jwt string) error - DeleteAccountJWT(jwt string) error -} - type AccountGetter interface { Get(ctx context.Context, accountRefName string, namespace string) (account *natsv1alpha1.Account, err error) } type Manager struct { accounts AccountGetter - natsClient NatsClient + natsClient nats.NatsClient secretClient SecretClient nauthNamespace string } -func NewManager(accounts AccountGetter, natsClient NatsClient, secretClient SecretClient, opts ...func(*Manager)) *Manager { +func NewManager(accounts AccountGetter, natsClient nats.NatsClient, secretClient SecretClient, opts ...func(*Manager)) *Manager { manager := &Manager{ accounts: accounts, natsClient: natsClient, @@ -67,11 +61,11 @@ func NewManager(accounts AccountGetter, natsClient NatsClient, secretClient Secr if err != nil { log.Fatalf("Failed create account manager. Failed to read namespace: %v", err) } - manager.nauthNamespace = string(controllerNamespace) + manager.nauthNamespace = strings.TrimSpace(string(controllerNamespace)) } if !manager.valid() { - log.Fatalf("Failed to crate Account manager. Missing required fields.") + log.Fatalf("Failed to create Account manager. Missing required fields.") return nil } @@ -124,7 +118,7 @@ func (a *Manager) Create(ctx context.Context, state *natsv1alpha1.Account) (*con accountSeed, _ := accountKeyPair.Seed() accountSecretValue := map[string]string{k8s.DefaultSecretKeyName: string(accountSeed)} - if err := a.secretClient.Apply(ctx, nil, accountSecretMeta, accountSecretValue); err != nil { + if err := a.secretClient.Apply(ctx, nil, accountSecretMeta, accountSecretValue, true); err != nil { return nil, err } @@ -142,7 +136,7 @@ func (a *Manager) Create(ctx context.Context, state *natsv1alpha1.Account) (*con accountSigningSeed, _ := accountSigningKeyPair.Seed() accountSignSeedSecretValue := map[string]string{k8s.DefaultSecretKeyName: string(accountSigningSeed)} - if err := a.secretClient.Apply(ctx, nil, accountSigningSecretMeta, accountSignSeedSecretValue); err != nil { + if err := a.secretClient.Apply(ctx, nil, accountSigningSecretMeta, accountSignSeedSecretValue, true); err != nil { return nil, err } @@ -151,6 +145,7 @@ func (a *Manager) Create(ctx context.Context, state *natsv1alpha1.Account) (*con natsClaims, err := newClaimsBuilder(ctx, getDisplayName(state), state.Spec, accountPublicKey, a.accounts). signingKey(accountSigningPublicKey). build() + if err != nil { accountName := fmt.Sprintf("%s-%s", state.GetNamespace(), state.GetName()) return nil, fmt.Errorf("failed to build NATS account claims for %s: %w", accountName, err) diff --git a/internal/account/mocks_test.go b/internal/account/mocks_test.go index dd05404..55ab5ce 100644 --- a/internal/account/mocks_test.go +++ b/internal/account/mocks_test.go @@ -23,7 +23,7 @@ type SecretStorerMock struct { } // ApplySecret implements ports.SecretStorer. -func (s *SecretStorerMock) Apply(ctx context.Context, secretOwner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string) error { +func (s *SecretStorerMock) Apply(ctx context.Context, secretOwner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string, update bool) error { args := s.Called(ctx, secretOwner, meta, valueMap) return args.Error(0) } diff --git a/internal/controller/account.go b/internal/controller/account.go index 8dc71dd..faea246 100644 --- a/internal/controller/account.go +++ b/internal/controller/account.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "os" + "time" "github.com/WirelessCar/nauth/internal/k8s" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -85,7 +86,7 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct err := r.Get(ctx, req.NamespacedName, natsAccount) if err != nil { if apierrors.IsNotFound(err) { - log.Info("resource not found. Ignoring since object must be deleted") + // log.Info("resource not found. Ignoring since object must be deleted") return ctrl.Result{}, nil } // Error reading the object - requeue the request. @@ -127,11 +128,10 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } if len(userList.Items) > 0 { - return r.reporter.error( - ctx, - natsAccount, - fmt.Errorf("cannot delete an account with associated users, found %d users", len(userList.Items)), - ) + log.Info("Cannot delete an account with associated users, found", "account", natsAccount, "users", len(userList.Items)) + return ctrl.Result{ + RequeueAfter: 2 * time.Second, + }, nil } if controllerutil.ContainsFinalizer(natsAccount, controllerAccountFinalizer) { diff --git a/internal/controller/user.go b/internal/controller/user.go index 328d544..33d3a9a 100644 --- a/internal/controller/user.go +++ b/internal/controller/user.go @@ -18,8 +18,10 @@ package controller import ( "context" + "errors" "fmt" "os" + "time" "k8s.io/client-go/tools/record" @@ -36,6 +38,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" natsv1alpha1 "github.com/WirelessCar/nauth/api/v1alpha1" + "github.com/WirelessCar/nauth/internal/k8s" ) type UserManager interface { @@ -147,6 +150,10 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. } if err := r.userManager.CreateOrUpdate(ctx, user); err != nil { + if errors.Is(err, k8s.NoErrRetryLater) { + log.Info("User creation was not successful", "reason", err) + return ctrl.Result{RequeueAfter: 2 * time.Second}, nil + } return r.reporter.error(ctx, user, err) } @@ -169,6 +176,7 @@ func (r *UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. return ctrl.Result{}, err } + log.Info("User created or updated") return r.reporter.status(ctx, user) } diff --git a/internal/k8s/account.go b/internal/k8s/account.go index 12b5d71..039d193 100644 --- a/internal/k8s/account.go +++ b/internal/k8s/account.go @@ -34,7 +34,7 @@ func (a *AccountClient) Get(ctx context.Context, accountRefName string, namespac } if !isReady(account) { - log.Error(err, "account is not ready", "namespace", namespace, "accountName", accountRefName) + log.Info("account is not ready", "namespace", namespace, "accountName", accountRefName) return nil, errors.Join(ErrAccountNotReady, err) } diff --git a/internal/k8s/errors.go b/internal/k8s/errors.go index bf2b519..25cac67 100644 --- a/internal/k8s/errors.go +++ b/internal/k8s/errors.go @@ -6,4 +6,5 @@ var ( ErrNoAccountFound = errors.New("no account found") ErrAccountNotReady = errors.New("account is not ready") ErrNotFound = errors.New("not found") + NoErrRetryLater = errors.New("try again later") ) diff --git a/internal/k8s/labels.go b/internal/k8s/labels.go index c52eb7d..629d6bc 100644 --- a/internal/k8s/labels.go +++ b/internal/k8s/labels.go @@ -5,6 +5,8 @@ const ( LabelAccountSignedBy = "account.nauth.io/signed-by" LabelUserID = "user.nauth.io/id" LabelUserAccountID = "user.nauth.io/account-id" + LabelUserSigningKeyID = "user.nauth.io/signing-key-id" + LabelUserName = "user.nauth.io/username" LabelUserSignedBy = "user.nauth.io/signed-by" LabelSecretType = "nauth.io/secret-type" LabelManaged = "nauth.io/managed" diff --git a/internal/k8s/secret/secret.go b/internal/k8s/secret/secret.go index 5b1a083..5b739a6 100644 --- a/internal/k8s/secret/secret.go +++ b/internal/k8s/secret/secret.go @@ -6,6 +6,7 @@ import ( "log" "maps" "os" + "strings" "github.com/WirelessCar/nauth/internal/k8s" v1 "k8s.io/api/core/v1" @@ -48,13 +49,13 @@ func NewClient(client client.Client, opts ...Option) *Client { if err != nil { log.Fatalf("Failed to read namespace: %v", err) } - secretClient.controllerNamespace = string(namespacePath) + secretClient.controllerNamespace = strings.TrimSpace(string(namespacePath)) } return secretClient } -func (k *Client) Apply(ctx context.Context, owner *Owner, meta metav1.ObjectMeta, valueMap map[string]string) error { +func (k *Client) Apply(ctx context.Context, owner *Owner, meta metav1.ObjectMeta, valueMap map[string]string, update bool) error { if !isManagedSecret(&meta) { return fmt.Errorf("label %s not supplied by secret %s/%s", k8s.LabelManaged, meta.Namespace, meta.Name) } @@ -77,6 +78,10 @@ func (k *Client) Apply(ctx context.Context, owner *Owner, meta metav1.ObjectMeta return fmt.Errorf("failed to create secret: %w", err) } } else { + if !update { + // Do not update the secret, we just needed to know it's here + return nil + } if !isManagedSecret(¤tSecret.ObjectMeta) { return fmt.Errorf("existing secret %s/%s not managed by nauth", meta.Namespace, meta.Name) } diff --git a/internal/k8s/secret/secret_test.go b/internal/k8s/secret/secret_test.go index 520d147..2775942 100644 --- a/internal/k8s/secret/secret_test.go +++ b/internal/k8s/secret/secret_test.go @@ -41,7 +41,7 @@ var _ = Describe("Secrets storer", func() { It("should successfully create and update an existing secret", func() { By("Creating a new secret from scratch") secret := map[string]string{"key": "value"} - err := secretStorer.Apply(ctx, nil, secretMeta, secret) + err := secretStorer.Apply(ctx, nil, secretMeta, secret, true) Expect(err).ToNot(HaveOccurred()) By("Retrieving the secret") @@ -52,7 +52,7 @@ var _ = Describe("Secrets storer", func() { By("Updating the secret with a new value") newSecret := map[string]string{"key": "new value"} - err = secretStorer.Apply(ctx, nil, secretMeta, newSecret) + err = secretStorer.Apply(ctx, nil, secretMeta, newSecret, true) Expect(err).ToNot(HaveOccurred()) By("Retrieving the updated secret") @@ -77,7 +77,7 @@ var _ = Describe("Secrets storer", func() { By("Trying to update the existing secret with a new value") newSecret := map[string]string{"key": "new value"} - err = secretStorer.Apply(ctx, nil, secretMeta, newSecret) + err = secretStorer.Apply(ctx, nil, secretMeta, newSecret, true) Expect(err).To(HaveOccurred()) Expect(err).To(Equal(fmt.Errorf("existing secret %s/%s not managed by nauth", namespace, resourceName))) @@ -110,7 +110,7 @@ var _ = Describe("Secrets storer", func() { It("should return success when deleting existing secret", func() { By("Creating a new secret from scratch") secret := map[string]string{"key": "value"} - err := secretStorer.Apply(ctx, nil, secretMeta, secret) + err := secretStorer.Apply(ctx, nil, secretMeta, secret, true) Expect(err).ToNot(HaveOccurred()) By("Deleting the secret") diff --git a/internal/k8s/secret_types.go b/internal/k8s/secret_types.go index a956452..a16ac4a 100644 --- a/internal/k8s/secret_types.go +++ b/internal/k8s/secret_types.go @@ -7,6 +7,7 @@ const ( const ( SecretTypeAccountRoot = "account-root" + SecretTypeUserSign = "user-sign" SecretTypeAccountSign = "account-sign" SecretTypeOperatorSign = "operator-sign" SecretTypeSystemAccountUserCreds = "system-account-user-creds" diff --git a/internal/nats/client.go b/internal/nats/client.go new file mode 100644 index 0000000..75eced2 --- /dev/null +++ b/internal/nats/client.go @@ -0,0 +1,9 @@ +package nats + +type NatsClient interface { + EnsureConnected(namespace string) error + Disconnect() + LookupAccountJWT(string) (string, error) + UploadAccountJWT(jwt string) error + DeleteAccountJWT(jwt string) error +} diff --git a/internal/user/claims.go b/internal/user/claims.go index d801c04..402b71e 100644 --- a/internal/user/claims.go +++ b/internal/user/claims.go @@ -49,6 +49,10 @@ func newClaimsBuilder( claim.Times = append(claim.Times, time) } claim.Locale = spec.UserLimits.Locale + } else { + if spec.UseSigningKey { + claim.Limits = jwt.Limits{} // Needed for scoped users + } } // NATS Limits diff --git a/internal/user/constants.go b/internal/user/constants.go new file mode 100644 index 0000000..c7c363c --- /dev/null +++ b/internal/user/constants.go @@ -0,0 +1,5 @@ +package user + +const ( + SecretNameUserSignTemplate = "%s-user-sign-%s" +) diff --git a/internal/user/mocks_test.go b/internal/user/mocks_test.go index 673de98..609b0d9 100644 --- a/internal/user/mocks_test.go +++ b/internal/user/mocks_test.go @@ -23,7 +23,7 @@ type SecretStorerMock struct { } // ApplySecret implements ports.SecretStorer. -func (s *SecretStorerMock) Apply(ctx context.Context, secretOwner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string) error { +func (s *SecretStorerMock) Apply(ctx context.Context, secretOwner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string, update bool) error { args := s.Called(ctx, secretOwner, meta, valueMap) return args.Error(0) } diff --git a/internal/user/user.go b/internal/user/user.go index b29249a..8b220c1 100644 --- a/internal/user/user.go +++ b/internal/user/user.go @@ -4,11 +4,15 @@ import ( "context" "errors" "fmt" + "log" + "os" + "strings" "sync" "github.com/WirelessCar/nauth/api/v1alpha1" "github.com/WirelessCar/nauth/internal/k8s" "github.com/WirelessCar/nauth/internal/k8s/secret" + "github.com/WirelessCar/nauth/internal/nats" "github.com/nats-io/jwt/v2" "github.com/nats-io/nkeys" v1 "k8s.io/api/core/v1" @@ -17,7 +21,7 @@ import ( ) type SecretClient interface { - Apply(ctx context.Context, owner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string) error + Apply(ctx context.Context, owner *secret.Owner, meta metav1.ObjectMeta, valueMap map[string]string, update bool) error Get(ctx context.Context, namespace string, name string) (map[string]string, error) GetByLabels(ctx context.Context, namespace string, labels map[string]string) (*v1.SecretList, error) Delete(ctx context.Context, namespace string, name string) error @@ -29,37 +33,271 @@ type AccountGetter interface { } type Manager struct { - accounts AccountGetter - secretClient SecretClient + accounts AccountGetter + natsClient nats.NatsClient + secretClient SecretClient + nauthNamespace string } -func NewManager(accounts AccountGetter, secretClient SecretClient) *Manager { - return &Manager{ +func NewManager(accounts AccountGetter, natsClient nats.NatsClient, secretClient SecretClient, opts ...func(*Manager)) *Manager { + manager := &Manager{ accounts: accounts, + natsClient: natsClient, secretClient: secretClient, } + + for _, opt := range opts { + opt(manager) + } + + if manager.nauthNamespace == "" { + controllerNamespace, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") + if err != nil { + log.Fatalf("Failed create account manager. Failed to read namespace: %v", err) + } + manager.nauthNamespace = strings.TrimSpace(string(controllerNamespace)) + } + return manager +} + +func WithNamespace(namespace string) func(*Manager) { + return func(manager *Manager) { + manager.nauthNamespace = namespace + } +} + +func (u *Manager) getOperatorSigningKeyPair(ctx context.Context) (nkeys.KeyPair, error) { + labels := map[string]string{ + k8s.LabelSecretType: k8s.SecretTypeOperatorSign, + } + operatorSecret, err := u.secretClient.GetByLabels(ctx, u.nauthNamespace, labels) + if err != nil { + return nil, fmt.Errorf("failed to get operator signing key: %w", err) + } + + if len(operatorSecret.Items) < 1 { + return nil, fmt.Errorf("missing operator signing key secret, make sure to label the secret with the label %s: %s", k8s.LabelSecretType, k8s.SecretTypeOperatorSign) + } + + if len(operatorSecret.Items) > 1 { + return nil, fmt.Errorf("multiple operator signing key secrets found, make sure only one secret has the label %s: %s", k8s.LabelSecretType, k8s.SecretTypeSystemAccountUserCreds) + } + + seed, ok := operatorSecret.Items[0].Data[k8s.DefaultSecretKeyName] + if !ok { + return nil, fmt.Errorf("secret for operator signing key seed was malformed") + } + + return nkeys.FromSeed(seed) } +// ensureSigningKeySecret returns a KeyPair from a Kubernetes secret, which is created if needed +func (u *Manager) ensureSigningKeySecret(ctx context.Context, namespace string, state *v1alpha1.User) (nkeys.KeyPair, error) { + signingKey, err := u.getUserSigningKeyPair(ctx, namespace, state.Name) + if err == nil { + return signingKey, nil + } + // Create or update the signing key secret + signingKey, _ = nkeys.CreateAccount() + accountSeed, _ := signingKey.Seed() + signingKeyPublicKey, _ := signingKey.PublicKey() + // signingPrivateKey, _ := sk.PrivateKey() + secretOwner := &secret.Owner{ + Owner: state, + } + secretMeta := metav1.ObjectMeta{ + Name: state.GetUserSigningKeySecretName(), + Namespace: state.GetNamespace(), + Labels: map[string]string{ + k8s.LabelUserSigningKeyID: signingKeyPublicKey, + k8s.LabelSecretType: k8s.SecretTypeUserSign, + k8s.LabelManaged: k8s.LabelManagedValue, + k8s.LabelUserName: state.Name, + }, + } + secretValue := map[string]string{ + k8s.DefaultSecretKeyName: string(accountSeed), + } + err = u.secretClient.Apply(ctx, secretOwner, secretMeta, secretValue, true) + if err != nil { + return nil, err + } + return signingKey, nil +} + +// deleteScoppingSigningKey removes a signing key from the account JWT +func (u *Manager) deleteScoppingSigningKey(ctx context.Context, namespace string, accountID string, state *v1alpha1.User) error { + signingKeyPair, err := u.ensureSigningKeySecret(ctx, namespace, state) + if err != nil { + return fmt.Errorf("cannot get signing key secret: %w", err) + } + signingKeyPublicKey, _ := signingKeyPair.PublicKey() + + // The secret is created, update the account JWT + err = u.natsClient.EnsureConnected(u.nauthNamespace) + if err != nil { + return fmt.Errorf("failed to connect to NATS cluster: %w", err) + } + defer u.natsClient.Disconnect() + accountJWT, err := u.natsClient.LookupAccountJWT(accountID) + if err != nil { + return fmt.Errorf("failed to lookup account jwt for account %s: %w", accountID, err) + } + if len(accountJWT) == 0 { + return fmt.Errorf("account jwt for account %s not found", accountID) + } + accountNatsClaims, err := jwt.DecodeAccountClaims(accountJWT) + if err != nil { + return fmt.Errorf("failed to decode account jwt for account %s: %w", accountID, err) + } + + // Add the new signing key to the list + if accountNatsClaims.SigningKeys == nil { + accountNatsClaims.SigningKeys = jwt.SigningKeys{} + } + accountNatsClaims.SigningKeys.Add(signingKeyPublicKey) + + delete(accountNatsClaims.SigningKeys, signingKeyPublicKey) + + // Get operator signing key + operatorSigningKeyPair, err := u.getOperatorSigningKeyPair(ctx) + if err != nil { + return fmt.Errorf("failed to get operator signing key pair from seed: %w", err) + } + + // Sign account JWT + newAccountJWT, err := accountNatsClaims.Encode(operatorSigningKeyPair) + if err != nil { + userResource := fmt.Sprintf("%s-%s", state.GetNamespace(), state.GetName()) + return fmt.Errorf("failed to sign account jwt for %s: %w", userResource, err) + } + + // Upload account JWT + if err := u.natsClient.UploadAccountJWT(newAccountJWT); err != nil { + return fmt.Errorf("failed to upload account jwt: %w", err) + } + return nil +} + +// ensureScoppingSigningKey creates a scopping signing key, making sure the Kubernetes secret is created, and the account JWT updated +func (u *Manager) ensureScoppingSigningKey(ctx context.Context, namespace string, accountID string, userPublicKey string, state *v1alpha1.User) (nkeys.KeyPair, error) { + signingKeyPair, err := u.ensureSigningKeySecret(ctx, namespace, state) + if err != nil { + return nil, fmt.Errorf("cannot get signing key secret: %w", err) + } + signingKeyPublicKey, _ := signingKeyPair.PublicKey() + + // The secret is created, update the account JWT + err = u.natsClient.EnsureConnected(u.nauthNamespace) + if err != nil { + return nil, fmt.Errorf("failed to connect to NATS cluster: %w", err) + } + defer u.natsClient.Disconnect() + accountJWT, err := u.natsClient.LookupAccountJWT(accountID) + if err != nil { + return nil, fmt.Errorf("failed to lookup account jwt for account %s: %w", accountID, err) + } + if len(accountJWT) == 0 { + return nil, fmt.Errorf("account jwt for account %s not found", accountID) + } + accountNatsClaims, err := jwt.DecodeAccountClaims(accountJWT) + if err != nil { + return nil, fmt.Errorf("failed to decode account jwt for account %s: %w", accountID, err) + } + + // Add the new signing key to the list + if accountNatsClaims.SigningKeys == nil { + accountNatsClaims.SigningKeys = jwt.SigningKeys{} + } + accountNatsClaims.SigningKeys.Add(signingKeyPublicKey) + + delete(accountNatsClaims.SigningKeys, signingKeyPublicKey) + + natsClaims := newClaimsBuilder(getDisplayName(state), state.Spec, userPublicKey, accountID).build() + scope := jwt.NewUserScope() + scope.Key = signingKeyPublicKey + scope.Role = state.Name + scope.Template.Pub = natsClaims.Pub + scope.Template.Sub = natsClaims.Sub + scope.Template.Permissions = natsClaims.Permissions + scope.Template.Limits = natsClaims.Limits + accountNatsClaims.SigningKeys.AddScopedSigner(scope) + + // Get operator signing key + operatorSigningKeyPair, err := u.getOperatorSigningKeyPair(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get operator signing key pair from seed: %w", err) + } + + // Sign account JWT + newAccountJWT, err := accountNatsClaims.Encode(operatorSigningKeyPair) + if err != nil { + userResource := fmt.Sprintf("%s-%s", state.GetNamespace(), state.GetName()) + return nil, fmt.Errorf("failed to sign account jwt for %s: %w", userResource, err) + } + + // Upload account JWT + if err := u.natsClient.UploadAccountJWT(newAccountJWT); err != nil { + return nil, fmt.Errorf("failed to upload account jwt: %w", err) + } + return signingKeyPair, nil +} + +/* +if useSigningKey = false + + find user secret, create if needed, update if needed + +if useSigningKey = true + + find user scoped key, create if needed, update if needed + if created of updated, update the account JWT + find user creds, create if needed (update not needed) +*/ func (u *Manager) CreateOrUpdate(ctx context.Context, state *v1alpha1.User) error { - account, err := u.accounts.Get(ctx, state.Spec.AccountName, state.Namespace) + acc, err := u.accounts.Get(ctx, state.Spec.AccountName, state.Namespace) if err != nil { - return err + return errors.Join(k8s.NoErrRetryLater, err) } - accountID := account.GetLabels()[k8s.LabelAccountID] - accountSigningKeyPair, err := u.getAccountSigningKeyPair(ctx, account.GetNamespace(), account.GetName(), accountID) + var signingKeyPair nkeys.KeyPair + var natsClaims *jwt.UserClaims + var signingKeyPublicKey string + + accountID := acc.GetLabels()[k8s.LabelAccountID] + accountSigningKeyPair, err := u.getAccountSigningKeyPair(ctx, acc.GetNamespace(), acc.GetName(), accountID) if err != nil { - return fmt.Errorf("failed to get signing key secret %s/%s: %w", account.GetNamespace(), account.GetName(), err) + return fmt.Errorf("failed to get signing key secret %s/%s: %w", acc.GetNamespace(), acc.GetName(), err) } - accountSigningKeyPublicKey, _ := accountSigningKeyPair.PublicKey() userKeyPair, _ := nkeys.CreateUser() userPublicKey, _ := userKeyPair.PublicKey() userSeed, _ := userKeyPair.Seed() - natsClaims := newClaimsBuilder(getDisplayName(state), state.Spec, userPublicKey, accountID). - build() - userJwt, err := natsClaims.Encode(accountSigningKeyPair) + // Get the user's signing key, create if needed + if state.Spec.UseSigningKey { + signingKeyPair, err = u.ensureScoppingSigningKey(ctx, acc.GetNamespace(), accountID, userPublicKey, state) + if err != nil { + return fmt.Errorf("failed to update the account JWT: %w", err) + } + signingKeyPublicKey, _ = signingKeyPair.PublicKey() + // Scopped users claims must be empty + natsClaims = newClaimsBuilder(getDisplayName(state), v1alpha1.UserSpec{ + AccountName: state.Spec.AccountName, + DisplayName: state.Spec.DisplayName, + Permissions: nil, + UserLimits: nil, + NatsLimits: nil, + }, userPublicKey, accountID).build() + } else { + // Sign using the account key + signingKeyPublicKey, _ = accountSigningKeyPair.PublicKey() + signingKeyPair = accountSigningKeyPair + natsClaims = newClaimsBuilder(getDisplayName(state), state.Spec, userPublicKey, accountID). + build() + } + + userJwt, err := natsClaims.Encode(signingKeyPair) if err != nil { userResource := fmt.Sprintf("%s-%s", state.GetNamespace(), state.GetName()) return fmt.Errorf("failed to sign user jwt for %s: %w", userResource, err) @@ -81,7 +319,7 @@ func (u *Manager) CreateOrUpdate(ctx context.Context, state *v1alpha1.User) erro secretValue := map[string]string{ k8s.UserCredentialSecretKeyName: string(userCreds), } - err = u.secretClient.Apply(ctx, secretOwner, secretMeta, secretValue) + err = u.secretClient.Apply(ctx, secretOwner, secretMeta, secretValue, !state.Spec.UseSigningKey) if err != nil { return err } @@ -94,8 +332,8 @@ func (u *Manager) CreateOrUpdate(ctx context.Context, state *v1alpha1.User) erro } state.GetLabels()[k8s.LabelUserID] = userPublicKey - state.GetLabels()[k8s.LabelUserAccountID] = account.GetLabels()[k8s.LabelAccountID] - state.GetLabels()[k8s.LabelUserSignedBy] = accountSigningKeyPublicKey + state.GetLabels()[k8s.LabelUserAccountID] = acc.GetLabels()[k8s.LabelAccountID] + state.GetLabels()[k8s.LabelUserSignedBy] = signingKeyPublicKey state.Status.ObservedGeneration = state.Generation state.Status.ReconcileTimestamp = metav1.Now() @@ -103,18 +341,69 @@ func (u *Manager) CreateOrUpdate(ctx context.Context, state *v1alpha1.User) erro return nil } +/* +if useSigningKey = false + + Delete user secret + +if useSigningKey = true + + Update Account JWT + Delete signing key secret + Delete secret +*/ func (u *Manager) Delete(ctx context.Context, state *v1alpha1.User) error { log := logf.FromContext(ctx) log.Info("Delete user", "userName", state.GetName()) - err := u.secretClient.Delete(ctx, state.Namespace, state.GetUserSecretName()) - if err != nil { + if state.Spec.UseSigningKey { + acc, err := u.accounts.Get(ctx, state.Spec.AccountName, state.Namespace) + if err != nil { + return errors.Join(k8s.NoErrRetryLater, err) + } + + accountID := acc.GetLabels()[k8s.LabelAccountID] + if err := u.deleteScoppingSigningKey(ctx, state.Namespace, accountID, state); err != nil { + return fmt.Errorf("failed to update the account JWT: %w", err) + } + + if err := u.secretClient.Delete(ctx, state.Namespace, state.GetUserSigningKeySecretName()); err != nil { + return fmt.Errorf("failed to delete user signing key secret %s/%s: %w", state.Namespace, state.GetUserSigningKeySecretName(), err) + } + } + if err := u.secretClient.Delete(ctx, state.Namespace, state.GetUserSecretName()); err != nil { return fmt.Errorf("failed to delete user secret %s/%s: %w", state.Namespace, state.GetUserSecretName(), err) } return nil } +func (u *Manager) getUserSigningKeyPair(ctx context.Context, namespace, userName string) (nkeys.KeyPair, error) { + labels := map[string]string{ + k8s.LabelSecretType: k8s.SecretTypeUserSign, + k8s.LabelManaged: k8s.LabelManagedValue, + k8s.LabelUserName: userName, + } + secrets, err := u.secretClient.GetByLabels(ctx, namespace, labels) + if err != nil { + return nil, fmt.Errorf("failed to get signing secret for user: %s-%s due to %w", namespace, userName, err) + } + + if len(secrets.Items) < 1 { + return nil, fmt.Errorf("no signing secret found for user: %s-%s", namespace, userName) + } + + if len(secrets.Items) > 1 { + return nil, fmt.Errorf("more than 1 signing secret found for user: %s-%s", namespace, userName) + } + + seed, ok := secrets.Items[0].Data[k8s.DefaultSecretKeyName] + if !ok { + return nil, fmt.Errorf("secret for user credentials seed was malformed") + } + return nkeys.FromSeed(seed) +} + func (u *Manager) getAccountSigningKeyPair(ctx context.Context, namespace, accountName, accountID string) (nkeys.KeyPair, error) { if keyPair, err := u.getAccountSigningKeyPairByAccountID(ctx, namespace, accountName, accountID); err == nil { return keyPair, nil @@ -134,6 +423,7 @@ func (u *Manager) getAccountSigningKeyPairByAccountID(ctx context.Context, names k8s.LabelSecretType: k8s.SecretTypeAccountSign, k8s.LabelManaged: k8s.LabelManagedValue, } + secrets, err := u.secretClient.GetByLabels(ctx, namespace, labels) if err != nil { return nil, fmt.Errorf("failed to get signing secret for account: %s-%s due to %w", namespace, accountName, err) diff --git a/internal/user/user_test.go b/internal/user/user_test.go index 5f1a18e..0f2b0d2 100644 --- a/internal/user/user_test.go +++ b/internal/user/user_test.go @@ -6,6 +6,7 @@ import ( "github.com/WirelessCar/nauth/api/v1alpha1" "github.com/WirelessCar/nauth/internal/k8s" + "github.com/nats-io/jwt/v2" "github.com/nats-io/nkeys" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -30,18 +31,21 @@ var _ = Describe("User manager", func() { userManager *Manager accountGetterMock *AccountGetterMock secretStorerMock *SecretStorerMock + natsClientMock *NATSClientMock ) BeforeEach(func() { By("creating the user manager") secretStorerMock = NewSecretStorerMock() accountGetterMock = NewAccountGetterMock() - userManager = NewManager(accountGetterMock, secretStorerMock) + natsClientMock = NewNATSClientMock() + userManager = NewManager(accountGetterMock, natsClientMock, secretStorerMock, WithNamespace(accountNamespace)) }) AfterEach(func() { secretStorerMock.AssertExpectations(GinkgoT()) accountGetterMock.AssertExpectations(GinkgoT()) + natsClientMock.AssertExpectations(GinkgoT()) }) It("creates a new user belonging to the correct account", func() { @@ -180,7 +184,92 @@ var _ = Describe("User manager", func() { Expect(user.Status.Claims.NatsLimits.Data).Should(Equal(user.Spec.NatsLimits.Data)) Expect(user.Status.Claims.NatsLimits.Payload).Should(Equal(user.Spec.NatsLimits.Payload)) }) + + It("creates a new scopped user from an account with legacy secrets, then delete it", func() { + By("providing a user specification") + user := GetNewScoppedUser() + + account := GetExistingAccount() + + By("mocking the secret storer") + + operatorKeyPair, _ := nkeys.CreateOperator() + operatorSeed, _ := operatorKeyPair.Seed() + operatorSignLabelsMock := map[string]string{k8s.LabelSecretType: k8s.SecretTypeOperatorSign} + operatorSignSecretMock := &corev1.SecretList{ + Items: []corev1.Secret{{Data: map[string][]byte{k8s.DefaultSecretKeyName: operatorSeed}}}, + } + secretStorerMock.On("GetByLabels", ctx, account.GetNamespace(), operatorSignLabelsMock).Return(operatorSignSecretMock, nil) + + secretStorerMock.On("GetByLabels", mock.Anything, account.GetNamespace(), mock.Anything).Return(&corev1.SecretList{}, nil) + + accountKeyPair, _ := nkeys.CreateAccount() + accountPublicKey, _ := accountKeyPair.PublicKey() + accountSeed, _ := accountKeyPair.Seed() + accountSecretValueMock := map[string]string{k8s.DefaultSecretKeyName: string(accountSeed)} + accountSecretNameMock := fmt.Sprintf(k8s.DeprecatedSecretNameAccountRootTemplate, account.GetName()) + secretStorerMock.On("Get", mock.Anything, account.GetNamespace(), accountSecretNameMock).Return(accountSecretValueMock, nil) + accountSecretLabelsMock := map[string]string{ + k8s.LabelAccountID: accountPublicKey, + k8s.LabelSecretType: k8s.SecretTypeAccountRoot, + k8s.LabelManaged: k8s.LabelManagedValue, + } + secretStorerMock.On("Label", mock.Anything, account.GetNamespace(), accountSecretNameMock, accountSecretLabelsMock).Return(nil) + + accountSigningKeyPair, _ := nkeys.CreateAccount() + accountSigningPublicKey, _ := accountSigningKeyPair.PublicKey() + accountSigningSeed, _ := accountSigningKeyPair.Seed() + accountSigningSecretValueMock := map[string]string{k8s.DefaultSecretKeyName: string(accountSigningSeed)} + accountSigningSecretNameMock := fmt.Sprintf(k8s.DeprecatedSecretNameAccountSignTemplate, account.GetName()) + secretStorerMock.On("Get", mock.Anything, account.GetNamespace(), accountSigningSecretNameMock).Return(accountSigningSecretValueMock, nil) + accountSigningSecretLabelsMock := map[string]string{ + k8s.LabelAccountID: accountPublicKey, + k8s.LabelSecretType: k8s.SecretTypeAccountSign, + k8s.LabelManaged: k8s.LabelManagedValue, + } + secretStorerMock.On("Label", mock.Anything, account.GetNamespace(), accountSigningSecretNameMock, accountSigningSecretLabelsMock).Return(nil) + claims := jwt.NewAccountClaims(accountPublicKey) + token, _ := claims.Encode(accountSigningKeyPair) + + By("mocking existing account") + account.Status.SigningKey = v1alpha1.KeyInfo{ + Name: accountSigningPublicKey, + } + account.Labels = map[string]string{ + k8s.LabelAccountID: accountPublicKey, + } + accountGetterMock.On("Get", ctx, accountName, accountNamespace).Return(*account, nil) + + By("mock storing account WT") + secretStorerMock.On("Apply", mock.Anything, mock.Anything, mock.Anything, mock.AnythingOfType("map[string]string")).Return(nil) + + By("mock getting the account JWT") + natsClientMock.On("LookupAccountJWT", mock.AnythingOfType("string")).Return(token, nil) + + By("mocking the NATS client") + natsClientMock.On("EnsureConnected", accountNamespace).Return(nil) + natsClientMock.On("Disconnect").Return() + natsClientMock.On("UploadAccountJWT", mock.Anything).Return(nil) + + // By("mock storing user credentials") + // secretStorerMock.On("Apply", mock.Anything, mock.Anything, mock.MatchedBy(func(s v1.ObjectMeta) bool { + // return s.GetName() == user.GetUserSecretName() && s.GetNamespace() == accountNamespace + // }), mock.AnythingOfType("map[string]string")).Return(nil) + + err := userManager.CreateOrUpdate(ctx, user) + + By("mock deleting secrets") + secretStorerMock.On("Delete", ctx, accountNamespace, mock.AnythingOfType("string")).Return(nil) + errDel := userManager.Delete(ctx, user) + + Expect(err).ToNot(HaveOccurred()) + Expect(errDel).ToNot(HaveOccurred()) + + Expect(user.GetLabels()).ToNot(BeNil()) + Expect(user.GetLabels()[k8s.LabelUserID]).Should(Satisfy(isUserPubKey)) + }) }) + }) func GetNewUser() *v1alpha1.User { @@ -201,6 +290,22 @@ func GetNewUser() *v1alpha1.User { } } +func GetNewScoppedUser() *v1alpha1.User { + return &v1alpha1.User{ + ObjectMeta: v1.ObjectMeta{ + Name: userName, + Namespace: accountNamespace, + }, + Spec: v1alpha1.UserSpec{ + UseSigningKey: true, + AccountName: accountName, + Permissions: nil, + UserLimits: nil, + NatsLimits: nil, + }, + } +} + func GetNewAccount() *v1alpha1.Account { return &v1alpha1.Account{ ObjectMeta: v1.ObjectMeta{ diff --git a/test/e2e/basic-test/00-assert-operator-secrets.yaml b/test/e2e/basic-test/00-assert-operator-secrets.yaml index 33f64a4..eb75a1a 100644 --- a/test/e2e/basic-test/00-assert-operator-secrets.yaml +++ b/test/e2e/basic-test/00-assert-operator-secrets.yaml @@ -44,7 +44,7 @@ apiVersion: v1 kind: Secret type: Opaque metadata: - name: operator-sau-creds + name: namespace: nats labels: nauth.io/secret-type: system-account-user-creds