Skip to content

Conversation

silva-fj
Copy link
Contributor

@silva-fj silva-fj commented Jun 18, 2025

Summary

Authentication Flow Updates

  • export_wallet and transfer_withdraw now use Authorization header authentication
  • All methods extract the OA from the user's JWT
  • Removed redundant user_email parameters

Refactored protected RPC methods to extract AccountId from authenticated users and remove email parameter:

  • omni_exportWallet
  • omni_transferWithdraw
  • omni_addWallet
  • omni_submitSwapOrder
  • omni_signLimitOrder
  • omni_notifyLimitOrderResult

Refactored related NativeTask's to use OA instead of Identity.

Note: We need to refactor the omni-account pallet to adjust how intent calls are dispatched. We currently send the Identity but we should send the actual OA. Because of this, some code has been commented out (as well as the integration tests)

silva-fj added 2 commits June 18, 2025 14:56
…ntication

- Add omni_transferWithdraw to PROTECTED_METHODS array
- Replace parameter-based auth (email_code, client_id) with JWT token authentication
- Use check_auth() for consistent authentication handling across protected methods
- Remove manual verify_auth() calls in favor of middleware-based authentication
- Align authentication mechanism with other protected methods like submit_swap_order
…tion

- Add omni_exportWallet to PROTECTED_METHODS array
- Replace parameter-based auth (email_code, client_id) with JWT token authentication
- Use check_auth() for consistent authentication handling across protected methods
- Remove manual verify_auth() calls in favor of middleware-based authentication
- Align authentication mechanism with other protected methods
Copy link

linear bot commented Jun 18, 2025

@silva-fj silva-fj changed the title refactor: standardize authentication for omni_transferWithdraw and omni_exportWallet Standardize authentication for omni_transferWithdraw and omni_exportWallet Jun 18, 2025
@silva-fj silva-fj requested review from Kailai-Wang and a team June 18, 2025 14:33
Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thanks - I think it requires a small refactoring of NativeTask

I assume checks against other methods would come in a follow-up PR?

silva-fj added 10 commits June 19, 2025 09:28
…untId in PumpxExportWallet

- Update NativeTask::PumpxExportWallet to use AccountId instead of Identity
- Remove user_email parameter from omni_exportWallet RPC method
- Extract omni account directly from authenticated user in RPC handlers
- Simplify native task handler by using AccountId directly instead of converting from Identity
- Update both omni and pumpx export wallet implementations for consistency

This simplifies the API by removing redundant parameters and improves security by using authenticated user data directly.
…AccountId in PumpxTransferWidthdraw

- Update NativeTask::PumpxTransferWidthdraw to use AccountId instead of Identity
- Remove user_email parameter from omni_transferWithdraw RPC method
- Extract omni account directly from authenticated user in RPC handlers
- Simplify native task handler by using AccountId directly instead of converting from Identity
- Update both omni and pumpx transfer withdraw implementations for consistency

This change maintains consistency with the PumpxExportWallet refactoring and improves security by using authenticated user data directly.
Replace Identity parameter with AccountId in PumpxAddWallet to simplify
authentication flow. This change:

- Updates NativeTask::PumpxAddWallet to accept AccountId directly
- Removes user_email parameter from omni add_wallet endpoint
- Extracts AccountId from authenticated user's omni_account token
- Converts Identity to omni_account in pumpx add_wallet endpoint
- Eliminates need for Identity conversion in native task handler
Replace Identity parameter with AccountId in PumpxSignLimitOrder to
simplify parameter handling. This change:

- Updates NativeTask::PumpxSignLimitOrder to accept AccountId directly
- Removes Identity-to-AccountId conversion logic in task handler
- Updates both omni and pumpx RPC endpoints to pass AccountId
- Eliminates error handling for invalid Identity types
Replace Identity with AccountId parameter in PumpxNotifyLimitOrderResult
task to maintain consistency with other native tasks and simplify the
implementation by removing the Identity-to-AccountId conversion logic.

Changes:
- Update NativeTask enum to use AccountId instead of Identity
- Remove Identity conversion workaround in native task handler
- Update RPC method implementations to pass AccountId directly
- Clean up unused Identity imports
Replace Identity with AccountId parameter in RequestIntent task to
standardize parameter types across native tasks and eliminate redundant
Identity-to-AccountId conversions.

Changes:
- Update NativeTask::RequestIntent to accept AccountId directly
- Remove Identity conversion logic in native task handler
- Temporarily disable SystemRemark and TransferNative intents
- Update RPC endpoints to pass AccountId instead of Identity
- Clean up unused imports and commented code
Temporarily disable the RequestIntent TransferNative test while the
omni-account pallet is being refactored to use AccountId instead of
Identity parameters.
@silva-fj silva-fj requested a review from Kailai-Wang June 19, 2025 19:50
Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.

Could you (or ask Verin) to make sure the apifox is up-to-date?

auth_type: Option<OmniAccountAuthType>,
) {
let call = parentchain_api_interface::tx().omni_account().dispatch_as_signed(
sender.hash().to_subxt_type(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC the problem here is the hash() right? We could later use the hash of the OmniAccount - either blake2_256 or another sha2 if we want it to be evm-friendly.

Can do together with AccountStore cleanup

@silva-fj silva-fj merged commit fc576a0 into dev Jun 24, 2025
23 checks passed
@silva-fj silva-fj deleted the p-1583-use-generic-user-idauth-and-client-idauth-in branch June 24, 2025 06:48
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.

5 participants