Merge pull request #1153 from kbarlowgw/master

Add 5m timeout to page.WaitForRequest
Versent · Nov 1, 2023 · 7cc41b5 · 7cc41b5
2 parents d088a19 + 915af83
commit 7cc41b5
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 6 deletions.
diff --git a/pkg/provider/browser/browser.go b/pkg/provider/browser/browser.go
@@ -13,18 +13,22 @@ import (
 
 var logger = logrus.WithField("provider", "browser")
 
+const DEFAULT_TIMEOUT float64 = 300000
+
 // Client client for browser based Identity Provider
 type Client struct {
 	Headless bool
 	// Setup alternative directory to download playwright browsers to
 	BrowserDriverDir string
+	Timeout          int
 }
 
 // New create new browser based client
 func New(idpAccount *cfg.IDPAccount) (*Client, error) {
 	return &Client{
 		Headless:         idpAccount.Headless,
 		BrowserDriverDir: idpAccount.BrowserDriverDir,
+		Timeout:          idpAccount.Timeout,
 	}, nil
 }
 
@@ -75,10 +79,10 @@ func (cl *Client) Authenticate(loginDetails *creds.LoginDetails) (string, error)
 		}
 	}()
 
-	return getSAMLResponse(page, loginDetails)
+	return getSAMLResponse(page, loginDetails, cl)
 }
 
-var getSAMLResponse = func(page playwright.Page, loginDetails *creds.LoginDetails) (string, error) {
+var getSAMLResponse = func(page playwright.Page, loginDetails *creds.LoginDetails, client *Client) (string, error) {
 	logger.WithField("URL", loginDetails.URL).Info("opening browser")
 
 	if _, err := page.Goto(loginDetails.URL); err != nil {
@@ -94,7 +98,7 @@ var getSAMLResponse = func(page playwright.Page, loginDetails *creds.LoginDetail
 	}
 
 	logger.Info("waiting ...")
-	r, _ := page.WaitForRequest(signin_re)
+	r, _ := page.WaitForRequest(signin_re, client.waitForRequestTimeout())
 	data, err := r.PostData()
 	if err != nil {
 		return "", err
@@ -123,3 +127,11 @@ func (cl *Client) Validate(loginDetails *creds.LoginDetails) error {
 
 	return nil
 }
+
+func (cl *Client) waitForRequestTimeout() playwright.PageWaitForRequestOptions {
+	timeout := float64(cl.Timeout)
+	if timeout < 30000 {
+		timeout = DEFAULT_TIMEOUT
+	}
+	return playwright.PageWaitForRequestOptions{Timeout: &timeout}
+}
diff --git a/pkg/provider/browser/browser_test.go b/pkg/provider/browser/browser_test.go
@@ -62,7 +62,7 @@ func TestNoBrowserDriverFail(t *testing.T) {
 	assert.ErrorContains(t, err, "could not start driver")
 }
 
-func fakeSAMLResponse(page playwright.Page, loginDetails *creds.LoginDetails) (string, error) {
+func fakeSAMLResponse(page playwright.Page, loginDetails *creds.LoginDetails, client *Client) (string, error) {
 	return response, nil
 }
 
@@ -96,6 +96,14 @@ func TestGetSAMLResponse(t *testing.T) {
 			</saml:EncryptedAssertion>
 	</samlp:Response>
 `
+
+	idpAccount := cfg.IDPAccount{
+		Headless: true,
+		Timeout:  100000,
+	}
+
+	client, err := New(&idpAccount)
+	assert.Nil(t, err)
 	params := url.Values{}
 	params.Add("foo1", "bar1")
 	params.Add("SAMLResponse", samlp)
@@ -107,12 +115,46 @@ func TestGetSAMLResponse(t *testing.T) {
 	regex, err := signinRegex()
 	assert.Nil(t, err)
 	page.Mock.On("Goto", url).Return(resp, nil)
-	page.Mock.On("WaitForRequest", regex).Return(req)
+	page.Mock.On("WaitForRequest", regex, client.waitForRequestTimeout()).Return(req)
 	req.Mock.On("PostData").Return(params.Encode(), nil)
 	// loginDetails := &creds.LoginDetails{
 	//	URL: url,
 	//}
-	// samlResp, err := getSAMLResponse(page, loginDetails)
+	// samlResp, err := getSAMLResponse(page, loginDetails, client)
 	// assert.Nil(t, err)
 	// assert.Equal(t, samlp, samlResp)
 }
+
+func TestWaitForRequestOptions(t *testing.T) {
+	timeout := float64(100000)
+	idpAccount := cfg.IDPAccount{
+		Headless: true,
+		Timeout:  int(timeout),
+	}
+
+	client, err := New(&idpAccount)
+	assert.Nil(t, err)
+
+	options := client.waitForRequestTimeout()
+	if *options.Timeout != timeout {
+		t.Errorf("Unexpected value for timeout [%.0f]: expected [%.0f]", *options.Timeout, timeout)
+	}
+}
+
+func TestWaitForRequestOptionsDefaultTimeout(t *testing.T) {
+	idpAccount := cfg.IDPAccount{
+		Headless: true,
+		Timeout:  1000,
+	}
+
+	client, err := New(&idpAccount)
+
+	if err != nil {
+		t.Errorf("Unable to create browser")
+	}
+
+	options := client.waitForRequestTimeout()
+	if *options.Timeout != DEFAULT_TIMEOUT {
+		t.Errorf("Unexpected value for timeout [%.0f]: expected [%.0f]", *options.Timeout, DEFAULT_TIMEOUT)
+	}
+}