mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-11-12 13:13:45 +01:00
[web] gracefully handle failed oauth token exchange for reference managers (#36377)
* [web] gracefully handle failed oauth token exchange for reference managers Mendeley/ReadCube redirect back to /:ref_provider/oauth/token-exchange with `error` and `error_description` instead of `code` when a user declines consent. completeAuth forwarded the whole query string on to third-party-references, whose strict zod schema (d8c1679bb08) accepts only oauth_token, oauth_verifier and code, so the exchange failed and the user got a 500. The same forwarding also sent `state` on every callback, success or failure. `state` is web's own CSRF token and is never read by third-party-references, so it tripped the log-only schema on every single token exchange, and would have broken linking outright under REQ_VALIDATION_MODE=enforce. Forward only the credentials the exchange needs, and show a translated notification in the Reference managers section rather than erroring. Validate ref_provider against the known providers so the provider name can be interpolated into the message without reflecting a URL segment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [web] fix providerName argument and validate before deriving refProvider Address Copilot review on #36377: - The token-exchange failure handler passed `false` positionally into the `providerName` slot of _redirectWithLinkingError, so a failed exchange would have rendered "your false account". Pass providerName and an explicit false. - _getRefProviderBackendKey(req) ran before parseReq, so refProvider was derived from raw, unvalidated req.params. Parse first and derive from the validated params via _toRefProviderBackendKey; _getRefProviderBackendKey now delegates to it so the other handlers are unchanged. The completeAuth tests kept their assertions inside ctx.res.callback without awaiting the controller, so the callback fired after the test had already returned and the assertions were never enforced — which is why the suite was green with the providerName bug in it. Await the controller and assert inline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [web] add logOnly to the completeAuth schema and guard the provider lookup logOnly returns the raw input on a schema failure, so ref_provider can be an arbitrary URL segment. Check it against the display-name table rather than trusting the schema: unknown providers 404 in both validation modes, and providerName is guaranteed non-null for the rest of the handler. Also covers the schema half under REQ_VALIDATION_MODE=enforce, so dropping ref_provider from completeAuthSchema fails a test rather than passing unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> GitOrigin-RevId: b98eb5414fe8b672f44aab8c3690f7637d434cd2
This commit is contained in:
committed by
Copybot
co-authored by
Claude Opus 5
parent
6e57b24811
commit
9cf4fa8bf5
@@ -34,6 +34,10 @@ async function settingsPage(req, res) {
|
||||
req.session,
|
||||
'projectSyncErrorMessage'
|
||||
)
|
||||
const referenceLinkingErrorMessage = popSessionValue(
|
||||
req.session,
|
||||
'referenceLinkingErrorMessage'
|
||||
)
|
||||
// Institution SSO
|
||||
let institutionLinked = _.get(req.session, ['saml', 'linked'])
|
||||
if (institutionLinked) {
|
||||
@@ -171,6 +175,7 @@ async function settingsPage(req, res) {
|
||||
thirdPartyIds: UserPagesController._restructureThirdPartyIds(user),
|
||||
projectSyncSuccessMessage,
|
||||
projectSyncErrorMessage,
|
||||
referenceLinkingErrorMessage,
|
||||
personalAccessTokens,
|
||||
emailAddressLimit: Settings.emailAddressLimit,
|
||||
isManagedAccount: !!req.managedBy,
|
||||
|
||||
@@ -43,6 +43,10 @@ block append meta
|
||||
meta(name='ol-github' data-type='json' content=github)
|
||||
meta(name='ol-projectSyncSuccessMessage' content=projectSyncSuccessMessage)
|
||||
meta(name='ol-projectSyncErrorMessage' content=projectSyncErrorMessage)
|
||||
meta(
|
||||
name='ol-referenceLinkingErrorMessage'
|
||||
content=referenceLinkingErrorMessage
|
||||
)
|
||||
meta(
|
||||
name='ol-personalAccessTokens'
|
||||
data-type='json'
|
||||
|
||||
@@ -25,6 +25,9 @@ function LinkingSection() {
|
||||
const cannotUseAi = getMeta('ol-cannot-use-ai')
|
||||
const projectSyncSuccessMessage = getMeta('ol-projectSyncSuccessMessage')
|
||||
const projectSyncErrorMessage = getMeta('ol-projectSyncErrorMessage')
|
||||
const referenceLinkingErrorMessage = getMeta(
|
||||
'ol-referenceLinkingErrorMessage'
|
||||
)
|
||||
|
||||
// hide linking widgets in CI
|
||||
const integrationLinkingWidgets = getMeta('ol-hideLinkingWidgets')
|
||||
@@ -133,6 +136,14 @@ function LinkingSection() {
|
||||
{hasReferencesLinkingSection ? (
|
||||
<>
|
||||
<h3 id="references">{t('reference_managers')}</h3>
|
||||
{referenceLinkingErrorMessage ? (
|
||||
<div className="notification-list">
|
||||
<Notification
|
||||
type="error"
|
||||
content={referenceLinkingErrorMessage}
|
||||
/>
|
||||
</div>
|
||||
) : null}
|
||||
<div className="settings-widgets-container">
|
||||
{referenceLinkingWidgets.map(
|
||||
({ import: importObject }, widgetIndex) => (
|
||||
|
||||
@@ -289,6 +289,7 @@ export interface Meta {
|
||||
| 'zotero_multiple_collections'
|
||||
| null
|
||||
}
|
||||
'ol-referenceLinkingErrorMessage': string
|
||||
'ol-ro-mirror-on-client-no-local-storage': boolean
|
||||
'ol-samlError': SAMLError | undefined
|
||||
'ol-script-log': ScriptLogType
|
||||
|
||||
@@ -2130,6 +2130,8 @@
|
||||
"reference_count_plural": "__count__ references",
|
||||
"reference_error_relink_hint": "If this error persists, try re-linking your account here:",
|
||||
"reference_format": "Reference format",
|
||||
"reference_manager_link_denied": "We couldn’t link your __provider__ account. To finish linking, allow __appName__ access when you’re asked.",
|
||||
"reference_manager_link_error": "Sorry, we weren’t able to link your __provider__ account. Please try again in a few moments.",
|
||||
"reference_manager_linked": "Reference manager linked",
|
||||
"reference_manager_lowercase": "reference manager",
|
||||
"reference_manager_searched_groups": "__provider__ search groups",
|
||||
|
||||
Reference in New Issue
Block a user