AWS EC2 Auth Backend #76
Conversation
- Mount configuration - Role configuration - Backend configuration
|
|
||
| componentDidMount() { | ||
| this.listEc2Roles(); | ||
| this.getEc2AuthConfig(); |
There was a problem hiding this comment.
In order to correctly display a role by URL such as http://127.0.0.1:8000/auth/aws-ec2/awsaccount1/test you need some code in componendDidMount.
Refer to app/components/Authentication/Radius/Radius.jsx:componentDidMount()
| } | ||
|
|
||
| componentDidUpdate(prevProps, prevState) { | ||
| if (this.state.newRoleConfig.role != prevState.newRoleConfig.role) { |
There was a problem hiding this comment.
If you use newRoleConfig.role directly in this function, it creates an inconvenient side effect. every character typed in the role name field in the new role name dialog will trigger 2 API calls to vault.
I suggest you use a separate state variable selectedRoleId.
Refer to app/components/Authentication/Radius/Radius.jsx:componentDidUpdate()
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This is missing code to handle switching to another aws-ec2 backend mounted on a different path.
Refer to app/components/Authentication/Radius/Radius.jsx:componentWillReceiveProps()
| let roles = resp.data.data.keys; | ||
| this.setState({ ec2Roles: _.valuesIn(roles) }); | ||
| }) | ||
| .catch(snackBarMessage); |
There was a problem hiding this comment.
Vault will return 404 here if no roles are configured. I recommend handling the exception to suppress the snackbar message in that case
| }) | ||
| }); | ||
| }) | ||
| .catch(snackBarMessage); |
There was a problem hiding this comment.
Again, vault returns 404 if backend has never been configured. Worth handling here and displaying a better message like This backend is not yet configured
|
|
||
| export default class AwsEc2AuthBackend extends React.Component { | ||
|
|
||
| ec2ConfigSchema = { |
There was a problem hiding this comment.
can we start placing this stuff in a config? Declaring undefined for everything doesn't seem right
| listEc2Roles() { | ||
| tokenHasCapabilities(['list'], `${this.state.baseVaultPath}/role`) | ||
| .then(() => { | ||
| callVaultApi('get', `${this.state.baseVaultPath}/role`, { list: true }, null) |
There was a problem hiding this comment.
es6 lets us define params...might be nice for readability to be like foo=null or bar=foo, to easily read what is going on here with the params
| .then(() => { | ||
| callVaultApi('get', `${this.state.baseVaultPath}/role`, { list: true }, null) | ||
| .then((resp) => { | ||
| let roles = resp.data.data.keys; |
| this.setState({ ec2Roles: _.valuesIn(roles) }); | ||
| }) | ||
| .catch((error) => { | ||
| if (error.response.status !== 404) { |
There was a problem hiding this comment.
again...status codes could be better
| }); | ||
| }) | ||
| .catch(() => { | ||
| snackBarMessage(new Error("Access denied")); |
There was a problem hiding this comment.
403 or something in the previous
| this.setState({ | ||
| configObj: update(this.state.configObj, | ||
| { | ||
| access_key: { $set: (config.access_key ? config.access_key : null) }, |
There was a problem hiding this comment.
why set it to null? if it is undefined use it and it will be falsey most likey where you make the check later
Users will be able to add new aws-ec2 mounts at whatever path, configure the backend, and add/manage roles.
This PR is still in progress. Looking to add options for the other bounds that Vault supports in addition to
bound_ami_idandbound_iam_role_arn, which are included now.