From 296ba908abf64070bc8f74532b541bdd09d74cef Mon Sep 17 00:00:00 2001 From: Alberto Ricart Date: Mon, 23 Dec 2024 11:11:21 -0600 Subject: [PATCH] various fixes regarding enabling/disabling/validation of jetstream on system account [FIX] added check for --js-enable=tier on system account (#685) [FIX] added bypass to allow --js-disable to work on system account in cases where it was enabled by specifying --js-enable=tier [FIX] enhanced validation to check for system accounts with enabled jetstream --- cmd/editaccount.go | 7 +++++- cmd/editaccount_test.go | 52 +++++++++++++++++++++++++++++++++++++++++ cmd/validate.go | 10 +++++++- cmd/validate_test.go | 28 +++++++++++++++++++++- 4 files changed, 94 insertions(+), 3 deletions(-) diff --git a/cmd/editaccount.go b/cmd/editaccount.go index 08ecf7ae..2ee34087 100644 --- a/cmd/editaccount.go +++ b/cmd/editaccount.go @@ -598,7 +598,12 @@ func (p *EditAccountParams) checkSystemAccount(ctx ActionCtx) error { return nil } - if p.claim.Limits.JetStreamTieredLimits != nil { + // allow the js to be disabled + if p.disableJetStream { + return nil + } + + if p.claim.Limits.JetStreamTieredLimits != nil || p.enableJetStream > -1 { return errors.New("system account cannot have JetStream limits - please rerun with --js-disable") } diff --git a/cmd/editaccount_test.go b/cmd/editaccount_test.go index 5bb84647..cd3ce384 100644 --- a/cmd/editaccount_test.go +++ b/cmd/editaccount_test.go @@ -524,3 +524,55 @@ func Test_EnableTierNoOtherFlag(t *testing.T) { require.Error(t, err) require.Equal(t, "rm-js-tier is exclusive of all other js options", err.Error()) } + +func Test_CannotEnableJsInSys(t *testing.T) { + ts := NewTestStore(t, "O") + defer ts.Done(t) + ts.AddAccount(t, "SYS") + _, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS") + require.NoError(t, err) + + _, _, err = ExecuteCmd(createEditAccount(), "--js-enable", "1") + require.Error(t, err) + + _, _, err = ExecuteCmd(createEditAccount(), "--js-disable") + require.NoError(t, err) + + sys, err := ts.Store.ReadAccountClaim("SYS") + require.NoError(t, err) + + require.False(t, sys.Limits.IsJSEnabled()) +} + +func Test_AllowSysToDisableJs(t *testing.T) { + ts := NewTestStore(t, "O") + defer ts.Done(t) + ts.AddAccount(t, "SYS") + _, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS") + require.NoError(t, err) + + sys, err := ts.Store.ReadAccountClaim("SYS") + require.NoError(t, err) + require.False(t, sys.Limits.IsJSEnabled()) + + sys.Limits.JetStreamTieredLimits = make(map[string]jwt.JetStreamLimits) + sys.Limits.JetStreamTieredLimits["R1"] = jwt.JetStreamLimits{DiskStorage: -1, MemoryStorage: -1} + + okp, err := ts.KeyStore.GetKeyPair(ts.GetOperatorPublicKey(t)) + require.NoError(t, err) + token, err := sys.Encode(okp) + require.NoError(t, err) + require.NoError(t, ts.Store.StoreRaw([]byte(token))) + + sys, err = ts.Store.ReadAccountClaim("SYS") + require.NoError(t, err) + require.True(t, sys.Limits.IsJSEnabled()) + + _, _, err = ExecuteCmd(createEditAccount(), "--js-disable") + require.NoError(t, err) + + sys, err = ts.Store.ReadAccountClaim("SYS") + require.NoError(t, err) + + require.False(t, sys.Limits.IsJSEnabled()) +} diff --git a/cmd/validate.go b/cmd/validate.go index e5faeafe..d3955541 100644 --- a/cmd/validate.go +++ b/cmd/validate.go @@ -1,5 +1,5 @@ /* - * Copyright 2018-2019 The NATS Authors + * Copyright 2018-2024 The NATS Authors * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at @@ -220,6 +220,14 @@ func (p *ValidateCmdParams) validate(ctx ActionCtx) error { if aci != nil { p.accountValidations[v] = aci } + if oc.SystemAccount == ac.Subject { + if ac.Limits.IsJSEnabled() { + if p.accountValidations[v] == nil { + p.accountValidations[v] = &jwt.ValidationResults{} + } + p.accountValidations[v].AddError("JetStream should not be enabled for system account") + } + } if !oc.DidSign(ac) { if p.accountValidations[v] == nil { p.accountValidations[v] = &jwt.ValidationResults{} diff --git a/cmd/validate_test.go b/cmd/validate_test.go index c817b226..99d93b47 100644 --- a/cmd/validate_test.go +++ b/cmd/validate_test.go @@ -1,5 +1,5 @@ /* - * Copyright 2018-2023 The NATS Authors + * Copyright 2018-2024 The NATS Authors * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at @@ -21,6 +21,7 @@ import ( "strings" "testing" + "github.com/nats-io/jwt/v2" "github.com/nats-io/nsc/v2/cmd/store" "github.com/stretchr/testify/require" ) @@ -201,3 +202,28 @@ func Test_ValidateInteractive(t *testing.T) { require.NoError(t, err) require.Contains(t, stderr, "Account \"B\"") } + +func Test_ValidateJsSys(t *testing.T) { + ts := NewTestStore(t, "O") + defer ts.Done(t) + ts.AddAccount(t, "SYS") + _, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS") + require.NoError(t, err) + + sys, err := ts.Store.ReadAccountClaim("SYS") + require.NoError(t, err) + require.False(t, sys.Limits.IsJSEnabled()) + + sys.Limits.JetStreamTieredLimits = make(map[string]jwt.JetStreamLimits) + sys.Limits.JetStreamTieredLimits["R1"] = jwt.JetStreamLimits{DiskStorage: -1, MemoryStorage: -1} + + okp, err := ts.KeyStore.GetKeyPair(ts.GetOperatorPublicKey(t)) + require.NoError(t, err) + token, err := sys.Encode(okp) + require.NoError(t, err) + require.NoError(t, ts.Store.StoreRaw([]byte(token))) + + _, stderr, err := ExecuteInteractiveCmd(HoistRootFlags(createValidateCommand()), []interface{}{1}) + require.Error(t, err) + require.Contains(t, stderr, "JetStream should not be enabled for system account") +}