feat: fetch CSRF token from MediaWiki Action API #165
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a POST API route to obtain a Wikibase CSRF token using OAuth credentials, introduces OAuth/CSRF schemas and InstanceId literals, and updates backend clients/services and frontend stores/composables/tests to use typed InstanceId and per-instance OAuth client handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API Route (/api/wikibase/:instanceId/csrf-token)
participant WB as WikibaseClient / Service
participant OC as OAuth client (per-instance cache)
participant MW as MediaWiki API
Client->>API: POST /:instanceId/csrf-token { credentials }
API->>WB: getCsrfToken(instanceId, credentials)
alt cached OAuth client exists
WB->>OC: use cached client (sign requests)
else no cached client
WB->>OC: initialize OAuth client with credentials
note right of OC: store per-instance client
end
WB->>MW: GET action=query&meta=tokens&type=csrf (OAuth-signed)
MW-->>WB: { query: { tokens: { csrftoken } } }
WB-->>API: { token: csrftoken }
API-->>Client: 200 { token: "<csrf>" }
rect rgba(255,230,230,0.6)
note over WB,API: Error path -> throw / return 4xx-5xx
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/api/wikibase/index.ts(2 hunks)backend/src/api/wikibase/schemas.ts(1 hunks)backend/src/services/wikibase-clients.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/services/wikibase-clients.ts (2)
backend/src/services/mediawiki-api.service.ts (1)
MediaWikiApiService(3-80)backend/src/api/wikibase/schemas.ts (4)
OAuthCredentials(4-17)OAuthCredentials(18-18)CSRFTokenResponse(20-26)CSRFTokenResponse(27-27)
backend/src/api/wikibase/index.ts (1)
backend/src/api/wikibase/schemas.ts (2)
OAuthCredentials(4-17)OAuthCredentials(18-18)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
backend/src/services/wikibase-clients.ts (1)
13-16: Configured timeout isn’t enforced by MediaWikiApiService.You set
timeout: 30000butMediaWikiApiService.request()doesn’t use it, so calls won’t actually time out. Either wire it up or drop the field to avoid false guarantees.Outside-changes snippet (mediawiki-api.service.ts) to enforce timeouts:
// inside request() const controller = new AbortController() const id = setTimeout(() => controller.abort(), this.config.timeout ?? 30000) try { const response = await fetch(url.toString(), { ...options, signal: controller.signal }) ... } finally { clearTimeout(id) }backend/src/api/wikibase/schemas.ts (2)
4-17: Tighten credential field validation.Prevent empty strings to fail fast before hitting the provider.
Apply:
- consumerKey: t.String({ + consumerKey: t.String({ + minLength: 1, description: 'Consumer key', }), - consumerSecret: t.String({ + consumerSecret: t.String({ + minLength: 1, description: 'Consumer secret', }), - accessToken: t.String({ + accessToken: t.String({ + minLength: 1, description: 'Access token', }), - accessTokenSecret: t.String({ + accessTokenSecret: t.String({ + minLength: 1, description: 'Access secret', }),
48-49: InstanceId literals are clear; consider centralizing mapping.Since adding a new instance requires edits across schema, endpoints, and clients, consider exporting a single source of truth (e.g., a
const INSTANCES = ['wikidata','commons'] as const) and derive both the union and endpoint map from it to avoid drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
backend/src/api/wikibase/index.ts(3 hunks)backend/src/api/wikibase/schemas.ts(2 hunks)backend/src/services/constraint-validation.service.ts(4 hunks)backend/src/services/mediawiki-api.service.ts(2 hunks)backend/src/services/wikibase-clients.ts(1 hunks)backend/src/services/wikibase.service.ts(6 hunks)backend/src/types/mediawiki-api.ts(1 hunks)backend/tests/api/project/project.wikibase.test.ts(2 hunks)frontend/src/features/data-processing/composables/useDataTypeCompatibility.ts(0 hunks)frontend/src/features/wikibase-schema/composables/__tests__/useSchemaApi.test.ts(18 hunks)frontend/src/features/wikibase-schema/composables/__tests__/useSchemaCompletenessIntegration.test.ts(3 hunks)frontend/src/features/wikibase-schema/composables/__tests__/useSchemaCompletenessValidation.test.ts(7 hunks)frontend/src/features/wikibase-schema/composables/__tests__/useSchemaValidationUI.test.ts(4 hunks)frontend/src/features/wikibase-schema/composables/useSchemaApi.ts(1 hunks)frontend/src/features/wikibase-schema/composables/useSchemaSelection.ts(1 hunks)frontend/src/features/wikibase-schema/stores/__tests__/schema.store.test.ts(1 hunks)frontend/src/features/wikibase-schema/stores/schema.store.ts(3 hunks)
💤 Files with no reviewable changes (1)
- frontend/src/features/data-processing/composables/useDataTypeCompatibility.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/src/features/wikibase-schema/composables/tests/useSchemaCompletenessValidation.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/api/wikibase/index.ts
🧰 Additional context used
🧬 Code graph analysis (7)
backend/src/services/mediawiki-api.service.ts (1)
backend/src/types/mediawiki-api.ts (1)
MediaWikiConfig(12-19)
frontend/src/features/wikibase-schema/stores/schema.store.ts (1)
backend/src/api/wikibase/schemas.ts (2)
InstanceId(48-48)InstanceId(49-49)
backend/src/services/wikibase-clients.ts (2)
backend/src/api/wikibase/schemas.ts (6)
InstanceId(48-48)InstanceId(49-49)OAuthCredentials(4-17)OAuthCredentials(18-18)CSRFTokenResponse(20-26)CSRFTokenResponse(27-27)backend/src/services/mediawiki-api.service.ts (1)
MediaWikiApiService(3-79)
backend/src/services/constraint-validation.service.ts (1)
backend/src/api/wikibase/schemas.ts (2)
InstanceId(48-48)InstanceId(49-49)
backend/tests/api/project/project.wikibase.test.ts (1)
backend/src/api/wikibase/schemas.ts (2)
InstanceId(48-48)InstanceId(49-49)
backend/src/services/wikibase.service.ts (2)
backend/src/api/wikibase/schemas.ts (2)
InstanceId(48-48)InstanceId(49-49)backend/src/types/wikibase-schema.ts (4)
PropertyId(11-16)PropertyId(17-17)ItemId(3-8)ItemId(9-9)
frontend/src/features/wikibase-schema/composables/useSchemaApi.ts (1)
backend/src/api/wikibase/schemas.ts (2)
InstanceId(48-48)InstanceId(49-49)
🔇 Additional comments (19)
frontend/src/features/wikibase-schema/composables/__tests__/useSchemaCompletenessIntegration.test.ts (3)
69-70: InstanceId literal matches store expectationUsing
'wikidata'here keeps the integration test aligned with the new InstanceId-based schema default, so the expectation reads cleanly.
107-107: Consistent literal for incomplete workflow setupSetting the store to
'wikidata'maintains consistency with the new canonical value while exercising the incomplete-statement path.
153-154: Validation state uses canonical instanceAssigning
'wikidata'before checking the validation state ensures this path reflects the canonical InstanceId and avoids mismatched expectations.frontend/src/features/wikibase-schema/stores/__tests__/schema.store.test.ts (1)
74-83: Updated default matches InstanceId-backed store stateConfirming the default expectation to
'wikidata'keeps the test aligned with the store’s new InstanceId initializer and reset behavior. Nice catch.frontend/src/features/wikibase-schema/composables/useSchemaApi.ts (1)
6-10: SchemaRequest now enforces InstanceIdImporting
InstanceIdand tightening the request type keeps the frontend contract in sync with the backend schema—good defensive typing.backend/tests/api/project/project.wikibase.test.ts (1)
7-8: Test data aligned with InstanceId literalsPulling in the new type and asserting
'commons'ensures the PUT path is exercising the expanded InstanceId domain. Looks solid.Also applies to: 389-392
frontend/src/features/wikibase-schema/composables/useSchemaSelection.ts (1)
50-56: Canonical wikibase preset preserved when starting freshSeeding new schemas with
'wikidata'keeps the selection flow consistent with the store’s typed default. All good.frontend/src/features/wikibase-schema/composables/__tests__/useSchemaValidationUI.test.ts (1)
140-214: Validation tests updated for InstanceId-backed storeSwitching the test setup to use the
'wikidata'literal keeps the validation expectations consistent with the new InstanceId handling. No gaps spotted.backend/src/types/mediawiki-api.ts (1)
14-14: LGTM: Required userAgent aligns with service changes.This change makes
userAgentrequired, which correctly aligns with the updated MediaWikiApiService that now usesthis.config.userAgentdirectly without fallback. Based on the web search results, MediaWiki APIs typically require a User-Agent header for requests, making this a sensible API contract improvement.backend/src/services/mediawiki-api.service.ts (2)
4-4: LGTM: Public config access enables per-instance usage.Making the config public allows external services to access the configuration, which aligns with the PR's objective of supporting per-instance OAuth workflows where different clients may need access to instance-specific configurations.
61-61: LGTM: Direct userAgent usage consistent with type change.The direct use of
this.config.userAgentis now safe since theMediaWikiConfiginterface makesuserAgentrequired. This removes the previous implicit fallback and ensures consistent User-Agent headers across requests.frontend/src/features/wikibase-schema/composables/__tests__/useSchemaApi.test.ts (2)
60-60: LGTM: Consistent adoption of 'wikidata' as default instance.The tests now consistently use
'wikidata'as the standard wikibase value, replacing previous URLs and empty strings. This aligns with the newInstanceIdtype system and provides better type safety and consistency across the application.Also applies to: 205-205, 211-211, 316-316, 337-337, 372-372, 381-381, 421-421, 447-447, 562-562, 579-579, 601-601, 618-618, 656-656, 683-683, 710-710, 761-761
232-234: Good use of TypeScript error directives for invalid test cases.The
@ts-expect-errorannotations appropriately mark invalid wikibase assignments for testing purposes, ensuring the type system catches invalid usage while allowing explicit testing of error conditions.Also applies to: 543-545
frontend/src/features/wikibase-schema/stores/schema.store.ts (1)
11-11: LGTM: Proper adoption of InstanceId type with sensible defaults.The changes correctly:
- Import the
InstanceIdtype from the backend schema definitions- Initialize the wikibase ref with the
'wikidata'literal that matches the union type- Reset to the same default value
This provides type safety while maintaining backwards compatibility with sensible defaults. Based on learnings from the search results, Wikidata is the most commonly used Wikibase instance, making it an appropriate default choice.
Also applies to: 26-26, 499-499
backend/src/services/constraint-validation.service.ts (1)
1-1: LGTM: Consistent InstanceId adoption across validation methods.The changes properly:
- Import the
InstanceIdtype from the centralized schema definitions- Update all public method signatures to use
InstanceIdfor type safety- Maintain existing caching and error handling logic
This ensures consistent typing across the Wikibase service layer and provides better type safety for instance-specific constraint validation operations.
Also applies to: 19-19, 398-398, 488-488
backend/src/services/wikibase-clients.ts (2)
6-9: SSRF risk addressed — endpoints are now server-controlled.Hardcoding per-instance API endpoints removes the earlier SSRF vector. Good fix.
3-3: Incorrect suggestion:import { OAuth } from 'oauth'isn’t valid
Theoauthpackage’s typings use CommonJS exports (no namedOAuthexport), soimport { OAuth } from 'oauth'will fail even withesModuleInteropenabled. Continue using the default import (or theimport OAuth = require('oauth')form) and reference the class directly (e.g.new OAuth(...)) rather than switching to a non‐existent named export.Likely an incorrect or invalid review comment.
backend/src/api/wikibase/schemas.ts (1)
20-27: CSRFTokenResponse shape looks correct.Matches MediaWiki’s
meta=tokens&type=csrfresponse.backend/src/services/wikibase.service.ts (1)
35-36: Typed InstanceId adoption verified — no remaining raw string callers.
Ran RG search across codebase; all calls to fetchAllProperties, searchProperties, getProperty, searchItems, and getItem now use typed instanceId, with no raw'wikidata'or'commons'arguments.
| private credentials: Partial<Record<InstanceId, OAuthCredentials>> = {} | ||
| private authenticatedClients: Partial<Record<InstanceId, OAuth.OAuth>> = {} |
There was a problem hiding this comment.
Do not retain OAuth credentials in memory.
this.credentials stores secrets beyond the request lifetime and gets overwritten per instance; unnecessary and a privacy/compliance risk. Remove it; pass creds per call only (you already do).
Apply:
- private credentials: Partial<Record<InstanceId, OAuthCredentials>> = {}
private authenticatedClients: Partial<Record<InstanceId, OAuth.OAuth>> = {}
...
- this.credentials[instanceId] = credentialsAlso applies to: 38-38
| this.authenticatedClients[instanceId] = new OAuth.OAuth( | ||
| this.endpoints[instanceId], | ||
| this.endpoints[instanceId], |
There was a problem hiding this comment.
Use MediaWiki OAuth1 endpoints, not the Action API URL.
OAuth.OAuth’s first two args must be the request/access token endpoints. Using /w/api.php is wrong (works only by accident because you’re not calling token flows). Also set a request timeout on the OAuth client.
Apply:
- this.authenticatedClients[instanceId] = new OAuth.OAuth(
- this.endpoints[instanceId],
- this.endpoints[instanceId],
+ const base = new URL(this.endpoints[instanceId]).origin
+ const requestTokenUrl = `${base}/w/index.php?title=Special:OAuth/initiate`
+ const accessTokenUrl = `${base}/w/index.php?title=Special:OAuth/token`
+ const oa = new OAuth.OAuth(
+ requestTokenUrl,
+ accessTokenUrl,
credentials.consumerKey,
credentials.consumerSecret,
'1.0',
null,
'HMAC-SHA1',
undefined,
{
'User-Agent': client.config.userAgent,
},
)
+ // Ensure outbound calls don’t hang indefinitely
+ oa.setClientOptions?.({ timeout: 30000 })
+ this.authenticatedClients[instanceId] = oaBased on learnings.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.authenticatedClients[instanceId] = new OAuth.OAuth( | |
| this.endpoints[instanceId], | |
| this.endpoints[instanceId], | |
| const base = new URL(this.endpoints[instanceId]).origin | |
| const requestTokenUrl = `${base}/w/index.php?title=Special:OAuth/initiate` | |
| const accessTokenUrl = `${base}/w/index.php?title=Special:OAuth/token` | |
| const oa = new OAuth.OAuth( | |
| requestTokenUrl, | |
| accessTokenUrl, | |
| credentials.consumerKey, | |
| credentials.consumerSecret, | |
| '1.0', | |
| null, | |
| 'HMAC-SHA1', | |
| undefined, | |
| { | |
| 'User-Agent': client.config.userAgent, | |
| }, | |
| ) | |
| // Ensure outbound calls don’t hang indefinitely | |
| oa.setClientOptions?.({ timeout: 30000 }) | |
| this.authenticatedClients[instanceId] = oa |
| async getCsrfToken( | ||
| instanceId: InstanceId, | ||
| credentials: OAuthCredentials, | ||
| ): Promise<CSRFTokenResponse> { | ||
| const authService = this.getAuthenticatedClient(instanceId, credentials) | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
| authService.get( | ||
| this.endpoints[instanceId] + | ||
| '?' + | ||
| new URLSearchParams({ | ||
| action: 'query', | ||
| meta: 'tokens', | ||
| format: 'json', | ||
| }).toString(), | ||
| credentials.accessToken, | ||
| credentials.accessTokenSecret, | ||
| (err, data) => { | ||
| if (err) { | ||
| console.error(err) | ||
| reject(err) | ||
| return | ||
| } | ||
| resolve(JSON.parse(data as string) as CSRFTokenResponse) | ||
| }, | ||
| ) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Be explicit with type=csrf and harden JSON parsing.
Add type=csrf to the query to avoid provider defaults changing; also guard JSON.parse errors and validate shape.
Apply:
authService.get(
this.endpoints[instanceId] +
'?' +
new URLSearchParams({
action: 'query',
meta: 'tokens',
+ type: 'csrf',
format: 'json',
}).toString(),
credentials.accessToken,
credentials.accessTokenSecret,
(err, data) => {
if (err) {
console.error(err)
reject(err)
return
}
- resolve(JSON.parse(data as string) as CSRFTokenResponse)
+ try {
+ const parsed = JSON.parse(data as string) as CSRFTokenResponse
+ if (!parsed?.query?.tokens?.csrftoken) {
+ return reject(new Error('Missing csrftoken in response'))
+ }
+ resolve(parsed)
+ } catch (e) {
+ reject(e)
+ }
},
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getCsrfToken( | |
| instanceId: InstanceId, | |
| credentials: OAuthCredentials, | |
| ): Promise<CSRFTokenResponse> { | |
| const authService = this.getAuthenticatedClient(instanceId, credentials) | |
| return new Promise((resolve, reject) => { | |
| authService.get( | |
| this.endpoints[instanceId] + | |
| '?' + | |
| new URLSearchParams({ | |
| action: 'query', | |
| meta: 'tokens', | |
| format: 'json', | |
| }).toString(), | |
| credentials.accessToken, | |
| credentials.accessTokenSecret, | |
| (err, data) => { | |
| if (err) { | |
| console.error(err) | |
| reject(err) | |
| return | |
| } | |
| resolve(JSON.parse(data as string) as CSRFTokenResponse) | |
| }, | |
| ) | |
| }) | |
| } | |
| async getCsrfToken( | |
| instanceId: InstanceId, | |
| credentials: OAuthCredentials, | |
| ): Promise<CSRFTokenResponse> { | |
| const authService = this.getAuthenticatedClient(instanceId, credentials) | |
| return new Promise((resolve, reject) => { | |
| authService.get( | |
| this.endpoints[instanceId] + | |
| '?' + | |
| new URLSearchParams({ | |
| action: 'query', | |
| meta: 'tokens', | |
| type: 'csrf', | |
| format: 'json', | |
| }).toString(), | |
| credentials.accessToken, | |
| credentials.accessTokenSecret, | |
| (err, data) => { | |
| if (err) { | |
| console.error(err) | |
| reject(err) | |
| return | |
| } | |
| try { | |
| const parsed = JSON.parse(data as string) as CSRFTokenResponse | |
| if (!parsed?.query?.tokens?.csrftoken) { | |
| return reject(new Error('Missing csrftoken in response')) | |
| } | |
| resolve(parsed) | |
| } catch (e) { | |
| reject(e) | |
| } | |
| }, | |
| ) | |
| }) | |
| } |
🤖 Prompt for AI Agents
In backend/src/services/wikibase-clients.ts around lines 56 to 83, the query
that requests tokens should explicitly include type=csrf and the JSON parsing
must be hardened: add type: 'csrf' to the URLSearchParams; wrap JSON.parse in a
try/catch and reject with a descriptive error if parsing fails; after parsing,
validate the parsed object has the expected shape (e.g., top-level
query.tokens.csrf or whatever CSRFTokenResponse requires) and reject if
validation fails; keep returning the parsed CSRFTokenResponse on success.
No description provided.