-
Notifications
You must be signed in to change notification settings - Fork 28
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
Whitelist #13
Whitelist #13
Conversation
swrap
commented
Sep 13, 2017
- Updated Whitlist
- Fixed up some test
Removed old whitelist-on text
@swrap I am interested in this patch, but it seems to be missing the implementation of the |
@nathanielc just added in the files |
@swrap That was fast thanks, I'll see if I can test this out on my deployment tonight. |
padlockcloud/whitelist.go
Outdated
// Returns whether email is whitelisted or not | ||
func (w *Whitelist) IsWhitelisted(email string) bool { | ||
b, ok := w.Emails[email] | ||
return b && ok |
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.
Just my 2 cents here on style as this isn't my project.
Go's spec defines a read from a map for an non existent entry to return the zero value of the type, which in this case is false
. So this function can be simplified to return w.Emails[emails]
without a loss of safety. And it improves clarity a bit in my opinion as there is less to figure out.
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.
Thanks I did not know that! I can fix that real quick.
@@ -46,6 +46,11 @@ func (h *RequestAuthToken) Handle(w http.ResponseWriter, r *http.Request, auth * | |||
return &BadRequest{"no email provided"} | |||
} | |||
|
|||
if h.whitelist != nil && h.whitelist.IsWhitelisted(email) == false { |
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.
This can be simplified to
if h.whitelist != nil && !h.whitelist.IsWhitelisted(email) {
@swrap Great job! Sorry, this flew under my radar until @nathanielc commented (thanks for the help btw). Gonna merge this now. |