317 lines
14 KiB
Markdown
317 lines
14 KiB
Markdown
# API Auth Access Fix — Unity Client
|
|
|
|
## TL;DR
|
|
|
|
> **Quick Summary**: Fix 3 critical bugs preventing the Unity client from successfully authenticating with the IchniOnline backend: wrong server port, GlobalResponse<T> field name mismatch (PascalCase vs camelCase) causing silent deserialization failure, and null pointer in third-party login flow.
|
|
>
|
|
> **Deliverables**:
|
|
> - `ApiResponse.cs` + `ApiClient.cs` — Atomic fix: fields renamed to camelCase, BaseUrl port corrected
|
|
> - `AuthService.cs` — null check added for third-party login response
|
|
>
|
|
> **Estimated Effort**: Quick (3 files, targeted changes)
|
|
> **Parallel Execution**: YES — all 3 tasks can run in parallel (independent files)
|
|
> **Critical Path**: None (no dependencies between tasks)
|
|
|
|
---
|
|
|
|
## Context
|
|
|
|
### Original Request
|
|
用户要求结合后端工程和 Apifox API 文档来对 `Assets/Scripts/Online` 目录里的 API 访问进行检查和修复，仅处理认证相关接口。
|
|
|
|
### Interview Summary
|
|
**Key Discussions**:
|
|
- Only auth-related endpoints in scope: login, register, session-key, third-party login
|
|
- Ignore beatmap and other APIs for now
|
|
- Only modify files within `Assets/Scripts/Online/`
|
|
|
|
**Research Findings**:
|
|
- Server (ASP.NET Core) uses `System.Text.Json` with camelCase policy → JSON keys: `code`, `message`, `data`
|
|
- Unity client uses `JsonUtility.FromJson` which is **case-sensitive** → C# field names must exactly match JSON keys
|
|
- Current `GlobalResponse<T>` has PascalCase `Code`/`Message`/`Data` → **silent deserialization failure**
|
|
- Server runs on `http://localhost:5308` (launchSettings.json) but Unity client has `http://localhost:60887` → **all requests fail**
|
|
- Third-party login can return `data: null` (unbound account) → **NullReferenceException** when accessing `result.Data.token`
|
|
|
|
### Metis Review
|
|
*Skipped — clear scope, direct bugs, user confirmed approach.*
|
|
|
|
---
|
|
|
|
## Work Objectives
|
|
|
|
### Core Objective
|
|
Fix the Unity client's auth API access so that all 4 auth endpoints (session-key, login, register, third-party login) successfully communicate with the IchniOnline backend.
|
|
|
|
### Concrete Deliverables
|
|
- `Assets/Scripts/Online/Network/Models/ApiResponse.cs` — GlobalResponse field names corrected
|
|
- `Assets/Scripts/Online/Network/ApiClient.cs` — BaseUrl port fixed, field access updated
|
|
- `Assets/Scripts/Online/Logic/AuthService.cs` — null-safe third-party login handling
|
|
|
|
### Definition of Done
|
|
- `IchniOnlineApiClient.Instance.BaseUrl` returns `http://localhost:5308`
|
|
- `JsonUtility.FromJson` correctly populates `GlobalResponse<T>.code`/`message`/`data`
|
|
- Third-party login with unbound account does not throw NullReferenceException
|
|
- All 3 auth flows (password login, TapTap login, register) return correct `ApiResult`
|
|
|
|
### Must Have
|
|
- Fix BaseUrl port from 60887 to 5308
|
|
- Fix `GlobalResponse<T>` field names to match JSON camelCase
|
|
- Add null check for `result.Data` in third-party login flow
|
|
- All changes within `Assets/Scripts/Online/` only
|
|
|
|
### Must NOT Have (Guardrails)
|
|
- Do NOT touch beatmap or non-auth API code
|
|
- Do NOT modify files outside `Assets/Scripts/Online/`
|
|
- Do NOT add Newtonsoft.Json or other new dependencies
|
|
- Do NOT refactor architecture (stay minimal, targeted fixes)
|
|
|
|
---
|
|
|
|
## Verification Strategy (MANDATORY)
|
|
|
|
> **ZERO HUMAN INTERVENTION** — ALL verification is agent-executed.
|
|
|
|
### Test Decision
|
|
- **Infrastructure exists**: NO
|
|
- **Automated tests**: None
|
|
- **Framework**: N/A
|
|
|
|
### QA Policy
|
|
Every task MUST include agent-executed QA scenarios. Evidence saved to `.omo/evidence/task-{N}-{scenario-slug}.{ext}`.
|
|
|
|
- **Library/Module verification**: Use Bash (bun/node REPL style) with `script-execute` to run C# test snippets that directly verify `JsonUtility.FromJson` deserialization behavior
|
|
- **Code review verification**: Read modified files and verify correctness by cross-referencing with server API contracts
|
|
|
|
---
|
|
|
|
## Execution Strategy
|
|
|
|
```
|
|
Wave 1 (Start Immediately):
|
|
├── Task 1: Fix ApiResponse.cs + ApiClient.cs — GlobalResponse camelCase + BaseUrl port [quick]
|
|
└── Task 2: Fix AuthService.cs — third-party login null check [quick]
|
|
|
|
Wave FINAL (After ALL tasks):
|
|
├── Task F1: QA verification (oracle)
|
|
└── Task F2: Scope fidelity check (deep)
|
|
|
|
Critical Path: Task 1 → F1, F2 (independent from Task 2)
|
|
Parallel Speedup: ~50% faster than sequential (Task 1 and Task 2 run in parallel)
|
|
Max Concurrent: 2 (both in Wave 1)
|
|
```
|
|
|
|
---
|
|
|
|
## TODOs
|
|
|
|
- [x] 1. Fix ApiResponse.cs + ApiClient.cs — GlobalResponse<T> camelCase + BaseUrl port
|
|
|
|
**What to do**:
|
|
- In `ApiResponse.cs`: Rename field `Code` → `code`, `Message` → `message`, `Data` → `data`
|
|
- In `ApiClient.cs`:
|
|
- Change `BaseUrl` default from `"http://localhost:60887"` to `"http://localhost:5308"`
|
|
- Update all references to `GlobalResponse<T>` fields from PascalCase to camelCase:
|
|
- `response.Code` → `response.code`
|
|
- `response.Message` → `response.message`
|
|
- `response.Data` → `response.data`
|
|
- `errorResponse.Code` → `errorResponse.code`
|
|
- `errorResponse.Message` → `errorResponse.message`
|
|
- Verify: `ResponseCode` enum values remain unchanged (Ok=10000, etc.)
|
|
- Verify no other PascalCase field access patterns exist in ApiClient.cs
|
|
|
|
> **Rationale for merging**: ApiResponse.cs declares the fields, ApiClient.cs consumes them. Both files must be changed atomically — if committed separately, the codebase is broken regardless of order. A single agent handles both files in one task to guarantee correctness.
|
|
|
|
**Must NOT do**:
|
|
- Do not change `ResponseCode` enum values or names
|
|
- Do not change `ApiResult<T>` class structure
|
|
- Do not add `[JsonProperty]` or other attributes
|
|
- Do not change the `BuildUrl`, `AddAuthHeader`, or `SendAsync` methods
|
|
- Do not change the method signatures of `GetAsync<T>` or `PostAsync<T>`
|
|
|
|
**Recommended Agent Profile**:
|
|
> - **Category**: `quick`
|
|
> - Reason: Two files, simple field rename + port change, no logic changes
|
|
> - **Skills**: none required
|
|
> - **Skills Evaluated but Omitted**: N/A
|
|
|
|
**Parallelization**:
|
|
> - **Can Run In Parallel**: NO (merged from previous Tasks 1+2 due to compilation dependency)
|
|
> - **Parallel Group**: Wave 1 (with Task 2)
|
|
> - **Blocks**: None
|
|
> - **Blocked By**: None (can start immediately)
|
|
|
|
**References**:
|
|
- `Assets/Scripts/Online/Network/Models/ApiResponse.cs` — Field declaration site
|
|
- `Assets/Scripts/Online/Network/ApiClient.cs` — Field usage site + BaseUrl
|
|
- Server `GlobalResponse.cs` at `/mnt/d/Projects/IchniOnline/IchniOnline.Server/Models/Responses/GlobalResponse.cs` — Reference (PascalCase with System.Text.Json camelCase policy)
|
|
- Backend `launchSettings.json` at `/mnt/d/Projects/IchniOnline/IchniOnline.Server/Properties/launchSettings.json` — Confirms port 5308
|
|
- Current JSON wire format: `{"code":10000,"message":"Success","data":{...}}` — JSON keys are lowercase
|
|
|
|
**Acceptance Criteria**:
|
|
- `ApiResponse.cs` field declarations use camelCase: `code`, `message`, `data`
|
|
- `ApiClient.cs` field references use camelCase: `response.code`, `response.message`, `response.data`
|
|
- `ApiClient.cs` BaseUrl line 20: `"http://localhost:5308"`
|
|
- `script-execute` confirms `JsonUtility.FromJson<GlobalResponse<string>>` correctly deserializes `{"code":10000,"message":"OK","data":"test"}`
|
|
|
|
**QA Scenarios (MANDATORY)**:
|
|
```
|
|
Scenario: Verify GlobalResponse deserialization works with camelCase JSON
|
|
Tool: script-execute (C# Roslyn)
|
|
Preconditions: Both files have been modified
|
|
Steps:
|
|
1. Use script-execute to run: string json = "{\\\"code\\\":10000,\\\"message\\\":\\\"OK\\\",\\\"data\\\":\\\"test\\\"}";
|
|
2. Deserialize: var obj = UnityEngine.JsonUtility.FromJson<GlobalResponse<string>>(json);
|
|
3. Assert: obj.code == (ResponseCode)10000
|
|
4. Assert: obj.message == "OK"
|
|
5. Assert: obj.data == "test"
|
|
Expected Result: All 3 fields are correctly populated from camelCase JSON keys
|
|
Failure Indicators: Any field remains default (0/null)
|
|
Evidence: .omo/evidence/task-1-globalresponse-deserialize.txt
|
|
|
|
Scenario: Verify GlobalResponseBase error deserialization
|
|
Tool: script-execute
|
|
Preconditions: Both files have been modified
|
|
Steps:
|
|
1. Use script-execute to run: string json = "{\\\"code\\\":10400,\\\"message\\\":\\\"Bad request\\\"}";
|
|
2. Deserialize as base: var obj = UnityEngine.JsonUtility.FromJson<GlobalResponseBase>(json);
|
|
3. Assert: obj.code == (ResponseCode)10400
|
|
4. Assert: obj.message == "Bad request"
|
|
Expected Result: Error response fields are correctly populated
|
|
Evidence: .omo/evidence/task-1-globalresponsebase-deserialize.txt
|
|
|
|
Scenario: Verify BaseUrl port is corrected
|
|
Tool: script-read
|
|
Preconditions: ApiClient.cs has been updated
|
|
Steps:
|
|
1. Read line 20 of ApiClient.cs
|
|
2. Assert: BaseUrl default value is "http://localhost:5308"
|
|
Expected Result: Port is corrected to 5308
|
|
Evidence: .omo/evidence/task-1-baseurl.txt
|
|
|
|
Scenario: Verify all GlobalResponse field references use camelCase in ApiClient.cs
|
|
Tool: grep
|
|
Preconditions: ApiClient.cs has been updated
|
|
Steps:
|
|
1. Search for pattern: "response\.Code" in ApiClient.cs — should return 0 matches
|
|
2. Search for pattern: "response\.code" — should return 2+ matches
|
|
3. Search for pattern: "errorResponse\.Code" — should return 0 matches
|
|
4. Search for pattern: "errorResponse\.code" — should return 2+ matches
|
|
Expected Result: All PascalCase field references replaced with camelCase
|
|
Evidence: .omo/evidence/task-1-field-access.txt
|
|
```
|
|
|
|
**Evidence to Capture**:
|
|
- [ ] script-execute output for deserialization test
|
|
- [ ] Line 20 read output
|
|
- [ ] grep results for field access patterns
|
|
|
|
**Commit**: YES
|
|
- Message: `fix(api): rename GlobalResponse<T> fields to camelCase, correct BaseUrl port to 5308`
|
|
- Files: `Assets/Scripts/Online/Network/Models/ApiResponse.cs`, `Assets/Scripts/Online/Network/ApiClient.cs`
|
|
- Pre-commit: Verify build compiles
|
|
|
|
---
|
|
|
|
- [x] 2. Fix AuthService.cs — third-party login null check
|
|
|
|
**What to do**:
|
|
- In `CompleteTapTapLoginAsync` method (~line 109): Add null check for `result.Data` before accessing `.token`
|
|
- Handle the case where third-party login succeeds (code=10000) but data is null (unbound account):
|
|
- If `result.IsSuccess && result.Data != null`: proceed normally (save session, set JWT, fire success)
|
|
- If `result.IsSuccess && result.Data == null`: fire `OnLoginFailed` with a clear message about account not being bound/linked
|
|
|
|
**Must NOT do**:
|
|
- Do not change the password login flow (LoginWithPasswordAsync)
|
|
- Do not change register flow
|
|
- Do not change the encryption logic
|
|
- Do not modify ThirdPartyServiceManager
|
|
|
|
**Recommended Agent Profile**:
|
|
> - **Category**: `quick`
|
|
> - Reason: Single null check addition in one method
|
|
> - **Skills**: none required
|
|
> - **Skills Evaluated but Omitted**: N/A
|
|
|
|
**Parallelization**:
|
|
> - **Can Run In Parallel**: YES
|
|
> - **Parallel Group**: Wave 1 (with Tasks 1, 2)
|
|
> - **Blocks**: None
|
|
> - **Blocked By**: None
|
|
|
|
**References**:
|
|
- `Assets/Scripts/Online/Logic/AuthService.cs` — File to edit
|
|
- Server `AuthController.cs` at `/mnt/d/Projects/IchniOnline/IchniOnline.Server/Controller/AuthController.cs` lines 70-77 — Shows the `ThirdPartyLogin` can return `GlobalResponse<LoginResponse>.Ok(null, "Account not bound")`
|
|
- `ApiResponse.cs` `ApiResult<T>` class — For understanding `IsSuccess`, `Data` properties
|
|
|
|
**Acceptance Criteria**:
|
|
- `CompleteTapTapLoginAsync` method has null guard before `result.Data.token` access
|
|
- When `result.IsSuccess && result.Data == null`: calls `OnLoginFailed` with message containing "not bound" or "unbound"
|
|
- When `result.IsSuccess && result.Data != null`: saves auth session, sets JWT, fires `OnLoginSuccess`
|
|
- Password login flow (`LoginWithPasswordAsync`) is unchanged
|
|
|
|
**QA Scenarios (MANDATORY)**:
|
|
```
|
|
Scenario: Verify null data handling for third-party login
|
|
Tool: script-read
|
|
Preconditions: AuthService.cs has been updated
|
|
Steps:
|
|
1. Read the CompleteTapTapLoginAsync method
|
|
2. Verify: there is a null check for result.Data before result.Data.token access
|
|
3. Verify: null case calls OnLoginFailed with appropriate message
|
|
Expected Result: NullReferenceException is prevented
|
|
Evidence: .omo/evidence/task-2-null-check.txt
|
|
|
|
Scenario: Verify password login flow is unchanged
|
|
Tool: grep
|
|
Preconditions: AuthService.cs has been updated
|
|
Steps:
|
|
1. Search for "result\.Data\.token" in AuthService.cs — should only appear in the TapTap completion method
|
|
2. Verify all occurrences have null guards
|
|
Expected Result: No unprotected access to result.Data.token
|
|
Evidence: .omo/evidence/task-2-password-flow.txt
|
|
```
|
|
|
|
**Evidence to Capture**:
|
|
- [ ] Read output showing the null check code
|
|
- [ ] grep results
|
|
|
|
**Commit**: YES
|
|
- Message: `fix(auth): add null check for third-party login response data`
|
|
- Files: `Assets/Scripts/Online/Logic/AuthService.cs`
|
|
- Pre-commit: Verify build compiles
|
|
|
|
---
|
|
|
|
## Final Verification Wave
|
|
|
|
- [ ] F1. **Plan Compliance Audit** — `oracle`
|
|
Read the plan end-to-end. For each "Must Have": verify implementation exists (read file, check values). For each "Must NOT Have": search codebase for forbidden patterns — reject with file:line if found. Check evidence files exist in .omo/evidence/. Compare deliverables against plan.
|
|
Output: `Must Have [N/N] | Must NOT Have [N/N] | Tasks [N/N] | VERDICT: APPROVE/REJECT`
|
|
|
|
- [ ] F2. **Scope Fidelity Check** — `deep`
|
|
For each task: read "What to do", read actual diff (git log/diff). Verify 1:1 — everything in spec was built (no missing), nothing beyond spec was built (no creep). Check "Must NOT do" compliance. Detect cross-task contamination.
|
|
Output: `Tasks [N/N compliant] | Contamination [CLEAN/N issues] | VERDICT`
|
|
|
|
---
|
|
|
|
## Commit Strategy
|
|
|
|
- **1**: `fix(api): rename GlobalResponse<T> fields to camelCase, correct BaseUrl port to 5308`
|
|
- Files: `ApiResponse.cs`, `ApiClient.cs`
|
|
- **2**: `fix(auth): add null check for third-party login response data`
|
|
- Files: `AuthService.cs`
|
|
|
|
---
|
|
|
|
## Success Criteria
|
|
|
|
### Verification Scenarios
|
|
1. `script-execute` confirms `JsonUtility.FromJson<GlobalResponse<string>>` correctly parses `{"code":10000,"message":"OK","data":"test"}`
|
|
2. `ApiClient.cs` line 20 shows `BaseUrl = "http://localhost:5308"`
|
|
3. `AuthService.cs` has null guard before `result.Data.token` access
|
|
|
|
### Final Checklist
|
|
- [ ] All "Must Have" present
|
|
- [ ] All "Must NOT Have" absent
|
|
- [ ] All 3 files modified correctly
|