Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 7 additions & 5 deletions server/grants/refresh.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,18 +180,20 @@ func (g *refresh) sessionID(ctx context.Context, refreshToken *storage.RefreshTo
return "", nil
}

ref, ok := offlineSessions.Refresh[refreshToken.ClientID]
if !ok || ref.SessionID == "" {
// Shared with introspection so the two cannot disagree about which session a
// token names (see tokens.RefreshReferenceSessionID).
sid := tokens.RefreshReferenceSessionID(offlineSessions, refreshToken.ClientID)
if sid == "" {
// Issued outside a browser flow — the password grant, or before sessions were
// turned on. There is no session to be bound to, so there is none to end.
return "", nil
}

if !bound {
return ref.SessionID, nil
return sid, nil
}

if !g.sessions.Alive(ctx, ref.SessionID) {
if !g.sessions.Alive(ctx, sid) {
// Through the store, not storage.DeleteRefresh: the token and the offline
// session's reference to it have to go together, or the admin API lists a
// token that no longer exists and fails trying to revoke it.
Expand All @@ -202,7 +204,7 @@ func (g *refresh) sessionID(ctx context.Context, refreshToken *storage.RefreshTo
"client_id", refreshToken.ClientID, "user_id", refreshToken.Claims.UserID)
return "", sessionEndedError()
}
return ref.SessionID, nil
return sid, nil
}

// sessionEndedError reports a refused refresh as invalid_grant, the one code RFC
Expand Down
56 changes: 51 additions & 5 deletions server/introspection/introspection.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,9 @@ type IntrospectionExtra struct {
// token at all when sessions are disabled.
//
// Note what this does not mean: introspection reports on the token, not on the
// session. A token from a session that has since ended still introspects as
// active until it expires, because nothing here consults session storage.
// session. For a standalone client a token from a session that has since ended
// still introspects as active until it expires. For a session-bound client the
// ended session takes the token with it, and introspection reports inactive.
SessionID string `json:"sid,omitempty"`

Email string `json:"email,omitempty"`
Expand Down Expand Up @@ -260,6 +261,44 @@ func (h *Handler) introspectRefreshToken(ctx context.Context, token string) (*In
return nil, newIntrospectInternalServerError()
}

client, err := h.Storage.GetClient(ctx, refresh.ClientID)
if err != nil {
// A deleted client cannot redeem the token; report inactive rather than 500.
// The endpoint takes no client authentication, so this is also the safer
// answer for an unauthenticated probe of a dangling token.
if errors.Is(err, storage.ErrNotFound) {
return nil, newIntrospectInactiveTokenError()
}
h.Logger.ErrorContext(ctx, "error while fetching client from storage", "err", err.Error())
return nil, newIntrospectInternalServerError()
}

// Standalone clients are not judged by their session. Skip the offline-session
// read: the endpoint is unauthenticated, and on Kubernetes that is one avoidable
// API call per request whose result would be discarded.
//
// When sessions are disabled the refresh grant skips its session check
// (sessionsEnabled). Gate here before GetOfflineSessions so a storage error
// cannot report inactive while the grant would still redeem the token.
if client.RefreshBoundToSession() && h.Sessions != nil && h.Sessions.Enabled() {
// A refresh token's sid lives on its offline-session reference, read the
// same way the refresh grant reads it (tokens.RefreshReferenceSessionID):
// the two must agree on whether the session the token is bound to still stands.
offlineSessions, err := h.Storage.GetOfflineSessions(ctx, refresh.Claims.UserID, refresh.ConnectorID)
if err != nil {
if !errors.Is(err, storage.ErrNotFound) {
h.Logger.ErrorContext(ctx, "failed to read offline session for sid", "err", err)
}
// The grant refuses a bound token whose session cannot be read;
// introspection reports it inactive.
return nil, newIntrospectInactiveTokenError()
}
sessionID := tokens.RefreshReferenceSessionID(offlineSessions, refresh.ClientID)
if !h.sessionAlive(ctx, client, sessionID) {
return nil, newIntrospectInactiveTokenError()
}
}

return &Introspection{
Active: true,
ClientID: refresh.ClientID,
Expand Down Expand Up @@ -293,11 +332,15 @@ func (h *Handler) introspectRefreshToken(ctx context.Context, token string) (*In
// RefreshTokenLifetime, the same declaration the refresh grant reads — the two must
// agree, or a standalone client refreshes into a token that is inactive from birth.
//
// When sessions are disabled the grant skips its check entirely (sessionsEnabled),
// so introspection must too: Manager.Alive returns false when Config is nil, which
// would otherwise flip every bound token inactive the moment sessions are turned off.
//
// The comparison is against the sid, not merely the existence of a session: signing
// out and back in makes a new session under the same subject, and the old token must
// not be revived by it.
func (h *Handler) sessionAlive(ctx context.Context, client storage.Client, subject, sessionID string) bool {
if sessionID == "" || !client.RefreshBoundToSession() {
func (h *Handler) sessionAlive(ctx context.Context, client storage.Client, sessionID string) bool {
if sessionID == "" || !client.RefreshBoundToSession() || h.Sessions == nil || !h.Sessions.Enabled() {
return true
}

Expand Down Expand Up @@ -325,11 +368,14 @@ func (h *Handler) introspectAccessToken(ctx context.Context, token string) (*Int

client, err := h.Storage.GetClient(ctx, clientID)
if err != nil {
if errors.Is(err, storage.ErrNotFound) {
return nil, newIntrospectInactiveTokenError()
}
h.Logger.ErrorContext(ctx, "error while fetching client from storage", "err", err.Error())
return nil, newIntrospectInternalServerError()
}

if !h.sessionAlive(ctx, client, idToken.Subject, claims.SessionID) {
if !h.sessionAlive(ctx, client, claims.SessionID) {
return nil, newIntrospectInactiveTokenError()
}

Expand Down
80 changes: 80 additions & 0 deletions server/server_introspection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,83 @@ func TestHandleIntrospect(t *testing.T) {
})
}
}

// A session-bound client's refresh token must introspect as inactive once the
// session it was issued under has ended, the same judgment the refresh grant
// makes. A standalone client's token outlives the session by design.
func TestHandleIntrospectRefreshTokenSessionBinding(t *testing.T) {
ctx := t.Context()

httpServer, s := newTestServerWithSessions(t, nil)
defer httpServer.Close()

mockTestStorage(t, s.storage)

// The refresh token was issued under a browser session.
require.NoError(t, s.storage.UpdateOfflineSessions(ctx, "1", "test",
func(old storage.OfflineSessions) (storage.OfflineSessions, error) {
old.Refresh["test"].SessionID = "sid"
return old, nil
}))

// That session has since ended by timeout: the row is still stored, but both
// expiries are in the past.
past := time.Now().Add(-time.Hour)
require.NoError(t, s.storage.CreateAuthSession(ctx, storage.AuthSession{
ID: "sid", Secret: testSessionSecret("sid"),
UserID: "1", ConnectorID: "test",
CreatedAt: past, LastActivity: past,
AbsoluteExpiry: past, IdleExpiry: past,
}))

refreshToken, err := internal.Marshal(&internal.RefreshToken{RefreshId: "test", Token: "bar"})
require.NoError(t, err)

introspect := func() string {
data := url.Values{}
data.Set("token", refreshToken)

u, err := url.Parse(s.issuerURL.String())
require.NoError(t, err)
u.Path = path.Join(u.Path, "token", "introspect")

req, _ := http.NewRequest("POST", u.String(), bytes.NewBufferString(data.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

rr := httptest.NewRecorder()
s.ServeHTTP(rr, req)
require.Equal(t, http.StatusOK, rr.Code)

result, err := io.ReadAll(rr.Body)
require.NoError(t, err)
return string(result)
}

// Standalone client: the ended session changes nothing.
require.Contains(t, introspect(), `"active":true`)

require.NoError(t, s.storage.UpdateClient(ctx, "test",
func(old storage.Client) (storage.Client, error) {
old.RefreshTokenLifetime = storage.RefreshTokenLifetimeSession
return old, nil
}))

// Session-bound client: the token ended with the session.
require.Equal(t, "{\"active\":false}\n", introspect())

// Bound client whose offline-session row is gone: grant refuses; introspection
// must report inactive (not 500).
require.NoError(t, s.storage.DeleteOfflineSessions(ctx, "1", "test"))
require.Equal(t, "{\"active\":false}\n", introspect())

// Bound client with a reference that carries no sid (minted outside a browser
// flow): there is no session to judge, so the token stays active.
require.NoError(t, s.storage.CreateOfflineSessions(ctx, storage.OfflineSessions{
UserID: "1",
ConnID: "test",
Refresh: map[string]*storage.RefreshTokenRef{
"test": {ID: "test", ClientID: "test"},
},
}))
require.Contains(t, introspect(), `"active":true`)
}
16 changes: 16 additions & 0 deletions server/tokens/refresh_session.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package tokens

import "github.com/dexidp/dex/storage"

// RefreshReferenceSessionID returns the browser session ID recorded on an
// offline session's refresh reference for clientID. The empty string means
// there is no reference, or the reference carries no sid (token minted outside
// a browser flow). The refresh grant and token introspection both read the sid
// through this helper so they cannot disagree about which session a token names.
func RefreshReferenceSessionID(offline storage.OfflineSessions, clientID string) string {
ref, ok := offline.Refresh[clientID]
if !ok || ref == nil {
return ""
}
return ref.SessionID
}
24 changes: 24 additions & 0 deletions server/tokens/refresh_session_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package tokens

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/dexidp/dex/storage"
)

func TestRefreshReferenceSessionID(t *testing.T) {
offline := storage.OfflineSessions{
Refresh: map[string]*storage.RefreshTokenRef{
"bound": {ID: "r1", ClientID: "bound", SessionID: "sid"},
"empty": {ID: "r2", ClientID: "empty", SessionID: ""},
"nil-ref": nil,
},
}

require.Equal(t, "sid", RefreshReferenceSessionID(offline, "bound"))
require.Equal(t, "", RefreshReferenceSessionID(offline, "empty"))
require.Equal(t, "", RefreshReferenceSessionID(offline, "nil-ref"))
require.Equal(t, "", RefreshReferenceSessionID(offline, "missing"))
}
Loading