Prevent the session expired error state from changing on tap (#466)

Co-authored-by: etoledom <[email protected]>

Sources/GravatarUI/SwiftUI/AvatarPicker/AvatarPickerView.swift

@@ -10,18 +10,66 @@ struct AvatarPickerView<ImageEditor: ImageEditorView>: View {
     @Environment(\.verticalSizeClass) var verticalSizeClass
     @Environment(\.horizontalSizeClass) var horizontalSizeClass
 
-    @StateObject var model: AvatarPickerViewModel
+    // Declare "@StateObject"s as private to prevent setting them from a
+    // memberwise initializer, which can conflict with the storage
+    // management that SwiftUI provides.
+    // https://developer.apple.com/documentation/swiftui/stateobject
+    @StateObject private var model: AvatarPickerViewModel
     @Binding var isPresented: Bool
+    @Binding var authToken: String?
 
     @State private var safariURL: URL?
     @State private var uploadError: FailedUploadInfo?
     @State private var isUploadErrorDialogPresented: Bool = false
 
-    var contentLayoutProvider: AvatarPickerContentLayoutProviding = AvatarPickerContentLayoutType.vertical
+    var contentLayoutProvider: AvatarPickerContentLayoutProviding
     var customImageEditor: ImageEditorBlock<ImageEditor>?
     var tokenErrorHandler: (() -> Void)?
     var avatarUpdatedHandler: (() -> Void)?
 
+    init(
+        email: Email,
+        authToken: Binding<String?>,
+        isPresented: Binding<Bool>,
+        contentLayoutProvider: AvatarPickerContentLayoutProviding = AvatarPickerContentLayoutType.vertical,
+        customImageEditor: ImageEditorBlock<ImageEditor>? = nil as NoCustomEditorBlock?,
+        tokenErrorHandler: (() -> Void)? = nil,
+        avatarUpdatedHandler: (() -> Void)? = nil
+    ) {
+        self._isPresented = isPresented
+        self.contentLayoutProvider = contentLayoutProvider
+        self.customImageEditor = customImageEditor
+        self.tokenErrorHandler = tokenErrorHandler
+        self.avatarUpdatedHandler = avatarUpdatedHandler
+        self._authToken = authToken
+        self._model = StateObject(wrappedValue: AvatarPickerViewModel(email: email, authToken: authToken.wrappedValue))
+    }
+
+    fileprivate init(
+        avatarImageModels: [AvatarImageModel],
+        selectedImageID: String? = nil,
+        profileModel: ProfileSummaryModel? = nil,
+        isPresented: Binding<Bool>,
+        contentLayoutProvider: AvatarPickerContentLayoutProviding = AvatarPickerContentLayoutType.vertical,
+        customImageEditor: ImageEditorBlock<ImageEditor>? = nil as NoCustomEditorBlock?,
+        tokenErrorHandler: (() -> Void)? = nil,
+        avatarUpdatedHandler: (() -> Void)? = nil
+    ) {
+        self._isPresented = isPresented
+        self.contentLayoutProvider = contentLayoutProvider
+        self.customImageEditor = customImageEditor
+        self.tokenErrorHandler = tokenErrorHandler
+        self.avatarUpdatedHandler = avatarUpdatedHandler
+        self._authToken = .constant(nil)
+        self._model = StateObject(
+            wrappedValue: AvatarPickerViewModel(
+                avatarImageModels: avatarImageModels,
+                selectedImageID: selectedImageID,
+                profileModel: profileModel
+            )
+        )
+    }
+
     public var body: some View {
         ZStack {
             VStack(spacing: 0) {
@@ -87,6 +135,9 @@ struct AvatarPickerView<ImageEditor: ImageEditorView>: View {
             SafariView(url: url)
                 .edgesIgnoringSafeArea(.all)
         }
+        .onChange(of: authToken ?? "") { newValue in
+            model.update(authToken: newValue)
+        }
     }
 
     private func header() -> some View {
@@ -507,30 +558,34 @@ private enum AvatarPicker {
         }
     }
 
-    let model = AvatarPickerViewModel(
-        avatarImageModels: [
-            .init(id: "0", source: .local(image: UIImage()), state: .loading),
-            .init(id: "1", source: .remote(url: "https://gravatar.com/userimage/110207384/aa5f129a2ec75162cee9a1f0c472356a.jpeg?size=256")),
-            .init(id: "2", source: .remote(url: "https://gravatar.com/userimage/110207384/db73834576b01b69dd8da1e29877ca07.jpeg?size=256")),
-            .init(id: "3", source: .remote(url: "https://gravatar.com/userimage/110207384/3f7095bf2580265d1801d128c6410016.jpeg?size=256")),
-            .init(id: "4", source: .remote(url: "https://gravatar.com/userimage/110207384/fbbd335e57862e19267679f19b4f9db8.jpeg?size=256")),
-            .init(id: "5", source: .remote(url: "https://gravatar.com/userimage/110207384/96c6950d6d8ce8dd1177a77fe738101e.jpeg?size=256")),
-            .init(id: "6", source: .remote(url: "https://gravatar.com/userimage/110207384/4a4f9385b0a6fa5c00342557a098f480.jpeg?size=256")),
-            .init(id: "7", source: .local(image: UIImage()), state: .error(supportsRetry: true, errorMessage: "Something went wrong.")),
-            .init(id: "8", source: .local(image: UIImage()), state: .error(supportsRetry: false, errorMessage: "Something went wrong.")),
-        ],
-        selectedImageID: "5",
-        profileModel: PreviewModel()
+    let avatarImageModels: [AvatarImageModel] = [
+        .init(id: "0", source: .local(image: UIImage()), state: .loading),
+        .init(id: "1", source: .remote(url: "https://gravatar.com/userimage/110207384/aa5f129a2ec75162cee9a1f0c472356a.jpeg?size=256")),
+        .init(id: "2", source: .remote(url: "https://gravatar.com/userimage/110207384/db73834576b01b69dd8da1e29877ca07.jpeg?size=256")),
+        .init(id: "3", source: .remote(url: "https://gravatar.com/userimage/110207384/3f7095bf2580265d1801d128c6410016.jpeg?size=256")),
+        .init(id: "4", source: .remote(url: "https://gravatar.com/userimage/110207384/fbbd335e57862e19267679f19b4f9db8.jpeg?size=256")),
+        .init(id: "5", source: .remote(url: "https://gravatar.com/userimage/110207384/96c6950d6d8ce8dd1177a77fe738101e.jpeg?size=256")),
+        .init(id: "6", source: .remote(url: "https://gravatar.com/userimage/110207384/4a4f9385b0a6fa5c00342557a098f480.jpeg?size=256")),
+        .init(id: "7", source: .local(image: UIImage()), state: .error(supportsRetry: true, errorMessage: "Something went wrong.")),
+        .init(id: "8", source: .local(image: UIImage()), state: .error(supportsRetry: false, errorMessage: "Something went wrong.")),
+    ]
+    let selectedImageID = "5"
+    let profileModel = PreviewModel()
+
+    return AvatarPickerView<NoCustomEditor>(
+        avatarImageModels: avatarImageModels,
+        selectedImageID: selectedImageID,
+        profileModel: profileModel,
+        isPresented: .constant(true),
+        contentLayoutProvider: AvatarPickerContentLayoutType.horizontal
     )
-
-    return AvatarPickerView<NoCustomEditor>(model: model, isPresented: .constant(true), contentLayoutProvider: AvatarPickerContentLayoutType.horizontal)
 }
 
 #Preview("Empty elements") {
-    AvatarPickerView<NoCustomEditor>(model: .init(avatarImageModels: [], profileModel: nil), isPresented: .constant(true))
+    AvatarPickerView<NoCustomEditor>(avatarImageModels: [], profileModel: nil, isPresented: .constant(true))
 }
 
 #Preview("Load from network") {
     /// Enter valid email and auth token.
-    AvatarPickerView<NoCustomEditor>(model: .init(email: .init(""), authToken: ""), isPresented: .constant(true))
+    AvatarPickerView<NoCustomEditor>(email: .init(""), authToken: .constant(""), isPresented: .constant(true))
 }

Sources/GravatarUI/SwiftUI/AvatarPicker/AvatarPickerViewModel.swift

@@ -47,7 +47,7 @@ class AvatarPickerViewModel: ObservableObject {
     @Published var profileModel: AvatarPickerProfileView.Model?
     @ObservedObject var toastManager: ToastManager = .init()
 
-    init(email: Email, authToken: String) {
+    init(email: Email, authToken: String?) {
         self.email = email
         avatarIdentifier = .email(email)
         self.authToken = authToken

Sources/GravatarUI/SwiftUI/OAuthSession/Keychain.swift

@@ -1,9 +1,9 @@
 import Foundation
 
 protocol SecureStorage: Sendable {
-    func setSecret(_ secret: String, for key: String) throws
+    func setSecret(_ secret: KeychainToken, for key: String) throws
     func deleteSecret(with key: String) throws
-    func secret(with key: String) throws -> String?
+    func secret(with key: String) throws -> KeychainToken?
 }
 
 struct Keychain: SecureStorage {
@@ -13,8 +13,8 @@ struct Keychain: SecureStorage {
         case unhandledError(status: OSStatus, message: String?)
     }
 
-    func setSecret(_ secret: String, for key: String) throws {
-        guard let tokenData = secret.data(using: .utf8) else {
+    func setSecret(_ secret: KeychainToken, for key: String) throws {
+        guard let tokenData = secret.data else {
             throw KeychainError.cannotConvertSecretIntoData
         }
 
@@ -31,7 +31,7 @@ struct Keychain: SecureStorage {
         }
     }
 
-    func secret(with key: String) throws -> String? {
+    func secret(with key: String) throws -> KeychainToken? {
         let query: [String: Any] = [
             kSecClass as String: kSecClassGenericPassword,
             kSecMatchLimit as String: kSecMatchLimitOne,
@@ -51,7 +51,7 @@ struct Keychain: SecureStorage {
         guard
             let existingItem = item as? [String: Any],
             let secretData = existingItem[kSecValueData as String] as? Data,
-            let secret = String(data: secretData, encoding: .utf8)
+            let secret = KeychainToken(data: secretData)
         else {
             throw KeychainError.unexpectedSecretData
         }

Sources/GravatarUI/SwiftUI/OAuthSession/KeychainToken.swift

@@ -0,0 +1,25 @@
+import Foundation
+
+struct KeychainToken: Codable {
+    let token: String
+    var isExpired: Bool = false
+
+    init?(data: Data) {
+        let decoder = JSONDecoder()
+        do {
+            let decodedToken = try decoder.decode(KeychainToken.self, from: data)
+            self = decodedToken
+        } catch {
+            return nil
+        }
+    }
+
+    init(token: String) {
+        self.token = token
+    }
+
+    var data: Data? {
+        let encoder = JSONEncoder()
+        return try? encoder.encode(self)
+    }
+}

Sources/GravatarUI/SwiftUI/OAuthSession/OAuthSession.swift

@@ -23,16 +23,34 @@ public struct OAuthSession: Sendable {
         (try? storage.secret(with: email.rawValue) ?? nil) != nil
     }
 
+    func hasValidSession(with email: Email) -> Bool {
+        guard let token = try? storage.secret(with: email.rawValue) else {
+            return false
+        }
+        return !token.isExpired
+    }
+
+    func markSessionAsExpired(with email: Email) {
+        guard var token = sessionToken(with: email), !token.isExpired else { return }
+        token.isExpired = true
+        overrideToken(token, for: email)
+    }
+
+    func overrideToken(_ token: KeychainToken, for email: Email) {
+        deleteSession(with: email)
+        try? storage.setSecret(token, for: email.rawValue)
+    }
+
     public func deleteSession(with email: Email) {
         try? storage.deleteSecret(with: email.rawValue)
     }
 
-    func sessionToken(with email: Email) -> String? {
+    func sessionToken(with email: Email) -> KeychainToken? {
         try? storage.secret(with: email.rawValue)
     }
 
     @discardableResult
-    func retrieveAccessToken(with email: Email) async throws -> String {
+    func retrieveAccessToken(with email: Email) async throws -> KeychainToken {
         guard let secrets = await Configuration.shared.oauthSecrets else {
             assertionFailure("Trying to retrieve access token without configuring oauth secrets.")
             throw OAuthError.notConfigured
@@ -41,12 +59,13 @@ public struct OAuthSession: Sendable {
         do {
             let url = try oauthURL(with: email, secrets: secrets)
             let callbackURL = try await authenticationSession.authenticate(using: url, callbackURLScheme: secrets.callbackScheme)
-            let token = try tokenResponse(from: callbackURL).token
-            guard try await CheckTokenAuthorizationService().isToken(token, authorizedFor: email) else {
+            let tokenText = try tokenResponse(from: callbackURL).token
+            guard try await CheckTokenAuthorizationService().isToken(tokenText, authorizedFor: email) else {
                 throw OAuthError.loggedInWithWrongEmail(email: email.rawValue)
             }
-            try storage.setSecret(token, for: email.rawValue)
-            return token
+            let newToken = KeychainToken(token: tokenText)
+            overrideToken(newToken, for: email)
+            return newToken
         } catch {
             throw OAuthError.from(error: error)
         }

Sources/GravatarUI/SwiftUI/ProfileEditor/QuickEditor.swift

@@ -24,13 +24,12 @@ struct QuickEditor<ImageEditor: ImageEditorView>: View {
     fileprivate typealias Constants = QuickEditorConstants
 
     @Environment(\.oauthSession) private var oauthSession
-    @State var hasSession: Bool = false
+    @State var token: String?
     @State var scope: QuickEditorScopeType
     @State var isAuthenticating: Bool = true
     @State var oauthError: OAuthError?
     @Binding var isPresented: Bool
     let email: Email
-    let token: String?
     var customImageEditor: ImageEditorBlock<ImageEditor>?
     var contentLayoutProvider: AvatarPickerContentLayoutProviding
     var avatarUpdatedHandler: (() -> Void)?
@@ -57,8 +56,6 @@ struct QuickEditor<ImageEditor: ImageEditorView>: View {
         NavigationView {
             if let token {
                 editorView(with: token)
-            } else if hasSession, let token = oauthSession.sessionToken(with: email) {
-                editorView(with: token)
             } else {
                 noticeView()
                     .accumulateIntrinsicHeight()
@@ -71,12 +68,13 @@ struct QuickEditor<ImageEditor: ImageEditorView>: View {
         switch scope {
         case .avatarPicker:
             AvatarPickerView(
-                model: .init(email: email, authToken: token),
+                email: email,
+                authToken: $token,
                 isPresented: $isPresented,
                 contentLayoutProvider: contentLayoutProvider,
                 customImageEditor: customImageEditor,
                 tokenErrorHandler: {
-                    oauthSession.deleteSession(with: email)
+                    oauthSession.markSessionAsExpired(with: email)
                     performAuthentication()
                 },
                 avatarUpdatedHandler: avatarUpdatedHandler
@@ -128,7 +126,7 @@ struct QuickEditor<ImageEditor: ImageEditorView>: View {
     func performAuthentication() {
         Task {
             isAuthenticating = true
-            if !oauthSession.hasSession(with: email) {
+            if !oauthSession.hasValidSession(with: email) {
                 do {
                     _ = try await oauthSession.retrieveAccessToken(with: email)
                     oauthError = nil
@@ -140,7 +138,7 @@ struct QuickEditor<ImageEditor: ImageEditorView>: View {
                     oauthError = nil
                 }
             }
-            hasSession = oauthSession.hasSession(with: email)
+            token = oauthSession.sessionToken(with: email)?.token
             isAuthenticating = false
         }
     }

Sources/GravatarUI/SwiftUI/View+Additions.swift

@@ -20,7 +20,8 @@ extension View {
         avatarUpdatedHandler: (() -> Void)? = nil
     ) -> some View {
         let avatarPickerView = AvatarPickerView(
-            model: AvatarPickerViewModel(email: Email(email), authToken: authToken),
+            email: Email(email),
+            authToken: .constant(authToken),
             isPresented: isPresented,
             contentLayoutProvider: AvatarPickerContentLayoutType.vertical,
             customImageEditor: customImageEditor,
@@ -40,7 +41,8 @@ extension View {
         avatarUpdatedHandler: (() -> Void)? = nil
     ) -> some View {
         let avatarPickerView = AvatarPickerView(
-            model: AvatarPickerViewModel(email: Email(email), authToken: authToken),
+            email: Email(email),
+            authToken: .constant(authToken),
             isPresented: isPresented,
             contentLayoutProvider: contentLayout,
             customImageEditor: customImageEditor,