-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(drivers/139): optimize login flow with cookie reuse and robust fallback #2067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…allback - Cookie Reuse Strategy: Introduced a fast-path login mechanism. If valid MailCookies (containing Os_SSo_Sid) are present, the driver attempts to skip the full password login (Step 1) and directly exchange the SID for a token (Step 2 -> Step 3). This significantly reduces risk control triggers and improves initialization speed. - Authorization Priority: Added a check to skip the entire login process if a valid Authorization string is already present in the configuration. - Robust Fallback: Implemented a fallback mechanism. If the fast-path (cookie reuse) fails (e.g., expired cookie), the driver automatically falls back to the full password login flow (Step 1 -> Step 2 -> Step 3) to ensure service availability. - Credential Validation: Refined validation logic. Now accepts configuration with only Authorization, or only MailCookies (for fast path), while strictly enforcing that if Username or Password is provided, all three credentials (including MailCookies) must be present to support the fallback password login. - Security: Ensured that when falling back to password login, only necessary cookies are sent (via sanitizeLoginCookies) to avoid polluting the request. - Code Cleanup: Removed unused imports and improved code formatting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request optimizes the 139 cloud drive driver's authentication flow to address login loop errors and risk control triggers. The changes implement a prioritized login strategy: Authorization → Cookie reuse → Password login fallback.
Changes:
- Introduced
validateAndInitCredentials()to implement flexible credential validation with priority-based login attempts - Added
sanitizeLoginCookies()to enforce strict cookie ordering and prevent authentication failures from stale session data - Enhanced
step1_password_login()to fetch fresh JSESSIONID, detect risk control errors, and properly manage cookie state - Refactored error handling in the
request()method to eliminate goto statements and streamline special case handling
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| drivers/139/driver.go | Simplified Init() to delegate credential validation to new helper function |
| drivers/139/util.go | Core changes including new validation logic, cookie sanitization, enhanced password login with risk control detection, and cleaner error handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err != nil && !strings.Contains(err.Error(), "auto redirect is disabled") { | ||
| return "", fmt.Errorf("step1 login request failed: %w", err) |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message matching on line 890 uses a substring check for "auto redirect is disabled". While this works, it's fragile as it depends on the exact error message from the resty library. Consider checking the error type or status code range (300-399) instead, which would be more robust against library updates.
| if err != nil && !strings.Contains(err.Error(), "auto redirect is disabled") { | |
| return "", fmt.Errorf("step1 login request failed: %w", err) | |
| if err != nil { | |
| // If there is an error and no response, treat it as a hard failure. | |
| if res == nil { | |
| return "", fmt.Errorf("step1 login request failed: %w", err) | |
| } | |
| // With NoRedirectPolicy, redirects (3xx) will appear as errors but still have a valid response. | |
| // Allow those through so we can handle the Location header manually. | |
| statusCode := res.StatusCode() | |
| if statusCode < 300 || statusCode > 399 { | |
| return "", fmt.Errorf("step1 login request failed: status %d: %w", statusCode, err) | |
| } |
| if name == "JSESSIONID" { | ||
| if newJSessionID != "" { | ||
| finalCookieParts = append(finalCookieParts, name+"="+newJSessionID) | ||
| } | ||
| continue |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When newJSessionID is empty (line 833), the JSESSIONID cookie will be omitted from the sanitized cookie string. This might be intentional to avoid sending stale session IDs, but the behavior should be documented. Consider adding a comment explaining why JSESSIONID is omitted when empty, or explicitly removing it from existingCookiesMap if that's the intent.
| // 3. Rebuild the cookie string. The order doesn't matter here, as sanitizeLoginCookies will reorder it later if needed. | ||
| var finalCookieParts []string | ||
| for name, value := range existingCookiesMap { | ||
| finalCookieParts = append(finalCookieParts, name+"="+value) | ||
| } |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iterating over a map in Go produces a non-deterministic order. This means the cookie string will be built in a random order each time, which could cause issues with services that expect cookies in a specific order. Consider using a slice or ordered map to maintain consistency, or document that the order is intentionally randomized here since sanitizeLoginCookies will impose order later if needed.
| // 3. Rebuild the cookie string. The order doesn't matter here, as sanitizeLoginCookies will reorder it later if needed. | |
| var finalCookieParts []string | |
| for name, value := range existingCookiesMap { | |
| finalCookieParts = append(finalCookieParts, name+"="+value) | |
| } | |
| // 3. Rebuild the cookie string in a deterministic order so that downstream | |
| // services that might rely on cookie order behave consistently. | |
| var finalCookieParts []string | |
| // Extract and sort cookie names to ensure deterministic order. | |
| var cookieNames []string | |
| for name := range existingCookiesMap { | |
| cookieNames = append(cookieNames, name) | |
| } | |
| sort.Strings(cookieNames) | |
| for _, name := range cookieNames { | |
| finalCookieParts = append(finalCookieParts, name+"="+existingCookiesMap[name]) | |
| } |
| // Priority 2: Try fast login using existing cookies (Step 2 -> Step 3) | ||
| // Extract SID from current MailCookies | ||
| cookies := strings.Split(d.MailCookies, ";") | ||
| for _, cookie := range cookies { | ||
| cookie = strings.TrimSpace(cookie) | ||
| // Check for Os_SSo_Sid | ||
| if strings.HasPrefix(cookie, "Os_SSo_Sid=") { | ||
| sid = strings.TrimPrefix(cookie, "Os_SSo_Sid=") | ||
| break | ||
| } | ||
| } | ||
|
|
||
| // Try Step 2 directly with existing SID and Cookies (using full cookies as implicit context) | ||
| if sid != "" { | ||
| log.Infof("139yun: attempting fast login using existing SID/Cookies (Step 2).") | ||
| token, err := d.step2_get_single_token(sid) |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fast login path extracts Os_SSo_Sid from cookies but doesn't verify that RMKEY exists before attempting step2_get_single_token. Looking at step2_get_single_token (line 987-989), it requires RMKEY to be present in MailCookies and returns an error if it's missing. Consider adding a check here to verify RMKEY exists before attempting the fast path, to provide clearer error messaging and avoid unnecessary API calls.
| func (d *Yun139) validateAndInitCredentials() error { | ||
| // More robust validation for MailCookies | ||
| trimmedCookies := strings.TrimSpace(d.MailCookies) | ||
| if trimmedCookies != "" { | ||
| d.MailCookies = trimmedCookies // Update with trimmed value | ||
| if !strings.Contains(d.MailCookies, "=") || len(strings.Split(d.MailCookies, "=")[0]) == 0 { | ||
| return fmt.Errorf("MailCookies format is invalid, please check your configuration") | ||
| } | ||
| } | ||
|
|
||
| // Priority 1: If Authorization exists, skip login process completely. | ||
| // We assume it's valid for now; validity will be checked by refreshToken() later in Init(). | ||
| if d.Authorization != "" { | ||
| log.Debugf("139yun: Authorization exists, skipping initialization login.") | ||
| return nil | ||
| } | ||
|
|
||
| // Validate all-or-nothing check for username and password | ||
| // "Cookies can exist alone, but if username or password is provided, all three must be provided" | ||
| hasUserOrPass := d.Username != "" || d.Password != "" | ||
| hasAll := d.MailCookies != "" && d.Username != "" && d.Password != "" | ||
|
|
||
| if hasUserOrPass && !hasAll { | ||
| return fmt.Errorf("if username or password is provided, all three (mail_cookies, username, password) must be provided") | ||
| } |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The metadata defines all fields (Authorization, Username, Password, MailCookies) with required:"true" tags, but the validation logic here allows several combinations where some of these fields can be empty. This creates a discrepancy between the UI form validation (which marks all as required) and the backend validation (which allows flexible combinations). Consider updating the meta.go field definitions to reflect the actual requirements, or add comments explaining why the tags differ from the runtime validation.
|
不好意思 刚刚回复前没看到你是新建了新的PR 但确实还是需要团队再看的 |
Description / 描述
本次 PR 对 139 云盘驱动的初始化和登录逻辑进行了更改,旨在解决用户报告的登录死循环错误(
Failed init storage: login with password failed...)以及触发风控的问题。问题背景:
经排查,原驱动逻辑存在以下缺陷:
ec=PML401010062等频率限制错误。JSESSIONID或Os_SSo_Sid)。这会导致服务器认为是异常会话,拒绝登录请求并重定向到错误页面(表现为重定向链接中缺失关键的sid参数)。MailCookies进行快速会话恢复,导致每次操作都必须“从零开始”。优化方案:
本次提交引入了更智能的登录策略,优先级顺序为:“Authorization 优先 -> Cookie 复用优先 -> 密码登录兜底”。
Changes / 变更内容
1. Authorization 优先策略
Authorization,驱动仍可能尝试去校验账号密码或执行登录。Authorization字段存在且非空,驱动将直接跳过整个登录流程。2. Cookie 复用策略 (快速通道)
MailCookies。如果从中提取到了有效的Os_SSo_Sid:3. 健壮的自动降级 (Fallback) 机制
4. 凭据校验逻辑细化
Authorization? -> 通过。MailCookies? -> 通过(仅尝试快速通道,适合不想保存明文密码的用户)。Username或Password? -> 必须三者齐全(MailCookies+ 账号 + 密码),以确保自动降级机制能正常工作。5. 安全与代码清理
sanitizeLoginCookies严格过滤请求头,剔除旧的 SessionID,只发送必要的设备标识 Cookie,防止脏数据导致登录失败。driver.go中未使用的strings包导入,并使用go fmt格式化了代码。Motivation and Context / 背景
关联 Issue: #1534
How Has This Been Tested? / 测试
我在本地使用多种账号状态进行了详细测试:
Authorization字符串。MailCookies(无 Auth),账号密码可选。MailCookies,有效的账号和密码。MailCookies,未填账号密码。Checklist / 检查清单
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签(如无权限或需要的标签不存在,请在描述中说明,管理员将后续处理)。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。
我已相应更新了相关仓库(若适用)。