[JWT] App startup #1473

nan-li · 2024-08-22T18:09:08Z

Description

One Line Summary

Handles basic-level app startup for Identity Verification - on new install and minor migrations on upgrades.

Details

Internal JWT Config management and listeners

One of the primary complexities is handling the state when an app is first installed or updated, but JWT is unknown. Once this value is received, it can be cached and future runs will be simple.
Make OSUserJwtConfig object to handle auth required status, etc.
User manager owns an instance of the OSUserJwtConfig and passes it to sub-components like Operation Repo and Executors who will basically be "paused" until they know if auth is required or not.
Operations can enqueue while the SDK is in an "unknown" state, they will just be blocked from sending.
Once remote params returns, the SDK learns if auth is required, and this value will be cached for the future. The OSUserJwtConfig fires listeners waiting on this value. If components learn that auth is on, they will process their queues and remove operations that are not valid. Then, the components will start operating normally.
There are lots of print()s to help debug, these will be removed eventually.

Request objects updated to consider JWT

User request instances already have a prepareForExecution() method that indicates if this request can be sent yet. For example, does the onesignal ID exist to make this request.
Piggyback off this existing method to check for auth and block sending this request if auth is required but invalid, or auth is unknown, etc.
We still keep the onesignal ID property on all requests as a way to know if the user has actually been created on the server and to check for the post-create cool-off period. When the request is sent, it will use either the onesignal ID or the external ID based on JWT on/off.
Requests Updated:
- OSRequestCreateUser
- OSRequestFetchUser
- OSRequestAddAliases
- OSRequestRemoveAlias
- OSRequestUpdateProperties
Requests Not Allowed with Identity Verification:
- OSRequestFetchIdentityBySubscription: This is dropped and the push subscription is included in any upcoming UserCreate calls.
- OSRequestIdentifyUser: This is translated into a UserCreate.

What has been tested in this PR

New install with Identity Verification on, and call login with correct JWT, add tag + email, login to another user with correct JWT, and add tag + email
New install with Identity Verification turned off
Updating a token using [OneSignal updateUserJwt:"externalUserId" withToken:"token"]
Migration from a previous version
Upgrading from player model
Please see Manual Testing below for more details

What is not included in this PR

Any subscriptions related changes needed including Push Subscription updates, fetch IAM, Live Activities, or any request outside of the User module
Error handling - 401 responses, firing invalid listeners, retrying
Logging into new users with valid JWT if the previous user has still not successfully logged in, future logins are blocked
Logout
There are existing tangled dependencies that are not ideal, and this PR includes more that could be refactored better.

Motivation

Support Identity Verification

Testing

Unit testing

🚧 WIP

Manual Testing

How to Test:

The Dev App is already pointing at staging with the bundle ID of com.onesignal.example.staging. Update the app ID to either 168... to test Identity Verification off, and 013... to test Identity Verification on. This is documented somewhere and please reach out for the keys and IDs.
Use the "Get Tags" button on the Dev App to print information for testing
Fill in an EID and token and use the Login button or Update button to login or update token.

New install - Identity Verification is off - no login is called

New install, see that no requests go through
Remote params returns and mock a response of "off"
Internal listeners fire and the anonymous user is created with the push subscription
Send tags works, update push sub works
Next cold start, Identity Verification is uncached, "off" is the starting state and things proceed as normal. No listeners fire.

New install - Identity Verification is ON - then login is called

New install, see that no requests go through
Remote params returns and mock a response of "on"
Internal listeners fire, no requests go through
Call login with a valid token
Create User is sent with the push subscription, and succeeds
Response is received and hydrated, send tags works
Fetch IAM and update push subscription does not work, it returns 401
Next cold start, Identity Verification is uncached, "on" is the starting state and things proceed as normal. No listeners fire. Fetch user by EID to hydrate works, session count is sent.

Upgrade from main - Identity Verification is ON

On main, turn off connection, do a new install and call:

[OneSignal login:@"nanaug23a"];
[OneSignal.User addTagWithKey:@"from" value:@"main"];
[OneSignal.User addEmail:@"test@domain.com"];

See that no requests go through due to connection
Update app to this branch and open app
The operations are uncached but not requests go through due to unknown auth
Remote params returns and mock a response of "on"
Internal listeners fire, no requests go through due to missing JWT token
Call [OneSignal updateUserJwt:@"nanaug23a" withToken:@"validToken"] for the external ID that was logged into.
Create User is sent with the push subscription, the tags is sent and the create email is sent. They are successful.

