Skip to content
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.

Sign up for GitHub

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

fix: added validation check for user credentials #228

Merged
merged 4 commits into from
Feb 9, 2024
Merged

fix: added validation check for user credentials #228

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba008f2
fix: added validation check for user credentials
tiwarishubham635 Feb 7, 2024
645c526
fix: added error code and more info in validate credentials
tiwarishubham635 Feb 8, 2024
4836dc7
fix: added inline comments
tiwarishubham635 Feb 8, 2024
bab0120
Merge branch 'main' into validate_credentials
tiwarishubham635 Feb 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,24 @@ func (c *Client) doWithErr(req *http.Request) (*http.Response, error) {
return res, nil
}

func isAlphanumeric(word string) bool {
return regexp.MustCompile(`^[a-zA-Z0-9]*$`).MatchString(word)
Copy link

Choose a reason for hiding this comment

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

It's not very efficient to do this every single time someone calls isAlphanumeric - make the regexp a package level variable that gets initialized one time.

Note, I believe this will also pass if word is the empty string. It would be pretty easy to add some basic smoke tests for this function.

}

func (c *Client) validateCredentials() error {
username, password := c.basicAuth()
err := &TwilioRestError{Status: 400}
if !isAlphanumeric(username) {
err.Message = "Invalid Username"
Copy link

Choose a reason for hiding this comment

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

This is going to be pretty vague and confusing for end users - if I get "Invalid Username" it might not be obvious to me where the problem is, especially if e.g. the user is pulling a username from an environment variable or a web form.

Can we make the error clearer about what the problem is?

return err
}
if !isAlphanumeric(password) {
err.Message = "Invalid Password"
return err
}
return nil
}

// SendRequest verifies, constructs, and authorizes an HTTP request.
func (c *Client) SendRequest(method string, rawURL string, data url.Values,
headers map[string]interface{}) (*http.Response, error) {
Expand All @@ -112,6 +130,11 @@ func (c *Client) SendRequest(method string, rawURL string, data url.Values,
valueReader = strings.NewReader(data.Encode())
}

credErr := c.validateCredentials()
Copy link

Choose a reason for hiding this comment

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

I believe that there are some Twilio endpoints that allow data to be retrieved without a valid username and password so it would be great to know if there is a product spec for this.

if credErr != nil {
return nil, credErr
}

req, err := http.NewRequest(method, u.String(), valueReader)
if err != nil {
return nil, err
Expand Down
46 changes: 46 additions & 0 deletions client/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,52 @@ func TestClient_SendRequestErrorWithDetails(t *testing.T) {
assert.Equal(t, details, twilioError.Details)
}

func TestClient_SendRequestUsernameError(t *testing.T) {
errorResponse := `{
"status": 400,
"code":20001,
"message":"Bad request",
"more_info":"https://www.twilio.com/docs/errors/20001",
}`
errorServer := httptest.NewServer(http.HandlerFunc(
func(resp http.ResponseWriter, req *http.Request) {
resp.WriteHeader(400)
_, _ = resp.Write([]byte(errorResponse))
}))
defer errorServer.Close()

newTestClient := NewClient("user1\nuser2", "pass")
resp, err := newTestClient.SendRequest("GET", errorServer.URL, nil, nil) //nolint:bodyclose
twilioError := err.(*twilio.TwilioRestError)
assert.Error(t, err)
assert.Contains(t, err.Error(), "Invalid Username")
assert.Equal(t, 400, twilioError.Status)
assert.Nil(t, resp)
}

func TestClient_SendRequestPasswordError(t *testing.T) {
errorResponse := `{
"status": 400,
"code":20001,
"message":"Bad request",
"more_info":"https://www.twilio.com/docs/errors/20001",
}`
errorServer := httptest.NewServer(http.HandlerFunc(
func(resp http.ResponseWriter, req *http.Request) {
resp.WriteHeader(400)
_, _ = resp.Write([]byte(errorResponse))
}))
defer errorServer.Close()

newTestClient := NewClient("user1", "pass1\npass2")
resp, err := newTestClient.SendRequest("GET", errorServer.URL, nil, nil) //nolint:bodyclose
twilioError := err.(*twilio.TwilioRestError)
assert.Error(t, err)
assert.Contains(t, err.Error(), "Invalid Password")
assert.Equal(t, 400, twilioError.Status)
assert.Nil(t, resp)
}

func TestClient_SendRequestWithRedirect(t *testing.T) {
redirectServer := httptest.NewServer(http.HandlerFunc(
func(writer http.ResponseWriter, request *http.Request) {
Expand Down
Loading