[#022] authenticate(anchor, publicKey) end-to-end helper#281
[#022] authenticate(anchor, publicKey) end-to-end helper#281Timothy2025-20 wants to merge 2 commits into
Conversation
- Add authenticate() function with challenge, sign, exchange
- Return { jwt, expiresAt } object
- Add distinct error types for each failure mode
- Add validateToken() helper for token verification
- Add E2E tests with mock anchor
Closes ezedike-evan#22
|
@Timothy2025-20 is attempting to deploy a commit to the ezedikeevan's projects Team on Vercel. A member of the Team first needs to authorize it. |
ezedike-evan
left a comment
There was a problem hiding this comment.
Thanks for the work here @Timothy2025-20, but this PR can't merge in its current state. Several issues need to be resolved.
1. Introduces axios as a hard dependency — not acceptable for this codebase
The existing codebase uses native fetch throughout (sep24.ts, sep38.ts, sep1.ts). This PR adds axios and axios-mock-adapter as dependencies, adds an AxiosInstance parameter to authenticate and validateToken, and imports Axios in the main lib/stellar/sep10.ts library file. That breaks the established pattern and adds ~50 KB to the bundle for no functional gain. The existing fetch-based implementation already handles all these cases. Remove the Axios dependency entirely.
2. Drops the JWT cache shared with the rest of the application
The existing code imports getCachedJwt, setCachedJwt, and invalidateCachedJwt from ./jwt-cache. This PR replaces that with a private let jwtCache = new Map() declared inline in sep10.ts. This breaks the shared cache contract — useAnchorAuth, sep38.ts, and other callers that depend on the singleton cache in jwt-cache.ts will no longer share state with authenticate. Revert to the ./jwt-cache imports.
3. Drops the network_passphrase validation
The existing fetchChallenge rejects challenges for any network other than Networks.PUBLIC with a typed ChallengeError('WRONG_NETWORK'). The PR silently falls back to Networks.PUBLIC with (data['network_passphrase'] as string) || Networks.PUBLIC, which means a testnet challenge would be silently accepted and signed as mainnet. This is a security regression. Restore the explicit rejection.
4. signChallengeWithSecret uses the XDR API incorrectly
The manual XDR manipulation via xdr.TransactionEnvelope, transaction.v1().tx().hash(), and new xdr.TransactionEnvelope.envelopeTypeTx(...) is not how the Stellar SDK is meant to be used for signing — TransactionBuilder.fromXDR + transaction.sign(keypair) is the correct and safe path. The current code will fail at runtime and the signChallengeWithSecret tests in the test file never actually exercise a valid XDR (they use 'AAAAAgAAA...' as a placeholder), so the test suite doesn't catch this.
5. The authenticate overload union is unsound
The unified authenticate(anchorUrlOrObject: string | ResolvedAnchor, publicKey: string, secretKeyOrSignal?: string | AbortSignal) signature returns Promise<Sep10AuthResult | Sep10Auth> — two different shapes. Every downstream caller would need to type-narrow the result. The two flows should remain as separate named exports (authenticateWithSecret / authenticateWithWallet — which you've added), and the default authenticate export should keep the existing signature (anchor: ResolvedAnchor, publicKey: string, signal?: AbortSignal): Promise<Sep10Auth> for backward compatibility.
6. Re-declares ResolvedAnchor and Sep10Auth locally
The file already imports these types from @/types. The PR removes that import and re-declares them inline with a looser shape (e.g. capabilities?: { sep10?: boolean } vs the full capabilities object in @/types). This will cause type conflicts in other files that import the canonical types. Remove the local re-declarations and restore the @/types import.
7. The test suite tests against Axios mocks, not the real code paths
Because authenticateWithSecret uses an injected httpClient (Axios), and the tests mock that client, the tests don't exercise the actual fetch path at all. The happy-path test also uses 'AAAAAgAAA...' as a valid XDR, which means signChallengeWithSecret is never reached without throwing — so the signing path has zero real coverage.
8. Missing newline at end of file
sep10.ts is missing a trailing newline, which will fail the repo's lint check.
What to do:
- Remove Axios; keep native
fetchthroughout. - Restore
./jwt-cacheimports. - Keep the
network_passphrasevalidation. - Use
TransactionBuilder.fromXDR+transaction.sign(keypair)for signing. - Keep the existing
authenticate(anchor: ResolvedAnchor, publicKey: string)signature; addauthenticateWithSecretas a separate named export if the secret-key path is needed. - Restore the
@/typesimport; remove local type re-declarations. - Update tests to mock
fetch(viavi.stubGlobal) as the rest of the test suite does, and use a real well-formed Stellar testnet XDR for the signing path. - Add a newline at end of file.
|
Three issues are still present. (1) Axios dependency: the implementation imports |
Description
Implements SEP-10 authentication helper that composes challenge request, signing, and token exchange into a single function.
Features
authenticate()function returning{ jwt, expiresAt }ChallengeError- Challenge request failsInvalidChallengeError- Invalid challenge receivedSigningError- Failed to sign challengeTokenExchangeError- Token exchange failsvalidateToken()helper for token verificationFiles Changed
lib/stellar/sep10.ts- Main authentication functiontests/sep10-e2e.spec.ts- E2E testsUsage Example