public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Auth: Update oauthtoken service to use remote cache and server lock (#90572)

Browse Source 
* update oauthtoken service to use remote cache and server lock

* remove token cache

* retry is lock is held by an in-flight refresh

* refactor token renewal to avoid race condition

* re-add refresh token expiry cache, but in SyncOauthTokenHook

* Add delta to the cache ttl

* Fix merge

* Change lockTimeConfig

* Always set the token from within the server lock

* Improvements

* early return when user is not authed by OAuth or refresh is disabled

* Allow more time for token refresh, tracing

* Retry on Mysql Deadlock error 1213

* Update pkg/services/authn/authnimpl/sync/oauth_token_sync.go

Co-authored-by: Dan Cech <dcech@grafana.com>

* Update pkg/services/authn/authnimpl/sync/oauth_token_sync.go

Co-authored-by: Dan Cech <dcech@grafana.com>

* Add settings for configuring min wait time between retries

* Add docs for the new setting

* Clean up

* Update docs/sources/setup-grafana/configure-grafana/_index.md

Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com>

---------

Co-authored-by: Mihaly Gyongyosi <mgyongyosi@users.noreply.github.com>
Co-authored-by: Christopher Moyer <35463610+chri2547@users.noreply.github.com>
This commit is contained in:
Dan Cech
2024-08-19 18:57:37 +02:00
committed by GitHub
co-authored by Christopher Moyer Mihaly Gyongyosi
parent 5ce9324801
commit 9020eb4b17
11 changed files with 300 additions and 307 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/authn/authnimpl/sync/oauth_token_sync.go
+52 -21
View File
@@ -3,12 +3,15 @@ package sync
import (
"context"
"errors"
"fmt"
"strings"
"time"
"github.com/grafana/authlib/claims"
"golang.org/x/oauth2"
"golang.org/x/sync/singleflight"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/login/social"
@@ -17,6 +20,8 @@ import (
"github.com/grafana/grafana/pkg/services/oauthtoken"
)
const maxOAuthTokenCacheTTL = 5 * time.Minute
func ProvideOAuthTokenSync(service oauthtoken.OAuthTokenService, sessionService auth.UserTokenService, socialService social.Service, tracer tracing.Tracer) *OAuthTokenSync {
return &OAuthTokenSync{
log.New("oauth_token.sync"),
@@ -25,6 +30,7 @@ func ProvideOAuthTokenSync(service oauthtoken.OAuthTokenService, sessionService
socialService,
new(singleflight.Group),
tracer,
localcache.New(maxOAuthTokenCacheTTL, 15*time.Minute),
}
}
@@ -35,6 +41,7 @@ type OAuthTokenSync struct {
socialService social.Service
singleflightGroup *singleflight.Group
tracer tracing.Tracer
cache *localcache.CacheService
}
func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, id *authn.Identity, _ *authn.Request) error {
@@ -56,44 +63,44 @@ func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, id *authn.Ident
return nil
}
ctxLogger := s.log.FromContext(ctx).New("userID", id.GetID())
userID, err := id.GetInternalID()
if err != nil {
s.log.FromContext(ctx).Error("Failed to refresh token. Invalid ID for identity", "type", id.GetIdentityType(), "err", err)
return nil
}
_, err, _ := s.singleflightGroup.Do(id.GetID(), func() (interface{}, error) {
ctxLogger := s.log.FromContext(ctx).New("userID", userID)
cacheKey := fmt.Sprintf("token-check-%s", id.GetID())
if _, ok := s.cache.Get(cacheKey); ok {
ctxLogger.Debug("Expiration check has been cached, no need to refresh")
return nil
}
_, err, _ = s.singleflightGroup.Do(cacheKey, func() (interface{}, error) {
ctxLogger.Debug("Singleflight request for OAuth token sync")
// FIXME: Consider using context.WithoutCancel instead of context.Background after Go 1.21 update
updateCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
updateCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second)
defer cancel()
if refreshErr := s.service.TryTokenRefresh(updateCtx, id); refreshErr != nil {
token, refreshErr := s.service.TryTokenRefresh(updateCtx, id)
if refreshErr != nil {
if errors.Is(refreshErr, context.Canceled) {
return nil, nil
}
token, _, err := s.service.HasOAuthEntry(ctx, id)
if err != nil {
ctxLogger.Error("Failed to get OAuth entry for verifying if token has already been refreshed", "id", id.ID, "error", err)
return nil, err
}
// if the access token has already been refreshed by another request (for example in HA scenario)
tokenExpires := token.OAuthExpiry.Round(0).Add(-oauthtoken.ExpiryDelta)
if !tokenExpires.Before(time.Now()) {
return nil, nil
}
ctxLogger.Error("Failed to refresh OAuth access token", "id", id.ID, "error", refreshErr)
if err := s.service.InvalidateOAuthTokens(ctx, token); err != nil {
ctxLogger.Warn("Failed to invalidate OAuth tokens", "id", id.ID, "error", err)
}
// log the user out
if err := s.sessionService.RevokeToken(ctx, id.SessionToken, false); err != nil {
ctxLogger.Warn("Failed to revoke session token", "id", id.ID, "tokenId", id.SessionToken.Id, "error", err)
}
s.cache.Delete(cacheKey)
return nil, refreshErr
}
s.cache.Set(cacheKey, true, getOAuthTokenCacheTTL(token))
return nil, nil
})
@@ -103,3 +110,27 @@ func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, id *authn.Ident
return nil
}
func getOAuthTokenCacheTTL(token *oauth2.Token) time.Duration {
ttl := maxOAuthTokenCacheTTL
if token == nil {
return ttl
}
if !token.Expiry.IsZero() {
d := time.Until(token.Expiry.Add(-oauthtoken.ExpiryDelta))
if d < ttl {
ttl = d
}
}
idTokenExpiry, err := oauthtoken.GetIDTokenExpiry(token)
if err == nil && !idTokenExpiry.IsZero() {
d := time.Until(idTokenExpiry.Add(-oauthtoken.ExpiryDelta))
if d < ttl {
ttl = d
}
}
return ttl
}
pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go
+25 -43
View File
@@ -8,9 +8,11 @@ import (
"github.com/grafana/authlib/claims"
"github.com/stretchr/testify/assert"
"golang.org/x/oauth2"
"golang.org/x/sync/singleflight"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/infra/localcache"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/login/social"
@@ -28,14 +30,12 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
identity *authn.Identity
oauthInfo *social.OAuthInfo
expectedHasEntryToken *login.UserAuth
expectHasEntryCalled bool
expectToken *login.UserAuth
expectedTryRefreshErr error
expectTryRefreshTokenCalled bool
expectRevokeTokenCalled bool
expectInvalidateOauthTokensCalled bool
expectRevokeTokenCalled bool
expectedErr error
}
@@ -52,33 +52,26 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
expectTryRefreshTokenCalled: false,
},
{
desc: "should invalidate access token and session token if token refresh fails",
identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectHasEntryCalled: true,
expectedTryRefreshErr: errors.New("some err"),
expectTryRefreshTokenCalled: true,
expectInvalidateOauthTokensCalled: true,
expectRevokeTokenCalled: true,
expectedHasEntryToken: &login.UserAuth{OAuthExpiry: time.Now().Add(-10 * time.Minute)},
expectedErr: authn.ErrExpiredAccessToken,
desc: "should invalidate access token and session token if token refresh fails",
identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectedTryRefreshErr: errors.New("some err"),
expectTryRefreshTokenCalled: true,
expectRevokeTokenCalled: true,
expectToken: &login.UserAuth{OAuthExpiry: time.Now().Add(-10 * time.Minute)},
expectedErr: authn.ErrExpiredAccessToken,
},
{
desc: "should refresh the token successfully",
identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectHasEntryCalled: false,
expectTryRefreshTokenCalled: true,
expectInvalidateOauthTokensCalled: false,
expectRevokeTokenCalled: false,
desc: "should refresh the token successfully",
identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectTryRefreshTokenCalled: true,
expectRevokeTokenCalled: false,
},
{
desc: "should not invalidate the token if the token has already been refreshed by another request (singleflight)",
identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectHasEntryCalled: true,
expectTryRefreshTokenCalled: true,
expectInvalidateOauthTokensCalled: false,
expectRevokeTokenCalled: false,
expectedHasEntryToken: &login.UserAuth{OAuthExpiry: time.Now().Add(10 * time.Minute)},
expectedTryRefreshErr: errors.New("some err"),
desc: "should not invalidate the token if the token has already been refreshed by another request (singleflight)",
identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectTryRefreshTokenCalled: true,
expectRevokeTokenCalled: false,
expectToken: &login.UserAuth{OAuthExpiry: time.Now().Add(10 * time.Minute)},
},
// TODO: address coverage of oauthtoken sync
@@ -87,24 +80,14 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
var (
hasEntryCalled bool
tryRefreshCalled bool
invalidateTokensCalled bool
revokeTokenCalled bool
tryRefreshCalled bool
revokeTokenCalled bool
)
service := &oauthtokentest.MockOauthTokenService{
HasOAuthEntryFunc: func(ctx context.Context, usr identity.Requester) (*login.UserAuth, bool, error) {
hasEntryCalled = true
return tt.expectedHasEntryToken, tt.expectedHasEntryToken != nil, nil
},
InvalidateOAuthTokensFunc: func(ctx context.Context, usr *login.UserAuth) error {
invalidateTokensCalled = true
return nil
},
TryTokenRefreshFunc: func(ctx context.Context, usr identity.Requester) error {
TryTokenRefreshFunc: func(ctx context.Context, usr identity.Requester) (*oauth2.Token, error) {
tryRefreshCalled = true
return tt.expectedTryRefreshErr
return nil, tt.expectedTryRefreshErr
},
}
@@ -132,13 +115,12 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
socialService: socialService,
singleflightGroup: new(singleflight.Group),
tracer: tracing.InitializeTracerForTest(),
cache: localcache.New(maxOAuthTokenCacheTTL, 15*time.Minute),
}
err := sync.SyncOauthTokenHook(context.Background(), tt.identity, nil)
assert.ErrorIs(t, err, tt.expectedErr)
assert.Equal(t, tt.expectHasEntryCalled, hasEntryCalled)
assert.Equal(t, tt.expectTryRefreshTokenCalled, tryRefreshCalled)
assert.Equal(t, tt.expectInvalidateOauthTokensCalled, invalidateTokensCalled)
assert.Equal(t, tt.expectRevokeTokenCalled, revokeTokenCalled)
})
}