From 1980d0f1cc57bb104fc00715f6e3fb5ea78f6746 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 15:17:50 -0400 Subject: [PATCH 01/15] Userpass: Support updating policies and password --- builtin/credential/userpass/backend.go | 2 ++ builtin/credential/userpass/path_users.go | 42 +++++++++++++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/builtin/credential/userpass/backend.go b/builtin/credential/userpass/backend.go index d2e62ab52a6f6..5505a1ab5e584 100644 --- a/builtin/credential/userpass/backend.go +++ b/builtin/credential/userpass/backend.go @@ -29,6 +29,8 @@ func Backend() *framework.Backend { Paths: append([]*framework.Path{ pathUsers(&b), + pathUserPolicies(&b), + pathUserPassword(&b), }, mfa.MFAPaths(b.Backend, pathLogin(&b))..., ), diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index abf4a44751f89..5ea25dccd5f16 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -43,14 +43,31 @@ func pathUsers(b *backend) *framework.Path { Callbacks: map[logical.Operation]framework.OperationFunc{ logical.DeleteOperation: b.pathUserDelete, logical.ReadOperation: b.pathUserRead, - logical.UpdateOperation: b.pathUserWrite, + logical.UpdateOperation: b.pathUserWrite, + logical.CreateOperation: b.pathUserWrite, }, + ExistenceCheck: b.userExistenceCheck, + HelpSynopsis: pathUserHelpSyn, HelpDescription: pathUserHelpDesc, } } +func (b *backend) userExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { + username := data.Get("name").(string) + if username == "" { + return false, fmt.Errorf("name cannot be empty") + } + + entry, err := req.Storage.Get("user/" + strings.ToLower(username)) + if err != nil { + return false, err + } + + return entry != nil, nil +} + func (b *backend) User(s logical.Storage, n string) (*UserEntry, error) { entry, err := s.Get("user/" + strings.ToLower(n)) if err != nil { @@ -68,6 +85,15 @@ func (b *backend) User(s logical.Storage, n string) (*UserEntry, error) { return &result, nil } +func (b *backend) SetUser(s logical.Storage, username string, userEntry *UserEntry) error { + entry, err := logical.StorageEntryJSON("user/"+username, userEntry) + if err != nil { + return err + } + + return s.Put(entry) +} + func (b *backend) pathUserDelete( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { err := req.Storage.Delete("user/" + strings.ToLower(d.Get("name").(string))) @@ -98,7 +124,15 @@ func (b *backend) pathUserRead( func (b *backend) pathUserWrite( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { name := strings.ToLower(d.Get("name").(string)) + if name == "" { + return nil, fmt.Errorf("missing username") + } + password := d.Get("password").(string) + if password == "" { + return nil, fmt.Errorf("missing password") + } + policies := strings.Split(d.Get("policies").(string), ",") for i, p := range policies { policies[i] = strings.TrimSpace(p) @@ -117,8 +151,7 @@ func (b *backend) pathUserWrite( return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil } - // Store it - entry, err := logical.StorageEntryJSON("user/"+name, &UserEntry{ + err = b.SetUser(req.Storage, name, &UserEntry{ PasswordHash: hash, Policies: policies, TTL: ttl, @@ -127,9 +160,6 @@ func (b *backend) pathUserWrite( if err != nil { return nil, err } - if err := req.Storage.Put(entry); err != nil { - return nil, err - } return nil, nil } From aa8926912a4ad268e173d505695f50c129ec058b Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 16:09:23 -0400 Subject: [PATCH 02/15] Tests for updating password and policies in userpass backend --- builtin/credential/userpass/backend_test.go | 110 +++++++++++++++++++- 1 file changed, 107 insertions(+), 3 deletions(-) diff --git a/builtin/credential/userpass/backend_test.go b/builtin/credential/userpass/backend_test.go index c0fc8bea41e40..441a104ce92f4 100644 --- a/builtin/credential/userpass/backend_test.go +++ b/builtin/credential/userpass/backend_test.go @@ -81,7 +81,7 @@ func TestBackend_basic(t *testing.T) { Backend: b, Steps: []logicaltest.TestStep{ testAccStepUser(t, "web", "password", "foo"), - testAccStepLogin(t, "web", "password"), + testAccStepLogin(t, "web", "password", []string{"default", "foo"}), }, }) } @@ -109,6 +109,98 @@ func TestBackend_userCrud(t *testing.T) { }) } +func TestBacken_userCreateOperation(t *testing.T) { + b, err := Factory(&logical.BackendConfig{ + Logger: nil, + System: &logical.StaticSystemView{ + DefaultLeaseTTLVal: testSysTTL, + MaxLeaseTTLVal: testSysMaxTTL, + }, + }) + if err != nil { + t.Fatalf("Unable to create backend: %s", err) + } + + logicaltest.Test(t, logicaltest.TestCase{ + Backend: b, + Steps: []logicaltest.TestStep{ + testUserCreateOperation(t, "web", "password", "foo"), + testAccStepLogin(t, "web", "password", []string{"default", "foo"}), + }, + }) +} + +func TestBackend_passwordUpdate(t *testing.T) { + b, err := Factory(&logical.BackendConfig{ + Logger: nil, + System: &logical.StaticSystemView{ + DefaultLeaseTTLVal: testSysTTL, + MaxLeaseTTLVal: testSysMaxTTL, + }, + }) + if err != nil { + t.Fatalf("Unable to create backend: %s", err) + } + + logicaltest.Test(t, logicaltest.TestCase{ + Backend: b, + Steps: []logicaltest.TestStep{ + testAccStepUser(t, "web", "password", "foo"), + testAccStepReadUser(t, "web", "foo"), + testAccStepLogin(t, "web", "password", []string{"default", "foo"}), + testUpdatePassword(t, "web", "newpassword"), + testAccStepLogin(t, "web", "newpassword", []string{"default", "foo"}), + }, + }) + +} + +func TestBackend_policiesUpdate(t *testing.T) { + b, err := Factory(&logical.BackendConfig{ + Logger: nil, + System: &logical.StaticSystemView{ + DefaultLeaseTTLVal: testSysTTL, + MaxLeaseTTLVal: testSysMaxTTL, + }, + }) + if err != nil { + t.Fatalf("Unable to create backend: %s", err) + } + + logicaltest.Test(t, logicaltest.TestCase{ + Backend: b, + Steps: []logicaltest.TestStep{ + testAccStepUser(t, "web", "password", "foo"), + testAccStepReadUser(t, "web", "foo"), + testAccStepLogin(t, "web", "password", []string{"default", "foo"}), + testUpdatePolicies(t, "web", "foo,bar"), + testAccStepReadUser(t, "web", "foo,bar"), + testAccStepLogin(t, "web", "password", []string{"bar", "default", "foo"}), + }, + }) + +} + +func testUpdatePassword(t *testing.T, user, password string) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.UpdateOperation, + Path: "users/password/" + user, + Data: map[string]interface{}{ + "password": password, + }, + } +} + +func testUpdatePolicies(t *testing.T, user, policies string) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.UpdateOperation, + Path: "users/policies/" + user, + Data: map[string]interface{}{ + "policies": policies, + }, + } +} + func testUsersWrite(t *testing.T, user string, data map[string]interface{}, expectError bool) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, @@ -139,7 +231,7 @@ func testLoginWrite(t *testing.T, user string, data map[string]interface{}, expe } } -func testAccStepLogin(t *testing.T, user string, pass string) logicaltest.TestStep { +func testAccStepLogin(t *testing.T, user string, pass string, policies []string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, Path: "login/" + user, @@ -148,7 +240,19 @@ func testAccStepLogin(t *testing.T, user string, pass string) logicaltest.TestSt }, Unauthenticated: true, - Check: logicaltest.TestCheckAuth([]string{"foo"}), + Check: logicaltest.TestCheckAuth(policies), + } +} + +func testUserCreateOperation( + t *testing.T, name string, password string, policies string) logicaltest.TestStep { + return logicaltest.TestStep{ + Operation: logical.CreateOperation, + Path: "users/" + name, + Data: map[string]interface{}{ + "password": password, + "policies": policies, + }, } } From e51661c714c2c7cbd2f6664163b861f70c49f748 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 16:12:55 -0400 Subject: [PATCH 03/15] Added paths to update policies and password --- .../credential/userpass/path_user_password.go | 80 +++++++++++++++++++ .../credential/userpass/path_user_policies.go | 69 ++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 builtin/credential/userpass/path_user_password.go create mode 100644 builtin/credential/userpass/path_user_policies.go diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go new file mode 100644 index 0000000000000..96a00afb24642 --- /dev/null +++ b/builtin/credential/userpass/path_user_password.go @@ -0,0 +1,80 @@ +package userpass + +import ( + "fmt" + "strings" + + "golang.org/x/crypto/bcrypt" + + "github.com/hashicorp/vault/logical" + "github.com/hashicorp/vault/logical/framework" +) + +func pathUserPassword(b *backend) *framework.Path { + return &framework.Path{ + Pattern: "users/password/" + framework.GenericNameRegex("name"), + Fields: map[string]*framework.FieldSchema{ + "name": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "Username for this user.", + }, + + "password": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "Password for this user.", + }, + }, + + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.UpdateOperation: b.pathUserPasswordUpdate, + }, + + HelpSynopsis: pathUserPasswordHelpSyn, + HelpDescription: pathUserPasswordHelpDesc, + } +} + +func (b *backend) pathUserPasswordUpdate( + req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + username := strings.ToLower(d.Get("name").(string)) + if username == "" { + return nil, fmt.Errorf("missing username") + } + + password := d.Get("password").(string) + if password == "" { + return nil, fmt.Errorf("missing password") + } + + userEntry, err := b.User(req.Storage, strings.ToLower(d.Get("name").(string))) + if err != nil { + return nil, err + } + if userEntry == nil { + return nil, nil + } + + // Generate a hash of the password + hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + return nil, err + } + + // Set the new password hash + userEntry.PasswordHash = hash + + // Store the UserEntry + err = b.SetUser(req.Storage, username, userEntry) + if err != nil { + return nil, err + } + return nil, nil +} + +const pathUserPasswordHelpSyn = ` +Reset user's password. +` + +const pathUserPasswordHelpDesc = ` +This endpoint allows resetting the user's password. +` diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go new file mode 100644 index 0000000000000..8a5f2f42c3b8d --- /dev/null +++ b/builtin/credential/userpass/path_user_policies.go @@ -0,0 +1,69 @@ +package userpass + +import ( + "fmt" + "strings" + + "github.com/hashicorp/vault/logical" + "github.com/hashicorp/vault/logical/framework" +) + +func pathUserPolicies(b *backend) *framework.Path { + return &framework.Path{ + Pattern: "users/policies/" + framework.GenericNameRegex("name"), + Fields: map[string]*framework.FieldSchema{ + "name": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "Username for this user.", + }, + "policies": &framework.FieldSchema{ + Type: framework.TypeString, + Description: "Comma-separated list of policies", + }, + }, + + Callbacks: map[logical.Operation]framework.OperationFunc{ + logical.UpdateOperation: b.pathUserPoliciesUpdate, + }, + + HelpSynopsis: pathUserPoliciesHelpSyn, + HelpDescription: pathUserPoliciesHelpDesc, + } +} +func (b *backend) pathUserPoliciesUpdate( + req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + username := strings.ToLower(d.Get("name").(string)) + if username == "" { + return nil, fmt.Errorf("missing username") + } + + policies := strings.Split(d.Get("policies").(string), ",") + for i, p := range policies { + policies[i] = strings.TrimSpace(p) + } + + userEntry, err := b.User(req.Storage, strings.ToLower(d.Get("name").(string))) + if err != nil { + return nil, err + } + if userEntry == nil { + return nil, nil + } + + userEntry.Policies = policies + + // Store the UserEntry + err = b.SetUser(req.Storage, username, userEntry) + if err != nil { + return nil, err + } + return nil, nil +} + +const pathUserPoliciesHelpSyn = ` +Update the policies associated with the username. +` + +const pathUserPoliciesHelpDesc = ` +This endpoint allows updating the policies associated with the username. +` From cad1ee3a85a638021a6e417d08bc3d5d661e7ce9 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 16:21:47 -0400 Subject: [PATCH 04/15] Reuse the variable instead of fetching 'name' again --- builtin/credential/userpass/path_user_password.go | 2 +- builtin/credential/userpass/path_user_policies.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index 96a00afb24642..e92e6e809065f 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -46,7 +46,7 @@ func (b *backend) pathUserPasswordUpdate( return nil, fmt.Errorf("missing password") } - userEntry, err := b.User(req.Storage, strings.ToLower(d.Get("name").(string))) + userEntry, err := b.User(req.Storage, username) if err != nil { return nil, err } diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index 8a5f2f42c3b8d..bad3fd521954a 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -42,7 +42,7 @@ func (b *backend) pathUserPoliciesUpdate( policies[i] = strings.TrimSpace(p) } - userEntry, err := b.User(req.Storage, strings.ToLower(d.Get("name").(string))) + userEntry, err := b.User(req.Storage, username) if err != nil { return nil, err } From 0e1769dd5d183e355a8bb3367ac5c63e869c3910 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 16:46:12 -0400 Subject: [PATCH 05/15] Change path structure of password and policies endpoints in userpass --- builtin/credential/userpass/backend_test.go | 6 +++--- builtin/credential/userpass/path_user_password.go | 2 +- builtin/credential/userpass/path_user_policies.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/credential/userpass/backend_test.go b/builtin/credential/userpass/backend_test.go index 441a104ce92f4..cdf62d52f7db5 100644 --- a/builtin/credential/userpass/backend_test.go +++ b/builtin/credential/userpass/backend_test.go @@ -109,7 +109,7 @@ func TestBackend_userCrud(t *testing.T) { }) } -func TestBacken_userCreateOperation(t *testing.T) { +func TestBackend_userCreateOperation(t *testing.T) { b, err := Factory(&logical.BackendConfig{ Logger: nil, System: &logical.StaticSystemView{ @@ -184,7 +184,7 @@ func TestBackend_policiesUpdate(t *testing.T) { func testUpdatePassword(t *testing.T, user, password string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, - Path: "users/password/" + user, + Path: "users/" + user + "/password", Data: map[string]interface{}{ "password": password, }, @@ -194,7 +194,7 @@ func testUpdatePassword(t *testing.T, user, password string) logicaltest.TestSte func testUpdatePolicies(t *testing.T, user, policies string) logicaltest.TestStep { return logicaltest.TestStep{ Operation: logical.UpdateOperation, - Path: "users/policies/" + user, + Path: "users/" + user + "/policies", Data: map[string]interface{}{ "policies": policies, }, diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index e92e6e809065f..b24e3c6c99b04 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -12,7 +12,7 @@ import ( func pathUserPassword(b *backend) *framework.Path { return &framework.Path{ - Pattern: "users/password/" + framework.GenericNameRegex("name"), + Pattern: "users/" + framework.GenericNameRegex("name") + "/password$", Fields: map[string]*framework.FieldSchema{ "name": &framework.FieldSchema{ Type: framework.TypeString, diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index bad3fd521954a..c9711530431b5 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -10,7 +10,7 @@ import ( func pathUserPolicies(b *backend) *framework.Path { return &framework.Path{ - Pattern: "users/policies/" + framework.GenericNameRegex("name"), + Pattern: "users/" + framework.GenericNameRegex("name") + "/policies$", Fields: map[string]*framework.FieldSchema{ "name": &framework.FieldSchema{ Type: framework.TypeString, From c29a12181636f20db741a5915512e2f2b7033439 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 17:05:23 -0400 Subject: [PATCH 06/15] Fetch and store UserEntry to properly handle both create and update --- builtin/credential/userpass/path_login.go | 4 +- .../credential/userpass/path_user_password.go | 2 +- .../credential/userpass/path_user_policies.go | 2 +- builtin/credential/userpass/path_users.go | 42 +++++++++---------- 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/builtin/credential/userpass/path_login.go b/builtin/credential/userpass/path_login.go index 3b6c67b724f23..b9ed5dbf20f88 100644 --- a/builtin/credential/userpass/path_login.go +++ b/builtin/credential/userpass/path_login.go @@ -39,7 +39,7 @@ func (b *backend) pathLogin( password := d.Get("password").(string) // Get the user and validate auth - user, err := b.User(req.Storage, username) + user, err := b.user(req.Storage, username) if err != nil { return nil, err } @@ -78,7 +78,7 @@ func (b *backend) pathLogin( func (b *backend) pathLoginRenew( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { // Get the user - user, err := b.User(req.Storage, req.Auth.Metadata["username"]) + user, err := b.user(req.Storage, req.Auth.Metadata["username"]) if err != nil { return nil, err } diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index b24e3c6c99b04..7cad9576378f1 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -46,7 +46,7 @@ func (b *backend) pathUserPasswordUpdate( return nil, fmt.Errorf("missing password") } - userEntry, err := b.User(req.Storage, username) + userEntry, err := b.user(req.Storage, username) if err != nil { return nil, err } diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index c9711530431b5..129c7683ed1ab 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -42,7 +42,7 @@ func (b *backend) pathUserPoliciesUpdate( policies[i] = strings.TrimSpace(p) } - userEntry, err := b.User(req.Storage, username) + userEntry, err := b.user(req.Storage, username) if err != nil { return nil, err } diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index 5ea25dccd5f16..ccf5167f05315 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -68,7 +68,7 @@ func (b *backend) userExistenceCheck(req *logical.Request, data *framework.Field return entry != nil, nil } -func (b *backend) User(s logical.Storage, n string) (*UserEntry, error) { +func (b *backend) user(s logical.Storage, n string) (*UserEntry, error) { entry, err := s.Get("user/" + strings.ToLower(n)) if err != nil { return nil, err @@ -106,7 +106,7 @@ func (b *backend) pathUserDelete( func (b *backend) pathUserRead( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - user, err := b.User(req.Storage, strings.ToLower(d.Get("name").(string))) + user, err := b.user(req.Storage, strings.ToLower(d.Get("name").(string))) if err != nil { return nil, err } @@ -123,45 +123,41 @@ func (b *backend) pathUserRead( func (b *backend) pathUserWrite( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - name := strings.ToLower(d.Get("name").(string)) - if name == "" { - return nil, fmt.Errorf("missing username") + username := strings.ToLower(d.Get("name").(string)) + user, err := b.user(req.Storage, username) + if err != nil { + return nil, err + } + // Due to existence check, user will only be nil if it's a create operation + if user == nil { + user = &UserEntry{} } password := d.Get("password").(string) if password == "" { return nil, fmt.Errorf("missing password") } - - policies := strings.Split(d.Get("policies").(string), ",") - for i, p := range policies { - policies[i] = strings.TrimSpace(p) - } - // Generate a hash of the password hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) if err != nil { return nil, err } + user.PasswordHash = hash + + policies := strings.Split(d.Get("policies").(string), ",") + for i, p := range policies { + policies[i] = strings.TrimSpace(p) + } + user.Policies = policies ttlStr := d.Get("ttl").(string) maxTTLStr := d.Get("max_ttl").(string) - ttl, maxTTL, err := b.SanitizeTTL(ttlStr, maxTTLStr) + user.TTL, user.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) if err != nil { return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil } - err = b.SetUser(req.Storage, name, &UserEntry{ - PasswordHash: hash, - Policies: policies, - TTL: ttl, - MaxTTL: maxTTL, - }) - if err != nil { - return nil, err - } - - return nil, nil + return nil, b.SetUser(req.Storage, username, user) } type UserEntry struct { From 5249c0d5e0c7ed57fb358bf3967e1f770de3e2c3 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 17:32:39 -0400 Subject: [PATCH 07/15] Refactor updating and creating userEntry into a helper function --- .../credential/userpass/path_user_password.go | 33 +--------- .../credential/userpass/path_user_policies.go | 31 +--------- builtin/credential/userpass/path_users.go | 61 +++++++++++-------- 3 files changed, 40 insertions(+), 85 deletions(-) diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index 7cad9576378f1..e5a7ca196fd0a 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -2,9 +2,6 @@ package userpass import ( "fmt" - "strings" - - "golang.org/x/crypto/bcrypt" "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" @@ -36,39 +33,11 @@ func pathUserPassword(b *backend) *framework.Path { func (b *backend) pathUserPasswordUpdate( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - username := strings.ToLower(d.Get("name").(string)) - if username == "" { - return nil, fmt.Errorf("missing username") - } - password := d.Get("password").(string) if password == "" { return nil, fmt.Errorf("missing password") } - - userEntry, err := b.user(req.Storage, username) - if err != nil { - return nil, err - } - if userEntry == nil { - return nil, nil - } - - // Generate a hash of the password - hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) - if err != nil { - return nil, err - } - - // Set the new password hash - userEntry.PasswordHash = hash - - // Store the UserEntry - err = b.SetUser(req.Storage, username, userEntry) - if err != nil { - return nil, err - } - return nil, nil + return b.userCreateUpdate(req, d) } const pathUserPasswordHelpSyn = ` diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index 129c7683ed1ab..e25a5f38dea2c 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -1,9 +1,6 @@ package userpass import ( - "fmt" - "strings" - "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" ) @@ -30,34 +27,10 @@ func pathUserPolicies(b *backend) *framework.Path { HelpDescription: pathUserPoliciesHelpDesc, } } + func (b *backend) pathUserPoliciesUpdate( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - username := strings.ToLower(d.Get("name").(string)) - if username == "" { - return nil, fmt.Errorf("missing username") - } - - policies := strings.Split(d.Get("policies").(string), ",") - for i, p := range policies { - policies[i] = strings.TrimSpace(p) - } - - userEntry, err := b.user(req.Storage, username) - if err != nil { - return nil, err - } - if userEntry == nil { - return nil, nil - } - - userEntry.Policies = policies - - // Store the UserEntry - err = b.SetUser(req.Storage, username, userEntry) - if err != nil { - return nil, err - } - return nil, nil + return b.userCreateUpdate(req, d) } const pathUserPoliciesHelpSyn = ` diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index ccf5167f05315..86d0bb4adcc14 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -121,43 +121,56 @@ func (b *backend) pathUserRead( }, nil } -func (b *backend) pathUserWrite( - req *logical.Request, d *framework.FieldData) (*logical.Response, error) { +func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { username := strings.ToLower(d.Get("name").(string)) - user, err := b.user(req.Storage, username) + userEntry, err := b.user(req.Storage, username) if err != nil { return nil, err } // Due to existence check, user will only be nil if it's a create operation - if user == nil { - user = &UserEntry{} + if userEntry == nil { + userEntry = &UserEntry{} } - password := d.Get("password").(string) - if password == "" { - return nil, fmt.Errorf("missing password") - } - // Generate a hash of the password - hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) - if err != nil { - return nil, err + // Set/update the values of UserEntry only if fields are supplied + if passwordRaw, ok := d.GetOk("password"); ok { + // Generate a hash of the password + hash, err := bcrypt.GenerateFromPassword([]byte(passwordRaw.(string)), bcrypt.DefaultCost) + if err != nil { + return nil, err + } + userEntry.PasswordHash = hash } - user.PasswordHash = hash - policies := strings.Split(d.Get("policies").(string), ",") - for i, p := range policies { - policies[i] = strings.TrimSpace(p) + if policiesRaw, ok := d.GetOk("policies"); ok { + policies := strings.Split(policiesRaw.(string), ",") + for i, p := range policies { + policies[i] = strings.TrimSpace(p) + } + userEntry.Policies = policies } - user.Policies = policies - ttlStr := d.Get("ttl").(string) - maxTTLStr := d.Get("max_ttl").(string) - user.TTL, user.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil + ttlStrRaw, ttlSet := d.GetOk("ttl") + maxTTLStrRaw, maxTTLSet := d.GetOk("max_ttl") + if ttlSet || maxTTLSet { + ttlStr := ttlStrRaw.(string) + maxTTLStr := maxTTLStrRaw.(string) + userEntry.TTL, userEntry.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil + } } - return nil, b.SetUser(req.Storage, username, user) + return nil, b.SetUser(req.Storage, username, userEntry) +} + +func (b *backend) pathUserWrite( + req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + password := d.Get("password").(string) + if password == "" { + return nil, fmt.Errorf("missing password") + } + return b.userCreateUpdate(req, d) } type UserEntry struct { From 91f4aab933d6f7693ddc7c238d1996710934ce6f Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 17:47:13 -0400 Subject: [PATCH 08/15] Input validations and field renaming --- builtin/credential/userpass/path_login.go | 14 +++++++++++--- builtin/credential/userpass/path_user_password.go | 4 ++-- builtin/credential/userpass/path_user_policies.go | 4 ++-- builtin/credential/userpass/path_users.go | 14 +++++++------- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/builtin/credential/userpass/path_login.go b/builtin/credential/userpass/path_login.go index b9ed5dbf20f88..28ef665fbbd6e 100644 --- a/builtin/credential/userpass/path_login.go +++ b/builtin/credential/userpass/path_login.go @@ -2,6 +2,7 @@ package userpass import ( "crypto/subtle" + "fmt" "strings" "github.com/hashicorp/vault/logical" @@ -11,9 +12,9 @@ import ( func pathLogin(b *backend) *framework.Path { return &framework.Path{ - Pattern: "login/" + framework.GenericNameRegex("name"), + Pattern: "login/" + framework.GenericNameRegex("username"), Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ + "username": &framework.FieldSchema{ Type: framework.TypeString, Description: "Username of the user.", }, @@ -35,8 +36,15 @@ func pathLogin(b *backend) *framework.Path { func (b *backend) pathLogin( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - username := strings.ToLower(d.Get("name").(string)) + username := strings.ToLower(d.Get("username").(string)) + if username == "" { + return nil, fmt.Errorf("missing username") + } + password := d.Get("password").(string) + if password == "" { + return nil, fmt.Errorf("missing password") + } // Get the user and validate auth user, err := b.user(req.Storage, username) diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index e5a7ca196fd0a..4398a7a23cd40 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -9,9 +9,9 @@ import ( func pathUserPassword(b *backend) *framework.Path { return &framework.Path{ - Pattern: "users/" + framework.GenericNameRegex("name") + "/password$", + Pattern: "users/" + framework.GenericNameRegex("username") + "/password$", Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ + "username": &framework.FieldSchema{ Type: framework.TypeString, Description: "Username for this user.", }, diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index e25a5f38dea2c..5a84372b9fe43 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -7,9 +7,9 @@ import ( func pathUserPolicies(b *backend) *framework.Path { return &framework.Path{ - Pattern: "users/" + framework.GenericNameRegex("name") + "/policies$", + Pattern: "users/" + framework.GenericNameRegex("username") + "/policies$", Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ + "username": &framework.FieldSchema{ Type: framework.TypeString, Description: "Username for this user.", }, diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index 86d0bb4adcc14..610ab1793b9d1 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -12,9 +12,9 @@ import ( func pathUsers(b *backend) *framework.Path { return &framework.Path{ - Pattern: "users/" + framework.GenericNameRegex("name"), + Pattern: "users/" + framework.GenericNameRegex("username"), Fields: map[string]*framework.FieldSchema{ - "name": &framework.FieldSchema{ + "username": &framework.FieldSchema{ Type: framework.TypeString, Description: "Username for this user.", }, @@ -55,9 +55,9 @@ func pathUsers(b *backend) *framework.Path { } func (b *backend) userExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { - username := data.Get("name").(string) + username := data.Get("username").(string) if username == "" { - return false, fmt.Errorf("name cannot be empty") + return false, fmt.Errorf("missing username") } entry, err := req.Storage.Get("user/" + strings.ToLower(username)) @@ -96,7 +96,7 @@ func (b *backend) SetUser(s logical.Storage, username string, userEntry *UserEnt func (b *backend) pathUserDelete( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - err := req.Storage.Delete("user/" + strings.ToLower(d.Get("name").(string))) + err := req.Storage.Delete("user/" + strings.ToLower(d.Get("username").(string))) if err != nil { return nil, err } @@ -106,7 +106,7 @@ func (b *backend) pathUserDelete( func (b *backend) pathUserRead( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - user, err := b.user(req.Storage, strings.ToLower(d.Get("name").(string))) + user, err := b.user(req.Storage, strings.ToLower(d.Get("username").(string))) if err != nil { return nil, err } @@ -122,7 +122,7 @@ func (b *backend) pathUserRead( } func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - username := strings.ToLower(d.Get("name").(string)) + username := strings.ToLower(d.Get("username").(string)) userEntry, err := b.user(req.Storage, username) if err != nil { return nil, err From 79ff36713b2deec0b1824c5a0c5ecdc2967ee19a Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Tue, 15 Mar 2016 22:19:31 -0400 Subject: [PATCH 09/15] Added API documentation for userpass backend --- website/source/docs/auth/userpass.html.md | 211 ++++++++++++++++++++++ 1 file changed, 211 insertions(+) diff --git a/website/source/docs/auth/userpass.html.md b/website/source/docs/auth/userpass.html.md index 2e4a929b51f9f..597e5bb8380ac 100644 --- a/website/source/docs/auth/userpass.html.md +++ b/website/source/docs/auth/userpass.html.md @@ -92,3 +92,214 @@ $ vault write auth/userpass/users/mitchellh \ The above creates a new user "mitchellh" with the password "foo" that will be associated with the "root" policy. This is the only configuration necessary. + +## API + +### /auth/userpass/users/[username] +#### POST + +
+
Description
+
+ Create a new user or update an existing user. +
+ +
Method
+
POST
+ +
URL
+
`/auth/userpass/users/`
+ +
Parameters
+
+
    +
  • + username + required + Username for this user. +
  • +
+
+
+
    +
  • + password + required + Password for this user. +
  • +
+
+
+
    +
  • + policies + optional + Comma-separated list of policies. + If set to empty string, only the `default` policy will be applicable to the user. +
  • +
+
+
+
    +
  • + ttl + optional + The lease duration which decides login expiration. +
  • +
+
+
+
    +
  • + max_ttl + optional + Maximum duration after which login should expire. +
  • +
+
+ +
Returns
+
`204` response code. +
+
+ +### /auth/userpass/users/[username]/password +#### POST +
+
Description
+
+ Update the password for an existing user. +
+ +
Method
+
POST
+ +
URL
+
`/auth/userpass/users//password`
+ +
Parameters
+
+
    +
  • + username + required + Username for this user. +
  • +
+
+
+
    +
  • + password + required + Password for this user. +
  • +
+
+ +
Returns
+
`204` response code. +
+
+ +### /auth/userpass/users/[username]/policies +#### POST +
+
Description
+
+ Update the policies associated with an existing user. +
+ +
Method
+
POST
+ +
URL
+
`/auth/userpass/users//policies`
+ +
Parameters
+
+
    +
  • + username + required + Username for this user. +
  • +
+
+
+
    +
  • + policies + optional + Comma-separated list of policies. + If this is field is not supplied, the policies will be unchanged. + If set to empty string, only the `default` policy will be applicable to the user. +
  • +
+
+ +
Returns
+
`204` response code. +
+
+ + +### /auth/userpass/login/[username] +#### POST +
+
Description
+
+ Update the policies associated with an existing user. +
+ +
Method
+
POST
+ +
URL
+
`/auth/userpass/users//policies`
+ +
Parameters
+
+
    +
  • + username + required + Username for this user. +
  • +
+
+
+
    +
  • + password + required + Password for this user. +
  • +
+
+ +
Returns
+
+ + ```javascript + { + "lease_id": "", + "renewable": false, + "lease_duration": 0, + "data": null, + "warnings": null, + "auth": { + "client_token": "64d2a8f2-2a2f-5688-102b-e6088b76e344", + "accessor": "18bb8f89-826a-56ee-c65b-1736dc5ea27d", + "policies": ["default"], + "metadata": { + "username": "vishal" + }, + "lease_duration": 7200, + "renewable": true + } + } + ``` + +
+
From b8e007c19566e7f739842592e21a7b1f1cd2433e Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 16 Mar 2016 11:26:33 -0400 Subject: [PATCH 10/15] Use helper for existence check. Avoid panic by fetching default values for field data --- builtin/credential/userpass/path_users.go | 12 ++++++------ website/source/docs/auth/userpass.html.md | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index 610ab1793b9d1..80b2d150f0e7f 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -60,12 +60,12 @@ func (b *backend) userExistenceCheck(req *logical.Request, data *framework.Field return false, fmt.Errorf("missing username") } - entry, err := req.Storage.Get("user/" + strings.ToLower(username)) + userEntry, err := b.user(req.Storage, username) if err != nil { return false, err } - return entry != nil, nil + return userEntry != nil, nil } func (b *backend) user(s logical.Storage, n string) (*UserEntry, error) { @@ -150,11 +150,11 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) userEntry.Policies = policies } - ttlStrRaw, ttlSet := d.GetOk("ttl") - maxTTLStrRaw, maxTTLSet := d.GetOk("max_ttl") + _, ttlSet := d.GetOk("ttl") + _, maxTTLSet := d.GetOk("max_ttl") if ttlSet || maxTTLSet { - ttlStr := ttlStrRaw.(string) - maxTTLStr := maxTTLStrRaw.(string) + ttlStr := d.Get("ttl").(string) + maxTTLStr := d.Get("max_ttl").(string) userEntry.TTL, userEntry.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) if err != nil { return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil diff --git a/website/source/docs/auth/userpass.html.md b/website/source/docs/auth/userpass.html.md index 597e5bb8380ac..769628353410f 100644 --- a/website/source/docs/auth/userpass.html.md +++ b/website/source/docs/auth/userpass.html.md @@ -102,6 +102,7 @@ necessary.
Description
Create a new user or update an existing user. + This path honors the distinction between the `create` and `update` capabilities inside ACL policies.
Method
From 59054298b81af1447a1fc3711f0ca29da3409d53 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 16 Mar 2016 11:39:52 -0400 Subject: [PATCH 11/15] Reduce the visibility of setUser --- builtin/credential/userpass/path_users.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index 80b2d150f0e7f..33bf27b25f7d7 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -85,7 +85,7 @@ func (b *backend) user(s logical.Storage, n string) (*UserEntry, error) { return &result, nil } -func (b *backend) SetUser(s logical.Storage, username string, userEntry *UserEntry) error { +func (b *backend) setUser(s logical.Storage, username string, userEntry *UserEntry) error { entry, err := logical.StorageEntryJSON("user/"+username, userEntry) if err != nil { return err @@ -161,7 +161,7 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) } } - return nil, b.SetUser(req.Storage, username, userEntry) + return nil, b.setUser(req.Storage, username, userEntry) } func (b *backend) pathUserWrite( From cfbab2c66f7cc968163d8a203bd27f7c99d85ab2 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 16 Mar 2016 13:39:20 -0400 Subject: [PATCH 12/15] Refactor updating user values --- .../credential/userpass/path_user_password.go | 29 +++++++++++- .../credential/userpass/path_user_policies.go | 26 ++++++++++- builtin/credential/userpass/path_users.go | 44 ++++++++++--------- 3 files changed, 75 insertions(+), 24 deletions(-) diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index 4398a7a23cd40..d2d2d534d6311 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -3,6 +3,8 @@ package userpass import ( "fmt" + "golang.org/x/crypto/bcrypt" + "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" ) @@ -33,11 +35,34 @@ func pathUserPassword(b *backend) *framework.Path { func (b *backend) pathUserPasswordUpdate( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { + + username := d.Get("username").(string) + + userEntry, err := b.user(req.Storage, username) + if err != nil { + return nil, err + } + + err = b.updateUserPassword(req, d, userEntry) + if err != nil { + return nil, err + } + + return nil, b.setUser(req.Storage, username, userEntry) +} + +func (b *backend) updateUserPassword(req *logical.Request, d *framework.FieldData, userEntry *UserEntry) error { password := d.Get("password").(string) if password == "" { - return nil, fmt.Errorf("missing password") + return fmt.Errorf("missing password") + } + // Generate a hash of the password + hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + return err } - return b.userCreateUpdate(req, d) + userEntry.PasswordHash = hash + return nil } const pathUserPasswordHelpSyn = ` diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index 5a84372b9fe43..d3124ce4f0714 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -1,6 +1,8 @@ package userpass import ( + "strings" + "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" ) @@ -30,7 +32,29 @@ func pathUserPolicies(b *backend) *framework.Path { func (b *backend) pathUserPoliciesUpdate( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { - return b.userCreateUpdate(req, d) + + username := d.Get("username").(string) + + userEntry, err := b.user(req.Storage, username) + if err != nil { + return nil, err + } + + err = b.updateUserPolicies(req, d, userEntry) + if err != nil { + return nil, err + } + + return nil, b.setUser(req.Storage, username, userEntry) +} + +func (b *backend) updateUserPolicies(req *logical.Request, d *framework.FieldData, userEntry *UserEntry) error { + policies := strings.Split(d.Get("policies").(string), ",") + for i, p := range policies { + policies[i] = strings.TrimSpace(p) + } + userEntry.Policies = policies + return nil } const pathUserPoliciesHelpSyn = ` diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index 33bf27b25f7d7..bc81018e463ba 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/vault/logical" "github.com/hashicorp/vault/logical/framework" - "golang.org/x/crypto/bcrypt" ) func pathUsers(b *backend) *framework.Path { @@ -132,33 +131,36 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) userEntry = &UserEntry{} } - // Set/update the values of UserEntry only if fields are supplied - if passwordRaw, ok := d.GetOk("password"); ok { - // Generate a hash of the password - hash, err := bcrypt.GenerateFromPassword([]byte(passwordRaw.(string)), bcrypt.DefaultCost) + // "password" will always be set here + err = b.updateUserPassword(req, d, userEntry) + if err != nil { + return nil, err + } + + if _, ok := d.GetOk("policies"); ok { + err = b.updateUserPolicies(req, d, userEntry) if err != nil { return nil, err } - userEntry.PasswordHash = hash } - if policiesRaw, ok := d.GetOk("policies"); ok { - policies := strings.Split(policiesRaw.(string), ",") - for i, p := range policies { - policies[i] = strings.TrimSpace(p) - } - userEntry.Policies = policies + ttlStr := "" + if ttlStrRaw, ok := d.GetOk("ttl"); ok { + ttlStr = ttlStrRaw.(string) + } else if req.Operation == logical.CreateOperation { + ttlStr = d.Get("ttl").(string) } - _, ttlSet := d.GetOk("ttl") - _, maxTTLSet := d.GetOk("max_ttl") - if ttlSet || maxTTLSet { - ttlStr := d.Get("ttl").(string) - maxTTLStr := d.Get("max_ttl").(string) - userEntry.TTL, userEntry.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) - if err != nil { - return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil - } + maxTTLStr := "" + if maxTTLStrRaw, ok := d.GetOk("max_ttl"); ok { + maxTTLStr = maxTTLStrRaw.(string) + } else if req.Operation == logical.CreateOperation { + maxTTLStr = d.Get("max_ttl").(string) + } + + userEntry.TTL, userEntry.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) + if err != nil { + return logical.ErrorResponse(fmt.Sprintf("err: %s", err)), nil } return nil, b.setUser(req.Storage, username, userEntry) From 6f2b428379f4e490a212468c96df74521eb07d6a Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 16 Mar 2016 14:21:14 -0400 Subject: [PATCH 13/15] Addessing review comments --- .../credential/userpass/path_user_password.go | 9 +++++ .../credential/userpass/path_user_policies.go | 10 ++++++ builtin/credential/userpass/path_users.go | 33 +++++++++---------- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index d2d2d534d6311..bc599f28375c1 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -24,6 +24,8 @@ func pathUserPassword(b *backend) *framework.Path { }, }, + ExistenceCheck: b.userPasswordExistenceCheck, + Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: b.pathUserPasswordUpdate, }, @@ -33,6 +35,10 @@ func pathUserPassword(b *backend) *framework.Path { } } +func (b *backend) userPasswordExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { + return true, nil +} + func (b *backend) pathUserPasswordUpdate( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { @@ -42,6 +48,9 @@ func (b *backend) pathUserPasswordUpdate( if err != nil { return nil, err } + if userEntry == nil { + return nil, fmt.Errorf("username does not exist") + } err = b.updateUserPassword(req, d, userEntry) if err != nil { diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index d3124ce4f0714..217646fca532a 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -1,6 +1,7 @@ package userpass import ( + "fmt" "strings" "github.com/hashicorp/vault/logical" @@ -21,6 +22,8 @@ func pathUserPolicies(b *backend) *framework.Path { }, }, + ExistenceCheck: b.userPoliciesExistenceCheck, + Callbacks: map[logical.Operation]framework.OperationFunc{ logical.UpdateOperation: b.pathUserPoliciesUpdate, }, @@ -30,6 +33,10 @@ func pathUserPolicies(b *backend) *framework.Path { } } +func (b *backend) userPoliciesExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { + return true, nil +} + func (b *backend) pathUserPoliciesUpdate( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { @@ -39,6 +46,9 @@ func (b *backend) pathUserPoliciesUpdate( if err != nil { return nil, err } + if userEntry == nil { + return nil, fmt.Errorf("username does not exist") + } err = b.updateUserPolicies(req, d, userEntry) if err != nil { diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index bc81018e463ba..a9bbaef211786 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -54,12 +54,7 @@ func pathUsers(b *backend) *framework.Path { } func (b *backend) userExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { - username := data.Get("username").(string) - if username == "" { - return false, fmt.Errorf("missing username") - } - - userEntry, err := b.user(req.Storage, username) + userEntry, err := b.user(req.Storage, data.Get("username").(string)) if err != nil { return false, err } @@ -67,8 +62,12 @@ func (b *backend) userExistenceCheck(req *logical.Request, data *framework.Field return userEntry != nil, nil } -func (b *backend) user(s logical.Storage, n string) (*UserEntry, error) { - entry, err := s.Get("user/" + strings.ToLower(n)) +func (b *backend) user(s logical.Storage, username string) (*UserEntry, error) { + if username == "" { + return nil, fmt.Errorf("missing username") + } + + entry, err := s.Get("user/" + strings.ToLower(username)) if err != nil { return nil, err } @@ -132,9 +131,11 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) } // "password" will always be set here - err = b.updateUserPassword(req, d, userEntry) - if err != nil { - return nil, err + if _, ok := d.GetOk("password"); ok { + err = b.updateUserPassword(req, d, userEntry) + if err != nil { + return nil, err + } } if _, ok := d.GetOk("policies"); ok { @@ -144,18 +145,14 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) } } - ttlStr := "" + ttlStr := userEntry.TTL.String() if ttlStrRaw, ok := d.GetOk("ttl"); ok { ttlStr = ttlStrRaw.(string) - } else if req.Operation == logical.CreateOperation { - ttlStr = d.Get("ttl").(string) } - maxTTLStr := "" + maxTTLStr := userEntry.MaxTTL.String() if maxTTLStrRaw, ok := d.GetOk("max_ttl"); ok { maxTTLStr = maxTTLStrRaw.(string) - } else if req.Operation == logical.CreateOperation { - maxTTLStr = d.Get("max_ttl").(string) } userEntry.TTL, userEntry.MaxTTL, err = b.SanitizeTTL(ttlStr, maxTTLStr) @@ -169,7 +166,7 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) func (b *backend) pathUserWrite( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { password := d.Get("password").(string) - if password == "" { + if req.Operation == logical.CreateOperation && password == "" { return nil, fmt.Errorf("missing password") } return b.userCreateUpdate(req, d) From daab5d6777416ec443cd1ab2562e08859c1c53cd Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 16 Mar 2016 14:27:01 -0400 Subject: [PATCH 14/15] Fix SanitizeTTL check --- logical/framework/backend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/logical/framework/backend.go b/logical/framework/backend.go index 73d77e741b5e2..e6749935aabb9 100644 --- a/logical/framework/backend.go +++ b/logical/framework/backend.go @@ -249,7 +249,7 @@ func (b *Backend) SanitizeTTL(ttlStr, maxTTLStr string) (ttl, maxTTL time.Durati return 0, 0, fmt.Errorf("\"max_ttl\" value must be less than allowed max lease TTL value '%s'", sysMaxTTL.String()) } } - if ttl > maxTTL { + if ttl > maxTTL && maxTTL != 0 { ttl = maxTTL } return From 4ae83b7cc84f59f08311e2107aa9ef4a84d7fba7 Mon Sep 17 00:00:00 2001 From: vishalnayak Date: Wed, 16 Mar 2016 14:53:53 -0400 Subject: [PATCH 15/15] Add comments to existence functions --- builtin/credential/userpass/path_login.go | 5 +---- builtin/credential/userpass/path_user_password.go | 2 ++ builtin/credential/userpass/path_user_policies.go | 2 ++ builtin/credential/userpass/path_users.go | 1 - 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/credential/userpass/path_login.go b/builtin/credential/userpass/path_login.go index 28ef665fbbd6e..458d35e8bb984 100644 --- a/builtin/credential/userpass/path_login.go +++ b/builtin/credential/userpass/path_login.go @@ -37,9 +37,6 @@ func pathLogin(b *backend) *framework.Path { func (b *backend) pathLogin( req *logical.Request, d *framework.FieldData) (*logical.Response, error) { username := strings.ToLower(d.Get("username").(string)) - if username == "" { - return nil, fmt.Errorf("missing username") - } password := d.Get("password").(string) if password == "" { @@ -52,7 +49,7 @@ func (b *backend) pathLogin( return nil, err } if user == nil { - return logical.ErrorResponse("unknown username or password"), nil + return logical.ErrorResponse("username does not exist"), nil } // Check for a password match. Check for a hash collision for Vault 0.2+, diff --git a/builtin/credential/userpass/path_user_password.go b/builtin/credential/userpass/path_user_password.go index bc599f28375c1..22c08239a800c 100644 --- a/builtin/credential/userpass/path_user_password.go +++ b/builtin/credential/userpass/path_user_password.go @@ -35,6 +35,8 @@ func pathUserPassword(b *backend) *framework.Path { } } +// By always returning true, this endpoint will be enforced to be invoked only upon UpdateOperation. +// The existence of user will be checked in the operation handler. func (b *backend) userPasswordExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { return true, nil } diff --git a/builtin/credential/userpass/path_user_policies.go b/builtin/credential/userpass/path_user_policies.go index 217646fca532a..73b9fe6d6a2fb 100644 --- a/builtin/credential/userpass/path_user_policies.go +++ b/builtin/credential/userpass/path_user_policies.go @@ -33,6 +33,8 @@ func pathUserPolicies(b *backend) *framework.Path { } } +// By always returning true, this endpoint will be enforced to be invoked only upon UpdateOperation. +// The existence of user will be checked in the operation handler. func (b *backend) userPoliciesExistenceCheck(req *logical.Request, data *framework.FieldData) (bool, error) { return true, nil } diff --git a/builtin/credential/userpass/path_users.go b/builtin/credential/userpass/path_users.go index a9bbaef211786..21fe4162559a4 100644 --- a/builtin/credential/userpass/path_users.go +++ b/builtin/credential/userpass/path_users.go @@ -130,7 +130,6 @@ func (b *backend) userCreateUpdate(req *logical.Request, d *framework.FieldData) userEntry = &UserEntry{} } - // "password" will always be set here if _, ok := d.GetOk("password"); ok { err = b.updateUserPassword(req, d, userEntry) if err != nil {