Automattic / Gravatar-SDK-iOS

Gravatar SDK is a Swift library that allows you to integrate Gravatar features into your own iOS applications.
https://gravatar.com
Mozilla Public License 2.0
52 stars 5 forks source link

Handle expired token (401 error) on avatar selection and upload #448

Closed andrewdmontgomery closed 1 month ago

andrewdmontgomery commented 1 month ago

Closes #447 #453

Description

If the QE is open while a token expires, the failure message is generic and doesn't provide a way for the user to re-authenticate.

This PR updates the UX, and error message handling, so that the user is informed that their session has expired, and they are presented with the ability to re-authenticate.

This PR also makes the following change:

    func map() -> ResponseErrorReason {
        switch self {
        case .URLSessionError(let error):
            return .URLSessionError(error: error)
        case .invalidHTTPStatusCodeError(let response, let data):
-          if response.statusCode == 400 {
+          if response.statusCode >= 400 {
                let error: ModelError? = try? data.decode()
                return .invalidHTTPStatusCode(response: response, errorPayload: error)
            } else {
                return .invalidHTTPStatusCode(response: response)
            }
        case .invalidURLResponseError(let response):
            return .invalidURLResponse(response: response)
        }
    }

This is technically accurate (all 400+ codes are "invalid". But there may be a good reason why we defined this as ==. So I wanted to call this out.

This PR also localizes one of the toast messages. It looks like other toast messages still need to be localized. I'll handle those in a separate PR.

Updating the OpenAPI spec

This PR now includes an updated OpenAPI spec that addresses the issue mentioned here.

~This PR temporarily updates the OpenAPI.spec – but only as a demonstration. This change to the OpenAPI spec should not be merged unless and until we have discussed it with the Backend team and Android.~

~When an expired token is used on the endpoint /me/avatars/{imageId}/email, the json response only includes the error key/value (with no business code provided):~

{
  "error": "You are not authorized to perform this action."
}

~We are currently using ModelError to represent this response, which is a model generated from the Error schema in the OpenAPI spec. That spec lists both the code and error as "required".~

~As a result, this JSON data in the response fails to decode as a ModelError, and we cannot pass the message to the client.~

~If using ModelError (Error schema) is appropriate for this endpoint, then either:~

~If using the ModelError is not appropriate, then we have some refactoring to do.~

Testing Steps

Testing Avatar Selection

  1. Launch the SwiftUI demo app
  2. Tap Profile editor with oauth
  3. Perform Oauth if necessary
  4. On your computer: a. Go here (with your test app Gravatar account): https://wordpress.com/me/security/connected-applications b. Click "Disconnect" for your test app
  5. Back on your device, attempt to choose a different avatar
  6. OBSERVE:
    • [ ] A toast message appears saying, You are not authorized to perform this action. (This string is provided by the backend in the response, and I believe it should be localized)
    • [ ] The grid of avatars is replaced with the error message "Session expired" and a "Log in" button

Simulator Screenshot - iPhone SE (3rd generation) - 2024-10-01 at 22 07 23

Testing Avatar Upload

  1. Follow the steps 1-4 above
  2. Back on your device, attempt to upload a new image
  3. OBSERVE:
    • [ ] A toast message appears saying, You are not authorized to perform this action. (This string is provided by the backend in the response, and I believe it should be localized)
    • [ ] The grid of avatars is replaced with the error message "Session expired" and a "Log in" button
wpmobilebot commented 1 month ago
Gravatar SwiftUI Prototype Build📲 You can test the changes from this Pull Request in Gravatar SwiftUI Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar SwiftUI Prototype Build Gravatar SwiftUI Prototype Build
Build Number1428
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-swiftui.prototype-build
Commit2d97c892208d160ace65100f340d022a82101ef6
App Center BuildGravatar SDK Demo - SwiftUI #234
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
wpmobilebot commented 1 month ago
Gravatar UIKit Prototype Build📲 You can test the changes from this Pull Request in Gravatar UIKit Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar UIKit Prototype Build Gravatar UIKit Prototype Build
Build Number1428
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-uikit.prototype-build
Commit2d97c892208d160ace65100f340d022a82101ef6
App Center BuildGravatar SDK Demo - UIKit #234
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.
pinarol commented 1 month ago

This is technically accurate (all 400+ codes are "invalid". But there may be a good reason why we defined this as ==. So I wanted to call this out.

That check is for deciding to decode the error into ModelError and we can only assume this is possible if the specs say so. Since we treat the decoding operation as "optional" we can probably move fwd with >=400 as well. But we can probably make it ">=400 and <500", i don't think 5XX errors are any fit for the ModelError anyway.

pinarol commented 1 month ago

This PR temporarily updates the OpenAPI.spec – but only as a demonstration. This change to the OpenAPI spec should not be merged unless and until we have discussed it with the Backend team and Android.

👍

andrewdmontgomery commented 1 month ago

The proposed change to the Error schema was accepted. A more comprehensive change is pending: https://github.a8c.com/Automattic/gravatar/pull/109013

I will wait for that PR to merge, and apply those changes to this PR.

etoledom commented 1 month ago

The proposed change to the Error schema was accepted. A more comprehensive change is pending: https://github.a8c.com/Automattic/gravatar/pull/109013

I will wait for that PR to merge, and apply those changes to this PR.

It has been merged 🎉