From f9cb6eab9a639df67cc59b4dd3140dc9b2501c79 Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 30 Aug 2019 17:19:19 +0200 Subject: [PATCH 01/11] Add Get function for directories --- pkg/secrethub/dir.go | 32 ++++++++++++++++++++++++++ pkg/secrethub/fakeclient/dir.go | 2 ++ pkg/secrethub/internals/http/client.go | 9 ++++++++ 3 files changed, 43 insertions(+) diff --git a/pkg/secrethub/dir.go b/pkg/secrethub/dir.go index 10ebd690..8496f3b8 100644 --- a/pkg/secrethub/dir.go +++ b/pkg/secrethub/dir.go @@ -9,6 +9,8 @@ import ( type DirService interface { // Create a directory at a given path. Create(path string) (*api.Dir, error) + // Get returns the directory on the given path. + Get(path string) (*api.Dir, error) // Delete removes the directory at the given path. Delete(path string) error // GetTree retrieves a directory at a given path and all of its descendants up to a given depth. @@ -27,6 +29,36 @@ type dirService struct { client *Client } +// Get returns the directory on the given path. +func (s dirService) Get(path string) (*api.Dir, error) { + p, err := api.NewDirPath(path) + if err != nil { + return nil, err + } + + blindName, err := s.client.convertPathToBlindName(p) + if err != nil { + return nil, errio.Error(err) + } + + encDir, err := s.client.httpClient.GetDir(p.GetNamespace(), p.GetRepo(), blindName) + if err != nil { + return nil, errio.Error(err) + } + + accountKey, err := s.client.getAccountKey() + if err != nil { + return nil, errio.Error(err) + } + + dir, err := encDir.Decrypt(accountKey) + if err != nil { + return nil, errio.Error(err) + } + + return dir, nil +} + // GetTree retrieves a directory tree at a given path. The contents to the given depth // are returned. When depth is -1 all contents of the directory are included in the tree. // When ancestors is true, the parent directories of the dir at the given path will also diff --git a/pkg/secrethub/fakeclient/dir.go b/pkg/secrethub/fakeclient/dir.go index fdb6289d..5d7dc9ba 100644 --- a/pkg/secrethub/fakeclient/dir.go +++ b/pkg/secrethub/fakeclient/dir.go @@ -4,6 +4,7 @@ package fakeclient import ( "github.com/secrethub/secrethub-go/internals/api" + "github.com/secrethub/secrethub-go/pkg/secrethub" ) // DirService is a mock of the DirService interface. @@ -11,6 +12,7 @@ type DirService struct { Creater DirCreater Deleter DirDeleter TreeGetter TreeGetter + secrethub.DirService } // Create implements the DirService interface Create function. diff --git a/pkg/secrethub/internals/http/client.go b/pkg/secrethub/internals/http/client.go index b7fcfffd..d6587288 100644 --- a/pkg/secrethub/internals/http/client.go +++ b/pkg/secrethub/internals/http/client.go @@ -52,6 +52,7 @@ const ( pathRepoKey = "%s/namespaces/%s/repos/%s/keys" pathRepoAccounts = "%s/namespaces/%s/repos/%s/accounts" pathRepoEvents = "%s/namespaces/%s/repos/%s/events" + pathRepoDir = "%s/namespaces/%s/repos/%s/dirs/%s" pathRepoDirSecrets = "%s/namespaces/%s/repos/%s/dirs/%s/secrets" pathRepoUsers = "%s/namespaces/%s/repos/%s/users" pathRepoUser = "%s/namespaces/%s/repos/%s/users/%s" @@ -335,6 +336,14 @@ func (c *Client) CreateDir(namespace, repoName string, in *api.CreateDirRequest) return out, errio.Error(err) } +// GetDir retrieves a directory encrypted for the authenticated user. +func (c *Client) GetDir(namespace, repoName, dirBlindName string) (*api.EncryptedDir, error) { + rawURL := fmt.Sprintf(pathRepoDir, c.base, namespace, repoName, dirBlindName) + out := &api.EncryptedDir{} + err := c.get(rawURL, true, out) + return out, err +} + // GetTree gets a directory and all of it subdirs and secrets recursively by blind name. // If depth is > 0 then the result is limited to depth // If ancestors = true then ancestors are added. From b1f31b3b769794fd23ed41444b24914dd15e1082 Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Sun, 1 Sep 2019 14:18:39 +0200 Subject: [PATCH 02/11] Don't return an unexpected server error when the response cannot be parsed This hides useful information in resolving the problem. --- pkg/secrethub/internals/http/encoding.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/pkg/secrethub/internals/http/encoding.go b/pkg/secrethub/internals/http/encoding.go index 95abb83e..131a9ee8 100644 --- a/pkg/secrethub/internals/http/encoding.go +++ b/pkg/secrethub/internals/http/encoding.go @@ -3,7 +3,6 @@ package http import ( "bytes" "encoding/json" - "fmt" "io/ioutil" "net/http" "strconv" @@ -98,21 +97,17 @@ func parseError(resp *http.Response) error { if err != nil { // Degrade with a best effort error message. log.Debugf("body: %v", string(bytes)) - return errio.UnexpectedStatusError( - fmt.Errorf("(cannot_parse_server_response) %d - %s: %v", - resp.StatusCode, - resp.Status, - err, - ), + return errHTTP.Code("cannot_parse_server_response").Errorf("%d - %s: %v", + resp.StatusCode, + resp.Status, + err, ) } if e.Message == "" { - return errio.UnexpectedStatusError( - fmt.Errorf("(unexpected_message_in_server_error) %d - %s. Response:\n%s", - resp.StatusCode, - resp.Status, - string(bytes), - ), + return errHTTP.Code("unexpected_message_in_server_error").Errorf("%d - %s. Response:\n%s", + resp.StatusCode, + resp.Status, + string(bytes), ) } From da95bd8ba55aaf70991e0586170e3579711b7bac Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Wed, 4 Sep 2019 14:42:25 +0200 Subject: [PATCH 03/11] Use the dirs/{dir_id} route to get a directory --- pkg/secrethub/dir.go | 17 ++++------------- pkg/secrethub/internals/http/client.go | 6 +++--- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/pkg/secrethub/dir.go b/pkg/secrethub/dir.go index 8496f3b8..a88a3935 100644 --- a/pkg/secrethub/dir.go +++ b/pkg/secrethub/dir.go @@ -2,6 +2,7 @@ package secrethub import ( "github.com/secrethub/secrethub-go/internals/api" + "github.com/secrethub/secrethub-go/internals/api/uuid" "github.com/secrethub/secrethub-go/internals/errio" ) @@ -10,7 +11,7 @@ type DirService interface { // Create a directory at a given path. Create(path string) (*api.Dir, error) // Get returns the directory on the given path. - Get(path string) (*api.Dir, error) + Get(id *uuid.UUID) (*api.Dir, error) // Delete removes the directory at the given path. Delete(path string) error // GetTree retrieves a directory at a given path and all of its descendants up to a given depth. @@ -30,18 +31,8 @@ type dirService struct { } // Get returns the directory on the given path. -func (s dirService) Get(path string) (*api.Dir, error) { - p, err := api.NewDirPath(path) - if err != nil { - return nil, err - } - - blindName, err := s.client.convertPathToBlindName(p) - if err != nil { - return nil, errio.Error(err) - } - - encDir, err := s.client.httpClient.GetDir(p.GetNamespace(), p.GetRepo(), blindName) +func (s dirService) Get(id *uuid.UUID) (*api.Dir, error) { + encDir, err := s.client.httpClient.GetDir(id) if err != nil { return nil, errio.Error(err) } diff --git a/pkg/secrethub/internals/http/client.go b/pkg/secrethub/internals/http/client.go index d6587288..62af34b0 100644 --- a/pkg/secrethub/internals/http/client.go +++ b/pkg/secrethub/internals/http/client.go @@ -10,6 +10,7 @@ import ( "github.com/op/go-logging" "github.com/secrethub/secrethub-go/internals/api" + "github.com/secrethub/secrethub-go/internals/api/uuid" "github.com/secrethub/secrethub-go/internals/auth" "github.com/secrethub/secrethub-go/internals/errio" ) @@ -52,7 +53,6 @@ const ( pathRepoKey = "%s/namespaces/%s/repos/%s/keys" pathRepoAccounts = "%s/namespaces/%s/repos/%s/accounts" pathRepoEvents = "%s/namespaces/%s/repos/%s/events" - pathRepoDir = "%s/namespaces/%s/repos/%s/dirs/%s" pathRepoDirSecrets = "%s/namespaces/%s/repos/%s/dirs/%s/secrets" pathRepoUsers = "%s/namespaces/%s/repos/%s/users" pathRepoUser = "%s/namespaces/%s/repos/%s/users/%s" @@ -337,8 +337,8 @@ func (c *Client) CreateDir(namespace, repoName string, in *api.CreateDirRequest) } // GetDir retrieves a directory encrypted for the authenticated user. -func (c *Client) GetDir(namespace, repoName, dirBlindName string) (*api.EncryptedDir, error) { - rawURL := fmt.Sprintf(pathRepoDir, c.base, namespace, repoName, dirBlindName) +func (c *Client) GetDir(id *uuid.UUID) (*api.EncryptedDir, error) { + rawURL := fmt.Sprintf(pathDir, c.base, id.String()) out := &api.EncryptedDir{} err := c.get(rawURL, true, out) return out, err From 414fa40f4aab4369ce89778411b8d5c693e6a390 Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Thu, 5 Sep 2019 09:53:20 +0200 Subject: [PATCH 04/11] Use values instead of pointers for uuid values that are always set On responses from the server, the UUID values are always given (except for the parent directory of a dir), therefore, we can return the values themselves instead of pointers. By doing so, the consuming applications don't have to dereference the pointers everywhere they are used. This change becomes more relevant now, as we are creating endpoints to get resources by their IDs, so the IDs will be consumed more often in the future. --- internals/api/account.go | 2 +- internals/api/acl.go | 10 +++---- internals/api/audit.go | 10 +++---- internals/api/credential.go | 2 +- internals/api/dir.go | 4 +-- internals/api/dir_test.go | 10 +++---- internals/api/dirfs.go | 18 ++++++------- internals/api/dirfs_test.go | 18 ++++++------- internals/api/encrypted_data_test.go | 4 +-- internals/api/org.go | 14 +++++----- internals/api/repo.go | 8 +++--- internals/api/repo_test.go | 4 +-- internals/api/secret.go | 24 ++++++++--------- internals/api/secret_key.go | 8 +++--- internals/api/secret_test.go | 40 ++++++++++++++-------------- internals/api/secret_version.go | 4 +-- internals/api/secret_version_test.go | 3 ++- internals/api/service.go | 2 +- internals/api/user.go | 2 +- internals/api/uuid/uuid.go | 16 ++++++----- internals/api/uuid_test.go | 10 +++++++ pkg/secrethub/acl.go | 4 +-- pkg/secrethub/crypto.go | 14 +++++----- pkg/secrethub/repo_user.go | 2 +- pkg/secrethub/secret_key.go | 2 +- pkg/secrethub/secret_version.go | 2 +- 26 files changed, 126 insertions(+), 111 deletions(-) create mode 100644 internals/api/uuid_test.go diff --git a/internals/api/account.go b/internals/api/account.go index 596c81ac..8973deac 100644 --- a/internals/api/account.go +++ b/internals/api/account.go @@ -18,7 +18,7 @@ var ( // Account represents an account on SecretHub. type Account struct { - AccountID *uuid.UUID `json:"account_id"` + AccountID uuid.UUID `json:"account_id"` Name AccountName `json:"name"` PublicKey []byte `json:"public_key"` AccountType string `json:"account_type"` diff --git a/internals/api/acl.go b/internals/api/acl.go index 45ffa002..8a1d230a 100644 --- a/internals/api/acl.go +++ b/internals/api/acl.go @@ -20,9 +20,9 @@ var ( // a directory and its children. type AccessRule struct { Account *Account `json:"account"` - AccountID *uuid.UUID `json:"account_id"` - DirID *uuid.UUID `json:"dir_id"` - RepoID *uuid.UUID `json:"repo_id"` + AccountID uuid.UUID `json:"account_id"` + DirID uuid.UUID `json:"dir_id"` + RepoID uuid.UUID `json:"repo_id"` Permission Permission `json:"permission"` CreatedAt time.Time `json:"created_at"` LastChangedAt time.Time `json:"last_changed_at"` @@ -59,8 +59,8 @@ func (car *CreateAccessRuleRequest) Validate() error { // effect of one or more access rules on the directory itself or its parent(s). type AccessLevel struct { Account *Account `json:"account"` - AccountID *uuid.UUID `json:"account_id"` - DirID *uuid.UUID `json:"dir_id"` + AccountID uuid.UUID `json:"account_id"` + DirID uuid.UUID `json:"dir_id"` Permission Permission `json:"permission"` } diff --git a/internals/api/audit.go b/internals/api/audit.go index 4cdaa0ae..04ad6df6 100644 --- a/internals/api/audit.go +++ b/internals/api/audit.go @@ -17,7 +17,7 @@ const ( // Audit represents an AuditEvent in SecretHub. type Audit struct { - EventID *uuid.UUID `json:"event_id"` + EventID uuid.UUID `json:"event_id"` Action AuditAction `json:"action"` IPAddress string `json:"ip_address"` LoggedAt time.Time `json:"logged_at"` @@ -31,8 +31,8 @@ type AuditAction string // AuditActor represents the Account of an AuditEvent type AuditActor struct { - ActorID *uuid.UUID `json:"id,omitempty"` - Deleted bool `json:"deleted,omitempty"` + ActorID uuid.UUID `json:"id,omitempty"` + Deleted bool `json:"deleted,omitempty"` // Type is `user` or `service`. When actor is deleted, type is always `account` Type string `json:"type"` User *User `json:"user,omitempty"` @@ -61,8 +61,8 @@ const ( // AuditSubject represents the Subject of an AuditEvent type AuditSubject struct { - SubjectID *uuid.UUID `json:"id,omitempty"` - Deleted bool `json:"deleted,omitempty"` + SubjectID uuid.UUID `json:"id,omitempty"` + Deleted bool `json:"deleted,omitempty"` // Type is `user`, `service`, `repo`, `secret`, `secret_version` or `secret_key`. When subject is deleted, user and service are indicated with type `account` Type AuditSubjectType `json:"type"` User *User `json:"user,omitempty"` diff --git a/internals/api/credential.go b/internals/api/credential.go index 3c033776..de4ddce6 100644 --- a/internals/api/credential.go +++ b/internals/api/credential.go @@ -38,7 +38,7 @@ const ( // Credential is used to authenticate to the API and to encrypt the account key. type Credential struct { - AccountID *uuid.UUID `json:"account_id"` + AccountID uuid.UUID `json:"account_id"` Type CredentialType `json:"type"` CreatedAt time.Time `json:"created_at"` Fingerprint string `json:"fingerprint"` diff --git a/internals/api/dir.go b/internals/api/dir.go index 6c50e5a1..eb62c43e 100644 --- a/internals/api/dir.go +++ b/internals/api/dir.go @@ -24,7 +24,7 @@ var ( // The names are encrypted and so are the names of SubDirs and Secrets. // The secrets contain no encrypted data, only the encrypted name. type EncryptedDir struct { - DirID *uuid.UUID `json:"dir_id"` + DirID uuid.UUID `json:"dir_id"` BlindName string `json:"blind_name"` EncryptedName crypto.CiphertextRSA `json:"encrypted_name"` ParentID *uuid.UUID `json:"parent_id"` @@ -56,7 +56,7 @@ func (ed *EncryptedDir) Decrypt(accountKey *crypto.RSAPrivateKey) (*Dir, error) // Dir represents an directory. // A dir belongs to a repo and contains other dirs and secrets. type Dir struct { - DirID *uuid.UUID `json:"dir_id"` + DirID uuid.UUID `json:"dir_id"` BlindName string `json:"blind_name"` Name string `json:"name"` ParentID *uuid.UUID `json:"parent_id"` diff --git a/internals/api/dir_test.go b/internals/api/dir_test.go index dc22ae4a..85d195ac 100644 --- a/internals/api/dir_test.go +++ b/internals/api/dir_test.go @@ -34,7 +34,7 @@ func TestCreateDirRequest_Validate(t *testing.T) { ParentBlindName: parentPathBlindName, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedName: testCiphertextRSA, }, }, @@ -45,7 +45,7 @@ func TestCreateDirRequest_Validate(t *testing.T) { createDirRequest: &api.CreateDirRequest{ BlindName: dirPathBlindName, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedName: testCiphertextRSA, }, }, @@ -80,11 +80,11 @@ func TestCreateDirRequest_Validate_UniqueEncryptedFor(t *testing.T) { EncryptedNames: []api.EncryptedNameRequest{ { - AccountID: accountID, + AccountID: UUID(accountID), EncryptedName: testCiphertextRSA, }, { - AccountID: accountID, + AccountID: UUID(accountID), EncryptedName: testCiphertextRSA, }, }, @@ -111,7 +111,7 @@ func getTestCreateDirRequest(t *testing.T) *api.CreateDirRequest { ParentBlindName: parentPathBlindName, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedName: testCiphertextRSA, }, }, diff --git a/internals/api/dirfs.go b/internals/api/dirfs.go index d506e1bd..3b6dd437 100644 --- a/internals/api/dirfs.go +++ b/internals/api/dirfs.go @@ -65,7 +65,7 @@ func (t EncryptedTree) Decrypt(accountKey *crypto.RSAPrivateKey) (*Tree, error) // This has to be prepopulated to be able to put childs under parents. dirMap := make(map[uuid.UUID]*Dir) for _, dir := range dirs { - dirMap[*dir.DirID] = dir + dirMap[dir.DirID] = dir } // All directories are looped and placed below the parent directory as a subdirectory. @@ -89,13 +89,13 @@ func (t EncryptedTree) Decrypt(accountKey *crypto.RSAPrivateKey) (*Tree, error) secretMap := make(map[uuid.UUID]*Secret) // All secrets are placed below every directory. for _, secret := range secrets { - dir, ok := dirMap[*secret.DirID] + dir, ok := dirMap[secret.DirID] if !ok { return nil, ErrParentDirNotAvailable } dir.Secrets = append(dir.Secrets, secret) - secretMap[*secret.SecretID] = secret + secretMap[secret.SecretID] = secret } return &Tree{ @@ -130,8 +130,8 @@ func (t Tree) DirCount() int { // AbsSecretPath returns the full path of secret. // This function makes the assumption that every secret has a ParentDir. // If not, an error will occur. -func (t Tree) AbsSecretPath(secretID *uuid.UUID) (*SecretPath, error) { - secret, ok := t.Secrets[*secretID] +func (t Tree) AbsSecretPath(secretID uuid.UUID) (*SecretPath, error) { + secret, ok := t.Secrets[secretID] if !ok { return nil, ErrSecretNotFound } @@ -149,18 +149,18 @@ func (t Tree) AbsSecretPath(secretID *uuid.UUID) (*SecretPath, error) { // AbsDirPath returns the full path of dir // This function makes the assumption that only the root dir has no parentID. // If not, an error will occur. -func (t Tree) AbsDirPath(dirID *uuid.UUID) (DirPath, error) { - if uuid.Equal(dirID, t.RootDir.DirID) { +func (t Tree) AbsDirPath(dirID uuid.UUID) (DirPath, error) { + if uuid.Equal(&dirID, &t.RootDir.DirID) { dirPath := t.ParentPath.JoinDir(t.RootDir.Name) return dirPath, nil } - dir, ok := t.Dirs[*dirID] + dir, ok := t.Dirs[dirID] if !ok { return "", ErrDirNotFound } - parentPath, err := t.AbsDirPath(dir.ParentID) + parentPath, err := t.AbsDirPath(*dir.ParentID) if err != nil { return "", errio.Error(err) } diff --git a/internals/api/dirfs_test.go b/internals/api/dirfs_test.go index cfec8a72..b970bb2d 100644 --- a/internals/api/dirfs_test.go +++ b/internals/api/dirfs_test.go @@ -23,13 +23,13 @@ func TestAbsDirPath(t *testing.T) { dir := &Dir{ DirID: uuid.New(), Name: "dir", - ParentID: repoDir.DirID, + ParentID: &repoDir.DirID, } subdir := &Dir{ DirID: uuid.New(), Name: "subdir", - ParentID: dir.DirID, + ParentID: &dir.DirID, } repoPath := DirPath("namespace/repo") @@ -37,7 +37,7 @@ func TestAbsDirPath(t *testing.T) { subdirPath := DirPath("namespace/repo/dir/subdir") cases := map[string]struct { - dirID *uuid.UUID + dirID uuid.UUID tree Tree expected DirPath err error @@ -48,7 +48,7 @@ func TestAbsDirPath(t *testing.T) { ParentPath: "namespace", RootDir: repoDir, Dirs: map[uuid.UUID]*Dir{ - *repoDir.DirID: repoDir, + repoDir.DirID: repoDir, }, Secrets: map[uuid.UUID]*Secret{}, }, @@ -61,7 +61,7 @@ func TestAbsDirPath(t *testing.T) { ParentPath: "namespace", RootDir: repoDir, Dirs: map[uuid.UUID]*Dir{ - *dir.DirID: dir, + dir.DirID: dir, }, Secrets: map[uuid.UUID]*Secret{}, }, @@ -74,8 +74,8 @@ func TestAbsDirPath(t *testing.T) { ParentPath: "namespace/repo", RootDir: dir, Dirs: map[uuid.UUID]*Dir{ - *dir.DirID: dir, - *subdir.DirID: subdir, + dir.DirID: dir, + subdir.DirID: subdir, }, Secrets: map[uuid.UUID]*Secret{}, }, @@ -88,8 +88,8 @@ func TestAbsDirPath(t *testing.T) { ParentPath: "namespace/repo", RootDir: dir, Dirs: map[uuid.UUID]*Dir{ - *dir.DirID: dir, - *subdir.DirID: subdir, + dir.DirID: dir, + subdir.DirID: subdir, }, Secrets: map[uuid.UUID]*Secret{}, }, diff --git a/internals/api/encrypted_data_test.go b/internals/api/encrypted_data_test.go index fe0ca3d4..87d2a3fb 100644 --- a/internals/api/encrypted_data_test.go +++ b/internals/api/encrypted_data_test.go @@ -10,7 +10,7 @@ import ( ) func TestEncryptedData_MarshalUnmarshalValidate(t *testing.T) { - encryptedDataRSAAccountKey := NewEncryptedDataRSAOAEP([]byte("rsa-ciphertext"), HashingAlgorithmSHA256, NewEncryptionKeyAccountKey(4096, *uuid.New())) + encryptedDataRSAAccountKey := NewEncryptedDataRSAOAEP([]byte("rsa-ciphertext"), HashingAlgorithmSHA256, NewEncryptionKeyAccountKey(4096, uuid.New())) cases := map[string]struct { in *EncryptedData @@ -24,7 +24,7 @@ func TestEncryptedData_MarshalUnmarshalValidate(t *testing.T) { in: NewEncryptedDataAESGCM([]byte("ciphertext"), []byte("nonce"), 96, NewEncryptionKeyLocal(256)), }, "aes with secret key": { - in: NewEncryptedDataAESGCM([]byte("ciphertext"), []byte("nonce"), 96, NewEncryptionKeySecretKey(256, *uuid.New())), + in: NewEncryptedDataAESGCM([]byte("ciphertext"), []byte("nonce"), 96, NewEncryptionKeySecretKey(256, uuid.New())), }, "rsa account key": { in: encryptedDataRSAAccountKey, diff --git a/internals/api/org.go b/internals/api/org.go index 64a51383..cb63367e 100644 --- a/internals/api/org.go +++ b/internals/api/org.go @@ -16,7 +16,7 @@ const ( // Org represents an organization account on SecretHub type Org struct { - OrgID *uuid.UUID `json:"org_id"` + OrgID uuid.UUID `json:"org_id"` Name string `json:"name"` Description string `json:"description"` CreatedAt time.Time `json:"created_at"` @@ -38,12 +38,12 @@ func (s SortOrgByName) Less(i, j int) bool { // OrgMember represents a user's membership of an organization. type OrgMember struct { - OrgID *uuid.UUID `json:"org_id"` - AccountID *uuid.UUID `json:"account_id"` - Role string `json:"role"` - CreatedAt time.Time `json:"created_at"` - LastChangedAt time.Time `json:"last_changed_at"` - User *User `json:"user,omitempty"` + OrgID uuid.UUID `json:"org_id"` + AccountID uuid.UUID `json:"account_id"` + Role string `json:"role"` + CreatedAt time.Time `json:"created_at"` + LastChangedAt time.Time `json:"last_changed_at"` + User *User `json:"user,omitempty"` } // SortOrgMemberByUsername makes a list of org members sortable. diff --git a/internals/api/repo.go b/internals/api/repo.go index af1b143d..73f6f4a0 100644 --- a/internals/api/repo.go +++ b/internals/api/repo.go @@ -28,7 +28,7 @@ var ( // Repo represents a repo on SecretHub. type Repo struct { - RepoID *uuid.UUID `json:"repo_id"` + RepoID uuid.UUID `json:"repo_id"` Owner string `json:"owner"` Name string `json:"name"` CreatedAt *time.Time `json:"created_at"` @@ -112,9 +112,9 @@ func (crr CreateRepoRequest) Validate() error { // RepoMember represents a member of a SecretHub repo. type RepoMember struct { - RepoID *uuid.UUID `json:"repo_id"` - AccountID *uuid.UUID `json:"account_id"` - CreatedAt time.Time `json:"created_at"` + RepoID uuid.UUID `json:"repo_id"` + AccountID uuid.UUID `json:"account_id"` + CreatedAt time.Time `json:"created_at"` } // CreateRepoMemberRequest contains the required fields for adding a user to a repo. diff --git a/internals/api/repo_test.go b/internals/api/repo_test.go index 857f2cae..4fb99992 100644 --- a/internals/api/repo_test.go +++ b/internals/api/repo_test.go @@ -51,7 +51,7 @@ func TestCreateRepoRequest_Validate(t *testing.T) { func TestInviteUserRequest_Validate_Success(t *testing.T) { inviteRequest := &api.InviteUserRequest{ - AccountID: accountIDUser1, + AccountID: &accountIDUser1, RepoMember: repoMemberRequest1, } @@ -65,7 +65,7 @@ func TestInviteUserRequest_Validate_Success(t *testing.T) { func TestInviteUserRequest_Validate_InvalidRepoMember(t *testing.T) { inviteRequest := &api.InviteUserRequest{ - AccountID: accountIDUser1, + AccountID: &accountIDUser1, RepoMember: &api.CreateRepoMemberRequest{}, } diff --git a/internals/api/secret.go b/internals/api/secret.go index 63cc15fd..1fe50fdd 100644 --- a/internals/api/secret.go +++ b/internals/api/secret.go @@ -40,9 +40,9 @@ var ( // EncryptedSecret represents an encrypted Secret // It does not contain the encrypted data. Only the encrypted name. type EncryptedSecret struct { - SecretID *uuid.UUID `json:"secret_id"` - DirID *uuid.UUID `json:"dir_id"` - RepoID *uuid.UUID `json:"repo_id"` + SecretID uuid.UUID `json:"secret_id"` + DirID uuid.UUID `json:"dir_id"` + RepoID uuid.UUID `json:"repo_id"` EncryptedName crypto.CiphertextRSA `json:"encrypted_name"` BlindName string `json:"blind_name"` VersionCount int `json:"version_count"` @@ -73,15 +73,15 @@ func (es *EncryptedSecret) Decrypt(accountKey *crypto.RSAPrivateKey) (*Secret, e // Secret represents a decrypted secret in SecretHub. type Secret struct { - SecretID *uuid.UUID `json:"secret_id"` - DirID *uuid.UUID `json:"dir_id"` - RepoID *uuid.UUID `json:"repo_id"` - Name string `json:"name"` - BlindName string `json:"blind_name"` - VersionCount int `json:"version_count"` - LatestVersion int `json:"latest_version"` - Status string `json:"status"` - CreatedAt time.Time `json:"created_at"` + SecretID uuid.UUID `json:"secret_id"` + DirID uuid.UUID `json:"dir_id"` + RepoID uuid.UUID `json:"repo_id"` + Name string `json:"name"` + BlindName string `json:"blind_name"` + VersionCount int `json:"version_count"` + LatestVersion int `json:"latest_version"` + Status string `json:"status"` + CreatedAt time.Time `json:"created_at"` } // HasName returns true when the secret version has the exact name. diff --git a/internals/api/secret_key.go b/internals/api/secret_key.go index 763d9c82..6671c13f 100644 --- a/internals/api/secret_key.go +++ b/internals/api/secret_key.go @@ -8,15 +8,15 @@ import ( // SecretKey represents a secret key that is intended to be used by a specific account. type SecretKey struct { - SecretKeyID *uuid.UUID `json:"secret_key_id"` - AccountID *uuid.UUID `json:"account_id"` + SecretKeyID uuid.UUID `json:"secret_key_id"` + AccountID uuid.UUID `json:"account_id"` Key *crypto.SymmetricKey `json:"key"` } // EncryptedSecretKey represents a secret key, encrypted for a specific account. type EncryptedSecretKey struct { - SecretKeyID *uuid.UUID `json:"secret_key_id"` - AccountID *uuid.UUID `json:"account_id"` + SecretKeyID uuid.UUID `json:"secret_key_id"` + AccountID uuid.UUID `json:"account_id"` EncryptedKey crypto.CiphertextRSA `json:"encrypted_key"` } diff --git a/internals/api/secret_test.go b/internals/api/secret_test.go index d435c67a..4115e95d 100644 --- a/internals/api/secret_test.go +++ b/internals/api/secret_test.go @@ -74,16 +74,16 @@ func TestCreateSecretRequest_Validate_Unique(t *testing.T) { EncryptedData: testCiphertextAES, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: accountID, + AccountID: &accountID, EncryptedName: testCiphertextRSA, }, { - AccountID: accountID, + AccountID: &accountID, EncryptedName: testCiphertextRSA, }, }, EncryptedKeys: []api.EncryptedKeyRequest{{ - AccountID: accountID, + AccountID: &accountID, EncryptedKey: testCiphertextRSA, }, }, @@ -96,16 +96,16 @@ func TestCreateSecretRequest_Validate_Unique(t *testing.T) { EncryptedData: testCiphertextAES, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: accountID, + AccountID: &accountID, EncryptedName: testCiphertextRSA, }, }, EncryptedKeys: []api.EncryptedKeyRequest{{ - AccountID: accountID, + AccountID: &accountID, EncryptedKey: testCiphertextRSA, }, { - AccountID: accountID, + AccountID: &accountID, EncryptedKey: testCiphertextRSA, }, }, @@ -136,12 +136,12 @@ func TestCreateSecretRequest_Validate_EncryptedNameAndKeyForEachAccount(t *testi EncryptedData: testCiphertextAES, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedName: testCiphertextRSA, }, }, EncryptedKeys: []api.EncryptedKeyRequest{{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedKey: testCiphertextRSA, }, }, @@ -160,17 +160,17 @@ func TestExistingNameMemberRequest_Validate(t *testing.T) { "success": { EncryptedNameRequest: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedName: testCiphertextRSA, }, - NodeID: uuid.New(), + NodeID: UUID(uuid.New()), }, expected: nil, }, "invalid node id": { EncryptedNameRequest: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedName: testCiphertextRSA, }, }, @@ -178,7 +178,7 @@ func TestExistingNameMemberRequest_Validate(t *testing.T) { }, "invalid account id": { EncryptedNameRequest: api.EncryptedNameForNodeRequest{ - NodeID: uuid.New(), + NodeID: UUID(uuid.New()), EncryptedNameRequest: api.EncryptedNameRequest{ EncryptedName: testCiphertextRSA, }, @@ -211,14 +211,14 @@ func TestSecretAccessRequest_Validate_AccountIDs(t *testing.T) { Request: api.SecretAccessRequest{ Name: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: uuid.New(), + AccountID: UUID(uuid.New()), EncryptedName: testCiphertextRSA, }, - NodeID: uuid.New(), + NodeID: UUID(uuid.New()), }, Keys: []api.SecretKeyMemberRequest{{ - AccountID: uuid.New(), - SecretKeyID: uuid.New(), + AccountID: UUID(uuid.New()), + SecretKeyID: UUID(uuid.New()), EncryptedKey: testCiphertextRSA, }, }, @@ -229,14 +229,14 @@ func TestSecretAccessRequest_Validate_AccountIDs(t *testing.T) { Request: api.SecretAccessRequest{ Name: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: testAccountID, + AccountID: &testAccountID, EncryptedName: testCiphertextRSA, }, - NodeID: uuid.New(), + NodeID: UUID(uuid.New()), }, Keys: []api.SecretKeyMemberRequest{{ - AccountID: testAccountID, - SecretKeyID: uuid.New(), + AccountID: &testAccountID, + SecretKeyID: UUID(uuid.New()), EncryptedKey: testCiphertextRSA, }, }, diff --git a/internals/api/secret_version.go b/internals/api/secret_version.go index 486f8df2..71b70cb6 100644 --- a/internals/api/secret_version.go +++ b/internals/api/secret_version.go @@ -34,7 +34,7 @@ var ( // EncryptedSecretVersion represents a version of an encrypted Secret. // It contains the encrypted data and the corresponding key. type EncryptedSecretVersion struct { - SecretVersionID *uuid.UUID `json:"secret_version_id"` + SecretVersionID uuid.UUID `json:"secret_version_id"` Secret *EncryptedSecret `json:"secret"` Version int `json:"version"` SecretKey *EncryptedSecretKey `json:"secret_key,omitempty"` @@ -77,7 +77,7 @@ func (esv *EncryptedSecretVersion) Decrypt(accountKey *crypto.RSAPrivateKey) (*S // SecretVersion represents a version of a Secret without any encrypted data. type SecretVersion struct { - SecretVersionID *uuid.UUID `json:"secret_version_id"` + SecretVersionID uuid.UUID `json:"secret_version_id"` Secret *Secret `json:"secret"` Version int `json:"version"` SecretKey *SecretKey `json:"secret_key,omitempty"` diff --git a/internals/api/secret_version_test.go b/internals/api/secret_version_test.go index edd7fbd6..f053faa5 100644 --- a/internals/api/secret_version_test.go +++ b/internals/api/secret_version_test.go @@ -44,8 +44,9 @@ func TestCreateSecretVersionRequest_Validate_MaxSize(t *testing.T) { t.Fatal(err) } + id := uuid.New() r := CreateSecretVersionRequest{ - SecretKeyID: uuid.New(), + SecretKeyID: &id, EncryptedData: ciphertext, } diff --git a/internals/api/service.go b/internals/api/service.go index 44dc70f7..c12a2b80 100644 --- a/internals/api/service.go +++ b/internals/api/service.go @@ -26,7 +26,7 @@ var ( // Service represents a service account on SecretHub. type Service struct { - AccountID *uuid.UUID `json:"account_id"` + AccountID uuid.UUID `json:"account_id"` ServiceID string `json:"service_id"` Repo *Repo `json:"repo"` Description string `json:"description"` diff --git a/internals/api/user.go b/internals/api/user.go index d40623f1..95fcead1 100644 --- a/internals/api/user.go +++ b/internals/api/user.go @@ -40,7 +40,7 @@ var ( // User represents a SecretHub user. type User struct { - AccountID *uuid.UUID `json:"account_id"` + AccountID uuid.UUID `json:"account_id"` PublicKey []byte `json:"public_key"` Username string `json:"username"` FullName string `json:"full_name"` diff --git a/internals/api/uuid/uuid.go b/internals/api/uuid/uuid.go index ea64a737..93b911af 100644 --- a/internals/api/uuid/uuid.go +++ b/internals/api/uuid/uuid.go @@ -11,18 +11,18 @@ type UUID struct { } // New generates a new UUID. -func New() *UUID { +func New() UUID { id := gid.NewV4() - return &UUID{id} + return UUID{id} } // FromString reads a UUID from a string -func FromString(str string) (*UUID, error) { +func FromString(str string) (UUID, error) { id, err := gid.FromString(str) if err != nil { - return nil, err + return UUID{}, err } - return &UUID{id}, nil + return UUID{id}, nil } // ToString converts UUID into string @@ -35,7 +35,11 @@ func (u *UUID) IsZero() bool { return u.UUID == gid.UUID([gid.Size]byte{0}) } -// Equal returns true if both argument uuids contain the same value. +// Equal returns true if both argument UUIDs contain the same value or if both are nil. func Equal(a *UUID, b *UUID) bool { + if a == nil || b == nil { + return a == b + } + return gid.Equal(a.UUID, b.UUID) } diff --git a/internals/api/uuid_test.go b/internals/api/uuid_test.go new file mode 100644 index 00000000..51cb3c80 --- /dev/null +++ b/internals/api/uuid_test.go @@ -0,0 +1,10 @@ +package api_test + +import ( + "github.com/secrethub/secrethub-go/internals/api/uuid" +) + +// UUID returns the pointer to the given uuid.UUID value. +func UUID(id uuid.UUID) *uuid.UUID { + return &id +} diff --git a/pkg/secrethub/acl.go b/pkg/secrethub/acl.go index ed6de4af..6cc8adb8 100644 --- a/pkg/secrethub/acl.go +++ b/pkg/secrethub/acl.go @@ -134,8 +134,8 @@ func (s accessRuleService) ListLevels(path string) ([]*api.AccessLevel, error) { rights := make(map[uuid.UUID][]*api.AccessRule) for _, rule := range rules { - list := rights[*rule.AccountID] - rights[*rule.AccountID] = append(list, rule) + list := rights[rule.AccountID] + rights[rule.AccountID] = append(list, rule) } result := make([]*api.AccessLevel, len(rights)) diff --git a/pkg/secrethub/crypto.go b/pkg/secrethub/crypto.go index f7e33c00..acf03f01 100644 --- a/pkg/secrethub/crypto.go +++ b/pkg/secrethub/crypto.go @@ -8,7 +8,7 @@ import ( ) func (c *Client) encryptDirFor(dir *api.Dir, accounts ...*api.Account) ([]api.EncryptedNameForNodeRequest, error) { - currentDir, err := encryptNameForNodeAccounts(dir.DirID, dir.Name, accounts...) + currentDir, err := encryptNameForNodeAccounts(&dir.DirID, dir.Name, accounts...) return currentDir, errio.Error(err) } @@ -49,10 +49,10 @@ func (c *Client) encryptSecretFor(secret *api.Secret, accounts ...*api.Account) encryptedName := api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: account.AccountID, + AccountID: &account.AccountID, EncryptedName: encryptedSecretName, }, - NodeID: secret.SecretID, + NodeID: &secret.SecretID, } encryptedKeys := make([]api.SecretKeyMemberRequest, len(decryptedKeys)) @@ -63,8 +63,8 @@ func (c *Client) encryptSecretFor(secret *api.Secret, accounts ...*api.Account) } encryptedKeys[keyIndex] = api.SecretKeyMemberRequest{ - AccountID: account.AccountID, - SecretKeyID: decryptedKey.SecretKeyID, + AccountID: &account.AccountID, + SecretKeyID: &decryptedKey.SecretKeyID, EncryptedKey: encryptedKey, } } @@ -114,7 +114,7 @@ func encryptNameForAccounts(name string, accounts ...*api.Account) ([]api.Encryp } encryptedNames[i] = api.EncryptedNameRequest{ - AccountID: account.AccountID, + AccountID: &account.AccountID, EncryptedName: ciphertext, } } @@ -137,7 +137,7 @@ func encryptKeyForAccounts(key *crypto.SymmetricKey, accounts ...*api.Account) ( } encryptedKeys[i] = api.EncryptedKeyRequest{ - AccountID: account.AccountID, + AccountID: &account.AccountID, EncryptedKey: encryptedSecretKey, } } diff --git a/pkg/secrethub/repo_user.go b/pkg/secrethub/repo_user.go index a8077393..7cd4eb4a 100644 --- a/pkg/secrethub/repo_user.go +++ b/pkg/secrethub/repo_user.go @@ -58,7 +58,7 @@ func (s repoUserService) Invite(path string, username string) (*api.RepoMember, } in := &api.InviteUserRequest{ - AccountID: account.AccountID, + AccountID: &account.AccountID, RepoMember: createRepoMember, } diff --git a/pkg/secrethub/secret_key.go b/pkg/secrethub/secret_key.go index a53bbf35..6ff61fa1 100644 --- a/pkg/secrethub/secret_key.go +++ b/pkg/secrethub/secret_key.go @@ -57,7 +57,7 @@ func (c *Client) createSecretKey(secretPath api.SecretPath) (*api.SecretKey, err } encryptedFor[i] = api.EncryptedKeyRequest{ - AccountID: account.AccountID, + AccountID: &account.AccountID, EncryptedKey: encryptedSecretKey, } } diff --git a/pkg/secrethub/secret_version.go b/pkg/secrethub/secret_version.go index 7f66441d..4fcf8725 100644 --- a/pkg/secrethub/secret_version.go +++ b/pkg/secrethub/secret_version.go @@ -167,7 +167,7 @@ func (c *Client) createSecretVersion(secretPath api.SecretPath, data []byte, sec in := &api.CreateSecretVersionRequest{ EncryptedData: encryptedData, - SecretKeyID: secretKey.SecretKeyID, + SecretKeyID: &secretKey.SecretKeyID, } blindName, err := c.convertPathToBlindName(secretPath) From db3f0e25ba1ea8826a5927dc91e8318803701eb1 Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 6 Sep 2019 09:00:28 +0200 Subject: [PATCH 05/11] Also use values instead of pointers for non-optional uuids in requests --- internals/api/dir.go | 2 +- internals/api/dir_test.go | 10 +++---- internals/api/dirfs.go | 2 +- internals/api/name.go | 8 +++--- internals/api/repo.go | 4 +-- internals/api/repo_test.go | 4 +-- internals/api/secret.go | 16 +++++------ internals/api/secret_key.go | 4 +-- internals/api/secret_test.go | 40 ++++++++++++++-------------- internals/api/secret_version.go | 4 +-- internals/api/secret_version_test.go | 2 +- internals/api/uuid/uuid.go | 8 ++---- internals/api/uuid_test.go | 10 ------- pkg/secrethub/crypto.go | 16 +++++------ pkg/secrethub/repo_user.go | 2 +- pkg/secrethub/secret_key.go | 2 +- pkg/secrethub/secret_version.go | 2 +- 17 files changed, 61 insertions(+), 75 deletions(-) delete mode 100644 internals/api/uuid_test.go diff --git a/internals/api/dir.go b/internals/api/dir.go index eb62c43e..2cfd1fde 100644 --- a/internals/api/dir.go +++ b/internals/api/dir.go @@ -98,7 +98,7 @@ func (cdr *CreateDirRequest) Validate() error { return err } - unique[*encryptedName.AccountID]++ + unique[encryptedName.AccountID]++ } for _, count := range unique { diff --git a/internals/api/dir_test.go b/internals/api/dir_test.go index 85d195ac..dc22ae4a 100644 --- a/internals/api/dir_test.go +++ b/internals/api/dir_test.go @@ -34,7 +34,7 @@ func TestCreateDirRequest_Validate(t *testing.T) { ParentBlindName: parentPathBlindName, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedName: testCiphertextRSA, }, }, @@ -45,7 +45,7 @@ func TestCreateDirRequest_Validate(t *testing.T) { createDirRequest: &api.CreateDirRequest{ BlindName: dirPathBlindName, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedName: testCiphertextRSA, }, }, @@ -80,11 +80,11 @@ func TestCreateDirRequest_Validate_UniqueEncryptedFor(t *testing.T) { EncryptedNames: []api.EncryptedNameRequest{ { - AccountID: UUID(accountID), + AccountID: accountID, EncryptedName: testCiphertextRSA, }, { - AccountID: UUID(accountID), + AccountID: accountID, EncryptedName: testCiphertextRSA, }, }, @@ -111,7 +111,7 @@ func getTestCreateDirRequest(t *testing.T) *api.CreateDirRequest { ParentBlindName: parentPathBlindName, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedName: testCiphertextRSA, }, }, diff --git a/internals/api/dirfs.go b/internals/api/dirfs.go index 3b6dd437..a40aeeed 100644 --- a/internals/api/dirfs.go +++ b/internals/api/dirfs.go @@ -150,7 +150,7 @@ func (t Tree) AbsSecretPath(secretID uuid.UUID) (*SecretPath, error) { // This function makes the assumption that only the root dir has no parentID. // If not, an error will occur. func (t Tree) AbsDirPath(dirID uuid.UUID) (DirPath, error) { - if uuid.Equal(&dirID, &t.RootDir.DirID) { + if uuid.Equal(dirID, t.RootDir.DirID) { dirPath := t.ParentPath.JoinDir(t.RootDir.Name) return dirPath, nil } diff --git a/internals/api/name.go b/internals/api/name.go index e6717564..5b989d1d 100644 --- a/internals/api/name.go +++ b/internals/api/name.go @@ -7,13 +7,13 @@ import ( // EncryptedNameRequest contains an EncryptedName for an Account. type EncryptedNameRequest struct { - AccountID *uuid.UUID `json:"account_id"` + AccountID uuid.UUID `json:"account_id"` EncryptedName crypto.CiphertextRSA `json:"encrypted_name"` } // Validate validates the EncryptedNameRequest to be valid. func (enr *EncryptedNameRequest) Validate() error { - if enr.AccountID == nil { + if enr.AccountID.IsZero() { return ErrInvalidAccountID } @@ -23,12 +23,12 @@ func (enr *EncryptedNameRequest) Validate() error { // EncryptedNameForNodeRequest contains an EncryptedName for an Account and the corresponding NodeID. type EncryptedNameForNodeRequest struct { EncryptedNameRequest - NodeID *uuid.UUID `json:"node_id"` + NodeID uuid.UUID `json:"node_id"` } // Validate validates the EncryptedNameForNodeRequest. func (nnr EncryptedNameForNodeRequest) Validate() error { - if nnr.NodeID == nil { + if nnr.NodeID.IsZero() { return ErrInvalidNodeID } diff --git a/internals/api/repo.go b/internals/api/repo.go index 73f6f4a0..257dbdda 100644 --- a/internals/api/repo.go +++ b/internals/api/repo.go @@ -138,13 +138,13 @@ func (req CreateRepoMemberRequest) Validate() error { // InviteUserRequest contains the required fields for inviting a user to a repo. type InviteUserRequest struct { - AccountID *uuid.UUID `json:"account_id"` + AccountID uuid.UUID `json:"account_id"` RepoMember *CreateRepoMemberRequest `json:"repo_member"` } // Validate validates a InviteUserRequest func (req InviteUserRequest) Validate() error { - if req.AccountID == nil { + if req.AccountID.IsZero() { return ErrInvalidAccountID } diff --git a/internals/api/repo_test.go b/internals/api/repo_test.go index 4fb99992..857f2cae 100644 --- a/internals/api/repo_test.go +++ b/internals/api/repo_test.go @@ -51,7 +51,7 @@ func TestCreateRepoRequest_Validate(t *testing.T) { func TestInviteUserRequest_Validate_Success(t *testing.T) { inviteRequest := &api.InviteUserRequest{ - AccountID: &accountIDUser1, + AccountID: accountIDUser1, RepoMember: repoMemberRequest1, } @@ -65,7 +65,7 @@ func TestInviteUserRequest_Validate_Success(t *testing.T) { func TestInviteUserRequest_Validate_InvalidRepoMember(t *testing.T) { inviteRequest := &api.InviteUserRequest{ - AccountID: &accountIDUser1, + AccountID: accountIDUser1, RepoMember: &api.CreateRepoMemberRequest{}, } diff --git a/internals/api/secret.go b/internals/api/secret.go index 1fe50fdd..8fbab4e9 100644 --- a/internals/api/secret.go +++ b/internals/api/secret.go @@ -119,8 +119,8 @@ func (csr *CreateSecretRequest) Validate() error { return err } - accounts[*encryptedName.AccountID]++ - unique[*encryptedName.AccountID]++ + accounts[encryptedName.AccountID]++ + unique[encryptedName.AccountID]++ } for _, count := range unique { @@ -140,8 +140,8 @@ func (csr *CreateSecretRequest) Validate() error { return err } - accounts[*encryptedKey.AccountID]-- - unique[*encryptedKey.AccountID]++ + accounts[encryptedKey.AccountID]-- + unique[encryptedKey.AccountID]++ } for _, count := range unique { @@ -204,18 +204,18 @@ func (r *SecretAccessRequest) Validate() error { // SecretKeyMemberRequest contains the request fields to grant access to a secret key. type SecretKeyMemberRequest struct { - AccountID *uuid.UUID `json:"account_id"` - SecretKeyID *uuid.UUID `json:"secret_key_id"` + AccountID uuid.UUID `json:"account_id"` + SecretKeyID uuid.UUID `json:"secret_key_id"` EncryptedKey crypto.CiphertextRSA `json:"encrypted_key"` } // Validate validates the request fields. func (skmr *SecretKeyMemberRequest) Validate() error { - if skmr.AccountID == nil { + if skmr.AccountID.IsZero() { return ErrInvalidAccountID } - if skmr.SecretKeyID == nil { + if skmr.SecretKeyID.IsZero() { return ErrInvalidKeyID } diff --git a/internals/api/secret_key.go b/internals/api/secret_key.go index 6671c13f..c6b0803f 100644 --- a/internals/api/secret_key.go +++ b/internals/api/secret_key.go @@ -57,13 +57,13 @@ func (r *CreateSecretKeyRequest) Validate() error { // EncryptedKeyRequest contains the request fields for re-encrypted for an account. type EncryptedKeyRequest struct { - AccountID *uuid.UUID `json:"account_id"` + AccountID uuid.UUID `json:"account_id"` EncryptedKey crypto.CiphertextRSA `json:"encrypted_key"` } // Validate validates the request fields. func (r *EncryptedKeyRequest) Validate() error { - if r.AccountID == nil { + if r.AccountID.IsZero() { return ErrInvalidAccountID } diff --git a/internals/api/secret_test.go b/internals/api/secret_test.go index 4115e95d..d435c67a 100644 --- a/internals/api/secret_test.go +++ b/internals/api/secret_test.go @@ -74,16 +74,16 @@ func TestCreateSecretRequest_Validate_Unique(t *testing.T) { EncryptedData: testCiphertextAES, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: &accountID, + AccountID: accountID, EncryptedName: testCiphertextRSA, }, { - AccountID: &accountID, + AccountID: accountID, EncryptedName: testCiphertextRSA, }, }, EncryptedKeys: []api.EncryptedKeyRequest{{ - AccountID: &accountID, + AccountID: accountID, EncryptedKey: testCiphertextRSA, }, }, @@ -96,16 +96,16 @@ func TestCreateSecretRequest_Validate_Unique(t *testing.T) { EncryptedData: testCiphertextAES, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: &accountID, + AccountID: accountID, EncryptedName: testCiphertextRSA, }, }, EncryptedKeys: []api.EncryptedKeyRequest{{ - AccountID: &accountID, + AccountID: accountID, EncryptedKey: testCiphertextRSA, }, { - AccountID: &accountID, + AccountID: accountID, EncryptedKey: testCiphertextRSA, }, }, @@ -136,12 +136,12 @@ func TestCreateSecretRequest_Validate_EncryptedNameAndKeyForEachAccount(t *testi EncryptedData: testCiphertextAES, EncryptedNames: []api.EncryptedNameRequest{{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedName: testCiphertextRSA, }, }, EncryptedKeys: []api.EncryptedKeyRequest{{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedKey: testCiphertextRSA, }, }, @@ -160,17 +160,17 @@ func TestExistingNameMemberRequest_Validate(t *testing.T) { "success": { EncryptedNameRequest: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedName: testCiphertextRSA, }, - NodeID: UUID(uuid.New()), + NodeID: uuid.New(), }, expected: nil, }, "invalid node id": { EncryptedNameRequest: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedName: testCiphertextRSA, }, }, @@ -178,7 +178,7 @@ func TestExistingNameMemberRequest_Validate(t *testing.T) { }, "invalid account id": { EncryptedNameRequest: api.EncryptedNameForNodeRequest{ - NodeID: UUID(uuid.New()), + NodeID: uuid.New(), EncryptedNameRequest: api.EncryptedNameRequest{ EncryptedName: testCiphertextRSA, }, @@ -211,14 +211,14 @@ func TestSecretAccessRequest_Validate_AccountIDs(t *testing.T) { Request: api.SecretAccessRequest{ Name: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: UUID(uuid.New()), + AccountID: uuid.New(), EncryptedName: testCiphertextRSA, }, - NodeID: UUID(uuid.New()), + NodeID: uuid.New(), }, Keys: []api.SecretKeyMemberRequest{{ - AccountID: UUID(uuid.New()), - SecretKeyID: UUID(uuid.New()), + AccountID: uuid.New(), + SecretKeyID: uuid.New(), EncryptedKey: testCiphertextRSA, }, }, @@ -229,14 +229,14 @@ func TestSecretAccessRequest_Validate_AccountIDs(t *testing.T) { Request: api.SecretAccessRequest{ Name: api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: &testAccountID, + AccountID: testAccountID, EncryptedName: testCiphertextRSA, }, - NodeID: UUID(uuid.New()), + NodeID: uuid.New(), }, Keys: []api.SecretKeyMemberRequest{{ - AccountID: &testAccountID, - SecretKeyID: UUID(uuid.New()), + AccountID: testAccountID, + SecretKeyID: uuid.New(), EncryptedKey: testCiphertextRSA, }, }, diff --git a/internals/api/secret_version.go b/internals/api/secret_version.go index 71b70cb6..f8c4b884 100644 --- a/internals/api/secret_version.go +++ b/internals/api/secret_version.go @@ -125,12 +125,12 @@ func (esv *EncryptedSecretVersion) ToAuditSubject() *AuditSubject { // secret version with a secret key. type CreateSecretVersionRequest struct { EncryptedData crypto.CiphertextAES `json:"encrypted_data"` - SecretKeyID *uuid.UUID `json:"secret_key_id"` + SecretKeyID uuid.UUID `json:"secret_key_id"` } // Validate validates the request fields. func (csvr *CreateSecretVersionRequest) Validate() error { - if csvr.SecretKeyID == nil { + if csvr.SecretKeyID.IsZero() { return ErrInvalidSecretKeyID } diff --git a/internals/api/secret_version_test.go b/internals/api/secret_version_test.go index f053faa5..20cb76a6 100644 --- a/internals/api/secret_version_test.go +++ b/internals/api/secret_version_test.go @@ -46,7 +46,7 @@ func TestCreateSecretVersionRequest_Validate_MaxSize(t *testing.T) { id := uuid.New() r := CreateSecretVersionRequest{ - SecretKeyID: &id, + SecretKeyID: id, EncryptedData: ciphertext, } diff --git a/internals/api/uuid/uuid.go b/internals/api/uuid/uuid.go index 93b911af..4df6d34f 100644 --- a/internals/api/uuid/uuid.go +++ b/internals/api/uuid/uuid.go @@ -35,11 +35,7 @@ func (u *UUID) IsZero() bool { return u.UUID == gid.UUID([gid.Size]byte{0}) } -// Equal returns true if both argument UUIDs contain the same value or if both are nil. -func Equal(a *UUID, b *UUID) bool { - if a == nil || b == nil { - return a == b - } - +// Equal returns true if both argument UUIDs contain the same value. +func Equal(a UUID, b UUID) bool { return gid.Equal(a.UUID, b.UUID) } diff --git a/internals/api/uuid_test.go b/internals/api/uuid_test.go deleted file mode 100644 index 51cb3c80..00000000 --- a/internals/api/uuid_test.go +++ /dev/null @@ -1,10 +0,0 @@ -package api_test - -import ( - "github.com/secrethub/secrethub-go/internals/api/uuid" -) - -// UUID returns the pointer to the given uuid.UUID value. -func UUID(id uuid.UUID) *uuid.UUID { - return &id -} diff --git a/pkg/secrethub/crypto.go b/pkg/secrethub/crypto.go index acf03f01..8512656c 100644 --- a/pkg/secrethub/crypto.go +++ b/pkg/secrethub/crypto.go @@ -8,7 +8,7 @@ import ( ) func (c *Client) encryptDirFor(dir *api.Dir, accounts ...*api.Account) ([]api.EncryptedNameForNodeRequest, error) { - currentDir, err := encryptNameForNodeAccounts(&dir.DirID, dir.Name, accounts...) + currentDir, err := encryptNameForNodeAccounts(dir.DirID, dir.Name, accounts...) return currentDir, errio.Error(err) } @@ -49,10 +49,10 @@ func (c *Client) encryptSecretFor(secret *api.Secret, accounts ...*api.Account) encryptedName := api.EncryptedNameForNodeRequest{ EncryptedNameRequest: api.EncryptedNameRequest{ - AccountID: &account.AccountID, + AccountID: account.AccountID, EncryptedName: encryptedSecretName, }, - NodeID: &secret.SecretID, + NodeID: secret.SecretID, } encryptedKeys := make([]api.SecretKeyMemberRequest, len(decryptedKeys)) @@ -63,8 +63,8 @@ func (c *Client) encryptSecretFor(secret *api.Secret, accounts ...*api.Account) } encryptedKeys[keyIndex] = api.SecretKeyMemberRequest{ - AccountID: &account.AccountID, - SecretKeyID: &decryptedKey.SecretKeyID, + AccountID: account.AccountID, + SecretKeyID: decryptedKey.SecretKeyID, EncryptedKey: encryptedKey, } } @@ -79,7 +79,7 @@ func (c *Client) encryptSecretFor(secret *api.Secret, accounts ...*api.Account) } // encryptNameForNodeAccounts encrypts the name for every account and returns a list of ExistingNameMemberRequests. -func encryptNameForNodeAccounts(nodeID *uuid.UUID, name string, accounts ...*api.Account) ([]api.EncryptedNameForNodeRequest, error) { +func encryptNameForNodeAccounts(nodeID uuid.UUID, name string, accounts ...*api.Account) ([]api.EncryptedNameForNodeRequest, error) { encryptedNames, err := encryptNameForAccounts(name, accounts...) if err != nil { return nil, err @@ -114,7 +114,7 @@ func encryptNameForAccounts(name string, accounts ...*api.Account) ([]api.Encryp } encryptedNames[i] = api.EncryptedNameRequest{ - AccountID: &account.AccountID, + AccountID: account.AccountID, EncryptedName: ciphertext, } } @@ -137,7 +137,7 @@ func encryptKeyForAccounts(key *crypto.SymmetricKey, accounts ...*api.Account) ( } encryptedKeys[i] = api.EncryptedKeyRequest{ - AccountID: &account.AccountID, + AccountID: account.AccountID, EncryptedKey: encryptedSecretKey, } } diff --git a/pkg/secrethub/repo_user.go b/pkg/secrethub/repo_user.go index 7cd4eb4a..a8077393 100644 --- a/pkg/secrethub/repo_user.go +++ b/pkg/secrethub/repo_user.go @@ -58,7 +58,7 @@ func (s repoUserService) Invite(path string, username string) (*api.RepoMember, } in := &api.InviteUserRequest{ - AccountID: &account.AccountID, + AccountID: account.AccountID, RepoMember: createRepoMember, } diff --git a/pkg/secrethub/secret_key.go b/pkg/secrethub/secret_key.go index 6ff61fa1..a53bbf35 100644 --- a/pkg/secrethub/secret_key.go +++ b/pkg/secrethub/secret_key.go @@ -57,7 +57,7 @@ func (c *Client) createSecretKey(secretPath api.SecretPath) (*api.SecretKey, err } encryptedFor[i] = api.EncryptedKeyRequest{ - AccountID: &account.AccountID, + AccountID: account.AccountID, EncryptedKey: encryptedSecretKey, } } diff --git a/pkg/secrethub/secret_version.go b/pkg/secrethub/secret_version.go index 4fcf8725..7f66441d 100644 --- a/pkg/secrethub/secret_version.go +++ b/pkg/secrethub/secret_version.go @@ -167,7 +167,7 @@ func (c *Client) createSecretVersion(secretPath api.SecretPath, data []byte, sec in := &api.CreateSecretVersionRequest{ EncryptedData: encryptedData, - SecretKeyID: &secretKey.SecretKeyID, + SecretKeyID: secretKey.SecretKeyID, } blindName, err := c.convertPathToBlindName(secretPath) From ee7fa63d5a1587fb56418773f6aaa8f04410d62c Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 6 Sep 2019 09:25:15 +0200 Subject: [PATCH 06/11] Accept a uuid value instead of pointer in Get function for directories --- pkg/secrethub/dir.go | 4 ++-- pkg/secrethub/internals/http/client.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/secrethub/dir.go b/pkg/secrethub/dir.go index a88a3935..6777b9ab 100644 --- a/pkg/secrethub/dir.go +++ b/pkg/secrethub/dir.go @@ -11,7 +11,7 @@ type DirService interface { // Create a directory at a given path. Create(path string) (*api.Dir, error) // Get returns the directory on the given path. - Get(id *uuid.UUID) (*api.Dir, error) + Get(id uuid.UUID) (*api.Dir, error) // Delete removes the directory at the given path. Delete(path string) error // GetTree retrieves a directory at a given path and all of its descendants up to a given depth. @@ -31,7 +31,7 @@ type dirService struct { } // Get returns the directory on the given path. -func (s dirService) Get(id *uuid.UUID) (*api.Dir, error) { +func (s dirService) Get(id uuid.UUID) (*api.Dir, error) { encDir, err := s.client.httpClient.GetDir(id) if err != nil { return nil, errio.Error(err) diff --git a/pkg/secrethub/internals/http/client.go b/pkg/secrethub/internals/http/client.go index 62af34b0..3ed3bad1 100644 --- a/pkg/secrethub/internals/http/client.go +++ b/pkg/secrethub/internals/http/client.go @@ -337,7 +337,7 @@ func (c *Client) CreateDir(namespace, repoName string, in *api.CreateDirRequest) } // GetDir retrieves a directory encrypted for the authenticated user. -func (c *Client) GetDir(id *uuid.UUID) (*api.EncryptedDir, error) { +func (c *Client) GetDir(id uuid.UUID) (*api.EncryptedDir, error) { rawURL := fmt.Sprintf(pathDir, c.base, id.String()) out := &api.EncryptedDir{} err := c.get(rawURL, true, out) From 6b966f8ab131fdb3d97e98f898903c5131c603ac Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 6 Sep 2019 09:42:56 +0200 Subject: [PATCH 07/11] Use AWS identity provider with SECRETHUB_IDENTITY_PROVIDER=aws envvar --- pkg/secrethub/client.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/secrethub/client.go b/pkg/secrethub/client.go index a2d5ce81..606e5c18 100644 --- a/pkg/secrethub/client.go +++ b/pkg/secrethub/client.go @@ -19,6 +19,11 @@ const ( userAgentPrefix = "SecretHub/v1 secrethub-go/" + ClientVersion ) +// Errors +var ( + ErrUnknownIdentityProvider = errClient.Code("unknown_identity_provider").ErrorPref("%s is not a supported identity provider. Valid options are `aws` and `key`.") +) + // ClientInterface is an interface that can be used to consume the SecretHub client and is implemented by secrethub.Client. type ClientInterface interface { // AccessRules returns a service used to manage access rules. @@ -110,7 +115,19 @@ func NewClient(with ...ClientOption) (*Client, error) { // Try to use default key credentials if none provided explicitly if client.decrypter == nil { - err := client.with(WithCredentials(credentials.UseKey(client.DefaultCredential()))) + identityProvider := os.Getenv("SECRETHUB_IDENTITY_PROVIDER") + + var provider credentials.Provider + switch identityProvider { + case "key", "": + provider = credentials.UseKey(client.DefaultCredential()) + case "aws": + provider = credentials.UseAWS() + default: + return nil, ErrUnknownIdentityProvider(identityProvider) + } + + err := client.with(WithCredentials(provider)) // nolint: staticcheck if err != nil { // TODO: log that default credential was not loaded. From 8da879257e651d6992357fdbb8bb5ac6d46282db Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 6 Sep 2019 13:15:36 +0200 Subject: [PATCH 08/11] Fix comments on getting a directory to reflect its parameter --- pkg/secrethub/dir.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/secrethub/dir.go b/pkg/secrethub/dir.go index 6777b9ab..d821cfb9 100644 --- a/pkg/secrethub/dir.go +++ b/pkg/secrethub/dir.go @@ -10,7 +10,7 @@ import ( type DirService interface { // Create a directory at a given path. Create(path string) (*api.Dir, error) - // Get returns the directory on the given path. + // Get returns the directory with the given ID. Get(id uuid.UUID) (*api.Dir, error) // Delete removes the directory at the given path. Delete(path string) error @@ -30,7 +30,7 @@ type dirService struct { client *Client } -// Get returns the directory on the given path. +// Get returns the directory with the given ID. func (s dirService) Get(id uuid.UUID) (*api.Dir, error) { encDir, err := s.client.httpClient.GetDir(id) if err != nil { From 5df69819c0fe8af28dba567427cb6e4fd5406d8e Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 6 Sep 2019 13:28:31 +0200 Subject: [PATCH 09/11] Make identity provider configuration case insensitive --- pkg/secrethub/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/secrethub/client.go b/pkg/secrethub/client.go index 606e5c18..87a0dffd 100644 --- a/pkg/secrethub/client.go +++ b/pkg/secrethub/client.go @@ -118,7 +118,7 @@ func NewClient(with ...ClientOption) (*Client, error) { identityProvider := os.Getenv("SECRETHUB_IDENTITY_PROVIDER") var provider credentials.Provider - switch identityProvider { + switch strings.ToLower(identityProvider) { case "key", "": provider = credentials.UseKey(client.DefaultCredential()) case "aws": From 1b6648d86779e7670a4a400ec46a5917febea275 Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 6 Sep 2019 13:41:07 +0200 Subject: [PATCH 10/11] Swap "" and "key" when matching the configured identity provider To make "" stand out more, as it might otherwise be overlooked. --- pkg/secrethub/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/secrethub/client.go b/pkg/secrethub/client.go index 87a0dffd..ee9af8a3 100644 --- a/pkg/secrethub/client.go +++ b/pkg/secrethub/client.go @@ -119,7 +119,7 @@ func NewClient(with ...ClientOption) (*Client, error) { var provider credentials.Provider switch strings.ToLower(identityProvider) { - case "key", "": + case "", "key": provider = credentials.UseKey(client.DefaultCredential()) case "aws": provider = credentials.UseAWS() From 298c0725d0c11d9e98c24e09277bb4ca957f18ff Mon Sep 17 00:00:00 2001 From: Simon Barendse Date: Fri, 6 Sep 2019 14:27:29 +0200 Subject: [PATCH 11/11] Rename dirs.Get to GetByID We'll use DoByID pattern for functions that take a UUID as parameter and use Do for functions that take the name (e.g. path) as a parameter. --- pkg/secrethub/dir.go | 6 +++--- pkg/secrethub/internals/http/client.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/secrethub/dir.go b/pkg/secrethub/dir.go index d821cfb9..227fb608 100644 --- a/pkg/secrethub/dir.go +++ b/pkg/secrethub/dir.go @@ -11,7 +11,7 @@ type DirService interface { // Create a directory at a given path. Create(path string) (*api.Dir, error) // Get returns the directory with the given ID. - Get(id uuid.UUID) (*api.Dir, error) + GetByID(id uuid.UUID) (*api.Dir, error) // Delete removes the directory at the given path. Delete(path string) error // GetTree retrieves a directory at a given path and all of its descendants up to a given depth. @@ -31,8 +31,8 @@ type dirService struct { } // Get returns the directory with the given ID. -func (s dirService) Get(id uuid.UUID) (*api.Dir, error) { - encDir, err := s.client.httpClient.GetDir(id) +func (s dirService) GetByID(id uuid.UUID) (*api.Dir, error) { + encDir, err := s.client.httpClient.GetDirByID(id) if err != nil { return nil, errio.Error(err) } diff --git a/pkg/secrethub/internals/http/client.go b/pkg/secrethub/internals/http/client.go index 3ed3bad1..7c471cc4 100644 --- a/pkg/secrethub/internals/http/client.go +++ b/pkg/secrethub/internals/http/client.go @@ -336,8 +336,8 @@ func (c *Client) CreateDir(namespace, repoName string, in *api.CreateDirRequest) return out, errio.Error(err) } -// GetDir retrieves a directory encrypted for the authenticated user. -func (c *Client) GetDir(id uuid.UUID) (*api.EncryptedDir, error) { +// GetDirByID retrieves a directory encrypted for the authenticated user. +func (c *Client) GetDirByID(id uuid.UUID) (*api.EncryptedDir, error) { rawURL := fmt.Sprintf(pathDir, c.base, id.String()) out := &api.EncryptedDir{} err := c.get(rawURL, true, out)