Repository: theupdateframework/go-tuf
Pull request #193 (2021-12-10 23:04:06)

[Delegations prereq 3] Add roles helpers
Splitting up https://github.com/theupdateframework/go-tuf/pull/175

Adds helpers such as `IsTopLevelRole` and `IsDelegatedTargetsRole`, which help make conditionals on role names/manifest names more readable.

coveralls: ## Pull Request Test Coverage Report for [Build 1565526747](https://coveralls.io/builds/44928207)

* **29** of **30**   **(96.67%)**  changed or added relevant lines in **4** files are covered.
* **5** unchanged lines in **1** file lost coverage.
* Overall coverage increased (+**1.6%**) to **71.769%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [repo.go](https://coveralls.io/builds/44928207/source?filename=repo.go#L323) | 5 | 6 | 83.33%
<!-- | **Total:** | **29** | **30** | **96.67%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [repo.go](https://coveralls.io/builds/44928207/source?filename=repo.go#L699) | 5 | 73.28% |
<!-- | **Total:** | **5** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/44928207/badge)](https://coveralls.io/builds/44928207) |
| :-- | --: |
| Change from base [Build 1565520123](https://coveralls.io/builds/44928156): |  1.6% |
| Covered Lines: | 933 |
| Relevant Lines: | 1300 |

---
##### 💛  - [Coveralls](https://coveralls.io)

mnm678: Oh, actually the tests are failing on this one, so we shouldn't merge yet. But otherwise it LGTM
hosseinsia: please add some description to the PR.

Patch:
diff --git a/internal/roles/roles.go b/internal/roles/roles.go
new file mode 100644
index 0000000..f7841c2
--- /dev/null
+++ b/internal/roles/roles.go
@@ -0,0 +1,41 @@
+package roles
+
+import (
+	"strconv"
+	"strings"
+)
+
+var TopLevelRoles = map[string]struct{}{
+	"root":      {},
+	"targets":   {},
+	"snapshot":  {},
+	"timestamp": {},
+}
+
+func IsTopLevelRole(name string) bool {
+	_, ok := TopLevelRoles[name]
+	return ok
+}
+
+func IsDelegatedTargetsRole(name string) bool {
+	return !IsTopLevelRole(name)
+}
+
+func IsTopLevelManifest(name string) bool {
+	return IsTopLevelRole(strings.TrimSuffix(name, ".json"))
+}
+
+func IsDelegatedTargetsManifest(name string) bool {
+	return !IsTopLevelManifest(name)
+}
+
+func IsVersionedManifest(name string) bool {
+	parts := strings.Split(name, ".")
+	// Versioned manifests have the form "x.role.json"
+	if len(parts) < 3 {
+		return false
+	}
+
+	_, err := strconv.Atoi(parts[0])
+	return err == nil
+}
diff --git a/repo.go b/repo.go
index 7d5e35a..94c2e93 100644
--- a/repo.go
+++ b/repo.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/secure-systems-lab/go-securesystemslib/cjson"
 	"github.com/theupdateframework/go-tuf/data"
+	"github.com/theupdateframework/go-tuf/internal/roles"
 	"github.com/theupdateframework/go-tuf/internal/signer"
 	"github.com/theupdateframework/go-tuf/pkg/keys"
 	"github.com/theupdateframework/go-tuf/sign"
@@ -199,7 +200,7 @@ func (r *Repo) GetThreshold(keyRole string) (int, error) {
 }
 
 func (r *Repo) SetThreshold(keyRole string, t int) error {
-	if !validMetadata(keyRole + ".json") {
+	if !roles.IsTopLevelRole(keyRole) {
 		// Delegations are not currently supported, so return an error if this is not a
 		// top-level metadata file.
 		return ErrInvalidRole{keyRole}
@@ -319,7 +320,7 @@ func (r *Repo) timestamp() (*data.Timestamp, error) {
 }
 
 func (r *Repo) ChangePassphrase(keyRole string) error {
-	if !verify.ValidRole(keyRole) {
+	if !roles.IsTopLevelRole(keyRole) {
 		return ErrInvalidRole{keyRole}
 	}
 
@@ -352,7 +353,7 @@ func (r *Repo) AddPrivateKey(role string, signer keys.Signer) error {
 }
 
 func (r *Repo) AddPrivateKeyWithExpires(keyRole string, signer keys.Signer, expires time.Time) error {
-	if !verify.ValidRole(keyRole) {
+	if !roles.IsTopLevelRole(keyRole) {
 		return ErrInvalidRole{keyRole}
 	}
 
@@ -451,7 +452,7 @@ func (r *Repo) RevokeKey(role, id string) error {
 }
 
 func (r *Repo) RevokeKeyWithExpires(keyRole, id string, expires time.Time) error {
-	if !verify.ValidRole(keyRole) {
+	if !roles.IsTopLevelRole(keyRole) {
 		return ErrInvalidRole{keyRole}
 	}
 
@@ -555,7 +556,7 @@ func (r *Repo) setMeta(roleFilename string, meta interface{}) error {
 
 func (r *Repo) Sign(roleFilename string) error {
 	role := strings.TrimSuffix(roleFilename, ".json")
-	if !verify.ValidRole(role) {
+	if !roles.IsTopLevelRole(role) {
 		return ErrInvalidRole{role}
 	}
 
@@ -591,7 +592,7 @@ func (r *Repo) Sign(roleFilename string) error {
 // The name must be a valid metadata file name, like root.json.
 func (r *Repo) AddOrUpdateSignature(roleFilename string, signature data.Signature) error {
 	role := strings.TrimSuffix(roleFilename, ".json")
-	if !verify.ValidRole(role) {
+	if !roles.IsTopLevelRole(role) {
 		return ErrInvalidRole{role}
 	}
 
@@ -695,15 +696,6 @@ func (r *Repo) SignedMeta(roleFilename string) (*data.Signed, error) {
 	return s, nil
 }
 
-func validMetadata(roleFilename string) bool {
-	for _, m := range topLevelMetadata {
-		if m == roleFilename {
-			return true
-		}
-	}
-	return false
-}
-
 func (r *Repo) AddTarget(path string, custom json.RawMessage) error {
 	return r.AddTargets([]string{path}, custom)
 }
diff --git a/verify/db.go b/verify/db.go
index 0684e6a..f809490 100644
--- a/verify/db.go
+++ b/verify/db.go
@@ -2,6 +2,7 @@ package verify
 
 import (
 	"github.com/theupdateframework/go-tuf/data"
+	"github.com/theupdateframework/go-tuf/internal/roles"
 	"github.com/theupdateframework/go-tuf/pkg/keys"
 )
 
@@ -44,7 +45,7 @@ func NewDelegationsVerifier(d *data.Delegations) (DelegationsVerifier, error) {
 		verifiers: make(map[string]keys.Verifier, len(d.Keys)),
 	}
 	for _, r := range d.Roles {
-		if _, ok := topLevelRoles[r.Name]; ok {
+		if _, ok := roles.TopLevelRoles[r.Name]; ok {
 			return DelegationsVerifier{}, ErrInvalidDelegatedRole
 		}
 		role := &data.Role{Threshold: r.Threshold, KeyIDs: r.KeyIDs}
@@ -72,25 +73,8 @@ func (db *DB) AddKey(id string, k *data.PublicKey) error {
 	return nil
 }
 
-var topLevelRoles = map[string]struct{}{
-	"root":      {},
-	"targets":   {},
-	"snapshot":  {},
-	"timestamp": {},
-}
-
-// ValidRole checks if a role is a top level role.
-func ValidRole(name string) bool {
-	return isTopLevelRole(name)
-}
-
-func isTopLevelRole(name string) bool {
-	_, ok := topLevelRoles[name]
-	return ok
-}
-
 func (db *DB) AddRole(name string, r *data.Role) error {
-	if !isTopLevelRole(name) {
+	if !roles.IsTopLevelRole(name) {
 		return ErrInvalidRole
 	}
 	return db.addRole(name, r)
diff --git a/verify/verify.go b/verify/verify.go
index 0d9cb36..2586a60 100644
--- a/verify/verify.go
+++ b/verify/verify.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/secure-systems-lab/go-securesystemslib/cjson"
 	"github.com/theupdateframework/go-tuf/data"
+	"github.com/theupdateframework/go-tuf/internal/roles"
 )
 
 type signedMeta struct {
@@ -25,7 +26,7 @@ func (db *DB) VerifyIgnoreExpiredCheck(s *data.Signed, role string, minVersion i
 		return err
 	}
 
-	if isTopLevelRole(role) {
+	if roles.IsTopLevelRole(role) {
 		// Top-level roles can only sign metadata of the same type (e.g. snapshot
 		// metadata must be signed by the snapshot role).
 		if !strings.EqualFold(sm.Type, role) {


Test patch:
diff --git a/internal/roles/roles_test.go b/internal/roles/roles_test.go
new file mode 100644
index 0000000..17ed265
--- /dev/null
+++ b/internal/roles/roles_test.go
@@ -0,0 +1,48 @@
+package roles
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsTopLevelRole(t *testing.T) {
+	assert.True(t, IsTopLevelRole("root"))
+	assert.True(t, IsTopLevelRole("targets"))
+	assert.True(t, IsTopLevelRole("timestamp"))
+	assert.True(t, IsTopLevelRole("snapshot"))
+	assert.False(t, IsTopLevelRole("bins"))
+}
+
+func TestIsDelegatedTargetsRole(t *testing.T) {
+	assert.False(t, IsDelegatedTargetsRole("root"))
+	assert.False(t, IsDelegatedTargetsRole("targets"))
+	assert.False(t, IsDelegatedTargetsRole("timestamp"))
+	assert.False(t, IsDelegatedTargetsRole("snapshot"))
+	assert.True(t, IsDelegatedTargetsRole("deleg"))
+}
+
+func TestIsTopLevelManifest(t *testing.T) {
+	assert.True(t, IsTopLevelManifest("root.json"))
+	assert.True(t, IsTopLevelManifest("targets.json"))
+	assert.True(t, IsTopLevelManifest("timestamp.json"))
+	assert.True(t, IsTopLevelManifest("snapshot.json"))
+	assert.False(t, IsTopLevelManifest("bins.json"))
+}
+
+func TestIsDelegatedTargetsManifest(t *testing.T) {
+	assert.False(t, IsDelegatedTargetsManifest("root.json"))
+	assert.False(t, IsDelegatedTargetsManifest("targets.json"))
+	assert.False(t, IsDelegatedTargetsManifest("timestamp.json"))
+	assert.False(t, IsDelegatedTargetsManifest("snapshot.json"))
+	assert.True(t, IsDelegatedTargetsManifest("bins.json"))
+}
+
+func TestIsVersionedManifest(t *testing.T) {
+	assert.False(t, IsVersionedManifest("a.b"))
+	assert.False(t, IsVersionedManifest("a.b.c"))
+	assert.False(t, IsVersionedManifest("a.b.json"))
+	assert.False(t, IsVersionedManifest("1.a"))
+	assert.True(t, IsVersionedManifest("1.a.json"))
+	assert.True(t, IsVersionedManifest("2.a.json"))
+}