Fixes for return_on_mfa code #92
Conversation
WalkthroughThe pull request introduces changes to the Multi-Factor Authentication (MFA) handling process in the Garth library. The modifications primarily affect the login mechanism across multiple files, including Changes
Sequence DiagramsequenceDiagram
participant User
participant Garth
participant SSO
User->>Garth: login(email, password)
Garth->>SSO: Attempt login
alt MFA Required
SSO-->>Garth: Return ("needs_mfa", client_state)
Garth-->>User: Prompt for MFA code
User->>Garth: Provide MFA code
Garth->>SSO: Resume login with MFA
SSO-->>Garth: Return OAuth1 and OAuth2 tokens
else Login Successful
SSO-->>Garth: Return OAuth1 and OAuth2 tokens
end
Garth-->>User: Complete login
Possibly related PRs
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🧰 Additional context used🪛 GitHub Actions: CIgarth/http.py[warning] Code formatting issues detected. File needs to be reformatted using Ruff formatter. [warning] Trailing whitespace found in file 🔇 Additional comments (4)
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
![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.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)garth/http.py(1 hunks)garth/sso.py(1 hunks)garth/version.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI
garth/http.py
[warning] Code formatting issues detected. File needs to be reformatted using Ruff formatter.
[warning] Trailing whitespace detected and needs to be removed.
🔇 Additional comments (3)
garth/http.py (1)
164-169: LGTM! The login method correctly handles the new return structure.The changes properly unpack and store the OAuth tokens from the SSO login response.
🧰 Tools
🪛 GitHub Actions: CI
[warning] Code formatting issues detected. File needs to be reformatted using Ruff formatter.
[warning] Trailing whitespace detected and needs to be removed.
garth/version.py (1)
1-1: LGTM! Version bump is appropriate.The increment to 0.5.3 aligns with semantic versioning for a backward-compatible bug fix.
README.md (1)
143-144: LGTM! Documentation accurately reflects the implementation changes.The example code has been updated to demonstrate the new MFA handling flow, making it clearer and more explicit.
Also applies to: 146-146
|
@matin Hi there, can you please have a quick look and merge if you have the time? |
|
@cyberjunky I apologize I haven't had a change to review. I'm adding @felipao-mx as a maintainer to review and merge |
With these changes I got it working, some returns where missing, I could not get it working with returning one result variable.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores