Repository: Versent/saml2aws (go)

Issue:
Getting playwright error when trying to saml2aws login
Hey,

Getting an error when I login to saml2aws.

Error: 
```
~ saml2aws login
Using IdP Account default to access Browser `URL`
To use saved password just hit enter.
? Username `username`
? Password

Authenticating as `username` ...
Error authenticating to IdP.: could not start driver: fork/exec /Users/`username`/Library/Caches/ms-playwright-go/1.20.0-beta-1647057403000/playwright.sh: no such file or directory
```

Steps:
```
brew install saml2aws
saml2aws configure
saml2aws login
```

Version: 
```
~ saml2aws --version
2.36.5
```

Pull request:
Browser IDP - Configure Browser IDP to automatically install Playwright Browser Drivers per old behavior <= 2.36.4
# Description
This PR seeks to have the browser IDP option to once again automatically download Browser Drivers as was the default behavior for saml2aws versions <= 2.36.4

This PR would functionally close #1005 

## Motivation

When saml2aws was upgraded to 2.36.5 and using the IDP option of `Browser`, one would get this failure message:
```
Error to authenticating to IdP: could not start driver: fork/exec /Users/alice/Library/Caches/ms-playwright-go/1.20.0-beta-1647057403000/playwright.sh: no such file or directory
```

It appears that as part of upgrading browser binaries and thus playwright as part of issue #952 and PR #982 introduced a version of [playwright-go](https://github.com/playwright-community/playwright-go/releases/tag/v0.2000.0) that had breaking change to the underlying playwright driver that will no longer automatically acquire browser drivers.

