Skip to content

fix(payouts): route merchant-authentication (access-token) calls through the payout connector#13150

Open
gopikrishna000 wants to merge 3 commits into
mainfrom
fix/itaubank-payout-access-token
Open

fix(payouts): route merchant-authentication (access-token) calls through the payout connector#13150
gopikrishna000 wants to merge 3 commits into
mainfrom
fix/itaubank-payout-access-token

Conversation

@gopikrishna000

Copy link
Copy Markdown
Contributor

Summary

During the merchant-authentication (access-token) flow, the access-token gateway unconditionally used the payment gRPC header builder (x-connector) and a payment reference — even when the caller was a payout. For payout access-token calls this selected the payment connector variant and carried a placeholder payment reference instead of the real payout reference, even though router_data.payout_id is available on the access-token RouterData (preserved through router_data_type_conversion).

What this does

Makes the access-token path payout-aware:

  • external_services — adds create_access_token_for_payouts, a sibling of create_access_token that uses build_unified_connector_service_grpc_headers_for_payouts (x-payout-connector).
  • gateway (access_token_gateway.rs) — when router_data.payout_id is set, builds the payout reference (UcsReferenceId::Payout) and resource id (UcsResourceId::PayoutAttempt) and dispatches to the payout sibling, following the existing payouts/gateway/* pattern. The payment path and the existing payout_transfer / payout_get paths are untouched.

Result

Payout access-token calls now select the payout connector variant, run its auth implementation, and carry the real payout reference.

Companion change

Pairs with the connector-side implementation of the Itaú payout merchant-auth flow: juspay/hyperswitch-prism#1806

…ugh the payout connector

During the merchant-authentication (access-token) flow, the access-token
gateway unconditionally used the payment gRPC header builder (x-connector)
and a payment reference, even when the caller was a payout. For payout
access-token calls this selected the payment connector variant and carried a
placeholder payment reference instead of the real payout reference.

Make the access-token path payout-aware:
- external_services: add create_access_token_for_payouts, which uses
  build_unified_connector_service_grpc_headers_for_payouts (x-payout-connector).
- gateway: when router_data.payout_id is set, build the payout reference and
  resource ids and call the payout sibling. Payments and the existing
  payout_transfer/payout_get paths are untouched.
@gopikrishna000 gopikrishna000 requested review from a team as code owners July 3, 2026 07:13
@semanticdiff-com

semanticdiff-com Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review changes with  SemanticDiff

Changed Files
File Status
  crates/router/src/core/payments/gateway/access_token_gateway.rs  48% smaller
  crates/external_services/src/grpc_client/unified_connector_service.rs  31% smaller
  crates/router/src/core/payments/access_token.rs  0% smaller

Comment on lines +1086 to +1127
/// Performs Create Access Token Granular for payouts
///
/// Identical to [`Self::create_access_token`] but attaches the payout connector
/// header (`x-payout-connector`) so the merchant-authentication flow resolves the
/// payout connector variant instead of the payment one.
pub async fn create_access_token_for_payouts(
&self,
create_access_token_request: payments_grpc::MerchantAuthenticationServiceCreateServerAuthenticationTokenRequest,
connector_auth_metadata: ConnectorAuthMetadata,
grpc_headers: GrpcHeadersUcs,
) -> UnifiedConnectorServiceResult<
tonic::Response<
payments_grpc::MerchantAuthenticationServiceCreateServerAuthenticationTokenResponse,
>,
> {
let mut request = tonic::Request::new(create_access_token_request);
let connector_name = connector_auth_metadata.connector_name.clone();
let metadata = build_unified_connector_service_grpc_headers_for_payouts(
connector_auth_metadata,
grpc_headers,
)?;
*request.metadata_mut() = metadata;

self.merchant_authentication_service_client
.clone()
.create_server_authentication_token(request)
.await
.map_err(|error| {
error_stack::Report::new(UnifiedConnectorServiceError::from_grpc_error(
&error,
&connector_name,
))
})
.inspect_err(|error| {
logger::error!(
grpc_error=?error,
method="create_server_authentication_token_for_payouts",
connector_name=?connector_name,
"UCS create server authentication token gRPC call failed"
)
})
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only difference between create_access_token_for_payouts and create_access_token is the metadata creation. so instead of duplicating the whole logic in create_access_token we can pass a field indicating payments or payouts, based on that we can create the metadata

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in d417197 — collapsed create_access_token_for_payouts into create_access_token with an is_payout flag that selects the payout vs payment header builder; both call sites updated (gateway passes is_payout, the payments/relay path passes false).

@XyneSpaces

Copy link
Copy Markdown

⚠️ Missing #[instrument] on async hot-path functions

Both create_access_token_for_payouts in unified_connector_service.rs (~1086-1128) and the async closure in access_token_gateway.rs (~131-175) lack #[instrument(skip_all, fields(...))]. These are on the payment/payout gateway path and require tracing instrumentation.

Add #[instrument(skip_all)] with relevant fields like connector_name, payout_id/payment_id to maintain observability parity with sibling methods.

@XyneSpaces

Copy link
Copy Markdown

Review Findings

[should-fix] Code duplication in access token methods

The new create_access_token_for_payouts method in UnifiedConnectorServiceClient duplicates most of the logic from create_access_token. The only difference is the metadata creation via build_unified_connector_service_grpc_headers_for_payouts. Consider refactoring to share the common logic.

[should-fix] is_payout detection logic

The detection of payout vs payment flows relies on router_data.payout_id.is_some(). Verify this is the canonical way to distinguish these flows and document any edge cases where a payout might not have a payout_id at this stage.

…stead of a payout sibling

Address review: collapse create_access_token_for_payouts into create_access_token
via an is_payout flag that selects the payout vs payment gRPC header builder,
removing the duplicated function body.
@XyneSpaces

Copy link
Copy Markdown

⚠️ Payout detection via payout_id.is_some() may be fragile

The logic at lines 102-170 detects payout flow using router_data.payout_id.is_some(). If payout_id is ever set for non-payout flows (e.g., in shared response types), this will incorrectly route through payout connector paths.

Consider using a more explicit flow discriminator or validating that is_payout is only set when the operation context explicitly indicates a payout flow, rather than relying on optional field presence.

@XyneSpaces

Copy link
Copy Markdown

⚠️ Boolean flag overload in UCS client interface

In crates/external_services/src/grpc_client/unified_connector_service.rs (~line 45):

The addition of is_payout: bool parameter to create_access_token is heading toward a boolean trap:

pub async fn create_access_token(
    &self,
    ...
    is_payout: bool,  // What does true mean again?
)

If more connector types are added (payouts, fraud_check, identity, etc.), this becomes an unmaintainable series of booleans.

Fix: Use an enum to make call sites self-documenting:

pub enum ConnectorType {
    Payment,
    Payout,
}

// Call site becomes:
create_access_token(..., ConnectorType::Payout)

@XyneSpaces

Copy link
Copy Markdown

💡 Consider consolidating payout detection logic

The is_payout flag is derived from router_data.payout_id.is_some() in the gateway but passed separately through the client. Ensure these stay synchronized. A mismatch between payout_id presence and is_payout flag could route payout access-token calls through payment headers or vice versa.

@XyneSpaces

Copy link
Copy Markdown

⚠️ Silent degradation on invalid attempt_id parsing

The inspect_err + .ok() pattern silently converts parse failures to None:

.inspect_err(|err| logger::warn!(...))
.ok()
.map(...)

Invalid attempt_id values are logged but don't fail the request. The gRPC call proceeds with incomplete resource identification headers. This could cause subtle downstream issues where UCS receives malformed payout/payment references.

Consider failing fast with a proper error instead of continuing with None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants