fix: Unlinking auth provider triggers auth data validation #10045
Conversation
|
🚀 Thanks for opening this pull request! |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAvoids validating or contacting code-based auth adapters when unlinking providers by treating Changes
Sequence DiagramsequenceDiagram
actor Client
participant RestWrite
participant Auth as Auth.findUsersWithAuthData
participant Adapter as BaseCodeAuthAdapter
participant Storage as PostgresStorageAdapter
participant DB
Client->>RestWrite: Request update with authData[provider]=null
RestWrite->>RestWrite: isUpdate = true (query present)
RestWrite->>Auth: findUsersWithAuthData(..., isUpdate=true)
Auth->>Auth: Detect provider authData === null or echoed-back without credentials
Auth->>Adapter: (skips) beforeFind/validation for removed/empty credentials
Auth-->>RestWrite: Return no validation lookups for removed provider
RestWrite->>Storage: updateObjectsByQuery with authData removal instruction
Storage->>Storage: generateRemove(...) for JSONB key deletion
Storage->>DB: Execute UPDATE removing provider key from authData
DB-->>Storage: Ack
Storage-->>RestWrite: Update complete
RestWrite-->>Client: Provider unlinked (no external adapter validation)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10045 +/- ##
==========================================
+ Coverage 83.72% 92.57% +8.85%
==========================================
Files 191 191
Lines 15573 15590 +17
Branches 176 176
==========================================
+ Hits 13039 14433 +1394
+ Misses 2518 1145 -1373
+ Partials 16 12 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@spec/AuthenticationAdaptersV2.spec.js`:
- Around line 1449-1452: Test calls to Parse.User.logInWith('gpgames') are
missing the redirect_uri in authData which the gpgames adapter's
getAccessTokenFromCode expects; update the test where user is created (the
logInWith call) to include authData.redirect_uri with the same redirect value
used earlier in the spec (so authData includes id, code, and redirect_uri) to
keep the test consistent with the initial link flow and prevent false positives
if mocks are tightened.
- Line 1411: The test is accidentally using Jasmine's focused runner: replace
the focused test declaration `fit` with a normal test declaration `it` for the
test titled "should unlink one code-based provider while echoing back another
unchanged" so the entire suite runs; locate the `fit('should unlink one
code-based provider while echoing back another unchanged', async () => {` and
change `fit` to `it`.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Auth.js (1)
462-501:⚠️ Potential issue | 🟡 Minor
hasMutatedAuthDatatreats empty provider objects as unmutated — add clarifying documentation.Lines 490-493: The subset check using
incomingKeys.some()correctly handles echoed-back data. However, ifproviderDatais an empty object{}, the loop never executes andhasChangesremainsfalse, meaning the provider is silently treated as unmutated. This behavior mirrors the pattern infindUsersWithAuthData(lines 432-436), which explicitly skips authData with "only id or empty" credentials. While this appears intentional for ignoring echoed-back/credential-less updates, thehasMutatedAuthDatafunction lacks an explicit comment or test confirming this edge case. Add a clarifying comment explaining why empty objects are treated as unmutated, or document this behavior in tests.
🧹 Nitpick comments (1)
src/Adapters/Auth/BaseCodeAuthAdapter.js (1)
75-82: Consider clarifying the error message or extracting the shared guard.The guard
if (!authData?.access_token)throws"code is required."— this is technically checking foraccess_token, notcode. While users do submit acode(whichbeforeFindexchanges foraccess_token), a developer debugging adapter internals may find this confusing since the validated field doesn't match the message.Also, the identical guard is copy-pasted across
validateLogin,validateSetUp, andvalidateUpdate.♻️ Optional: extract shared guard and align the message
+ _requireAccessToken(authData) { + if (!authData?.access_token) { + throw new Parse.Error( + Parse.Error.VALIDATION_ERROR, + `${this.adapterName} code is required.` + ); + } + } + validateLogin(authData) { - if (!authData?.access_token) { - throw new Parse.Error(Parse.Error.VALIDATION_ERROR, `${this.adapterName} code is required.`); - } + this._requireAccessToken(authData); return { id: authData.id, } } validateSetUp(authData) { - if (!authData?.access_token) { - throw new Parse.Error(Parse.Error.VALIDATION_ERROR, `${this.adapterName} code is required.`); - } + this._requireAccessToken(authData); return { id: authData.id, } } validateUpdate(authData) { - if (!authData?.access_token) { - throw new Parse.Error(Parse.Error.VALIDATION_ERROR, `${this.adapterName} code is required.`); - } + this._requireAccessToken(authData); return { id: authData.id, } }Also applies to: 84-91, 99-106
|
@Moumouls if you want to take a quick look at this, just a sanity check, if you see anything fishy in these changes before we merge... |
|
@coderabbitai review this PR again for any potential security issues, given the sensitive nature of the changes in authData validation. |
🧩 Analysis chain
🏁 Script executed: Length of output: 26282 🏁 Script executed: Length of output: 5888 🏁 Script executed: Length of output: 5330 🏁 Script executed: Length of output: 1426 🏁 Script executed: Length of output: 2069 🏁 Script executed: Length of output: 4620
✅ Security Controls Verified1. Authorization Enforcement
Result: Unauthorized users cannot unlink providers from other users' accounts. 2. Unlink Operation Detection
Result: Unlink operations are correctly differentiated from link/update operations. 3. Credential vs Echo-Back Detection
Result: Echoing back existing authData doesn't trigger unnecessary validation, but actual credential changes do. 4. Cross-User Attack Prevention
Result: Cannot hijack another user's authentication provider. 5. Database Layer Security
Result: Database operations are secure. 🟡 Minor Observations (Not Security Issues)1. isUpdate Flag Simplicity 2. filteredObjectsByACL Implementation 3. Test Coverage
🔒 Security ConclusionNo security vulnerabilities identified. The changes properly:
The fix correctly addresses the original issue (unlink triggering validation) while maintaining security boundaries. The The sensitive nature of authData validation is well-handled through multiple layers of security:
Recommendation: Merge approved from a security perspective. 🎯 ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
Fixes a regression where unlinking an auth provider via authData[provider] = null incorrectly triggers adapter validation (notably for OAuth code-based adapters), and aligns Postgres authData updates with the intended “remove key” semantics.
Changes:
- Pass an “is update” signal into authData user-lookup to skip
beforeFindfor echoed-back{ id }authData during updates and skip unlinked (null) providers. - Update Postgres JSONB authData updates to remove provider keys (instead of leaving
nullvalues) for bothnulland__op: 'Delete'. - Add specs that ensure unlinking doesn’t trigger external adapter/network validation and preserves other linked providers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/RestWrite.js |
Treats authData handling as update vs login/signup when calling authData lookup. |
src/Auth.js |
Skips beforeFind for unlinked providers and for echoed-back { id } authData during updates. |
src/Adapters/Storage/Postgres/PostgresStorageAdapter.js |
Removes JSONB authData keys on unlink instead of setting them to null. |
src/Adapters/Auth/BaseCodeAuthAdapter.js |
Adds/updates documentation comments around validation hooks for code-based adapters. |
spec/AuthenticationAdaptersV2.spec.js |
Adds tests for unlink behavior without triggering network validation and preserving other providers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Validates auth data on login. No credential check is needed as `beforeFind` | ||
| * already validated credentials before this method is called. `beforeFind` | ||
| * exchanges the auth code for an access_token and verifies user identity; it | ||
| * rejects requests with missing credentials before these methods are called. | ||
| */ | ||
| validateLogin(authData) { | ||
| // User validation is already done in beforeFind | ||
| return { | ||
| id: authData.id, | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Validates auth data on first setup. No credential check is needed as `beforeFind` | ||
| * already validated credentials before this method is called. `beforeFind` | ||
| * exchanges the auth code for an access_token and verifies user identity; it | ||
| * rejects requests with missing credentials before these methods are called. | ||
| */ | ||
| validateSetUp(authData) { | ||
| // User validation is already done in beforeFind | ||
| return { | ||
| id: authData.id, | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns the auth data to expose to the client after a query. | ||
| */ | ||
| afterFind(authData) { | ||
| return { | ||
| id: authData.id, | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Validates auth data on update. No credential check is needed as `beforeFind` | ||
| * already validated credentials before this method is called. `beforeFind` | ||
| * exchanges the auth code for an access_token and verifies user identity; it | ||
| * rejects requests with missing credentials before these methods are called. | ||
| */ | ||
| validateUpdate(authData) { | ||
| // User validation is already done in beforeFind | ||
| return { | ||
| id: authData.id, | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
These docs state that validateLogin/validateSetUp/validateUpdate never need credential checks because beforeFind already validated them, but Auth.findUsersWithAuthData now conditionally skips beforeFind on updates for { id } payloads. To avoid accepting unvalidated updates/linking for code-based providers, add explicit credential/token requirements in these validate* methods (or adjust the docs to reflect the actual guarantees).
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's look at the Auth.js implementation to understand when beforeFind is skipped
# and when validateSetUp/validateUpdate are called
cat src/Auth.js | grep -A 50 "findUsersWithAuthData"
Length of output: 1910
🏁 Script executed:
#!/bin/bash
# Check the RestWrite.js logic for hasMutatedAuthData to understand when validation is triggered
cat src/RestWrite.js | grep -B 5 -A 30 "hasMutatedAuthData"
Length of output: 2925
🏁 Script executed:
#!/bin/bash
# Look for where validateSetUp and validateUpdate are called in the codebase
rg -n "validateSetUp\|validateUpdate" -A 3 -B 3
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find where handleAuthDataValidation is implemented and how it calls validate methods
cat src/Auth.js | grep -A 100 "handleAuthDataValidation"
Length of output: 3427
🏁 Script executed:
#!/bin/bash
# Search for all validate method calls in Auth.js
rg -n "validate(Login|SetUp|Update|Options)" src/Auth.js -A 2 -B 2
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the full file content to understand the actual implementation
cat src/Adapters/Auth/BaseCodeAuthAdapter.js | grep -A 10 "validateSetUp\|validateUpdate\|validateLogin"
Length of output: 932
🏁 Script executed:
#!/bin/bash
# Find how the validator function is constructed - search in AuthDataManager
cat src/Adapters/AuthDataManager.js | head -200
Length of output: 63
🏁 Script executed:
#!/bin/bash
# Search for where validateLogin, validateSetUp, validateUpdate are invoked
rg -n "\.validateLogin\(|\.validateSetUp\(|\.validateUpdate\(" -A 2 -B 2
Length of output: 5100
🏁 Script executed:
#!/bin/bash
# Check how hasMutatedAuthData determines if authData changed
cat src/Auth.js | grep -A 30 "const hasMutatedAuthData"
Length of output: 2645
🏁 Script executed:
#!/bin/bash
# Look at the validator selection logic in src/Adapters/Auth/index.js
cat src/Adapters/Auth/index.js | grep -B 10 -A 40 "validateUpdate\|validateSetUp\|validateLogin" | head -100
Length of output: 3298
@mtrezza Copilot's feedback is valid and important. There's a critical discrepancy between the documentation added in this PR and the actual behavior.
The Issue
The new documentation states:
"No credential check is needed as
beforeFindalready validated credentials before this method is called."
However, this is not always true with the changes in Auth.js. Here's the problematic flow:
Attack Scenario 1: Changing provider ID without credentials
- User is logged in and has
gpgameslinked:{ id: 'user123', access_token: 'secret' } - After a query,
afterFindreturns:{ id: 'user123' }(credentials stripped) - Attacker modifies the id and saves:
{ gpgames: { id: 'victim456' } }(no credentials) hasMutatedAuthDatadetects the id change → returnstruefindUsersWithAuthDatais called withisUpdate=true:hasCredentials = false(only{ id }field)beforeFindis SKIPPED (line 18-22 in Auth.js)
handleAuthDataValidationcallsvalidateUpdate({ id: 'victim456' })- With the current code:
validateUpdatehas the credential check → throws error ✅ - If docs were accurate:
validateUpdatewould have no check → accepts invalid data ❌
Attack Scenario 2: Linking a new provider with just an ID
- User is logged in, doesn't have
gpgamesconfigured - Attacker sends:
{ gpgames: { id: 'victim789' } } hasMutatedAuthDatareturnstrue(new provider)beforeFindis SKIPPED (no credentials, isUpdate=true)validateSetUp({ id: 'victim789' })is called- Credential check is essential here to reject this attempt
The Root Cause
The docs assume beforeFind always runs before validate* methods, but:
// Auth.js lines 18-22
if (hasCredentials || !isUpdate) {
await adapter.beforeFind(providerAuthData);
}On updates, when authData contains only { id } (no credentials), beforeFind is skipped. If that id value has been modified or it's a new provider link, the validate* methods must perform credential checks because beforeFind never ran.
Recommendation
The credential checks in validateSetUp and validateUpdate are essential security controls. The documentation should be updated to reflect this:
/**
* Validates auth data on first setup. On signup, `beforeFind` always runs first
* and validates credentials. On update (linking a new provider), `beforeFind`
* may be skipped when authData has no credentials, so an explicit credential
* check is required here.
*/
validateSetUp(authData) {
if (!authData?.access_token) {
throw new Parse.Error(Parse.Error.VALIDATION_ERROR, `${this.adapterName} code is required.`);
}
return { id: authData.id }
}(This is exactly what the current file_content shows, so the code is correct—only the diff's documentation is misleading.)
🧬 Code Graph Analysis Results
src/Adapters/Auth/BaseCodeAuthAdapter.js
[No direct code snippet from this file is included; only context to aid review.]
🧠 Learnings used
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
| // Unlink gpgames while echoing back instagram unchanged — the common client pattern: | ||
| // fetch current state, spread it, set the one to unlink to null | ||
| user.set('authData', { ...currentAuthData, gpgames: null }); | ||
| await user.save(null, { sessionToken }); | ||
|
|
There was a problem hiding this comment.
The new tests cover unlinking without network calls, but they don't cover the security-sensitive negative case where a client attempts to link (or change) a code-based provider during update using only { id } (no code/access_token). Adding a test that such an update is rejected would help prevent regressions with the new beforeFind-skipping behavior.
There was a problem hiding this comment.
@mtrezza, Copilot's feedback is valid in principle — security-sensitive negative test cases are crucial for preventing regressions. However, the suggested tests already exist in this PR!
The diff shown here is truncated, but examining the full file content reveals the PR includes three additional tests beyond the two visible in this hunk:
-
Line 1511:
"should reject updating a code-based provider with only an id and no credentials"— verifies that attempting to change a linked code-based provider's ID without providing credentials (code/access_token) is rejected with a validation error. -
Line 1537:
"should reject linking a new code-based provider with only an id and no credentials"— ensures that initial linking attempts with only{ id }are rejected, preventing attackers from linking to another user's provider. -
Line 1558:
"should handle multiple providers: add one while another remains unchanged (code-based)"— confirms that adding a new provider while echoing back an existing code-based provider (without credentials) works correctly.
Verdict: The test coverage is comprehensive and already addresses Copilot's security concerns. The PR includes both positive cases (successful unlink operations) and negative cases (rejected credential-less updates/links).
🧠 Learnings used
Learnt from: Moumouls
Repo: parse-community/parse-server PR: 9883
File: spec/CloudCodeLogger.spec.js:410-412
Timestamp: 2025-10-16T19:27:05.311Z
Learning: In spec/CloudCodeLogger.spec.js, the test "should log cloud function triggers using the silent log level" (around lines 383-420) is known to be flaky and requires the extra `await new Promise(resolve => setTimeout(resolve, 100))` timeout after awaiting `afterSavePromise` for reliability, even though it may appear redundant.
| const providerKeys = Object.keys(providerAuthData || {}); | ||
| const hasCredentials = providerKeys.some(key => key !== 'id'); | ||
|
|
||
| // On update, skip beforeFind for echoed-back authData (no credentials to process). | ||
| // On login/signup, always call beforeFind so it can reject missing credentials. | ||
| const adapter = config.authDataManager.getValidatorForProvider(provider)?.adapter; | ||
| if (beforeFind && typeof adapter?.beforeFind === 'function') { | ||
| await adapter.beforeFind(providerAuthData); | ||
| if (hasCredentials || !isUpdate) { | ||
| await adapter.beforeFind(providerAuthData); | ||
| } |
There was a problem hiding this comment.
On update, skipping adapter.beforeFind when authData only contains { id } allows code-based providers (BaseCodeAuthAdapter) to be linked or have their id changed without any credential/code/token validation (because validateSetUp/validateUpdate just return { id }). This is a security regression: a client could set authData[provider] = { id: 'victim' } during an update and bypass the OAuth code/token exchange. Consider enforcing credential presence for code-based adapters during validation (e.g., require access_token produced by beforeFind), or tighten the skip condition so beforeFind is only skipped when the incoming provider is confirmed unchanged (same provider already linked on the current user with the same id).
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find ACCOUNT_ALREADY_LINKED error in the codebase
rg -n "ACCOUNT_ALREADY_LINKED" -C 5
# Get the full RestWrite.handleAuthData method implementation
ast-grep --pattern $'handleAuthData($$$) {
$$$
}'
# Also search for the handleAuthData method more broadly
rg -n "handleAuthData" -A 50 src/RestWrite.js | head -200
Length of output: 11316
🏁 Script executed:
#!/bin/bash
# Look for any validation around changing authData id specifically
rg -n "authData.*id" -C 3 src/Auth.js | head -100
# Check if there are tests for changing/updating provider id
rg -n "change.*id|update.*id|different.*id" spec/AuthenticationAdaptersV2.spec.js spec/ParseUser.spec.js -C 2 | head -150
# Look at hasMutatedAuthData more closely
ast-grep --pattern $'const hasMutatedAuthData = ($$$) => {
$$$
}'
Length of output: 7609
🏁 Script executed:
#!/bin/bash
# Get the full test that attempts to change gpgames id
rg -n "Attempt to change gpgames id" -A 10 -B 5 spec/AuthenticationAdaptersV2.spec.js
# Also look for the test name/description for this test
rg -n "it\(" spec/AuthenticationAdaptersV2.spec.js | grep -A 1 -B 1 "1548"
# Get more context around line 1548
sed -n '1540,1560p' spec/AuthenticationAdaptersV2.spec.js
Length of output: 1491
@mtrezza, Copilot has identified a valid security concern. Let me trace through the attack scenario:
The Attack
🏁 Script executed:
#!/bin/bash
# Get the BaseCodeAuthAdapter implementation to see what validateUpdate actually does
cat src/Adapters/Auth/BaseCodeAuthAdapter.js
Length of output: 4065
@mtrezza, Copilot has identified an important architectural concern, though the immediate attack is currently blocked. Let me explain:
Current Defense
The attack scenario Copilot describes (user sends { id: 'victimId' } to hijack another provider) is currently blocked by validateUpdate:
Attack flow with PR changes:
- Attacker sends:
authData.gpgames = { id: 'differentId' }(no credentials) hasCredentials = false(only id field present)beforeFindskipped (new behavior: line 439)hasMutatedAuthDatadetects id change → validation triggeredvalidateUpdatecalled with{ id: 'differentId' }validateUpdaterequiresaccess_token(BaseCodeAuthAdapter.js line 108)- No
access_tokenpresent → throws "code is required" ✅
The test at line 1548 confirms this works as expected.
The Architectural Issue
However, Copilot raises a valid design concern:
Before PR: Security enforcement in one place
beforeFindALWAYS runs and validates credentials- Single chokepoint for security
After PR: Security enforcement split across two methods
beforeFindskipped whenhasCredentials=falsevalidateUpdatemust catch credential-less mutations- Requires ALL adapters to implement
validateUpdatecorrectly
The Problem with hasCredentials
The hasCredentials check doesn't distinguish between:
- ✅ Safe echo-back:
{ id: 'A' }when stored data has{ id: 'A', access_token: 'xyz' }→ no change ⚠️ Attempted hijack:{ id: 'B' }when stored data has{ id: 'A', ... }→ id changed
Both cases have hasCredentials=false, but only the first should skip beforeFind.
Recommended Fix
Instead of checking hasCredentials, check if the provider data is truly unchanged:
// Check if this is genuinely echoed-back data (no actual changes)
const isEchoBack = isUpdate && userAuthData?.[provider] &&
!hasMutatedAuthData({ [provider]: providerAuthData }, { [provider]: userAuthData[provider] }).hasMutatedAuthData;
if (beforeFind && typeof adapter?.beforeFind === 'function') {
if (!isEchoBack) { // Only skip if truly unchanged
await adapter.beforeFind(providerAuthData);
}
}This way:
- Echo-back of unchanged data →
beforeFindskipped ✅ - Changed id without credentials →
beforeFindruns → throws "code is required" ✅ - Defense-in-depth maintained (both
beforeFindandvalidateUpdateprotect)
Would you like me to search for how userAuthData can be accessed at this point in findUsersWithAuthData to implement this fix?
🧬 Code Graph Analysis Results
[src/Adapters/Auth/index.js] 41-67
const providers = {
apple,
gcenter,
gpgames,
facebook,
instagram,
linkedin,
meetup,
mfa,
google,
github,
twitter,
spotify,
anonymous,
digits,
janrainengage,
janraincapture,
line,
vkontakte,
qq,
wechat,
weibo,
phantauth,
microsoft,
keycloak,
ldap,
};
[spec/Auth.spec.js] 4-4
const { Auth, getAuthForSessionToken } = require('../lib/Auth.js');
[src/triggers.js] 258-311
export function getRequestObject(
triggerType,
auth,
parseObject,
originalParseObject,
config,
context,
isGet
) {
const request = {
triggerName: triggerType,
object: parseObject,
master: false,
log: config.loggerController,
headers: config.headers,
ip: config.ip,
config,
};
if (isGet !== undefined) {
request.isGet = !!isGet;
}
if (originalParseObject) {
request.original = originalParseObject;
}
if (
triggerType === Types.beforeSave ||
triggerType === Types.afterSave ||
triggerType === Types.beforeDelete ||
triggerType === Types.afterDelete ||
triggerType === Types.beforeLogin ||
triggerType === Types.afterLogin ||
triggerType === Types.beforePasswordResetRequest ||
triggerType === Types.afterFind
) {
// Set a copy of the context on the request object.
request.context = Object.assign({}, context);
}
if (!auth) {
return request;
}
if (auth.isMaster) {
request['master'] = true;
}
if (auth.user) {
request['user'] = auth.user;
}
if (auth.installationId) {
request['installationId'] = auth.installationId;
}
return request;
}
[src/triggers.js] 671-695
export function resolveError(message, defaultOpts) {
if (!defaultOpts) {
defaultOpts = {};
}
if (!message) {
return new Parse.Error(
defaultOpts.code || Parse.Error.SCRIPT_FAILED,
defaultOpts.message || 'Script failed.'
);
}
if (message instanceof Parse.Error) {
return message;
}
const code = defaultOpts.code || Parse.Error.SCRIPT_FAILED;
// If it's an error, mark it as a script failed
if (typeof message === 'string') {
return new Parse.Error(code, message);
}
const error = new Parse.Error(code, message.message || message);
if (message instanceof Error) {
error.stack = message.stack;
}
return error;
}
…e-server into fix/authData-unlink
2 participants
Pull Request
Issue
Unlinking auth provider triggers auth data validation.
Closes: #9856
Closes: #9855
Approach
Without the fix, the added test fails:
This is a significant simplification over #9856.
The change in
src/Adapters/Storage/Postgres/PostgresStorageAdapter.jsalso fixes a bug for Postgres (unrelated to the issue reported in #9855, but apparent with the fix implemented here). The change fixes unlinking auth data in Postgres by properly removing JSON keys instead of setting them to null. Before, when auth data had__op: 'Delete', it set the value tonullin the JSONB field using json_object_set_key — the key remained with anullvalue. After, it uses a newgenerateRemovehelper that uses Postgres's operator (jsonb - key) to actually remove the key from the JSONB object. Both explicit__op: 'Delete'andnullvalues trigger key removal.Summary by CodeRabbit
New Features
Tests
Improvements
Documentation