Per the breaking change [notice](https://github.com/playwright-community/playwright-go/releases/tag/v0.2000.0) in playwright-go, this PR seeks to add the API Install method of acquiring the browser driver.

## Alternatives
Alternatively, instead of baking in the install, the README could be updated to reflect this requirement, and have the user on their own acquired the driver via either the playwright-go CLI or the upstream Playwright Typescript project.

This however does add  friction to use saml2aws, especially if one is using a package manager like Homebrew or Chocolatey, i.e. needing to on top of acquiring saml2aws via the documented means to also separately acquire Go Tools or NPM/NPX/Node to acquire playwright driver.

## Additional Comments
The author recognizes the implementation is somewhat naive in that it gives no configuration to avoid downloading the drivers which might desirable, e.g. avoid re-downloading drivers for CI/CD tests


Patch:
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index 1528e68..6622e3f 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -24,7 +24,6 @@ jobs:
 
       - name: Test
         run: |
-          go run github.com/playwright-community/playwright-go/cmd/playwright install
           go test -v ./... -coverprofile=${{ matrix.os }}_coverage.txt -covermode=atomic
 
       - name: Upload coverage report
diff --git a/README.md b/README.md
index de530ae..5a94e24 100644
--- a/README.md
+++ b/README.md
@@ -248,6 +248,7 @@ Commands:
                                  The file that will cache the credentials retrieved from AWS. When not specified, will use the default AWS credentials file location. (env: SAML2AWS_CREDENTIALS_FILE)
         --cache-saml             Caches the SAML response (env: SAML2AWS_CACHE_SAML)
         --cache-file=CACHE-FILE  The location of the SAML cache file (env: SAML2AWS_SAML_CACHE_FILE)
+        --download-browser-driver  Automatically download browsers for Browser IDP. (env: SAML2AWS_AUTO_BROWSER_DOWNLOAD)
         --disable-sessions         Do not use Okta sessions. Uses Okta sessions by default. (env: SAML2AWS_OKTA_DISABLE_SESSIONS)
         --disable-remember-device  Do not remember Okta MFA device. Remembers MFA device by default. (env: SAML2AWS_OKTA_DISABLE_REMEMBER_DEVICE)
 
@@ -544,6 +545,18 @@ region                  = us-east-1
 
 To use this you will need to export `AWS_DEFAULT_PROFILE=customer-test` environment variable to target `test`.
 
+### Playwright Browser Drivers for Browser IDP
+
+If you are using the Browser Identity Provider, on first invocation of `saml2aws login` you need to remember to install
+the browser drivers in order for playwright-go to work. Otherwise you will see the following error message:
+
+`Error authenticating to IDP.: could not start driver: fork/exec  ... no such file or directory`
+
+To install the drivers, you can:
+* Pass `--download-browser-driver` to `saml2aws login`
+* Set in your shell environment `SAML2AWS_AUTO_BROWSER_DOWNLOAD=true`
+* Set `download_browser_driver = true` in your saml2aws config file, i.e. `~/.saml2aws`
+
 ## Advanced Configuration (Multiple AWS account access but SAML authenticate against a single 'SSO' AWS account)
 
 Example:
diff --git a/cmd/saml2aws/commands/login.go b/cmd/saml2aws/commands/login.go
index 97bbb7b..0a7766a 100644
--- a/cmd/saml2aws/commands/login.go
+++ b/cmd/saml2aws/commands/login.go
@@ -224,6 +224,12 @@ func resolveLoginDetails(account *cfg.IDPAccount, loginFlags *flags.LoginExecFla
 		loginDetails.MFAIPAddress = loginFlags.CommonFlags.MFAIPAddress
 	}
 
+	if loginFlags.DownloadBrowser {
+		loginDetails.DownloadBrowser = loginFlags.DownloadBrowser
+	} else if account.DownloadBrowser {
+		loginDetails.DownloadBrowser = account.DownloadBrowser
+	}
+
 	// log.Printf("loginDetails %+v", loginDetails)
 
 	// if skip prompt was passed just pass back the flag values
diff --git a/cmd/saml2aws/main.go b/cmd/saml2aws/main.go
index e974e40..328400d 100644
--- a/cmd/saml2aws/main.go
+++ b/cmd/saml2aws/main.go
@@ -114,6 +114,7 @@ func main() {
 	cmdLogin.Flag("credentials-file", "The file that will cache the credentials retrieved from AWS. When not specified, will use the default AWS credentials file location. (env: SAML2AWS_CREDENTIALS_FILE)").Envar("SAML2AWS_CREDENTIALS_FILE").StringVar(&commonFlags.CredentialsFile)
 	cmdLogin.Flag("cache-saml", "Caches the SAML response (env: SAML2AWS_CACHE_SAML)").Envar("SAML2AWS_CACHE_SAML").BoolVar(&commonFlags.SAMLCache)
 	cmdLogin.Flag("cache-file", "The location of the SAML cache file (env: SAML2AWS_SAML_CACHE_FILE)").Envar("SAML2AWS_SAML_CACHE_FILE").StringVar(&commonFlags.SAMLCacheFile)
+	cmdLogin.Flag("download-browser-driver", "Automatically download browsers for Browser IDP. (env: SAML2AWS_AUTO_BROWSER_DOWNLOAD)").Envar("SAML2AWS_AUTO_BROWSER_DOWNLOAD").BoolVar(&loginFlags.DownloadBrowser)
 	cmdLogin.Flag("disable-sessions", "Do not use Okta sessions. Uses Okta sessions by default. (env: SAML2AWS_OKTA_DISABLE_SESSIONS)").Envar("SAML2AWS_OKTA_DISABLE_SESSIONS").BoolVar(&commonFlags.DisableSessions)
 	cmdLogin.Flag("disable-remember-device", "Do not remember Okta MFA device. Remembers MFA device by default. (env: SAML2AWS_OKTA_DISABLE_REMEMBER_DEVICE)").Envar("SAML2AWS_OKTA_DISABLE_REMEMBER_DEVICE").BoolVar(&commonFlags.DisableRememberDevice)
 
diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go
index aac86f2..d547edf 100644
--- a/pkg/cfg/cfg.go
+++ b/pkg/cfg/cfg.go
@@ -56,9 +56,11 @@ type IDPAccount struct {
 	SAMLCache             bool   `ini:"saml_cache"`
 	SAMLCacheFile         string `ini:"saml_cache_file"`
 	TargetURL             string `ini:"target_url"`
-	DisableRememberDevice bool   `ini:"disable_remember_device"` // used by Okta
-	DisableSessions       bool   `ini:"disable_sessions"`        // used by Okta
-	Headless              bool   `ini:"headless"`                // used by browser
+	DisableRememberDevice bool   `ini:"disable_remember_device"`      // used by Okta
+	DisableSessions       bool   `ini:"disable_sessions"`             // used by Okta
+	DownloadBrowser       bool   `ini:"download_browser_driver"`      // used by browser
+	BrowserDriverDir      string `ini:"browser_driver_dir,omitempty"` // used by browser; hide from user if not set
+	Headless              bool   `ini:"headless"`                     // used by browser
 	Prompter              string `ini:"prompter"`
 }
 
diff --git a/pkg/creds/creds.go b/pkg/creds/creds.go
index 0f846ff..5aa6971 100644
--- a/pkg/creds/creds.go
+++ b/pkg/creds/creds.go
@@ -4,6 +4,7 @@ package creds
 type LoginDetails struct {
 	ClientID          string // used by OneLogin
 	ClientSecret      string // used by OneLogin
+	DownloadBrowser   bool   // used by Browser
 	MFAIPAddress      string // used by OneLogin
 	Username          string
 	Password          string
diff --git a/pkg/flags/flags.go b/pkg/flags/flags.go
index 9c741cb..60d8640 100644
--- a/pkg/flags/flags.go
+++ b/pkg/flags/flags.go
@@ -39,6 +39,7 @@ type CommonFlags struct {
 // LoginExecFlags flags for the Login / Exec commands
 type LoginExecFlags struct {
 	CommonFlags       *CommonFlags
+	DownloadBrowser   bool
 	Force             bool
 	DuoMFAOption      string
 	ExecProfile       string
diff --git a/pkg/provider/browser/browser.go b/pkg/provider/browser/browser.go
index 83314c9..4bea173 100644
--- a/pkg/provider/browser/browser.go
+++ b/pkg/provider/browser/browser.go
@@ -17,16 +17,33 @@ var logger = logrus.WithField("provider", "browser")
 // Client client for browser based Identity Provider
 type Client struct {
 	Headless bool
+	// Setup alternative directory to download playwright browsers to
+	BrowserDriverDir string
 }
 
 // New create new browser based client
 func New(idpAccount *cfg.IDPAccount) (*Client, error) {
-	return &Client{Headless: idpAccount.Headless}, nil
+	return &Client{
+		Headless:         idpAccount.Headless,
+		BrowserDriverDir: idpAccount.BrowserDriverDir,
+	}, nil
 }
 
 func (cl *Client) Authenticate(loginDetails *creds.LoginDetails) (string, error) {
+	runOptions := playwright.RunOptions{}
+	if cl.BrowserDriverDir != "" {
+		runOptions.DriverDirectory = cl.BrowserDriverDir
+	}
+
+	// Optionally download browser drivers if specified
+	if loginDetails.DownloadBrowser {
+		err := playwright.Install(&runOptions)
+		if err != nil {
+			return "", err
+		}
+	}
 
-	pw, err := playwright.Run()
+	pw, err := playwright.Run(&runOptions)
 	if err != nil {
 		return "", err
 	}


Test patch:
diff --git a/pkg/provider/browser/browser_test.go b/pkg/provider/browser/browser_test.go
index edd2aed..c21cde7 100644
--- a/pkg/provider/browser/browser_test.go
+++ b/pkg/provider/browser/browser_test.go
@@ -39,13 +39,29 @@ func TestValidate(t *testing.T) {
 	client, err := New(account)
 	assert.Nil(t, err)
 	loginDetails := &creds.LoginDetails{
-		URL: "https://google.com/",
+		URL:             "https://google.com/",
+		DownloadBrowser: true,
 	}
 	resp, err := client.Authenticate(loginDetails)
 	assert.Nil(t, err)
 	assert.Equal(t, resp, response)
 }
 
+// Test that if download directory does not have browsers, it fails with expected error message
+func TestNoBrowserDriverFail(t *testing.T) {
+	account := &cfg.IDPAccount{
+		Headless:         true,
+		BrowserDriverDir: t.TempDir(), // set up a directory we know won't have drivers
+	}
+	loginDetails := &creds.LoginDetails{
+		URL: "https://google.com/",
+	}
+	client, _ := New(account)
+	_, err := client.Authenticate(loginDetails)
+	assert.Error(t, err)
+	assert.ErrorContains(t, err, "could not start driver")
+}
+
 func fakeSAMLResponse(page playwright.Page, loginDetails *creds.LoginDetails) (string, error) {
 	return response, nil
 }