19 · The login screen that remembers too much
A login view model with a "remember me" switch that stores far more than it should, in the wrong places — review it.
Shape: review this PR · Reported: PhonePe asked how to share keychain data between apps; a candidate writes that storing tokens in the keychain wasn't the end of the conversation · Verified: the fix's login logic run against a fake server and token store, Swift 6.4
"A teammate added 'remember me' to the login screen. It works, QA signed it off, and it's going out on Thursday. Review it before it does."
import Foundation
struct LoginResponse: Decodable {
let token: String
}
final class LoginViewModel {
var email = ""
var password = ""
var rememberMe = false
var errorMessage: String?
var isLoggedIn = false
private var token: String?
init() {
let defaults = UserDefaults.standard
if defaults.bool(forKey: "rememberMe") {
email = defaults.string(forKey: "email") ?? ""
password = defaults.string(forKey: "password") ?? ""
}
token = defaults.string(forKey: "authToken")
isLoggedIn = token != nil
}
func login() {
NSLog("Logging in \(email) with password \(password)")
var components = URLComponents(string: "http://api.example.com/v1/login")!
components.queryItems = [
URLQueryItem(name: "email", value: email),
URLQueryItem(name: "password", value: password),
]
var request = URLRequest(url: components.url!)
request.httpMethod = "POST"
URLSession.shared.dataTask(with: request) { data, response, _ in
guard let data, let http = response as? HTTPURLResponse else { return }
switch http.statusCode {
case 200:
let token = try! JSONDecoder().decode(LoginResponse.self, from: data).token
self.token = token
UserDefaults.standard.set(token, forKey: "authToken")
if self.rememberMe {
UserDefaults.standard.set(true, forKey: "rememberMe")
UserDefaults.standard.set(self.email, forKey: "email")
UserDefaults.standard.set(self.password, forKey: "password")
}
self.isLoggedIn = true
case 404:
self.errorMessage = "No account exists for \(self.email)."
case 401:
self.errorMessage = "Wrong password for \(self.email)."
default:
self.errorMessage = "Something went wrong."
}
}.resume()
}
func avatarURL() -> URL {
URL(string: "https://api.example.com/v1/me/avatar?token=\(token ?? "")")!
}
func logout() {
UserDefaults.standard.removeObject(forKey: "authToken")
isLoggedIn = false
}
}And in the same PR, Info.plist, so the http:// URL works:
<key>NSAppTransportSecurity</key>
<dict>
<key>NSAllowsArbitraryLoads</key>
<true/>
</dict>A hint, if you're stuck
- Follow the password from the text field. List every place it ends up — memory, disk, logs, the network, other people's servers.
- Do the same for the token.
- Read the two error messages as an attacker with a list of email addresses would.
- What happens when the user taps "Log in" five times quickly? What happens after logout?
The key — what I expect a senior to find
try!on the response. A 200 with an unexpected body — a maintenance page, a renamed field — crashes the app on the login screen. Decode withtryand show an error.- UI state written from a background queue. The completion handler runs on
URLSession's queue and setsisLoggedInanderrorMessage, which drive the UI. That's a data race and wrong-thread UI. Swift 6 mode warns (capture of 'self' with non-Sendable type 'LoginViewModel' in a '@Sendable' closure); Swift 5 mode says nothing. Make the class@MainActorand useasync/await. - "Remember me" stores the password itself, in plain text.
UserDefaultsis an unencrypted plist in the app's container. It goes into backups, and anyone with the backup or a jailbroken phone reads it.initfills it back into the field on every launch, andlogout()never deletes it. Never store the password; remembering the user means keeping the token. - The token in
UserDefaults. Same plist, same problem: whoever reads it is logged in as the user, no password needed. It belongs in the keychain. - Clear-text HTTP, enabled app-wide.
http://sends the email and password readable to anyone on the same café Wi-Fi.NSAllowsArbitraryLoadsswitches off App Transport Security (Apple's HTTPS-only default) for every request in the app. Delete it and usehttps://. - The password in the URL. Even over HTTPS, URLs end up in server, proxy and CDN logs and in crash reports. Credentials go in the request body. It's also a user-visible bug:
URLComponentsleaves+as is (password=p@ss+w0rd%26x), and many servers read+in a query as a space. - The token in the avatar URL. Same leak, plus the URL becomes a
URLCachekey on disk and image loaders often log it. The token is also pasted in unencoded, so a+or&in it breaks the URL. Send it in anAuthorizationheader. - The password logged.
NSLogwrites to the system log, which a connected Mac reads in Console and which lands in sysdiagnose files. It also treats the string as a format: a password ofx%dy%@zwas logged asx0y(null)z. Don't log credentials at all. - Error messages reveal who has an account. "No account exists" versus "Wrong password" lets anyone test a list of emails — account enumeration. One message for both: "Email or password is incorrect."
- The password stays in memory after login. It sits in
passwordfor as long as the screen lives. Clear it as soon as the request is built. - No protection against hammering. Five quick taps send five requests, and a server's rate limit (
429 Too Many Requests) gets "Something went wrong." Block a second submit while one is in flight. - Logout leaves the user half logged in. The token stays in memory in
token, soavatarURL()keeps working, and the server is never told. - Failures are silent. No network, no data:
guard … else { return }. The button does nothing and the user taps again. - The error never clears.
errorMessageis set on a failure and never reset, so a retry that succeeds still shows the old error under a logged-in screen. - Unticking "remember me" forgets nothing. Log in once with it on, once with it off: the first login's data stays stored. Unticking has to delete what was saved.
isLoggedIntrusts any stored token. A token from months ago, already expired, makes the app look logged in until the first request fails. The fix keeps this; say what you'd add (below).- Untestable.
UserDefaults.standardandURLSession.sharedare reached for directly, so none of the above can be checked in a unit test.
The idea behind it
Every secret has a blast radius: the set of places it can be read from. A good login flow keeps that set as small as possible.
A password is the worst secret to leak. People reuse it on other sites, so leaking it hurts the user far beyond your app. So the app should touch it for as short a time as it can: read it from the field, send it once, forget it.
A token is a stand-in the server hands back. It proves you logged in without repeating the password. It can be limited — it can expire, and the server can cancel it. That's why "remember me" should keep the token, never the password.
Where to keep it? UserDefaults is a settings file, not a safe. The keychain is the system's encrypted store for small secrets. Each item has an accessibility class — the rule for when the system unlocks it:
- WhenUnlocked — only while the phone is unlocked.
- AfterFirstUnlock — after the first unlock since the phone was switched on, even when it's locked again. Background work needs this.
- ThisDeviceOnly — added to either: the item is never moved to a new phone through a backup.
Think of a hotel. The password is your passport: show it once at the desk, then put it away. The token is the key card: it opens your room, expires when you check out, and the desk can cancel it. You keep the card in the room safe, not on the lobby table.
The fix
Same class, same properties. login() becomes async, the token moves to the keychain behind one protocol, and the NSAllowsArbitraryLoads entry is deleted from Info.plist.
import Foundation
struct LoginResponse: Decodable {
let token: String
}
protocol TokenStore { // key 4: the one new seam; the app passes a keychain store
func token() -> String?
func set(_ token: String) throws
func remove()
}
@MainActor // key 2
final class LoginViewModel {
var email = ""
var password = ""
var rememberMe = false
var errorMessage: String?
var isLoggedIn = false
private var token: String?
private var isSubmitting = false // key 11
private let session: URLSession
private let tokenStore: TokenStore
init(session: URLSession = .shared, tokenStore: TokenStore) { // key 17
self.session = session
self.tokenStore = tokenStore
email = UserDefaults.standard.string(forKey: "email") ?? "" // key 3: never the password
token = tokenStore.token()
isLoggedIn = token != nil
}
func login() async {
guard !isSubmitting else { return } // key 11: one request per tap burst
isSubmitting = true
defer { isSubmitting = false }
errorMessage = nil // key 14
var request = URLRequest(url: URL(string: "https://api.example.com/v1/login")!) // key 5
request.httpMethod = "POST"
request.setValue("application/json", forHTTPHeaderField: "Content-Type")
request.httpBody = try? JSONEncoder().encode(["email": email, "password": password]) // key 6
password = "" // key 10
do {
let (data, response) = try await session.data(for: request)
switch (response as? HTTPURLResponse)?.statusCode {
case 200:
let token = try JSONDecoder().decode(LoginResponse.self, from: data).token // key 1
try remember(token)
self.token = token
isLoggedIn = true
case 401, 404:
errorMessage = "Email or password is incorrect." // key 9
default:
errorMessage = "Couldn't sign in. Please try again."
}
} catch {
errorMessage = "Couldn't sign in. Please try again." // key 13
}
}
func avatarRequest() -> URLRequest { // key 7
var request = URLRequest(url: URL(string: "https://api.example.com/v1/me/avatar")!)
if let token { request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") }
return request
}
func logout() {
tokenStore.remove() // key 12
for key in ["authToken", "rememberMe", "email", "password"] {
UserDefaults.standard.removeObject(forKey: key)
}
token = nil
isLoggedIn = false
}
private func remember(_ token: String) throws {
if rememberMe {
try tokenStore.set(token) // key 3, 4: only the token
UserDefaults.standard.set(email, forKey: "email")
} else {
tokenStore.remove() // key 15: unticking forgets
UserDefaults.standard.removeObject(forKey: "email")
}
}
}Why each piece:
- "Remember me" now means "stay signed in". The original filled in the email and password. The fix keeps the token, and still pre-fills the email — an email isn't a secret. The password is never written anywhere.
TokenStoreis the one new seam. The view model never seesSecItem…, and tests pass a dictionary. TheURLSessionwas already a dependency; it's now passed in, with.sharedas the default.isSubmittingwithdefer. The flag is set before the firstawait, so a second tap on the main actor sees it and returns;deferresets it on every path out.
Said out loud, not coded: the keychain store is about ten lines — a generic-password query (kSecClass, service, account), SecItemDelete then SecItemAdd with kSecAttrAccessibleWhenUnlockedThisDeviceOnly (delete first, or a second save fails with errSecDuplicateItem), and SecItemCopyMatching to read; honour 429 and Retry-After; revoke the token on the server at logout; check the token's expiry before trusting isLoggedIn; AfterFirstUnlock if a background refresh needs the token; Logger with privacy: .private for other personal data.
Now write the tests
"Good. Now write me the tests that would have stopped this PR — the security ones first."
What I'd test, and why
- A 401 and a 404 show the same message — the leak the review led with: the screen must not tell anyone which emails have an account.
- A double tap sends one request — two
login()calls at once, one request on the wire. - "Remember me" stores only the token — the token lands in the store, the password field is cleared, and nothing is written to
UserDefaultsunderpasswordorauthToken. - The credentials go in a JSON
POSTbody, not the URL — the URL is exactly the HTTPS endpoint with no query, and the body decodes to the email and password,+and&intact. - Logout forgets the session — the store is empty, the remembered email is gone, and the avatar request no longer carries an
Authorizationheader.
I wouldn't unit-test the real keychain store. It is a thin wrapper over Apple's SecItem calls, and the real keychain needs a signed app. One test in the app target, or a check by hand, covers it.
The seam. TokenStore gets an in-memory fake. For the network I don't add a protocol: the test gives the view model a URLSession whose configuration routes every request to a URLProtocol subclass — a fake server inside URLSession that answers with canned status codes and records each request. The suite is .serialized because that fake keeps one shared log.
import Foundation
import Synchronization
import Testing
/// A fake server inside URLSession: answers with canned status codes and records each request.
final class StubServer: URLProtocol {
struct Log { var statuses: [Int] = []; var requests: [URLRequest] = []; var bodies: [Data] = [] }
static let log = Mutex(Log())
override class func canInit(with request: URLRequest) -> Bool { true }
override class func canonicalRequest(for request: URLRequest) -> URLRequest { request }
override func startLoading() {
// Inside URLProtocol the body arrives as a stream, not as httpBody.
let body = request.httpBodyStream.map { stream in
stream.open(); defer { stream.close() }
var data = Data(), buffer = [UInt8](repeating: 0, count: 1024)
while case let n = stream.read(&buffer, maxLength: buffer.count), n > 0 { data.append(buffer, count: n) }
return data
} ?? Data()
let status = Self.log.withLock { log in
log.requests.append(request); log.bodies.append(body)
return log.statuses.isEmpty ? 500 : log.statuses.removeFirst()
}
let response = HTTPURLResponse(url: request.url!, statusCode: status, httpVersion: nil, headerFields: nil)!
client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed)
client?.urlProtocol(self, didLoad: Data(#"{"token":"t-123"}"#.utf8))
client?.urlProtocolDidFinishLoading(self)
}
override func stopLoading() {}
}
/// The real keychain is Apple's code and needs a signed app; a dictionary stands in for it.
final class InMemoryTokenStore: TokenStore {
var stored: String?
func token() -> String? { stored }
func set(_ token: String) throws { stored = token }
func remove() { stored = nil }
}
@MainActor
@Suite(.serialized) // one StubServer log, shared
struct LoginViewModelTests {
let store = InMemoryTokenStore()
func makeViewModel(answering statuses: Int...) -> LoginViewModel {
StubServer.log.withLock { $0 = StubServer.Log(statuses: statuses) }
let configuration = URLSessionConfiguration.ephemeral
configuration.protocolClasses = [StubServer.self]
let viewModel = LoginViewModel(session: URLSession(configuration: configuration), tokenStore: store)
viewModel.email = "ana@example.com"
viewModel.password = "p@ss+w0rd&x"
return viewModel
}
@Test
func unknownEmailAndWrongPasswordLookTheSame() async {
let viewModel = makeViewModel(answering: 404, 401)
await viewModel.login()
let unknownEmail = viewModel.errorMessage
viewModel.password = "another try"
await viewModel.login()
#expect(unknownEmail == "Email or password is incorrect.")
#expect(viewModel.errorMessage == unknownEmail)
}
@Test
func doubleTapSendsOneRequest() async {
let viewModel = makeViewModel(answering: 200, 200)
async let firstTap: Void = viewModel.login()
async let secondTap: Void = viewModel.login()
_ = await (firstTap, secondTap)
#expect(StubServer.log.withLock { $0.requests.count } == 1)
#expect(viewModel.isLoggedIn)
}
@Test
func rememberMeStoresOnlyTheToken() async {
let viewModel = makeViewModel(answering: 200)
viewModel.rememberMe = true
await viewModel.login()
#expect(store.stored == "t-123")
#expect(viewModel.password.isEmpty)
#expect(UserDefaults.standard.string(forKey: "password") == nil)
#expect(UserDefaults.standard.string(forKey: "authToken") == nil)
}
@Test
func credentialsGoInAJSONPostBodyNotTheURL() async throws {
let viewModel = makeViewModel(answering: 401)
await viewModel.login()
let (request, body) = try #require(StubServer.log.withLock { log in
log.requests.first.map { ($0, log.bodies[0]) }
})
#expect(request.url?.absoluteString == "https://api.example.com/v1/login")
#expect(request.httpMethod == "POST")
#expect(request.value(forHTTPHeaderField: "Content-Type") == "application/json")
let sent = try JSONDecoder().decode([String: String].self, from: body)
#expect(sent == ["email": "ana@example.com", "password": "p@ss+w0rd&x"])
}
@Test
func logoutForgetsTheSession() async {
let viewModel = makeViewModel(answering: 200)
viewModel.rememberMe = true
await viewModel.login()
viewModel.logout()
#expect(!viewModel.isLoggedIn)
#expect(store.stored == nil)
#expect(UserDefaults.standard.string(forKey: "email") == nil)
#expect(viewModel.avatarRequest().value(forHTTPHeaderField: "Authorization") == nil)
}
}One thing that trips people up: inside a URLProtocol, the request's body arrives as httpBodyStream, not httpBody, so the fake reads the stream.
Ran with Swift 6.4: 5 tests, all passed.
What I'd ask next
- "How would two of our apps share the login?" — The PhonePe question. Put both apps in the same keychain access group (the Keychain Sharing capability, same team ID) and set
kSecAttrAccessGroupon the item. Both apps read the same token. Apps from different teams can't share keychain items. - "Is the keychain enough?" — The Medium story's point. The keychain protects the token at rest on this device. It doesn't stop a stolen token being replayed from elsewhere. Keep access tokens short-lived with a refresh token, revoke on logout, and for high-value actions bind the session to the device — a key in the Secure Enclave or App Attest.
- "Does the keychain survive deleting the app?" — Often yes: keychain items can outlive the app. Store an "installed" flag in
UserDefaults(which is deleted with the app) and, on a fresh install, wipe the keychain items before reading them. - "Face ID for 'remember me'?" — Add a
SecAccessControlwith.biometryCurrentSetto the item. The system then shows Face ID when the item is read; you never handle the biometric yourself. - "What should logout also do?" — Call the server to revoke the token, clear cookies and
URLCache, and drop any per-user caches, so the next person sees nothing of the last.