Repository: nats-io/nsc (go)

Issue:
nsc pull will replace a v2 operator with a v1
if you `nsc pull -A`, it is possible to pull a v1 operator `/jwt/v1/operator` instead of `/jwt/v2/operator` when the account server supports both.

Likely if the operator is v2 and the pulled operator is v2, the pull should be rejected (we may need a flag to specify the operator URL to pull).

Pull request description:
`nsc pull -A` overwrites v2 operator with a v1
made operator pulls check v2 before v1, this is just a temporary solution, as the correct thing is to change the account server to handle a query string parameter to determine if the client is looking for a v2 JWT.
FIX #388

Patch:
diff --git a/cmd/pull.go b/cmd/pull.go
index 902a13d..6d70437 100644
--- a/cmd/pull.go
+++ b/cmd/pull.go
@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2019 The NATS Authors
+ * Copyright 2018-2021 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
@@ -102,6 +102,10 @@ func (j *PullJob) Token() (string, error) {
 }
 
 func (j *PullJob) Run() {
+	if j.PullStatus != nil {
+		// already ran
+		return
+	}
 	s, err := store.PullAccount(j.ASU)
 	if err != nil {
 		j.Err = err
@@ -172,9 +176,16 @@ func (p *PullParams) setupJobs(ctx ActionCtx) error {
 		if err != nil {
 			return err
 		}
-		j := PullJob{ASU: u, Name: oc.Name, LocalClaim: oc}
-		p.Jobs = append(p.Jobs, &j)
-
+		// in ngs we are in v2 operator, so lets try /jwt/v2/operator first
+		uv2 := strings.Replace(u, "/jwt/v1/operator", "/jwt/v2/operator", 1)
+		j := PullJob{ASU: uv2, Name: oc.Name, LocalClaim: oc}
+		j.Run()
+		if j.Err == nil {
+			p.Jobs = append(p.Jobs, &j)
+		} else {
+			j = PullJob{ASU: u, Name: oc.Name, LocalClaim: oc}
+			p.Jobs = append(p.Jobs, &j)
+		}
 		tc := GetConfig()
 		accounts, err := tc.ListAccounts()
 		if err != nil {
diff --git a/cmd/store/store.go b/cmd/store/store.go
index 2937433..a22a797 100644
--- a/cmd/store/store.go
+++ b/cmd/store/store.go
@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2020 The NATS Authors
+ * Copyright 2018-2021 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
@@ -343,6 +343,9 @@ func PullAccount(u string) (Status, error) {
 	if err != nil {
 		return nil, fmt.Errorf("error pulling %q: %v", u, err)
 	}
+	if r.StatusCode > 299 {
+		return nil, fmt.Errorf("error pulling %q: %d", u, r.StatusCode)
+	}
 	defer r.Body.Close()
 	var buf bytes.Buffer
 	_, err = io.Copy(&buf, r.Body)
diff --git a/cmd/wellknownoperator.go b/cmd/wellknownoperator.go
index 4781478..2356f75 100644
--- a/cmd/wellknownoperator.go
+++ b/cmd/wellknownoperator.go
@@ -36,7 +36,7 @@ var wellKnownOperators KnownOperators
 
 func defaultWellKnownOperators() KnownOperators {
 	return KnownOperators{
-		{Name: "synadia", URL: "https://www.synadia.com", AccountServerURL: "https://api.synadia.io/jwt/v1/synadia"},
+		{Name: "synadia", URL: "https://www.synadia.com", AccountServerURL: "https://api.synadia.io/jwt/v2/synadia"},
 	}
 }
 


Test patch:
diff --git a/cmd/addoperator_test.go b/cmd/addoperator_test.go
index 14589a7..ce3e70e 100644
--- a/cmd/addoperator_test.go
+++ b/cmd/addoperator_test.go
@@ -227,7 +227,7 @@ func Test_AddWellKnownOperator(t *testing.T) {
 
 	// create the managed operator store
 	_, opk, okp := CreateOperatorKey(t)
-	as, _ := RunTestAccountServerWithOperatorKP(t, okp, 2)
+	as, _ := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
 	defer as.Close()
 
 	// add an entry to well known
diff --git a/cmd/generateactivation_test.go b/cmd/generateactivation_test.go
index 21b385f..4fc46f0 100644
--- a/cmd/generateactivation_test.go
+++ b/cmd/generateactivation_test.go
@@ -229,7 +229,7 @@ func Test_GenerateActivationUsingSigningKey(t *testing.T) {
 
 func Test_InteractiveGenerateActivationPush(t *testing.T) {
 	_, _, okp := CreateOperatorKey(t)
-	as, m := RunTestAccountServerWithOperatorKP(t, okp, 2)
+	as, m := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
 	defer as.Close()
 
 	ts := NewTestStoreWithOperator(t, "T", okp)
diff --git a/cmd/init_test.go b/cmd/init_test.go
index 01e8db9..5188272 100644
--- a/cmd/init_test.go
+++ b/cmd/init_test.go
@@ -182,7 +182,7 @@ func Test_InitWellKnownV1Operator(t *testing.T) {
 
 	_, _, okp := CreateOperatorKey(t)
 	// run a jwt account server
-	as, _ := RunTestAccountServerWithOperatorKP(t, okp, 1)
+	as, _ := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 1})
 	defer as.Close()
 
 	// add an entry to well known
diff --git a/cmd/migrate_test.go b/cmd/migrate_test.go
index cd07752..07f08e9 100644
--- a/cmd/migrate_test.go
+++ b/cmd/migrate_test.go
@@ -94,7 +94,7 @@ func Test_MigrateSingleInteractive(t *testing.T) {
 
 func Test_MigrateManaged(t *testing.T) {
 	_, opk, okp := CreateOperatorKey(t)
-	as, m := RunTestAccountServerWithOperatorKP(t, okp, 2)
+	as, m := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
 	defer as.Close()
 
 	// create the managed operator store
diff --git a/cmd/pull_test.go b/cmd/pull_test.go
index 6f31ddd..7d89978 100644
--- a/cmd/pull_test.go
+++ b/cmd/pull_test.go
@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2019 The NATS Authors
+ * Copyright 2018-2021 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
@@ -192,3 +192,43 @@ func Test_SyncNewerFromNatsResolver(t *testing.T) {
 	require.NotEmpty(t, claimOrig.ID)
 	require.Equal(t, claimOrig.ID, claim2.ID)
 }
+
+func Test_V2OperatorDoesntFail(t *testing.T) {
+	_, _, okp := CreateOperatorKey(t)
+	as, m := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2, OperatorOnlyIfV2: true})
+	defer as.Close()
+
+	ts := NewTestStoreWithOperator(t, "T", okp)
+	defer ts.Done(t)
+	err := ts.Store.StoreRaw(m["operator"])
+	require.NoError(t, err)
+
+	// edit the jwt
+	_, _, err = ExecuteCmd(createPullCmd(), "-A")
+	require.NoError(t, err)
+
+	oc, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+	require.Equal(t, oc.Version, 2)
+}
+
+func Test_V1OperatorDoesntFail(t *testing.T) {
+	_, _, okp := CreateOperatorKey(t)
+	as, m := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
+	defer as.Close()
+
+	ts := NewTestStoreWithOperator(t, "T", okp)
+	defer ts.Done(t)
+	err := ts.Store.StoreRaw(m["operator"])
+	require.NoError(t, err)
+
+	// edit the jwt
+	stdout, stderr, err := ExecuteCmd(createPullCmd(), "-A")
+	t.Log(stdout)
+	t.Log(stderr)
+	require.NoError(t, err)
+
+	oc, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+	require.Equal(t, oc.Version, 2)
+}
diff --git a/cmd/push_test.go b/cmd/push_test.go
index 4b9d800..00c93d9 100644
--- a/cmd/push_test.go
+++ b/cmd/push_test.go
@@ -32,7 +32,7 @@ import (
 
 func Test_SyncOK(t *testing.T) {
 	_, _, okp := CreateOperatorKey(t)
-	as, m := RunTestAccountServerWithOperatorKP(t, okp, 2)
+	as, m := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
 	defer as.Close()
 
 	ts := NewTestStoreWithOperator(t, "T", okp)
@@ -56,7 +56,7 @@ func Test_SyncOK(t *testing.T) {
 
 func Test_SyncNoURL(t *testing.T) {
 	_, _, okp := CreateOperatorKey(t)
-	as, m := RunTestAccountServerWithOperatorKP(t, okp, 2)
+	as, m := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
 	ts := NewTestStoreWithOperatorJWT(t, string(m["operator"]))
 	ts.AddAccount(t, "A")
 	as.Close()
@@ -104,7 +104,7 @@ func Test_SyncManaged(t *testing.T) {
 
 func Test_SyncManualServer(t *testing.T) {
 	_, _, okp := CreateOperatorKey(t)
-	as, m := RunTestAccountServerWithOperatorKP(t, okp, 2)
+	as, m := RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
 	defer as.Close()
 
 	// remove the account server
diff --git a/cmd/util_test.go b/cmd/util_test.go
index 8e65457..57f6d23 100644
--- a/cmd/util_test.go
+++ b/cmd/util_test.go
@@ -666,7 +666,12 @@ func Test_Util(t *testing.T) {
 	require.Equal(t, pk, ac2.Issuer)
 }
 
-func RunTestAccountServerWithOperatorKP(t *testing.T, okp nkeys.KeyPair, vers int) (*httptest.Server, map[string][]byte) {
+type TasOpts struct {
+	Vers             int
+	OperatorOnlyIfV2 bool
+}
+
+func RunTestAccountServerWithOperatorKP(t *testing.T, okp nkeys.KeyPair, opts TasOpts) (*httptest.Server, map[string][]byte) {
 	storage := make(map[string][]byte)
 	opk, err := okp.PublicKey()
 	require.NoError(t, err)
@@ -677,13 +682,29 @@ func RunTestAccountServerWithOperatorKP(t *testing.T, okp nkeys.KeyPair, vers in
 			w.Write([]byte(err.Error()))
 		}
 		getHandler := func(w http.ResponseWriter, r *http.Request) {
+			var data []byte
 			id := filepath.Base(r.RequestURI)
-			data := storage[id]
+			data = storage[id]
+
+			// if we don't have v2, and they ask for v2, ignore it
+			if r.RequestURI == "/jwt/v2/operator" && !opts.OperatorOnlyIfV2 {
+				data = nil
+			}
+			if r.RequestURI == "/jwt/v1/operator" && opts.OperatorOnlyIfV2 {
+				// option to only answer to v2 - they asked v1
+				data = nil
+			}
+
 			if data == nil {
 				w.WriteHeader(http.StatusNotFound)
 			}
 			w.Header().Add("Content-Type", "application/jwt")
 			w.Write(data)
+			if data == nil {
+				t.Log(r.RequestURI, "not found")
+			} else {
+				t.Log(r.RequestURI, "OK")
+			}
 		}
 
 		updateAccountHandler := func(w http.ResponseWriter, r *http.Request) {
@@ -771,7 +792,7 @@ func RunTestAccountServerWithOperatorKP(t *testing.T, okp nkeys.KeyPair, vers in
 		}
 	}))
 
-	if vers == 1 {
+	if opts.Vers == 1 {
 		oc := jwt1.NewOperatorClaims(opk)
 		oc.Name = "T"
 		oc.Subject = opk
@@ -801,5 +822,5 @@ func RunTestAccountServerWithOperatorKP(t *testing.T, okp nkeys.KeyPair, vers in
 // Runs a TestAccountServer returning the server and the underlying storage
 func RunTestAccountServer(t *testing.T) (*httptest.Server, map[string][]byte) {
 	_, _, okp := CreateOperatorKey(t)
-	return RunTestAccountServerWithOperatorKP(t, okp, 2)
+	return RunTestAccountServerWithOperatorKP(t, okp, TasOpts{Vers: 2})
 }