refactor(server): resolve every grant's connector in the pipeline

Address review: the refresh grant re-implemented the connector-authorization
invariant (ConnectorAllowed + GrantTypeAllowed) inline and held its own connector
cache, duplicating the endpoint's resolve step.

ConnectorID now takes the context and client and may look the id up in storage, so
grants whose connector is recorded on a stored token — refresh reads it off the
refresh token — still source it through the same hook. The endpoint resolves the
connector and enforces the invariant once, in one place, and hands it to Authorize.
The refresh grant drops its connector cache and inline checks; it validates the
token in ConnectorID, stashes it on the request, and reuses it in Authorize.

Also replace the hand-rolled contains helper with slices.Contains.

Signed-off-by: maksim.nabokikh <max.nabokih@gmail.com>

Author: nabokihms

server/grants/authcode.go

@@ -35,10 +35,10 @@ func (g *authorizationCode) ScopePolicy() ScopePolicy {
 }
 
 // ConnectorID is empty: the connector is recorded on the stored auth code and
-// was already authorized at /auth, so it is resolved inside Authorize rather than
-// through the endpoint's connector step.
-func (g *authorizationCode) ConnectorID(req *Request) string {
-	return ""
+// was already authorized at /auth. The grant resolves it (without re-running the
+// invariant) only to decide on a refresh token, inside ExchangeAuthCode.
+func (g *authorizationCode) ConnectorID(ctx context.Context, req *Request, client storage.Client) (string, *oauth2.Error) {
+	return "", nil
 }
 
 // handle an access token request https://tools.ietf.org/html/rfc6749#section-4.1.3

server/grants/clientcredentials.go

@@ -39,8 +39,8 @@ func (g *clientCredentials) ScopePolicy() ScopePolicy {
 }
 
 // ConnectorID is empty: client_credentials involves no connector.
-func (g *clientCredentials) ConnectorID(req *Request) string {
-	return ""
+func (g *clientCredentials) ConnectorID(ctx context.Context, req *Request, client storage.Client) (string, *oauth2.Error) {
+	return "", nil
 }
 
 func (g *clientCredentials) Authorize(ctx context.Context, req *Request, client storage.Client, conn connectors.Connector) (*Result, error) {

server/grants/grants.go

@@ -42,6 +42,10 @@ type Request struct {
 	SubjectToken       string
 	SubjectTokenType   string
 	RequestedTokenType string
+
+	// resolvedRefresh is the validated refresh token, looked up while resolving
+	// the connector and reused by the refresh grant's Authorize.
+	resolvedRefresh *storage.RefreshToken
 }
 
 // parseRequest reads the whole token request form once. Client credentials come
@@ -96,10 +100,11 @@ type Grant interface {
 	// this grant.
 	ScopePolicy() ScopePolicy
 	// ConnectorID is the connector this grant authenticates against, or "" when
-	// it uses none (client_credentials). The endpoint resolves it and enforces
-	// the connector-authorization invariant before Authorize, so a grant cannot
-	// forget the check.
-	ConnectorID(req *Request) string
+	// it uses none (client_credentials). The grant may read it from the request
+	// or look it up in storage. The endpoint then resolves it and enforces the
+	// connector-authorization invariant before Authorize, so a grant cannot
+	// forget the check. Returning an error rejects the request.
+	ConnectorID(ctx context.Context, req *Request, client storage.Client) (string, *oauth2.Error)
 	// Authorize turns the validated request into the authorization to issue
 	// tokens for, proving the resource owner's identity against conn (the zero
 	// Connector when ConnectorID is ""). Returning an *oauth2.Error makes the
@@ -163,7 +168,7 @@ func NewEndpoint(issuer *tokens.Issuer, s storage.Storage, conns *connectors.Cac
 		&password{logger: logger, connectorID: passwordConnector},
 		&tokenExchange{issuer: issuer, logger: logger},
 		&authorizationCode{storage: s, connectors: conns, now: now, logger: logger},
-		&refresh{storage: s, connectors: conns, issuer: issuer, policy: refreshPolicy, sessionsEnabled: sessionsEnabled, now: now, logger: logger},
+		&refresh{storage: s, issuer: issuer, policy: refreshPolicy, sessionsEnabled: sessionsEnabled, now: now, logger: logger},
 	)
 	return e
 }
@@ -207,7 +212,12 @@ func (e *Endpoint) Dispatch(w http.ResponseWriter, r *http.Request, grantType st
 
 	// 3. Resolve the grant's connector and enforce the connector-authorization
 	// invariant. A grant that uses no connector resolves to the zero Connector.
-	conn, oerr := e.resolveConnector(ctx, grant, req, client)
+	connID, oerr := grant.ConnectorID(ctx, req, client)
+	if oerr != nil {
+		e.writeError(ctx, w, oerr)
+		return true
+	}
+	conn, oerr := e.resolveConnector(ctx, connID, client, grant.GrantType())
 	if oerr != nil {
 		e.writeError(ctx, w, oerr)
 		return true
@@ -293,13 +303,12 @@ func (e *Endpoint) validateScopes(ctx context.Context, client storage.Client, re
 	return nil
 }
 
-// resolveConnector enforces the connector-authorization invariant for the grant
-// and returns the opened connector: the client must allow the connector, and the
-// connector must permit the grant type. A grant that uses no connector
-// (ConnectorID == "") resolves to the zero Connector. Running here, before
-// Authorize, means no grant can forget the check.
-func (e *Endpoint) resolveConnector(ctx context.Context, grant Grant, req *Request, client storage.Client) (connectors.Connector, *oauth2.Error) {
-	connID := grant.ConnectorID(req)
+// resolveConnector enforces the connector-authorization invariant and returns the
+// opened connector: the client must allow the connector, and the connector must
+// permit the grant type. connID == "" (a grant that uses no connector) resolves
+// to the zero Connector. Running here, before Authorize, means no grant can
+// forget the check.
+func (e *Endpoint) resolveConnector(ctx context.Context, connID string, client storage.Client, grantType string) (connectors.Connector, *oauth2.Error) {
 	if connID == "" {
 		return connectors.Connector{}, nil
 	}
@@ -313,8 +322,8 @@ func (e *Endpoint) resolveConnector(ctx context.Context, grant Grant, req *Reque
 		e.logger.ErrorContext(ctx, "failed to get connector", "connector_id", connID, "err", err)
 		return connectors.Connector{}, &oauth2.Error{Type: oauth2.InvalidRequest, Description: "Requested connector does not exist.", Status: http.StatusBadRequest}
 	}
-	if !connectors.GrantTypeAllowed(conn.GrantTypes, grant.GrantType()) {
-		e.logger.ErrorContext(ctx, "connector does not allow grant", "connector_id", connID, "grant_type", grant.GrantType())
+	if !connectors.GrantTypeAllowed(conn.GrantTypes, grantType) {
+		e.logger.ErrorContext(ctx, "connector does not allow grant", "connector_id", connID, "grant_type", grantType)
 		return connectors.Connector{}, &oauth2.Error{Type: oauth2.InvalidRequest, Description: "Requested connector does not support this grant type.", Status: http.StatusBadRequest}
 	}
 	return conn, nil

server/grants/password.go

@@ -43,8 +43,8 @@ func (g *password) ScopePolicy() ScopePolicy {
 }
 
 // ConnectorID is the connector the password grant is configured to use.
-func (g *password) ConnectorID(req *Request) string {
-	return g.connectorID
+func (g *password) ConnectorID(ctx context.Context, req *Request, client storage.Client) (string, *oauth2.Error) {
+	return g.connectorID, nil
 }
 
 func (g *password) Authorize(ctx context.Context, req *Request, client storage.Client, conn connectors.Connector) (*Result, error) {

server/grants/refresh.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"log/slog"
 	"net/http"
+	"slices"
 	"time"
 
 	"github.com/dexidp/dex/connector"
@@ -21,7 +22,6 @@ import (
 // than minting a new one, so it mints its own instead of the standard Issue.
 type refresh struct {
 	storage         storage.Storage
-	connectors      *connectors.Cache
 	issuer          *tokens.Issuer
 	policy          *tokens.RefreshStrategy
 	sessionsEnabled bool
@@ -43,38 +43,34 @@ func (g *refresh) ScopePolicy() ScopePolicy {
 	return ScopePolicy{}
 }
 
-// ConnectorID is empty: the connector is recorded on the stored refresh token, so
-// it is resolved and re-checked inside Authorize on every refresh.
-func (g *refresh) ConnectorID(req *Request) string {
-	return ""
-}
-
-func (g *refresh) Authorize(ctx context.Context, req *Request, client storage.Client, conn connectors.Connector) (*Result, error) {
+// ConnectorID validates the refresh token and reports the connector recorded on
+// it, so the endpoint resolves and re-checks that connector on every refresh: a
+// client's allowed connectors, or a connector's grant types, may have been
+// tightened after the token was issued. The validated token is stashed for
+// Authorize.
+func (g *refresh) ConnectorID(ctx context.Context, req *Request, client storage.Client) (string, *oauth2.Error) {
 	token, oerr := parseRefreshToken(req.RefreshToken)
 	if oerr != nil {
-		return nil, oerr
+		return "", oerr
 	}
 
 	refreshToken, err := tokens.LookupRefreshToken(ctx, g.storage, g.policy, g.logger, &client.ID, token)
 	if err != nil {
-		return nil, refreshLookupError(err)
+		return "", refreshLookupError(err)
 	}
 
-	// Resolve the connector and re-check authorization on every refresh: the
-	// connector may have been removed from the client's allowed list, or had the
-	// refresh grant revoked, after this token was issued.
-	if !connectors.ConnectorAllowed(client.AllowedConnectors, refreshToken.ConnectorID) {
-		g.logger.WarnContext(ctx, "connector not allowed for client", "client_id", client.ID, "connector_id", refreshToken.ConnectorID)
-		return nil, &oauth2.Error{Type: oauth2.InvalidGrant, Description: "Connector not allowed for this client.", Status: http.StatusBadRequest}
-	}
-	upstream, err := g.connectors.Get(ctx, refreshToken.ConnectorID)
-	if err != nil {
-		g.logger.ErrorContext(ctx, "connector not found", "connector_id", refreshToken.ConnectorID, "err", err)
-		return nil, &oauth2.Error{Type: oauth2.InvalidRequest, Status: http.StatusInternalServerError}
-	}
-	if !connectors.GrantTypeAllowed(upstream.GrantTypes, oauth2.GrantTypeRefreshToken) {
-		g.logger.ErrorContext(ctx, "connector does not allow refresh token grant", "connector_id", refreshToken.ConnectorID)
-		return nil, &oauth2.Error{Type: oauth2.InvalidRequest, Description: "Connector does not support refresh tokens.", Status: http.StatusBadRequest}
+	req.resolvedRefresh = refreshToken
+	return refreshToken.ConnectorID, nil
+}
+
+// Authorize rotates the refresh token, re-reads the identity against the resolved
+// connector, and returns the token set to issue (with the rotated refresh token).
+func (g *refresh) Authorize(ctx context.Context, req *Request, client storage.Client, conn connectors.Connector) (*Result, error) {
+	refreshToken := req.resolvedRefresh
+	// Already validated in ConnectorID; re-parse only to hand Rotate the raw token.
+	token, oerr := parseRefreshToken(req.RefreshToken)
+	if oerr != nil {
+		return nil, oerr
 	}
 
 	scopes, oerr := g.refreshScopes(req, refreshToken)
@@ -109,7 +105,7 @@ func (g *refresh) Authorize(ctx context.Context, req *Request, client storage.Cl
 		if err != nil {
 			return connector.Identity{}, err
 		}
-		return g.refreshWithConnector(ctx, upstream, connectorData, scopes, tokens.IdentityFromClaims(refreshToken.Claims))
+		return g.refreshWithConnector(ctx, conn, connectorData, scopes, tokens.IdentityFromClaims(refreshToken.Claims))
 	}
 
 	rawNewToken, ident, err := g.issuer.Refresh.Rotate(ctx, refreshToken, token, g.policy, freshIdentity)
@@ -167,7 +163,7 @@ func (g *refresh) refreshScopes(req *Request, refreshToken *storage.RefreshToken
 
 	var unauthorized []string
 	for _, scope := range req.Scopes {
-		if !contains(refreshToken.Scopes, scope) {
+		if !slices.Contains(refreshToken.Scopes, scope) {
 			unauthorized = append(unauthorized, scope)
 		}
 	}
@@ -246,12 +242,3 @@ func refreshLookupError(err error) *oauth2.Error {
 		return &oauth2.Error{Type: oauth2.InvalidRequest, Status: http.StatusInternalServerError}
 	}
 }
-
-func contains(arr []string, item string) bool {
-	for _, v := range arr {
-		if v == item {
-			return true
-		}
-	}
-	return false
-}

server/grants/tokenexchange.go

@@ -38,8 +38,8 @@ func (g *tokenExchange) ScopePolicy() ScopePolicy {
 }
 
 // ConnectorID reads the required connector_id parameter (an RFC 8693 extension).
-func (g *tokenExchange) ConnectorID(req *Request) string {
-	return req.ConnectorID
+func (g *tokenExchange) ConnectorID(ctx context.Context, req *Request, client storage.Client) (string, *oauth2.Error) {
+	return req.ConnectorID, nil
 }
 
 func (g *tokenExchange) Authorize(ctx context.Context, req *Request, client storage.Client, conn connectors.Connector) (*Result, error) {