Added Webfinger lookup for actor profile #660
Conversation
|
Warning Rate limit exceeded@vershwal has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
## Walkthrough
A new asynchronous function, `lookupActorProfile`, was introduced in `src/lookup-helpers.ts` to perform a WebFinger lookup for a given handle and return the actor's profile URL as a `URL` object or `null`. The function normalizes the input handle, constructs a WebFinger resource string, and queries `lookupWebFinger`. It prioritizes returning a profile-page link, falling back to an ActivityPub self link if necessary, with appropriate logging and error handling. Corresponding unit tests were added in `src/lookup-helpers.unit.test.ts` to cover various scenarios, including successful lookups, missing links, and error cases.
## Possibly related PRs
- TryGhost/ActivityPub#520: Implements a custom WebFinger HTTP handler on the server side, which relates to this PR's client-side WebFinger lookup functionality, both addressing WebFinger protocol processing.
## Suggested reviewers
- mike182uk✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lookup-helpers.ts (1)
110-162: Nicely implemented WebFinger lookup for actor profilesThe function follows a logical workflow with proper error handling and fallback mechanisms. It first tries to find the profile-page link and falls back to the ActivityPub self link if needed.
A few suggestions for improvement:
Consider extracting the common WebFinger lookup logic from both
lookupAPIdByHandleandlookupActorProfileto reduce code duplication. Both functions share similar handle normalization, WebFinger lookup, and error handling patterns.+async function performWebFingerLookup(ctx, handle, options = {}) { + try { + // Remove leading @ if present + const cleanHandle = handle.startsWith('@') ? handle.slice(1) : handle; + const resource = `acct:${cleanHandle}`; + + const webfingerData = await lookupWebFinger(resource, { + allowPrivateAddress: + process.env.ALLOW_PRIVATE_ADDRESS === 'true' && + ['development', 'testing'].includes(process.env.NODE_ENV || ''), + ...options + }); + + if (!webfingerData?.links) { + ctx.data.logger.info(`No links found in WebFinger response for handle ${handle}`); + return null; + } + + return { webfingerData, cleanHandle }; + } catch (err) { + ctx.data.logger.error(`Error looking up WebFinger for handle ${handle} - ${err}`); + return null; + } +}Consider using the same string formatting approach consistently throughout the file. Other functions use the format
'Message with {placeholder}'with{ placeholder: value }as the second argument, while this function uses template literals:- `No links found in WebFinger response for handle ${handle}`, + 'No links found in WebFinger response for handle {handle}', + { handle },src/lookup-helpers.unit.test.ts (1)
132-281: Comprehensive test suite for the new functionThe test suite for
lookupActorProfileis thorough and covers all critical scenarios:
- Returning profile page URL when available
- Falling back to self link when profile page is not available
- Handling handles with leading '@'
- Properly handling error cases and logging
Consider adding a test case for when the WebFinger response contains links but they're empty (vs. the response having no
linksproperty at all):+ it('should return null when WebFinger response has empty links array', async () => { + const mockWebFingerResponse = { + links: [] + }; + + ( + lookupWebFinger as unknown as ReturnType<typeof vi.fn> + ).mockResolvedValue(mockWebFingerResponse); + + const result = await lookupActorProfile( + mockCtx as unknown as Context<ContextData>, + 'user@example.com', + ); + + expect(result).toBeNull(); + expect(mockCtx.data.logger.info).toHaveBeenCalledWith( + 'No ActivityPub profile found in WebFinger response for handle user@example.com', + ); + });This would validate behavior when the WebFinger response includes an empty links array, which is a distinct case from having no links property at all.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lookup-helpers.ts(1 hunks)src/lookup-helpers.unit.test.ts(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/lookup-helpers.ts (1)
src/app.ts (1)
ContextData(186-190)
src/lookup-helpers.unit.test.ts (2)
src/app.ts (1)
ContextData(186-190)src/lookup-helpers.ts (1)
lookupActorProfile(110-162)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build, Test and Push
🔇 Additional comments (2)
src/lookup-helpers.unit.test.ts (2)
1-4: Updated imports look goodThe imports correctly include the new dependencies and the
lookupActorProfilefunction.
40-40: Type casting updates ensure proper typingGood updates to ensure consistent typing with
Context<ContextData>across all test cases.Also applies to: 60-60, 83-83, 96-96, 124-124
| return null; | ||
| } | ||
|
|
||
| return new URL(selfLink.href); |
There was a problem hiding this comment.
selfLink.href should be an URL according to spec, but maybe safer to put that in a try/catch block, as we're fetching external data?
Ref https://linear.app/ghost/issue/PROD-1736