search

AdRoll / hologram

Easy, painless AWS credentials on developer laptops.
Apache License 2.0
803 stars 42 forks source link

Add a configurable username attribute (still defaults to 'cn') #30

Closed copumpkin closed 9 years ago

copumpkin commented 9 years ago

This should address #18. Mostly just passing the config field through the code to relevant places. I've tested it locally with sAMAccountName against Active Directory. The one piece I haven't tested is the addSSHKey logic, because I don't know how to invoke it! Happy to make changes if that ends up not working properly.

frangarciam commented 9 years ago

Thanks for the PR! That ticks one task off my todo list :)

I just gave it a cursory glance (will do a full review on Monday) and it looks good, the only comment I have is that if I use it with a current config file, the attribute will be "", not "cn". You'd only need to check if the attribute is an empty string after parsing the config (in main.go) and set it to "cn" to make it a proper default, so it should be a simple enough change.

copumpkin commented 9 years ago

Good point! I'll fix that when I'm next at my computer :)

On Mar 27, 2015, at 18:20, Fran Garcia notifications@github.com wrote:

Thanks for the PR! That ticks one task off my todo list :)

I just gave it a cursory glance (will do a full review on Monday) and it looks good, the only comment I have is that if I use it with a current config file, the attribute will be "", not "cn". You'd only need to check if the attribute is an empty string after parsing the config (in main.go) and set it to "cn" to make it a proper default, so it should be a simple enough change.

— Reply to this email directly or view it on GitHub.

copumpkin commented 9 years ago

Should be fixed!

frangarciam commented 9 years ago

Thanks! Everything looks good :+1: