Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow @ to be part of key name in TOTP secret engine #5652

Merged
merged 4 commits into from
Oct 31, 2018
Merged

Allow @ to be part of key name in TOTP secret engine #5652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7c148d7
Allow @ to be part of key name in TOTP secret engine
vishalnayak Oct 30, 2018
40a4004
Allow @ for key name while generating the code
vishalnayak Oct 31, 2018
c577a01
Merge branch 'master-oss' into totp-key-name-regex
vishalnayak Oct 31, 2018
1ad931f
Merge branch 'master' into totp-key-name-regex
vishalnayak Oct 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 74 additions & 0 deletions builtin/logical/totp/backend_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"testing"
"time"

"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/logical"
logicaltest "github.com/hashicorp/vault/logical/testing"
"github.com/mitchellh/mapstructure"
Expand Down Expand Up @@ -38,6 +39,79 @@ func generateCode(key string, period uint, digits otplib.Digits, algorithm otpli
return totpToken, err
}

func TestBackend_KeyName(t *testing.T) {
config := logical.TestBackendConfig()
config.StorageView = &logical.InmemStorage{}
b, err := Factory(context.Background(), config)
if err != nil {
t.Fatal(err)
}

tests := []struct {
Name string
KeyName string
Fail bool
}{
{
"without @",
"sample",
false,
},
{
"with @ in the beginning",
"@sample.com",
true,
},
{
"with @ in the end",
"sample.com@",
true,
},
{
"with @ in between",
"sample@sample.com",
false,
},
{
"with multiple @",
"sample@sample@@sample.com",
false,
},
}
var resp *logical.Response
for _, tc := range tests {
resp, err = b.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "keys/" + tc.KeyName,
Operation: logical.UpdateOperation,
Storage: config.StorageView,
Data: map[string]interface{}{
"generate": true,
"account_name": "vault",
"issuer": "hashicorp",
},
})
if tc.Fail {
if err == nil {
t.Fatalf("expected an error for test %q", tc.Name)
}
continue
} else if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: test name: %q\nresp: %#v\nerr: %v", tc.Name, resp, err)
}
resp, err = b.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "code/" + tc.KeyName,
Operation: logical.ReadOperation,
Storage: config.StorageView,
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: test name: %q\nresp: %#v\nerr: %v", tc.Name, resp, err)
}
if resp.Data["code"].(string) == "" {
t.Fatalf("failed to generate code for test %q", tc.Name)
}
}
}

func TestBackend_readCredentialsDefaultValues(t *testing.T) {
config := logical.TestBackendConfig()
config.StorageView = &logical.InmemStorage{}
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/totp/path_code.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

func pathCode(b *backend) *framework.Path {
return &framework.Path{
Pattern: "code/" + framework.GenericNameRegex("name"),
Pattern: "code/" + framework.GenericNameWithAtRegex("name"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we likely to see this used elsewhere? Should this really be in the framework package?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unlikely that we see it elsewhere. I originally had it in the totp package itself, but then moved to framework to keep it close to GenericNameRegex to increases chances of changes to that reflected in this as well.

Fields: map[string]*framework.FieldSchema{
"name": &framework.FieldSchema{
Type: framework.TypeString,
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/totp/path_keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func pathListKeys(b *backend) *framework.Path {

func pathKeys(b *backend) *framework.Path {
return &framework.Path{
Pattern: "keys/" + framework.GenericNameRegex("name"),
Pattern: "keys/" + framework.GenericNameWithAtRegex("name"),
Fields: map[string]*framework.FieldSchema{
"name": {
Type: framework.TypeString,
Expand Down
6 changes: 6 additions & 0 deletions logical/framework/path.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ func GenericNameRegex(name string) string {
return fmt.Sprintf("(?P<%s>\\w(([\\w-.]+)?\\w)?)", name)
}

// GenericNameWithAtRegex returns a generic regex that allows alphanumeric
// characters along with -, . and @.
func GenericNameWithAtRegex(name string) string {
return fmt.Sprintf("(?P<%s>\\w(([\\w-.@]+)?\\w)?)", name)
}

// Helper which returns a regex string for optionally accepting the a field
// from the API URL
func OptionalParamRegex(name string) string {
Expand Down