Skip to content

Commit

Permalink
fix: Do not use connector data from the refresh token field (#2729)
Browse files Browse the repository at this point in the history
Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>
  • Loading branch information
nabokihms authored Dec 19, 2022
1 parent 3bfe0d9 commit 6d9ca8d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
8 changes: 5 additions & 3 deletions server/refreshhandlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ func (s *Server) refreshWithConnector(ctx context.Context, rCtx *refreshContext,
// TODO(ericchiang): We may want a strict mode where connectors that don't implement
// this interface can't perform refreshing.
if refreshConn, ok := rCtx.connector.Connector.(connector.RefreshConnector); ok {
// Set connector data to the one received from an offline session
ident.ConnectorData = rCtx.connectorData
s.logger.Debugf("connector data before refresh: %s", ident.ConnectorData)

newIdent, err := refreshConn.Refresh(ctx, parseScopes(rCtx.scopes), ident)
Expand Down Expand Up @@ -245,7 +247,6 @@ func (s *Server) updateRefreshToken(ctx context.Context, rCtx *refreshContext) (
Email: rCtx.storageToken.Claims.Email,
EmailVerified: rCtx.storageToken.Claims.EmailVerified,
Groups: rCtx.storageToken.Claims.Groups,
ConnectorData: rCtx.connectorData,
}

refreshTokenUpdater := func(old storage.RefreshToken) (storage.RefreshToken, error) {
Expand All @@ -255,6 +256,7 @@ func (s *Server) updateRefreshToken(ctx context.Context, rCtx *refreshContext) (
switch {
case !rotationEnabled && reusingAllowed:
// If rotation is disabled and the offline session was updated not so long ago - skip further actions.
old.ConnectorData = nil
return old, nil

case rotationEnabled && reusingAllowed:
Expand All @@ -269,7 +271,7 @@ func (s *Server) updateRefreshToken(ctx context.Context, rCtx *refreshContext) (

// Do not update last used time for offline session if token is allowed to be reused
lastUsed = old.LastUsed
ident.ConnectorData = nil
old.ConnectorData = nil
return old, nil

case rotationEnabled && !reusingAllowed:
Expand All @@ -286,7 +288,7 @@ func (s *Server) updateRefreshToken(ctx context.Context, rCtx *refreshContext) (
old.LastUsed = lastUsed

// ConnectorData has been moved to OfflineSession
old.ConnectorData = []byte{}
old.ConnectorData = nil

// Call only once if there is a request which is not in the reuse interval.
// This is required to avoid multiple calls to the external IdP for concurrent requests.
Expand Down
11 changes: 11 additions & 0 deletions server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -871,6 +871,17 @@ func TestOAuth2CodeFlow(t *testing.T) {
if respDump, err = httputil.DumpResponse(resp, true); err != nil {
t.Fatal(err)
}

tokens, err := s.storage.ListRefreshTokens()
if err != nil {
t.Fatalf("failed to get existed refresh token: %v", err)
}

for _, token := range tokens {
if /* token was updated */ token.ObsoleteToken != "" && token.ConnectorData != nil {
t.Fatalf("token connectorDatawith id %q field is not nil: %s", token.ID, token.ConnectorData)
}
}
})
}
}
Expand Down

0 comments on commit 6d9ca8d

Please sign in to comment.