Upgrade from player model - Identity Verification is ON

This is the test process I did:

On player-model-main, do a new install and I did not set external ID.
The player is created. Check the player ID and push token in the dashboard. Kill app.
Update app to this branch and open app
The cached data is read but no user requests are made. Get IAM is automatically sent.
Remote params returns and mock a response of "on". The OSRequestFetchIdentityBySubscription that was previously generated is dropped.
Nothing happens.
Then login with a JWT but forget to include the push subscription ID
Create User is sent with the local push subscription read from cache (this was the player) and includes the player ID and push token, but receive a 401 due to missing push subscription ID in claims.
Update the jwt token with the subscription ID
Create User is re-sent successfully. Fetch user happens.

Affected code checklist

Checklist

Overview

I have filled out all REQUIRED sections above
PR does one thing
Any Public API changes are explained in the PR details and conform to existing APIs

Testing

I have included test coverage for these changes, or explained why they are not needed
All automated tests pass, or I explained why that is not possible
I have personally tested this on my device, or explained why that is not possible

Final pass

Code is as readable as possible.
I have reviewed this PR myself, ensuring it meets each checklist item

This change is

* Add text fields and buttons for testing * Use staging * remove app clips * Make Client more verbose with headers

For debugging

* Make OSUserJwtConfig object to handle auth status etc * User manager owns an instance of the JWT Config

* Add `OSUD_USE_IDENTITY_VERIFICATION` flag to OneSignalCommonDefines

public updateUserJwt method

* User request instances already have a `prepareForExecution()` method that indicates if this request can be sent yet. For example, does the onesignal ID exist to make this request. * Use this existing method to check for auth and block sending this request if auth is required but invalid, or auth is unknown, etc. * OSRequestFetchUser will have onesignalId as a property, regardless of JWT on or off. The aliasLabel and aliasId pair is already expected to be onesignal ID, so make it explicit. * Keeping the onesignal ID can be used to know if the user has actually been created on the server and to check for the post-create cool-off period. The request will be translated to use the onesignal ID or the external ID based on JWT on/off. Requests Updated: * OSRequestCreateUser * OSRequestFetchUser * OSRequestAddAliases * OSRequestRemoveAlias * OSRequestUpdateProperties Requests Not Allowed with Identity Verification: * OSRequestFetchIdentityBySubscription * OSRequestIdentifyUser

* Identity Model Store Listener will ignore it so no Deltas will be enqueued

* As a refresher, the Identity Model Repo holds all Identity Models present during an app run, which can include past users that have pending updates, while the Identity Model Store contains only the current user's Identity Model. * Let the OSIdentityModelRepo be the listener for changes to JWT token updates, and it needs to pass on that information to the User Manager who manages the JWT Config and fires other listeners of token changes.

Updates to: * OSPropertyOperationExecutor * OSIdentityOperationExecutor * OSUserExecutor Updates to OSSubscriptionOperationExecutor is still to be done.

* Refactor from a static shared instance to a instance managed by User Manager * Operation Repo does not flush while Identity Verification is unknown * Currently it is not going to process Deltas based on JWT. Executors will.

nan-li · 2024-08-26T21:42:04Z

iOS_SDK/OneSignalSDK/OneSignalUser/Source/Requests/OSRequestAddAliases.swift

