Skip to content

Commit

Permalink
Merge pull request #241 from rusq/okta
Browse files Browse the repository at this point in the history
Playwright error handling and increased login timeout (5min)
  • Loading branch information
rusq committed Oct 18, 2023
2 parents 7089bd8 + 68c2035 commit cf51095
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 27 deletions.
15 changes: 9 additions & 6 deletions auth/browser.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/url"
"os"
"strings"
"time"

"github.com/rusq/slackdump/v2/auth/auth_ui"
"github.com/rusq/slackdump/v2/auth/browser"
Expand All @@ -20,9 +21,10 @@ type BrowserAuth struct {
}

type browserOpts struct {
workspace string
browser browser.Browser
flow BrowserAuthUI
workspace string
browser browser.Browser
flow BrowserAuthUI
loginTimeout time.Duration
}

type BrowserAuthUI interface {
Expand All @@ -33,8 +35,9 @@ type BrowserAuthUI interface {
func NewBrowserAuth(ctx context.Context, opts ...Option) (BrowserAuth, error) {
var br = BrowserAuth{
opts: browserOpts{
flow: defaultFlow,
browser: browser.Bfirefox,
flow: defaultFlow,
browser: browser.Bfirefox,
loginTimeout: browser.DefLoginTimeout,
},
}
for _, opt := range opts {
Expand All @@ -55,7 +58,7 @@ func NewBrowserAuth(ctx context.Context, opts ...Option) (BrowserAuth, error) {
br.opts.workspace = wsp
}

auther, err := browser.New(br.opts.workspace, browser.OptBrowser(br.opts.browser))
auther, err := browser.New(br.opts.workspace, browser.OptBrowser(br.opts.browser), browser.OptTimeout(br.opts.loginTimeout))
if err != nil {
return br, err
}
Expand Down
13 changes: 13 additions & 0 deletions auth/browser/browser.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@ package browser
import (
"fmt"
"strings"
"time"

"github.com/playwright-community/playwright-go"
)

// DefLoginTimeout is the default Slack login timeout
const DefLoginTimeout = 5 * time.Minute

//go:generate stringer -type Browser -trimprefix=B browser.go
type Browser int

Expand All @@ -26,6 +30,15 @@ func OptBrowser(b Browser) Option {
}
}

func OptTimeout(d time.Duration) Option {
return func(c *Client) {
if d < 0 {
return
}
c.loginTimeout = float64(d.Milliseconds())
}
}

func (e *Browser) Set(v string) error {
v = strings.ToLower(v)
for i := 0; i < len(_Browser_index)-1; i++ {
Expand Down
43 changes: 30 additions & 13 deletions auth/browser/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ const slackDomain = ".slack.com"

// Client is the client for Browser Auth Provider.
type Client struct {
workspace string
pageClosed chan bool // will receive a notification that the page is closed prematurely.
br Browser
workspace string
pageClosed chan bool // will receive a notification that the page is closed prematurely.
br Browser
loginTimeout float64 // slack login page timeout in milliseconds.
}

var Logger logger.Interface = logger.Default
Expand All @@ -34,7 +35,12 @@ func New(workspace string, opts ...Option) (*Client, error) {
if workspace == "" {
return nil, errors.New("workspace can't be empty")
}
cl := &Client{workspace: workspace, pageClosed: make(chan bool, 1), br: Bfirefox}
cl := &Client{
workspace: workspace,
pageClosed: make(chan bool, 1),
br: Bfirefox,
loginTimeout: float64(DefLoginTimeout.Milliseconds()),
}
for _, opt := range opts {
opt(cl)
}
Expand Down Expand Up @@ -100,6 +106,7 @@ func (cl *Client) Authenticate(ctx context.Context) (string, []*http.Cookie, err
if err != nil {
return "", nil, err
}
// page close sentinel.
page.On("close", func() { trace.Log(ctx, "user", "page closed"); close(cl.pageClosed) })

uri := fmt.Sprintf("https://%s"+slackDomain, cl.workspace)
Expand All @@ -110,8 +117,11 @@ func (cl *Client) Authenticate(ctx context.Context) (string, []*http.Cookie, err
}

var r playwright.Request
if err := cl.withBrowserGuard(ctx, func() {
r, err = page.ExpectRequest(uri+"/api/api.features*", func() error { return nil })
if err := cl.withBrowserGuard(ctx, func() error {
r, err = page.ExpectRequest(uri+"/api/api.features*", func() error { return nil }, playwright.PageExpectRequestOptions{
Timeout: &cl.loginTimeout,
})
return err
}); err != nil {
return "", nil, err
}
Expand All @@ -132,20 +142,27 @@ func (cl *Client) Authenticate(ctx context.Context) (string, []*http.Cookie, err
return token, convertCookies(state.Cookies), nil
}

func (cl *Client) withBrowserGuard(ctx context.Context, fn func()) error {
done := make(chan struct{})
var errPageClosed = errors.New("browser closed or timed out")

// withBrowserGuard starts the function fn in a goroutine, and waits for it to
// finish. If the context is canceled, or the page is closed, it returns
// the appropriate error.
func (cl *Client) withBrowserGuard(ctx context.Context, fn func() error) error {
var (
errC = make(chan error)
)
go func() {
defer close(done)
fn()
defer close(errC)
errC <- fn()
}()
select {
case <-ctx.Done():
return ctx.Err()
case <-cl.pageClosed:
return errors.New("browser closed or timed out")
case <-done:
return errPageClosed
case err := <-errC:
return err
}
return nil
}

func convertCookies(pwc []playwright.Cookie) []*http.Cookie {
Expand Down
3 changes: 3 additions & 0 deletions auth/browser/extractors.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ var (

// extractToken extracts token from the request.
func extractToken(r playwright.Request) (string, error) {
if r == nil {
return "", errors.New("no request")
}
if r.Method() == http.MethodGet {
return extractTokenGet(r.URL())
} else if r.Method() == http.MethodPost {
Expand Down
15 changes: 14 additions & 1 deletion auth/option.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package auth

import "github.com/rusq/slackdump/v2/auth/browser"
import (
"time"

"github.com/rusq/slackdump/v2/auth/browser"
)

type options struct {
*browserOpts
Expand Down Expand Up @@ -28,3 +32,12 @@ func BrowserWithBrowser(b browser.Browser) Option {
o.browserOpts.browser = b
}
}

func BrowserWithTimeout(d time.Duration) Option {
return func(o *options) {
if d < 0 {
return
}
o.browserOpts.loginTimeout = d
}
}
11 changes: 7 additions & 4 deletions cmd/slackdump/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"path/filepath"
"runtime/trace"
"syscall"
"time"

"github.com/joho/godotenv"
"github.com/rusq/dlog"
Expand Down Expand Up @@ -52,10 +53,11 @@ var secrets = []string{".env", ".env.txt", "secrets.txt"}

// params is the command line parameters
type params struct {
appCfg config.Params
creds app.SlackCreds
authReset bool
browser browser.Browser
appCfg config.Params
creds app.SlackCreds
authReset bool
browser browser.Browser
browserTimeout time.Duration

traceFile string // trace file
logFile string //log file, if not specified, outputs to stderr.
Expand Down Expand Up @@ -253,6 +255,7 @@ func parseCmdLine(args []string) (params, error) {
fs.StringVar(&p.creds.Cookie, "cookie", osenv.Secret(envSlackCookie, ""), "d= cookie `value` or a path to a cookie.txt file (environment: "+envSlackCookie+")")
fs.BoolVar(&p.authReset, "auth-reset", false, "reset EZ-Login 3000 authentication.")
fs.Var(&p.browser, "browser", "set the browser to use for authentication: 'chromium' or 'firefox' (default: firefox)")
fs.DurationVar(&p.browserTimeout, "browser-timeout", browser.DefLoginTimeout, "browser login timeout")
fs.StringVar(&p.workspace, "w", "", "set the Slack `workspace` name. If not specifed, the slackdump will show an\ninteractive prompt.")

// operation mode
Expand Down
10 changes: 7 additions & 3 deletions cmd/slackdump/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/assert"

"github.com/rusq/slackdump/v2"
"github.com/rusq/slackdump/v2/auth/browser"
"github.com/rusq/slackdump/v2/internal/app"
"github.com/rusq/slackdump/v2/internal/app/config"
"github.com/rusq/slackdump/v2/internal/structures"
Expand Down Expand Up @@ -63,15 +64,17 @@ func Test_checkParameters(t *testing.T) {
Token: "x",
Cookie: "d",
},
browserTimeout: browser.DefLoginTimeout,
appCfg: config.Params{
ListFlags: config.ListFlags{
Users: false,
Channels: true,
},
FilenameTemplate: defFilenameTemplate,
Input: config.Input{List: &structures.EntityList{}},
Output: config.Output{Filename: "-", Format: "text"},
Options: slackdump.DefOptions,

Input: config.Input{List: &structures.EntityList{}},
Output: config.Output{Filename: "-", Format: "text"},
Options: slackdump.DefOptions,
}},
false,
},
Expand All @@ -83,6 +86,7 @@ func Test_checkParameters(t *testing.T) {
Token: "x",
Cookie: "d",
},
browserTimeout: browser.DefLoginTimeout,
appCfg: config.Params{
ListFlags: config.ListFlags{
Channels: false,
Expand Down

0 comments on commit cf51095

Please sign in to comment.