Skip to content

feat(go-sdk): Automatically refresh expired authentication tokens #1608

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(go-sdk): Automatically refresh expired authentication tokens #1608

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 31 additions & 2 deletions go/rtl/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,29 @@ func (t *transportWithHeaders) RoundTrip(req *http.Request) (*http.Response, err
type AuthSession struct {
Config ApiSettings
Client http.Client
token *oauth2.Token
source oauth2.TokenSource
}

func (s *AuthSession) IsActive() bool {
if s.token == nil {
return false
}
leeway := time.Second * 10
return s.token.Expiry.After(time.Now().Add(leeway))
}

func (s *AuthSession) Login() (*oauth2.Token, error) {
if s.IsActive() {
return s.token, nil
}

token, err := s.source.Token()
if err != nil {
return nil, err
}
s.token = token
return token, nil
}
Comment on lines +46 to 65
Copy link

Choose a reason for hiding this comment

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

high

The AuthSession is likely to be used concurrently, but the new methods IsActive and Login introduce data races when accessing the s.token field. IsActive reads s.token while Login writes to it, without any synchronization.

To make AuthSession thread-safe, you should add a sync.RWMutex to the struct:

import "sync"

type AuthSession struct {
	// ... other fields
	token  *oauth2.Token
	source oauth2.TokenSource
	mu     sync.RWMutex
}

Then, you can update IsActive and Login to use this mutex. The implementation below uses a read-lock for checking the token and a write-lock for refreshing it, which is efficient. It also uses the double-checked locking pattern to prevent unnecessary locking when the token is valid.

I've also replaced the magic number 10 * time.Second with a constant for better readability.

const tokenLeeway = 10 * time.Second

func (s *AuthSession) IsActive() bool {
	s.mu.RLock()
	defer s.mu.RUnlock()
	if s.token == nil {
		return false
	}
	return s.token.Expiry.After(time.Now().Add(tokenLeeway))
}

func (s *AuthSession) Login() (*oauth2.Token, error) {
	// First, check with a read lock to avoid contention if the token is valid.
	s.mu.RLock()
	if s.token != nil && s.token.Expiry.After(time.Now().Add(tokenLeeway)) {
		token := s.token
		s.mu.RUnlock()
		return token, nil
	}
	s.mu.RUnlock()

	// If the token is invalid or nil, acquire a write lock to refresh it.
	s.mu.Lock()
	defer s.mu.Unlock()

	// Re-check after obtaining the write lock, in case another goroutine refreshed it.
	if s.token != nil && s.token.Expiry.After(time.Now().Add(tokenLeeway)) {
		return s.token, nil
	}

	token, err := s.source.Token()
	if err != nil {
		return nil, err
	}
	s.token = token
	return token, nil
}


func NewAuthSession(config ApiSettings) *AuthSession {
Expand Down Expand Up @@ -74,21 +97,27 @@ func NewAuthSessionWithTransport(config ApiSettings, transport http.RoundTripper
&http.Client{Transport: appIdHeaderTransport},
)

source := oauthConfig.TokenSource(ctx)

// Make use of oauth2 transport to handle token management
oauthTransport := &oauth2.Transport{
Source: oauthConfig.TokenSource(ctx),
Source: source,
// Will set "x-looker-appid" Header on all other requests
Base: appIdHeaderTransport,
}

return &AuthSession{
Config: config,
Client: http.Client{Transport: oauthTransport},
source: source,
}
}

func (s *AuthSession) Do(result interface{}, method, ver, path string, reqPars map[string]interface{}, body interface{}, options *ApiSettings) error {

_, err := s.Login()
if err != nil {
return err
}
// prepare URL
u := fmt.Sprintf("%s/api%s%s", s.Config.BaseUrl, ver, path)

Expand Down
Loading