Skip to content
This repository was archived by the owner on Mar 15, 2024. It is now read-only.

Conversation

@abhide
Copy link
Contributor

@abhide abhide commented Aug 8, 2019

Tickets: TE-34

@abhide abhide requested a review from michaelw August 8, 2019 20:45
return nil, err
}
username := fmt.Sprintf("vault_%s_%s_%s_%d", name, req.DisplayName, userUUID, time.Now().UnixNano())
username := fmt.Sprintf("%s_%s", role.UserPrefix, userUUID)
Copy link
Contributor

Choose a reason for hiding this comment

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

userUUID might not be unique, e.g., did you test same user requesting two emphemeral creds with overlapping lifetime?

Copy link
Contributor

Choose a reason for hiding this comment

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

also, should we keep requestor name in there for tracking? (at least unless explicitly overridden)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A new UUID is generated for userUUID for each new user. Check 3 lines above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added req.DisplayName to username

Accidentally, delete `go install` from `make build` target;
while moving to go modules.
@abhide abhide force-pushed the user-prefix branch 3 times, most recently from 3092b58 to 04e4da8 Compare August 12, 2019 22:28
Copy link
Contributor

@michaelw michaelw left a comment

Choose a reason for hiding this comment

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

we might want to document that defaultUserPrefix is a special value now (or not make it special)

@michaelw michaelw merged commit a8e992e into master Aug 13, 2019
@michaelw michaelw deleted the user-prefix branch August 19, 2021 01:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants