-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Initial Implementation #1
Changes from 3 commits
9ebb9a6
1ad98a1
7a03f33
fb64bc5
110ab66
0b39bfd
ecb43aa
950f372
bced1df
28c410c
e84347d
bdc5375
78aef98
681e221
65e1059
2f3a01e
23a967c
0efd193
385357c
284933b
575ab78
e22fb44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
package plugin | ||
package rediselasticache | ||
|
||
import ( | ||
"os" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,28 @@ | ||
package plugin | ||
package rediselasticache | ||
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"regexp" | ||
"strings" | ||
"unicode" | ||
|
||
"github.com/hashicorp/go-hclog" | ||
|
||
"github.com/aws/aws-sdk-go/aws" | ||
"github.com/aws/aws-sdk-go/aws/credentials" | ||
"github.com/aws/aws-sdk-go/aws/session" | ||
"github.com/aws/aws-sdk-go/service/elasticache" | ||
"github.com/hashicorp/go-hclog" | ||
"github.com/hashicorp/vault/sdk/database/dbplugin/v5" | ||
"github.com/hashicorp/vault/sdk/database/helper/credsutil" | ||
"github.com/mitchellh/mapstructure" | ||
) | ||
|
||
var nonAlphanumericRegex = regexp.MustCompile("[^a-zA-Z\\d]+") | ||
var ( | ||
nonAlphanumericHyphenRegex = regexp.MustCompile("[^a-zA-Z\\d-]+") | ||
doubleHyphenRegex = regexp.MustCompile("-{2,}") | ||
) | ||
|
||
// Verify interface is implemented | ||
var _ dbplugin.Database = (*redisElastiCacheDB)(nil) | ||
|
@@ -88,18 +92,21 @@ func (r *redisElastiCacheDB) NewUser(_ context.Context, req dbplugin.NewUserRequ | |
return dbplugin.NewUserResponse{}, fmt.Errorf("unable to generate username: %w", err) | ||
} | ||
|
||
accessString := strings.Join(req.Statements.Commands[:], " ") | ||
accessString, err := parseCreationCommands(req.Statements.Commands) | ||
if err != nil { | ||
return dbplugin.NewUserResponse{}, fmt.Errorf("unable to parse access string: %w", err) | ||
} | ||
|
||
userId := generateUserId(username) | ||
userId := normaliseId(username) | ||
|
||
output, err := r.client.CreateUser(&elasticache.CreateUserInput{ | ||
AccessString: &accessString, | ||
AccessString: aws.String(accessString), | ||
Engine: aws.String("Redis"), | ||
NoPasswordRequired: aws.Bool(false), | ||
Passwords: []*string{&req.Password}, | ||
Tags: []*elasticache.Tag{}, | ||
UserId: &userId, | ||
UserName: &username, | ||
UserId: aws.String(userId), | ||
UserName: aws.String(username), | ||
}) | ||
if err != nil { | ||
return dbplugin.NewUserResponse{}, fmt.Errorf("unable to create new user: %w", err) | ||
|
@@ -111,7 +118,7 @@ func (r *redisElastiCacheDB) NewUser(_ context.Context, req dbplugin.NewUserRequ | |
func (r *redisElastiCacheDB) UpdateUser(_ context.Context, req dbplugin.UpdateUserRequest) (dbplugin.UpdateUserResponse, error) { | ||
r.logger.Debug("updating AWS ElastiCache Redis user", "username", req.Username) | ||
|
||
userId := generateUserId(req.Username) | ||
userId := normaliseId(req.Username) | ||
|
||
_, err := r.client.ModifyUser(&elasticache.ModifyUserInput{ | ||
UserId: &userId, | ||
|
@@ -127,7 +134,7 @@ func (r *redisElastiCacheDB) UpdateUser(_ context.Context, req dbplugin.UpdateUs | |
func (r *redisElastiCacheDB) DeleteUser(_ context.Context, req dbplugin.DeleteUserRequest) (dbplugin.DeleteUserResponse, error) { | ||
r.logger.Debug("deleting AWS ElastiCache Redis user", "username", req.Username) | ||
|
||
userId := generateUserId(req.Username) | ||
userId := normaliseId(req.Username) | ||
|
||
_, err := r.client.DeleteUser(&elasticache.DeleteUserInput{ | ||
UserId: &userId, | ||
|
@@ -139,19 +146,56 @@ func (r *redisElastiCacheDB) DeleteUser(_ context.Context, req dbplugin.DeleteUs | |
return dbplugin.DeleteUserResponse{}, nil | ||
} | ||
|
||
// The ID can have up to 40 characters, and must begin with a letter. | ||
func parseCreationCommands(commands []string) (string, error) { | ||
if len(commands) == 0 { | ||
return "on ~* +@read", nil | ||
} | ||
|
||
accessString := "" | ||
for _, command := range commands { | ||
var rules []string | ||
err := json.Unmarshal([]byte(command), &rules) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
if len(rules) > 0 { | ||
accessString += strings.Join(rules, " ") | ||
accessString += " " | ||
} | ||
} | ||
|
||
if strings.HasPrefix(accessString, "off ") || strings.Contains(accessString, " off ") || strings.HasSuffix(accessString, " off") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could occur in another context. For example, if an eshop business caches special offers in Redis using the key pattern: It really needs to be the keyword "off" all by itself. Would it be more readable with a word-boundary regex? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for verifying! That makes sense -- assuming key patterns cannot contain spaces. Yeah my initial thought was to use a regex. But that is just a nitpick. I am good with either one. |
||
return "", errors.New("creation of disabled or 'off' users is forbidden") | ||
} | ||
|
||
if !(strings.HasPrefix(accessString, "on ") || strings.Contains(accessString, " on ") || strings.HasSuffix(accessString, " on")) { | ||
accessString = "on " + accessString | ||
} | ||
|
||
accessString = strings.TrimSpace(accessString) | ||
|
||
return accessString, nil | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For people who already reviewed, this is a behaviour change that was introduced in between the updates. We talked and decided that the format for the creation statements should be a list of strings instead of a string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM. The tests helped me understand the behavior here. Nice job on those! |
||
|
||
// All Elasticache IDs can have up to 40 characters, and must begin with a letter. | ||
// It should not end with a hyphen or contain two consecutive hyphens. | ||
// Valid characters: A-Z, a-z, 0-9, and -(hyphen). | ||
func generateUserId(username string) string { | ||
userId := nonAlphanumericRegex.ReplaceAllString(username, "") | ||
func normaliseId(raw string) string { | ||
normalized := nonAlphanumericHyphenRegex.ReplaceAllString(raw, "") | ||
normalized = doubleHyphenRegex.ReplaceAllString(normalized, "") | ||
|
||
if len(normalized) > 40 { | ||
normalized = normalized[len(normalized)-40:] | ||
} | ||
|
||
if len(userId) > 40 { | ||
userId = userId[len(userId)-40:] | ||
if unicode.IsNumber(rune(normalized[0])) { | ||
normalized = string(rune('A'-17+normalized[0])) + normalized[1:] | ||
} | ||
|
||
if unicode.IsNumber(rune(userId[0])) { | ||
userId = string(rune('A'-17+userId[0])) + userId[1:] | ||
if strings.HasSuffix(normalized, "-") { | ||
normalized = normalized[:len(normalized)-1] + "x" | ||
} | ||
|
||
return userId | ||
return normalized | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: A small comment on what this function does would help future readers understand quickly. Details you shared in files#r954459565 would be perfect.