+    /// Needs `onesignal_id` without JWT on or `external_id` with valid JWT to send this request
    func prepareForExecution(newRecordsState: OSNewRecordsState) -> Bool {
-        if let onesignalId = identityModel.onesignalId,
-           newRecordsState.canAccess(onesignalId),
-           let appId = OneSignalConfigManager.getAppId()
-        {
-            self.addJWTHeader(identityModel: identityModel)
-            self.path = "apps/\(appId)/users/by/\(OS_ONESIGNAL_ID)/\(onesignalId)/identity"
-            return true
-        } else {
+        let alias = getAlias(identityModel: identityModel)
+        guard
+            let onesignalId = identityModel.onesignalId,
+            newRecordsState.canAccess(onesignalId),
+            let aliasIdToUse = alias.id,
+            let appId = OneSignalConfigManager.getAppId(),
+            addJWTHeaderIsValid(identityModel: identityModel)
+        else {
            return false
        }


consider removing the repetition, OSUserRequest can be a class instead of protocol or there can be another helper that prepare calls

emawby

discussed changes to the requests to not duplicate code in prepareForExecution

nan-li · 2024-08-27T00:35:49Z

Updated: tested player model migration and seems to work as well.

* Set most things to not require auth / JWT off.

* Multiple requests are making the same user-related requirement checks. * Put this into a helper function. * Since each request sets its own path at execution time with an appropriate app ID and alias, the app ID and alias are two separate checks.

nan-li · 2024-08-29T18:23:16Z

Badge tests still flaky: OneSignalNotificationsTests.testDontclearBadgesWhenAppBecomesActive()

nan-li mentioned this pull request Aug 22, 2024

WIP Jwt app startup #1467

Closed

18 tasks

emawby self-requested a review August 22, 2024 18:22

emawby self-assigned this Aug 22, 2024

nan-li force-pushed the internal_dump_method branch from 0ba7ad5 to 039479d Compare August 22, 2024 18:31

nan-li force-pushed the identity_verification_app_startup branch 4 times, most recently from 6fe9b03 to 4827f36 Compare August 22, 2024 19:56

nan-li force-pushed the internal_dump_method branch 2 times, most recently from 186c65d to 35677bc Compare August 22, 2024 23:26

nan-li force-pushed the identity_verification_app_startup branch from 4827f36 to a012272 Compare August 23, 2024 15:41

nan-li added 15 commits August 23, 2024 08:56

[dev app] for testing, revert later

3f83a50

* Add text fields and buttons for testing * Use staging * remove app clips * Make Client more verbose with headers

internal dump method helper filled out

aa7f3e8

For debugging

Add internal JWT Config management and listener

408ef55

* Make OSUserJwtConfig object to handle auth status etc * User manager owns an instance of the JWT Config

mock remote params returning use JWT = true

7af6373

Add JWT constants to Common Defines

ec84c2c

* Add `OSUD_USE_IDENTITY_VERIFICATION` flag to OneSignalCommonDefines

public updateUserJwt method

d3eb08d

public updateUserJwt method

Helper methods to add auth headers

ca6b132

Update Event Producer to accept multiple subscribers

2d1b317

Add JWT Token to Identity Model

d81c460

* Identity Model Store Listener will ignore it so no Deltas will be enqueued

Update Executors to consider JWT

e7059de

Updates to: * OSPropertyOperationExecutor * OSIdentityOperationExecutor * OSUserExecutor Updates to OSSubscriptionOperationExecutor is still to be done.

Operation Repo - refactor and update for JWT

5c98665

* Refactor from a static shared instance to a instance managed by User Manager * Operation Repo does not flush while Identity Verification is unknown * Currently it is not going to process Deltas based on JWT. Executors will.

User Manger - updates to login

e019d3d

TODOs

a0baad8

nan-li force-pushed the identity_verification_app_startup branch from a012272 to a0baad8 Compare August 23, 2024 15:56

nan-li force-pushed the internal_dump_method branch from 35677bc to 039479d Compare August 23, 2024 18:54

nan-li requested review from jkasten2 and jinliu9508 August 23, 2024 19:51

nan-li requested review from shepherd-l and jennantilla August 23, 2024 19:51

nan-li commented Aug 26, 2024

View reviewed changes

emawby requested changes Aug 26, 2024

View reviewed changes

nan-li added 2 commits August 29, 2024 08:32

[tests] fix tests to build and pass, after changes

ed888cd

* Set most things to not require auth / JWT off.

nan-li requested a review from emawby August 29, 2024 18:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JWT] App startup #1473

[JWT] App startup #1473

nan-li commented Aug 22, 2024 •

edited

Loading

nan-li Aug 26, 2024

emawby left a comment

nan-li commented Aug 27, 2024

nan-li commented Aug 29, 2024

[JWT] App startup #1473

Are you sure you want to change the base?

[JWT] App startup #1473

Conversation

nan-li commented Aug 22, 2024 • edited Loading

Description

One Line Summary

Details

Internal JWT Config management and listeners

Request objects updated to consider JWT

What has been tested in this PR

What is not included in this PR

Motivation

Testing

Unit testing

Manual Testing

Affected code checklist

Checklist

Overview

Testing

Final pass

nan-li Aug 26, 2024

Choose a reason for hiding this comment

emawby left a comment

Choose a reason for hiding this comment

nan-li commented Aug 27, 2024

nan-li commented Aug 29, 2024

nan-li commented Aug 22, 2024 •

edited

Loading