fix(auth): restore iOS note save/reload reliability#174
Merged
Conversation
Return JWT tokens on auth responses and add bearer-token fallback auth in middleware/helpers so iOS note saves and reloads remain authenticated across device sessions. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughiOS now persists bearer tokens returned by signup/login into Keychain, attaches Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant "iOS Client" as IOS
participant Keychain
participant "Backend API" as API
participant Middleware
participant "Auth Service" as Auth
User->>IOS: Login/Signup (credentials)
IOS->>API: POST /auth/login or /signup\nHeaders: X-Still-Point-Client: ios
API->>Auth: verify credentials & generate token
Auth-->>API: user + token
API-->>IOS: { user, token } (token included because header == ios)
IOS->>Keychain: save(token)
Keychain-->>IOS: saved
rect rgba(100,200,100,0.5)
Note over IOS,API: Authenticated request flow
User->>IOS: Request protected resource
IOS->>Keychain: load()
Keychain-->>IOS: token
IOS->>API: GET /resource\nAuthorization: Bearer <token>
API->>Middleware: validate request
Middleware->>Auth: verify token (from Authorization or cookie)
Auth-->>Middleware: verified
Middleware-->>API: proceed
API-->>IOS: protected resource
end
User->>IOS: Logout
IOS->>API: POST /auth/logout
IOS->>Keychain: clear()
Keychain-->>IOS: cleared
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
ios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swift (1)
18-20: Set explicit Keychain accessibility for the auth token.Consider adding
kSecAttrAccessible(e.g.,kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly) so token protection is explicit instead of relying on defaults.🔐 Suggested tweak
var attributes = query attributes[kSecValueData as String] = data +attributes[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly SecItemAdd(attributes as CFDictionary, nil)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swift` around lines 18 - 20, When saving the auth token in AuthTokenStore (around the attributes/query construction before SecItemAdd), add an explicit kSecAttrAccessible entry to the attributes dictionary (for example kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly) so the token's Keychain accessibility is explicit; update the same attributes used in the SecItemAdd call (the variable named attributes) to include this accessibility constant before calling SecItemAdd.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift`:
- Around line 36-38: The current auth-response handling only saves a token when
response.token is present (AuthTokenStore.save(token)) but never clears a
previously stored token when response.token is nil; update both spots (the if
let response.token blocks) to add an else branch that removes the stored token
from the keychain via the AuthTokenStore API (e.g., call AuthTokenStore.clear()
or AuthTokenStore.deleteStoredToken() — use the existing deletion method in
AuthTokenStore) so stale tokens are removed when the response contains no token.
- Around line 52-55: The logout() method currently only calls
AuthTokenStore.clear() after a successful post call, so local tokens/cookies
remain if the request fails; move the local cleanup into a guaranteed path by
invoking AuthTokenStore.clear() regardless of network outcome (for example, add
a defer { AuthTokenStore.clear() } at the start of logout() or otherwise ensure
AuthTokenStore.clear() is executed before returning/throwing) while keeping the
post("/api/auth/logout", body: Optional<String>.none) call to perform the
server-side logout.
In `@ios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swift`:
- Around line 8-21: The Keychain calls in save(_:), and the other Keychain
helpers that call SecItemDelete/SecItemAdd/SecItemCopyMatching are ignoring the
returned OSStatus; capture each OSStatus result and handle non-errSecSuccess
cases instead of discarding them. Update save(_ token: String) to capture the
return values of SecItemDelete and SecItemAdd, and either throw an error or
return a Bool so callers can react; include a clear log message with the
OSStatus (or SecCopyErrorMessageString output) when the call fails. Do the same
for the corresponding load/delete functions that call
SecItemCopyMatching/SecItemDelete so failures are propagated to the auth flow
rather than silently ignored.
In `@src/lib/auth.ts`:
- Around line 57-61: The current logic gives cookie precedence (tokenFromCookie)
over an Authorization Bearer (tokenFromBearer), which can let a stale cookie
override a valid bearer token; change the selection so the Bearer header wins:
when computing token (currently assigned from tokenFromCookie ??
tokenFromBearer), invert the precedence to use tokenFromBearer ??
tokenFromCookie and keep the existing Bearer detection/trim logic (see
COOKIE_NAME, tokenFromCookie, tokenFromBearer, and the token assignment) so
valid Authorization headers are preferred over cookies.
In `@src/middleware.ts`:
- Around line 32-37: The code currently prefers cookie tokens over Bearer tokens
causing stale cookies to override valid Authorization headers; change the
precedence so tokenFromBearer is chosen first: when computing token (currently
assigned from tokenFromCookie ?? tokenFromBearer) use tokenFromBearer ??
tokenFromCookie instead, keeping the existing Authorization parsing
(authorization?.startsWith("Bearer ") ? authorization.slice("Bearer
".length).trim() : null) and COOKIE_NAME usage unchanged so trimming/null
handling remains intact.
---
Nitpick comments:
In `@ios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swift`:
- Around line 18-20: When saving the auth token in AuthTokenStore (around the
attributes/query construction before SecItemAdd), add an explicit
kSecAttrAccessible entry to the attributes dictionary (for example
kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly) so the token's Keychain
accessibility is explicit; update the same attributes used in the SecItemAdd
call (the variable named attributes) to include this accessibility constant
before calling SecItemAdd.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd363d39-7563-4247-9809-3b43708b0a9c
📒 Files selected for processing (7)
ios/StillPointShared/Sources/StillPointShared/APIClient.swiftios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swiftios/StillPointShared/Sources/StillPointShared/DTOs/DTOs.swiftsrc/app/api/auth/login/route.tssrc/app/api/auth/signup/route.tssrc/lib/auth.tssrc/middleware.ts
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Address CodeRabbit/Bugbot findings by making bearer auth header-first, tightening keychain handling, and returning JWTs only for explicit iOS client requests. Made-with: Cursor
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
ios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swift (1)
8-29:⚠️ Potential issue | 🟠 MajorPropagate token persistence failures to callers instead of only logging.
Lines 8-29 and Lines 57-67 still swallow Keychain failures; the auth flow can’t react, so login can appear successful while bearer auth is effectively broken.
🔧 Proposed fix
enum AuthTokenStore { @@ - static func save(_ token: String) { + `@discardableResult` + static func save(_ token: String) -> Bool { @@ let deleteStatus = SecItemDelete(query as CFDictionary) if deleteStatus != errSecSuccess && deleteStatus != errSecItemNotFound { logKeychainFailure(operation: "delete-before-save", status: deleteStatus) + return false } @@ let addStatus = SecItemAdd(attributes as CFDictionary, nil) if addStatus != errSecSuccess { logKeychainFailure(operation: "save", status: addStatus) + return false } + return true } @@ - static func clear() { + `@discardableResult` + static func clear() -> Bool { @@ let status = SecItemDelete(query as CFDictionary) if status != errSecSuccess && status != errSecItemNotFound { logKeychainFailure(operation: "clear", status: status) + return false } + return true }Also applies to: 57-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swift` around lines 8 - 29, AuthTokenStore.save currently swallows Keychain errors; change its API to propagate failures (e.g., make static func save(_ token: String) throws or return Result/Bool) and throw/return a meaningful error when SecItemDelete or SecItemAdd return a non-success status (include the OSStatus in the error), and update callers to handle the thrown/returned error; apply the same change to the other affected methods around lines 57-67 (the other Keychain read/write/delete helpers) and use or augment logKeychainFailure to record the failure details while still propagating an error to the caller.
🧹 Nitpick comments (1)
ios/StillPointShared/Sources/StillPointShared/APIClient.swift (1)
121-129: Consider centralizing request header setup to reduce drift across HTTP methods.The same header setup pattern is repeated in
get/post/patch/delete; a shared builder would make future auth-header changes less error-prone.♻️ Refactor sketch
+ private func makeRequest(method: String, path: String) -> URLRequest { + let url = baseURL.appendingPathComponent(path) + var request = URLRequest(url: url) + request.httpMethod = method + request.setValue("application/json", forHTTPHeaderField: "Content-Type") + request.setValue("ios", forHTTPHeaderField: "X-Still-Point-Client") + applyAuthorizationHeader(to: &request) + return request + } @@ - let url = baseURL.appendingPathComponent(path) - var request = URLRequest(url: url) - request.httpMethod = "GET" - request.setValue("application/json", forHTTPHeaderField: "Content-Type") - request.setValue("ios", forHTTPHeaderField: "X-Still-Point-Client") - applyAuthorizationHeader(to: &request) + var request = makeRequest(method: "GET", path: path)Also applies to: 131-142, 144-153, 155-163
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift` around lines 121 - 129, Multiple HTTP methods (get/post/patch/delete) duplicate header and request setup; create a single helper (e.g., buildRequest(path: String, method: String) or makeRequest(path: String, method: HTTPMethod)) that constructs the URLRequest, sets "Content-Type", "X-Still-Point-Client", applies applyAuthorizationHeader(to:), and returns the prepared request for use by execute(_:). Replace the repeated setup in get/post/patch/delete with calls to this new helper and keep execute(request) unchanged so future header/auth changes are made in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swift`:
- Around line 8-29: AuthTokenStore.save currently swallows Keychain errors;
change its API to propagate failures (e.g., make static func save(_ token:
String) throws or return Result/Bool) and throw/return a meaningful error when
SecItemDelete or SecItemAdd return a non-success status (include the OSStatus in
the error), and update callers to handle the thrown/returned error; apply the
same change to the other affected methods around lines 57-67 (the other Keychain
read/write/delete helpers) and use or augment logKeychainFailure to record the
failure details while still propagating an error to the caller.
---
Nitpick comments:
In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift`:
- Around line 121-129: Multiple HTTP methods (get/post/patch/delete) duplicate
header and request setup; create a single helper (e.g., buildRequest(path:
String, method: String) or makeRequest(path: String, method: HTTPMethod)) that
constructs the URLRequest, sets "Content-Type", "X-Still-Point-Client", applies
applyAuthorizationHeader(to:), and returns the prepared request for use by
execute(_:). Replace the repeated setup in get/post/patch/delete with calls to
this new helper and keep execute(request) unchanged so future header/auth
changes are made in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72b914c9-412b-424d-a48b-b61ed266eb35
📒 Files selected for processing (6)
ios/StillPointShared/Sources/StillPointShared/APIClient.swiftios/StillPointShared/Sources/StillPointShared/AuthTokenStore.swiftsrc/app/api/auth/login/route.tssrc/app/api/auth/signup/route.tssrc/lib/auth.tssrc/middleware.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/app/api/auth/signup/route.ts
- src/app/api/auth/login/route.ts
- src/lib/auth.ts
- src/middleware.ts
Make AuthTokenStore save/clear return success status, fail fast when token persistence fails during login/signup, and centralize iOS request header setup to keep auth behavior consistent. Made-with: Cursor
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2b80e10. Configure here.
This was referenced Apr 24, 2026
Merged
This was referenced May 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
middlewareandgetCurrentUser) so authenticated API calls from native iOS do not depend on cookie behaviortokenin login/signup responses and store it in iOS Keychain; sendAuthorization: Beareron all iOS API requestsCloses #171. Related to #60 (same symptom class: iOS note save failures tied to auth/response expectations).
Done Definition (AC)
Test plan
npm run buildxcodebuild -project ios/StillPoint.xcodeproj -scheme StillPoint -destination 'generic/platform=iOS Simulator' build CODE_SIGNING_ALLOWED=NOMade with Cursor
Summary by CodeRabbit
New Features
Behavior Changes