refactor(server): apply code-review fixes

- Replace hand-rolled membership loops with slices.Contains in
  shouldIssueRefreshToken, tokens.HasOpenID and tokens.CrossClientTrusted.
- Hoist the constant password/client_credentials scope policies to package vars
  so ScopePolicy() no longer allocates the maps on every request.
- Harden Endpoint.writeError against a typed-nil *oauth2.Error so a future grant
  returning one cannot panic the token endpoint.
- Correct the ConnectorID doc: returning "" skips the connector step, which
  authorization_code does deliberately (gated at /auth).

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

Author: nabokihms

server/grants/clientcredentials.go

@@ -22,20 +22,22 @@ func (g *clientCredentials) RequiresClientAuth() bool {
 	return true
 }
 
+var clientCredentialsScopePolicy = ScopePolicy{
+	Standard: map[string]bool{
+		tokens.ScopeOpenID:  true,
+		tokens.ScopeEmail:   true,
+		tokens.ScopeProfile: true,
+		tokens.ScopeGroups:  true,
+	},
+	Rejected: map[string]string{
+		tokens.ScopeOfflineAccess: "client_credentials grant does not support offline_access scope.",
+		tokens.ScopeFederatedID:   "client_credentials grant does not support federated:id scope.",
+	},
+	ErrorType: oauth2.InvalidScope,
+}
+
 func (g *clientCredentials) ScopePolicy() ScopePolicy {
-	return ScopePolicy{
-		Standard: map[string]bool{
-			tokens.ScopeOpenID:  true,
-			tokens.ScopeEmail:   true,
-			tokens.ScopeProfile: true,
-			tokens.ScopeGroups:  true,
-		},
-		Rejected: map[string]string{
-			tokens.ScopeOfflineAccess: "client_credentials grant does not support offline_access scope.",
-			tokens.ScopeFederatedID:   "client_credentials grant does not support federated:id scope.",
-		},
-		ErrorType: oauth2.InvalidScope,
-	}
+	return clientCredentialsScopePolicy
 }
 
 // ConnectorID is empty: client_credentials involves no connector.

server/grants/grants.go

@@ -7,6 +7,7 @@ import (
 	"log/slog"
 	"net/http"
 	"net/url"
+	"slices"
 	"strings"
 	"time"
 
@@ -103,11 +104,14 @@ type Grant interface {
 	// ScopePolicy reports how the endpoint validates the requested scopes for
 	// this grant.
 	ScopePolicy() ScopePolicy
-	// ConnectorID is the connector this grant authenticates against, or "" when
-	// 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 is the connector this grant authenticates against; the endpoint
+	// resolves it and enforces the connector-authorization invariant (client
+	// allows it, connector allows the grant type) before Authorize. The grant may
+	// read it from the request or look it up in storage; returning an error
+	// rejects the request. Returning "" skips the step — for a grant that uses no
+	// connector (client_credentials, device_code), or one already authorized
+	// elsewhere (authorization_code was gated at /auth and resolves its connector
+	// inside Authorize only to decide on a refresh token).
 	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
@@ -373,9 +377,10 @@ func (e *Endpoint) authenticateClient(ctx context.Context, w http.ResponseWriter
 // writeError writes err as an OAuth2 error response. An *oauth2.Error carries its
 // own type/description/status; anything else is reported as a server error.
 func (e *Endpoint) writeError(ctx context.Context, w http.ResponseWriter, err error) {
-	oerr := &oauth2.Error{Type: oauth2.ServerError, Status: http.StatusInternalServerError}
-	if !errors.As(err, &oerr) {
+	var oerr *oauth2.Error
+	if !errors.As(err, &oerr) || oerr == nil {
 		e.logger.ErrorContext(ctx, "token request failed", "err", err)
+		oerr = &oauth2.Error{Type: oauth2.ServerError, Status: http.StatusInternalServerError}
 	}
 	if werr := oauth2.WriteError(w, oerr.Type, oerr.Description, oerr.Status); werr != nil {
 		e.logger.ErrorContext(ctx, "failed to write token error response", "err", werr)
@@ -392,10 +397,5 @@ func shouldIssueRefreshToken(conn connectors.Connector, scopes []string) bool {
 	if !connectors.GrantTypeAllowed(conn.GrantTypes, oauth2.GrantTypeRefreshToken) {
 		return false
 	}
-	for _, scope := range scopes {
-		if scope == tokens.ScopeOfflineAccess {
-			return true
-		}
-	}
-	return false
+	return slices.Contains(scopes, tokens.ScopeOfflineAccess)
 }

server/grants/password.go

@@ -27,19 +27,21 @@ func (g *password) RequiresClientAuth() bool {
 	return true
 }
 
+var passwordScopePolicy = ScopePolicy{
+	Standard: map[string]bool{
+		tokens.ScopeOpenID:        true,
+		tokens.ScopeOfflineAccess: true,
+		tokens.ScopeEmail:         true,
+		tokens.ScopeProfile:       true,
+		tokens.ScopeGroups:        true,
+		tokens.ScopeFederatedID:   true,
+	},
+	RequireOpenID: true,
+	ErrorType:     oauth2.InvalidRequest,
+}
+
 func (g *password) ScopePolicy() ScopePolicy {
-	return ScopePolicy{
-		Standard: map[string]bool{
-			tokens.ScopeOpenID:        true,
-			tokens.ScopeOfflineAccess: true,
-			tokens.ScopeEmail:         true,
-			tokens.ScopeProfile:       true,
-			tokens.ScopeGroups:        true,
-			tokens.ScopeFederatedID:   true,
-		},
-		RequireOpenID: true,
-		ErrorType:     oauth2.InvalidRequest,
-	}
+	return passwordScopePolicy
 }
 
 // ConnectorID is the connector the password grant is configured to use.

server/tokens/crossclient.go

@@ -2,6 +2,7 @@ package tokens
 
 import (
 	"context"
+	"slices"
 
 	"github.com/dexidp/dex/storage"
 )
@@ -20,10 +21,5 @@ func CrossClientTrusted(ctx context.Context, s storage.Storage, clientID, peerID
 		}
 		return false, err
 	}
-	for _, id := range peer.TrustedPeers {
-		if id == clientID {
-			return true, nil
-		}
-	}
-	return false, nil
+	return slices.Contains(peer.TrustedPeers, clientID), nil
 }

server/tokens/scopes.go

@@ -1,6 +1,7 @@
 package tokens
 
 import (
+	"slices"
 	"strings"
 
 	"github.com/dexidp/dex/connector"
@@ -21,12 +22,7 @@ const (
 // HasOpenID reports whether the openid scope was requested, i.e. whether an
 // ID token should be issued.
 func HasOpenID(scopes []string) bool {
-	for _, scope := range scopes {
-		if scope == ScopeOpenID {
-			return true
-		}
-	}
-	return false
+	return slices.Contains(scopes, ScopeOpenID)
 }
 
 // ParseCrossClientScope extracts the peer client ID from a cross-client audience