feat: Add login with additional auth data #1858
Conversation
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
📝 WalkthroughWalkthroughAdds a POST-based /login flow that accepts additional parameters (e.g., authData) by introducing a new PFRESTUserCommand factory, extending PFUserController and PFUser APIs to pass authData parameters, sanitizing transient MFA authData from persisted user state, and adding unit tests for the new flow. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant PFUser
participant PFUserController
participant PFRESTUserCommand
participant ParseServer
App->>PFUser: logInWithUsernameInBackground(username,password,authData)
PFUser->>PFUserController: logInCurrentUserAsyncWithUsername:password:parameters:revocableSession:
PFUserController->>PFRESTUserCommand: logInUserCommandWithParameters:revocableSession:error:
PFRESTUserCommand->>ParseServer: POST /login { username, password, authData }
ParseServer-->>PFRESTUserCommand: Response (user dictionary, session)
PFRESTUserCommand-->>PFUserController: Response dictionary
PFUserController->>PFUserController: Sanitize authData (remove mfa)
PFUserController->>PFUser: Construct PFUser from sanitized data
PFUserController-->>App: Return BFTask resolving with PFUser
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
Parse/Parse/Source/PFUser.h (2)
170-177: Docstring should include params and server-version notePlease align the doc with neighboring APIs by documenting parameters and noting the minimum Parse Server version for MFA.
/** Logs in a user with username and password and additional authentication data (e.g., MFA). The authData keys must follow the Parse Server spec, for example: @{ @"mfa": @{ @"token": authCode } } - This data is only sent as part of the login request and is not persisted on the PFUser instance. + This data is only sent as part of the login request and is not persisted on the PFUser instance. + + @param username The username of the user. + @param password The password of the user. + @param authData Optional additional auth data to be sent only during login (e.g., MFA). + + @discussion Requires Parse Server 6.0+ for MFA/authData during login. Transient keys such as "mfa" + received from the server response are sanitized and not persisted on the PFUser instance. */
182-186: Expand the block variant doc for consistencyMatch the style of other block-based APIs by documenting parameters and behavior.
-/** Block variant of login with additional authData. */ +/** + Block variant of login with additional authData. + + @param username The username of the user. + @param password The password of the user. + @param authData Optional additional auth data to be sent only during login (e.g., MFA). + @param block The block to execute. It has the signature: ^(PFUser *user, NSError *error). + */Parse/Tests/Unit/UserCommandTests.m (1)
44-61: Strengthen header assertionAssert the exact header content, not just the count, to avoid silent regressions in header names/values.
- XCTAssertEqual(command.additionalRequestHeaders.count, 1); + XCTAssertEqualObjects(command.additionalRequestHeaders, (@{ @"X-Parse-Revocable-Session" : @"1" }));Parse/Parse/Internal/Commands/PFRESTUserCommand.h (1)
26-34: Clarify parameter type and add server-version note
- Use generics for parameters to improve readability and type hints.
- Document that POST /login with additional auth requires Parse Server 6+.
/** Creates a login command with a JSON body, allowing additional parameters such as authData. This posts to the login route and is required for features like MFA where additional authentication data must be supplied alongside username/password. + Requires Parse Server 6.0+ for MFA/authData during login. */ -+ (instancetype)logInUserCommandWithParameters:(NSDictionary *)parameters ++ (instancetype)logInUserCommandWithParameters:(NSDictionary<NSString *, id> *)parameters revocableSession:(BOOL)revocableSessionEnabled error:(NSError **)error;Parse/Parse/Source/PFUser.m (1)
369-374: Consider extracting MFA sanitization logic into a reusable methodThe MFA sanitization logic appears in multiple locations (lines 369-374, 655-660). This duplication could lead to maintenance issues if the sanitization requirements change.
Consider extracting this into a private helper method:
+- (NSDictionary *)_sanitizeAuthDataRemovingMFA:(NSDictionary *)authData { + if ([authData isKindOfClass:[NSDictionary class]] && authData[@"mfa"]) { + NSMutableDictionary *mutable = [authData mutableCopy]; + [mutable removeObjectForKey:@"mfa"]; + return [mutable copy]; + } + return authData; +}Then use it in both locations:
- // Remove transient MFA auth provider from persisted state - if ([newAuthData isKindOfClass:[NSDictionary class]] && newAuthData[@"mfa"]) { - NSMutableDictionary *mutable = [newAuthData mutableCopy]; - [mutable removeObjectForKey:@"mfa"]; - newAuthData = [mutable copy]; - } + // Remove transient MFA auth provider from persisted state + newAuthData = [self _sanitizeAuthDataRemovingMFA:newAuthData];Parse/Parse/Internal/User/Controller/PFUserController.m (1)
136-137: Consider validating required parametersWhile the code handles nil username/password by providing empty strings, this might lead to less informative error messages from the server.
Consider validating required parameters before making the request:
- NSMutableDictionary *merged = [@{ @"username": username ?: @"", - @"password": password ?: @"" } mutableCopy]; + if (!username || !password) { + return [BFTask taskWithError:[PFErrorUtilities errorWithCode:kPFErrorUsernameMissing + message:@"Username and password are required."]]; + } + NSMutableDictionary *merged = [@{ @"username": username, + @"password": password } mutableCopy];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Parse/Parse/Internal/Commands/PFRESTUserCommand.h(1 hunks)Parse/Parse/Internal/Commands/PFRESTUserCommand.m(1 hunks)Parse/Parse/Internal/User/Controller/PFUserController.h(1 hunks)Parse/Parse/Internal/User/Controller/PFUserController.m(2 hunks)Parse/Parse/Source/PFUser.h(1 hunks)Parse/Parse/Source/PFUser.m(3 hunks)Parse/Tests/Unit/UserCommandTests.m(1 hunks)Parse/Tests/Unit/UserControllerTests.m(1 hunks)
🔇 Additional comments (7)
Parse/Parse/Internal/Commands/PFRESTUserCommand.m (1)
68-79: LGTM: POST /login with body parametersThe new factory cleanly mirrors the existing GET variant, sets the revocable-session header, and uses POST for body parameters (authData). No issues found.
Parse/Tests/Unit/UserControllerTests.m (1)
238-271: AI summary inconsistency: test not removedThe AI summary mentions removal of testLogInCurrentUserWithUsernamePasswordNullResult, but it’s still present here. No change requested; just flagging the discrepancy.
Parse/Parse/Source/PFUser.m (2)
853-864: Well-implemented login API with authData supportThe new login API correctly handles optional authData parameter and maintains backward compatibility. The implementation properly delegates to the controller with the parameters dictionary only when authData is present.
866-871: LGTM: Block-based variant follows established patternsThe block-based convenience method correctly delegates to the task-based variant and maintains consistency with other SDK methods.
Parse/Parse/Internal/User/Controller/PFUserController.h (1)
41-48: Clear documentation for the new login methodThe documentation comment effectively explains the purpose and usage of the parameters dictionary, particularly for MFA support. The method signature is well-designed to maintain backward compatibility.
Parse/Parse/Internal/User/Controller/PFUserController.m (2)
129-167: Well-structured login implementation with proper parameter handlingThe implementation correctly:
- Merges username/password with additional parameters
- Prevents authData persistence by only sending it in the request body
- Handles error cases consistently with existing login methods
- Saves the user as the current object after successful login
The comment on line 139-140 clearly explains the design decision.
69-81: AuthData sanitization is sufficient; no other transient providers foundA repository-wide search confirms that
"mfa"is the only transient key inauthData. The current logic already removes"mfa"and correctly preserves any other keys (or drops the entireauthDatawhen empty). No changes required.
New Pull Request Checklist
Issue Description
Closes: #1839
Approach
A
logInWithUsernameInBackgroundfunction is added that takes additionalauthData.In order to prevent sending the
authData.mfaobject on every subsequent PFUser update, theauthData.mfakey is removed from the response payload on login so it is not kept in memory for future saves. This is needed because otherwise Parse Server would be callingvalidateUpdateinside the Auth Adapter which fails without a valid TOTP.The login function can be called like this:
PFUser.logInWithUsername(inBackground: username, password: password, authData: ["mfa": ["token": authCode]])TODOs before merging
Summary by CodeRabbit