Skip to content

Commit

Permalink
Store UserExecutor requests that need auth
Browse files Browse the repository at this point in the history 
This PR adds a pendingAuthRequests dictionary that stores the requests that are waiting for an updated JWT keyed on externalId.

When a requests fails with a 401 due to JWT or fails when preparing for execution we remove the request from the request queue and add it to the pending dictionary.

Once we get the onJWTUpdated callback for that externalId we requeue the pending requests and try again.

Also update tests to account for the callback object change and add tests for the new case
  • Loading branch information
@emawby
emawby committed Sep 11, 2024
1 parent 1d0b97f commit f3cb3d8
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 33 deletions.
54 changes: 43 additions & 11 deletions iOS_SDK/OneSignalSDK/OneSignalUser/Source/Executors/OSUserExecutor.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import OneSignalOSCore
*/
class OSUserExecutor {
var userRequestQueue: [OSUserRequest] = []
var pendingAuthRequests: [String: [OSUserRequest]] = [String:[OSUserRequest]]()
private let newRecordsState: OSNewRecordsState
let jwtConfig: OSUserJwtConfig

Expand Down Expand Up @@ -289,6 +290,22 @@ extension OSUserExecutor {
appendToQueue(request)
executePendingRequests()
}

func pendRequestUntilAuthUpdated(_ request: OSUserRequest, externalId: String?) {
self.dispatchQueue.async {
self.userRequestQueue.removeAll(where: { $0 == request})
guard let externalId = externalId else {
return
}
var requests = self.pendingAuthRequests[externalId] ?? []
let inQueue = requests.contains(where: {$0 == request})
guard !inQueue else {
return
}
requests.append(request)
self.pendingAuthRequests[externalId] = requests
}
}

func executeCreateUserRequest(_ request: OSRequestCreateUser) {
guard !request.sentToClient else {
Expand All @@ -301,6 +318,11 @@ extension OSUserExecutor {
request.pushSubscriptionModel = pushSubscriptionModel
request.updatePushSubscriptionModel(pushSubscriptionModel)
}

guard request.addJWTHeaderIsValid(identityModel: request.identityModel) else {
pendRequestUntilAuthUpdated(request, externalId:request.identityModel.externalId)
return
}

guard request.prepareForExecution(newRecordsState: newRecordsState)
else {
Expand Down Expand Up @@ -344,7 +366,7 @@ extension OSUserExecutor {
OneSignalLog.onesignalLog(.LL_ERROR, message: "OSUserExecutor no externalId for unauthorized request.")
return
}
self.handleUnauthorizedError(externalId: externalId, error: nsError)
self.handleUnauthorizedError(externalId: externalId, error: nsError, request: request)
request.sentToClient = false
} else if responseType != .retryable {
// A failed create user request would leave the SDK in a bad state
Expand All @@ -361,8 +383,9 @@ extension OSUserExecutor {
}
}

func handleUnauthorizedError(externalId: String, error: NSError) {
func handleUnauthorizedError(externalId: String, error: NSError, request: OSUserRequest) {
if (jwtConfig.isRequired ?? false) {
self.pendRequestUntilAuthUpdated(request, externalId: externalId)
OneSignalUserManagerImpl.sharedInstance.invalidateJwtForExternalId(externalId: externalId, error: error)
}
}
Expand All @@ -376,6 +399,7 @@ extension OSUserExecutor {

/**
For migrating legacy players from 3.x to 5.x. This request will fetch the identity object for a subscription ID, and we will use the returned onesignalId to fetch and hydrate the local user.
ECM can this ever succeed with identity verification on?
*/
func executeFetchIdentityBySubscriptionRequest(_ request: OSRequestFetchIdentityBySubscription) {
guard !request.sentToClient else {
Expand Down Expand Up @@ -485,7 +509,7 @@ extension OSUserExecutor {
// This will hydrate the OneSignal ID for any pending requests
self.createUser(aliasLabel: request.aliasLabel, aliasId: request.aliasId, identityModel: request.identityModelToUpdate)
}
} else if responseType == .invalid || responseType == .unauthorized {
} else if responseType == .invalid || responseType == .unauthorized { //Identify User should never be called with identity verification on
// Failed, no retry
self.removeFromQueue(request)
self.executePendingRequests()
Expand Down Expand Up @@ -574,7 +598,7 @@ extension OSUserExecutor {
OneSignalUserManagerImpl.sharedInstance._logout()
} else if responseType == .unauthorized && (self.jwtConfig.isRequired ?? false) {
if let externalId = request.identityModel.externalId {
self.handleUnauthorizedError(externalId: externalId, error: nsError)
self.handleUnauthorizedError(externalId: externalId, error: nsError, request: request)
}
request.sentToClient = false
} else if responseType != .retryable {
Expand Down Expand Up @@ -707,15 +731,23 @@ extension OSUserExecutor: OSUserJwtConfigListener {
}

func onJwtUpdated(externalId: String, token: String?) {
/*
ECM
Do we actually even need this callback?
Requests that are invalidated do not pass prepare for execution
Once they are valid they will pass prepare for execution.
We could use this callback to optimize sending requests immediately
*/
reQueuePendingRequestsForExternalId(externalId: externalId)
print("❌ OSUserExecutor onJwtUpdated for \(externalId) to \(String(describing: token))")
}

private func reQueuePendingRequestsForExternalId(externalId: String) {
self.dispatchQueue.async {
guard let requests = self.pendingAuthRequests[externalId] else {
return
}
for request in requests {
self.userRequestQueue.append(request)
}
self.pendingAuthRequests[externalId] = nil
OneSignalUserDefaults.initShared().saveCodeableData(forKey: OS_USER_EXECUTOR_USER_REQUEST_QUEUE_KEY, withValue: self.userRequestQueue)
self.executePendingRequests(withDelay: true)
}
}

private func removeInvalidRequests() {
self.dispatchQueue.async {
Expand Down
5 changes: 4 additions & 1 deletion iOS_SDK/OneSignalSDK/OneSignalUserMocks/MockUserDefines.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ public let userB_EUID = "test_user_b_external_id"
public let testPushSubId = "test_push_subscription_id"
public let testEmailSubId = "test_email_subscription_id"
public let testPushToken = "2b7347630b72265c83b1c1d2227f563ce6169d5aaf274b06f1a1fadf3a04be69"
public let userA_JwtToken = "eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiIwMTM5YmQ2Zi00NTFmLTQzOGMtODg4Ni00ZTBmMGZlM2EwODUiLCJleHAiOjE3MjUzOTY3NTksImlkZW50aXR5Ijp7ImV4dGVybmFsX2lkIjoiZWxsaW90MTE0MCJ9LCJzdWJzY3JpcHRpb25zIjpbeyJ0eXBlIjoiRW1haWwiLCJ0b2tlbiI6InRlc3RAZG9tYWluLmNvbSJ9LHsidHlwZSI6IlNNUyIsInRva2VuIjoiKzEyMzQ1Njc4In1dfQ.wmtt8mH7wYpxmUDyx_l8ktfF4Eg-6y_4iOSsIEl3AxuQ5pEriCIRj-3P-NmSPO3jsSAGPeBRZQ-rRS5j-LbN1w"
public let userA_InvalidJwtToken = "byJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiIwMTM5YmQ2Zi00NTFmLTQzOGMtODg4Ni00ZTBmMGZlM2EwODUiLCJleHAiOjE3MjUzOTY3NTksImlkZW50aXR5Ijp7ImV4dGVybmFsX2lkIjoiZWxsaW90MTE0MCJ9LCJzdWJzY3JpcHRpb25zIjpbeyJ0eXBlIjoiRW1haWwiLCJ0b2tlbiI6InRlc3RAZG9tYWluLmNvbSJ9LHsidHlwZSI6IlNNUyIsInRva2VuIjoiKzEyMzQ1Njc4In1dfQ.wmtt8mH7wYpxmUDyx_l8ktfF4Eg-6y_4iOSsIEl3AxuQ5pEriCIRj-3P-NmSPO3jsSAGPeBRZQ-rRS5j-LbN1w"

public let userA_ValidJwtToken = "eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiIwMTM5YmQ2Zi00NTFmLTQzOGMtODg4Ni00ZTBmMGZlM2EwODUiLCJleHAiOjE3MjUzOTY3NTksImlkZW50aXR5Ijp7ImV4dGVybmFsX2lkIjoiZWxsaW90MTE0MCJ9LCJzdWJzY3JpcHRpb25zIjpbeyJ0eXBlIjoiRW1haWwiLCJ0b2tlbiI6InRlc3RAZG9tYWluLmNvbSJ9LHsidHlwZSI6IlNNUyIsInRva2VuIjoiKzEyMzQ1Njc4In1dfQ.wmtt8mH7wYpxmUDyx_l8ktfF4Eg-6y_4iOSsIEl3AxuQ5pEriCIRj-3P-NmSPO3jsSAGPeBRZQ-rRS5j-LbN1w"
public let userB_ValidJwtToken = "fyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiIwMTM5YmQ2Zi00NTFmLTQzOGMtODg4Ni00ZTBmMGZlM2EwODUiLCJleHAiOjE3MjUzOTY3NTksImlkZW50aXR5Ijp7ImV4dGVybmFsX2lkIjoiZWxsaW90MTE0MCJ9LCJzdWJzY3JpcHRpb25zIjpbeyJ0eXBlIjoiRW1haWwiLCJ0b2tlbiI6InRlc3RAZG9tYWluLmNvbSJ9LHsidHlwZSI6IlNNUyIsInRva2VuIjoiKzEyMzQ1Njc4In1dfQ.wmtt8mH7wYpxmUDyx_l8ktfF4Eg-6y_4iOSsIEl3AxuQ5pEriCIRj-3P-NmSPO3jsSAGPeBRZQ-rRS5j-LbN1w"

public let userA_email = "userA@onesignal.com"

Expand Down
8 changes: 3 additions & 5 deletions iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/IdentityExecutorTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ final class IdentityExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let aliases = userA_Aliases
MockUserRequests.setAddAliasesResponse(with: mocks.client, aliases: aliases)
mocks.identityExecutor.enqueueDelta(OSDelta(name: OS_ADD_ALIAS_DELTA, identityModelId: user.identityModel.modelId, model: user.identityModel, property: "aliases", value:aliases))
Expand All @@ -120,14 +120,13 @@ final class IdentityExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let aliases = userA_Aliases
MockUserRequests.setUnauthorizedAddAliasFailureResponse(with: mocks.client, aliases: aliases)
mocks.identityExecutor.enqueueDelta(OSDelta(name: OS_ADD_ALIAS_DELTA, identityModelId: user.identityModel.modelId, model: user.identityModel, property: "aliases", value:aliases))

var invalidatedCallbackWasCalled = false
OneSignalUserManagerImpl.sharedInstance.User.onJwtInvalidated { event in
XCTAssertTrue(event.message == "token has invalid claims: token is expired")
invalidatedCallbackWasCalled = true
}

Expand All @@ -147,14 +146,13 @@ final class IdentityExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let aliases = userA_Aliases
MockUserRequests.setUnauthorizedRemoveAliasFailureResponse(with: mocks.client, aliasLabel: userA_AliasLabel)
mocks.identityExecutor.enqueueDelta(OSDelta(name: OS_REMOVE_ALIAS_DELTA, identityModelId: user.identityModel.modelId, model: user.identityModel, property: "aliases", value:aliases))

var invalidatedCallbackWasCalled = false
OneSignalUserManagerImpl.sharedInstance.User.onJwtInvalidated { event in
XCTAssertTrue(event.message == "token has invalid claims: token is expired")
invalidatedCallbackWasCalled = true
}

Expand Down
5 changes: 2 additions & 3 deletions iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/PropertyExecutorTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ final class PropertyExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let tags = ["testUserA" : "true"]
MockUserRequests.setAddTagsResponse(with: mocks.client, tags: tags)
mocks.propertyExecutor.enqueueDelta(OSDelta(name: OS_UPDATE_PROPERTIES_DELTA, identityModelId: user.identityModel.modelId, model: OSPropertiesModel(changeNotifier: OSEventProducer()), property: "tags", value:tags))
Expand All @@ -120,7 +120,7 @@ final class PropertyExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken



Expand All @@ -130,7 +130,6 @@ final class PropertyExecutorTests: XCTestCase {

var invalidatedCallbackWasCalled = false
OneSignalUserManagerImpl.sharedInstance.User.onJwtInvalidated { event in
XCTAssertTrue(event.message == "token has invalid claims: token is expired")
invalidatedCallbackWasCalled = true
}

Expand Down
11 changes: 4 additions & 7 deletions iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/SubscriptionsExecutorTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ final class SubscriptionExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let email = userA_email
MockUserRequests.setAddEmailResponse(with: mocks.client, email: email)
mocks.subscriptionExecutor.enqueueDelta(OSDelta(name: OS_ADD_SUBSCRIPTION_DELTA, identityModelId: user.identityModel.modelId, model: OSSubscriptionModel(type: .email, address: email, subscriptionId: nil, reachable: true, isDisabled: false, changeNotifier: OSEventProducer()), property: OSSubscriptionType.email.rawValue, value:email))
Expand All @@ -120,14 +120,13 @@ final class SubscriptionExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let email = userA_email
MockUserRequests.setUnauthorizedAddEmailFailureResponse(with: mocks.client, email: email)
mocks.subscriptionExecutor.enqueueDelta(OSDelta(name: OS_ADD_SUBSCRIPTION_DELTA, identityModelId: user.identityModel.modelId, model: OSSubscriptionModel(type: .email, address: email, subscriptionId: nil, reachable: true, isDisabled: false, changeNotifier: OSEventProducer()), property: OSSubscriptionType.email.rawValue, value:email))

var invalidatedCallbackWasCalled = false
OneSignalUserManagerImpl.sharedInstance.User.onJwtInvalidated { event in
XCTAssertTrue(event.message == "token has invalid claims: token is expired")
invalidatedCallbackWasCalled = true
}

Expand All @@ -147,14 +146,13 @@ final class SubscriptionExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let email = userA_email
MockUserRequests.setUnauthorizedRemoveEmailFailureResponse(with: mocks.client, email: email)
mocks.subscriptionExecutor.enqueueDelta(OSDelta(name: OS_REMOVE_SUBSCRIPTION_DELTA, identityModelId: user.identityModel.modelId, model: OSSubscriptionModel(type: .email, address: email, subscriptionId: testEmailSubId, reachable: true, isDisabled: false, changeNotifier: OSEventProducer()), property: OSSubscriptionType.email.rawValue, value:email))

var invalidatedCallbackWasCalled = false
OneSignalUserManagerImpl.sharedInstance.User.onJwtInvalidated { event in
XCTAssertTrue(event.message == "token has invalid claims: token is expired")
invalidatedCallbackWasCalled = true
}

Expand All @@ -174,14 +172,13 @@ final class SubscriptionExecutorTests: XCTestCase {
OneSignalUserManagerImpl.sharedInstance.operationRepo.paused = true

let user = mocks.setUserManagerInternalUser(externalId: userA_EUID, onesignalId: userA_OSID)
user.identityModel.jwtBearerToken = userA_JwtToken
user.identityModel.jwtBearerToken = userA_InvalidJwtToken
let token = testPushToken
MockUserRequests.setUnauthorizedUpdateSubscriptionFailureResponse(with: mocks.client, token: token)
mocks.subscriptionExecutor.enqueueDelta(OSDelta(name: OS_UPDATE_SUBSCRIPTION_DELTA, identityModelId: user.identityModel.modelId, model: OSSubscriptionModel(type: .push, address: token, subscriptionId: testPushSubId, reachable: true, isDisabled: false, changeNotifier: OSEventProducer()), property: "token", value:token))

var invalidatedCallbackWasCalled = false
OneSignalUserManagerImpl.sharedInstance.User.onJwtInvalidated { event in
XCTAssertTrue(event.message == "token has invalid claims: token is expired")
invalidatedCallbackWasCalled = true
}

Expand Down
Loading

0 comments on commit f3cb3d8

Please sign in to comment.