feat: Add QR login functionality with 2FA support #397
Conversation
pamod-madubashana
commented
Jan 26, 2026
pamod-madubashana
commented
- Implement start_qr_login and start_continuous_qr_login methods
- Add proper DC migration handling during QR login flow
- Handle SESSION_PASSWORD_NEEDED errors for 2FA accounts
- Add get_password_token public method for 2FA flows
- Update integration test to use environment variables
- Fix import_login_token to switch home DC before importing tokens
- Add QR login status enum with proper error handling
- Implement start_qr_login and start_continuous_qr_login methods - Add proper DC migration handling during QR login flow - Handle SESSION_PASSWORD_NEEDED errors for 2FA accounts - Add get_password_token public method for 2FA flows - Update integration test to use environment variables - Fix import_login_token to switch home DC before importing tokens - Add QR login status enum with proper error handling
…iable configuration - Implement QR login with proper DC migration handling - Add 2FA support with SESSION_PASSWORD_NEEDED error handling - Update test to use environment variables for API credentials - Fix type mismatch in wait_for_qr_login function call - Add get_password_token method for 2FA flows
| use grammers_session::types::{PeerInfo, UpdateState, UpdatesState}; | ||
| use grammers_tl_types as tl; | ||
|
|
||
| use base64::Engine; |
There was a problem hiding this comment.
New formatting would remove the empty line above and sort. CI doesn't enforce it but I'd prefer the change anyway.
| #[derive(Debug, Clone, PartialEq)] | ||
| pub enum QrLoginStatus { | ||
| Idle, | ||
| Waiting, |
There was a problem hiding this comment.
As a user, how are Idle and Waiting different? Also, I think these all need to be documented.
| Waiting, | ||
| Expired, | ||
| Success, | ||
| PasswordRequired(Option<String>), // 2FA required, with optional hint |
There was a problem hiding this comment.
Should probably be a struct variant with { hint: Option<String> } for it to be better self-documenting. Since it is pub.
| Expired, | ||
| Success, | ||
| PasswordRequired(Option<String>), // 2FA required, with optional hint | ||
| Error(String), |
There was a problem hiding this comment.
This seems odd, to have a String as a Error. I don't know what this contains as a user.
| pub struct QrLoginInfo { | ||
| pub qr_url: String, | ||
| pub expires_unix: u64, | ||
| pub expires_in_seconds: i64, |
| pub async fn import_login_token( | ||
| &self, | ||
| token: Vec<u8>, | ||
| dc_id: i32, |
There was a problem hiding this comment.
Users should not care about this. So it shouldn't be an explicit parameter.
| /// Finalize QR login by completing the authorization process | ||
| pub async fn finalize_qr_login( | ||
| &self, | ||
| auth: tl::types::auth::Authorization, |
There was a problem hiding this comment.
Client methods should not interface with raw types without explicitly calling them raw. So they're not OK as parameters.
| } | ||
|
|
||
| /// Fetches 2FA password token (hint, SRP params) so the app can prompt for password. | ||
| pub async fn get_password_token(&self) -> Result<PasswordToken, InvocationError> { |
There was a problem hiding this comment.
Why two methods? And how does this differ from the existing password handling? Can the existing password handling not be reused?
| } | ||
|
|
||
| /// Finalize QR login by completing the authorization process | ||
| pub async fn finalize_qr_login( |
There was a problem hiding this comment.
Seems odd to need this conversion step. By now the login has already finished.
| } | ||
| } | ||
|
|
||
| // Add a comment about how to run this test |
|
Thanks for the review, and sorry for the issues in my previous attempt. QR login is essential for my Telegram-based project, so I’ll address the concerns and submit a cleaner update. |
2 participants
