Conversation
7e18f19 to
db27d29
Compare
| this(httpClient, realtimeConfigs, context, null); | ||
| } | ||
|
|
||
| public RealtimeManager(@NonNull HttpClient httpClient, @NonNull RealtimeConfigs realtimeConfigs, |
There was a problem hiding this comment.
Do we need this additional constructor? shouldnt we just have one and mock the auth in tests? (We should add tests to RT in any case)
| } | ||
| result = super.postSync(url, postBody, true); | ||
| String jwt = super.resolveJwt(this.customerId); | ||
| if (AuthJwtResolver.isMissingRequiredJwt(Optimove.getConfig().getAuthTokenProvider(), this.customerId, jwt)) { |
There was a problem hiding this comment.
See line 99, could this cause all visitors to start failing requests? (userId is either userId or visitor here)
| } | ||
|
|
||
| public RequestBuilder<T> userJwt(@Nullable String jwt) { | ||
| return this; |
There was a problem hiding this comment.
Are we intentionally dropping this?
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
|
|
||
| public final class AuthManager { |
There was a problem hiding this comment.
Ideally we would want to inject it to all components that need it.
It is now possible in OptistreamHandler and RT.
It isnt possible with the singleton/static components.
We have 3 options:
- Leave it like it is now - 5 instantiations with Optimove.getConfig().getAuthTokenProvider()
- Inject where DI pattern is used (OptistreamHandler and RT) and use Optimove.getAuthManager() for the singletons instead
- Use Optimove.getAuthManager() for all
I think 2 is cleanest (sooner or later we would need to stop letting random components seek the values they need in other singletons and get them using DI instead). 3 is pragmatic IMO. Let me know what you think.
|
@graciecooper Can we add some more test coverage for the auth flows (Embedded, grouping etc) or that would require significant refactoring? |
| error != null ? error.getMessage() : "null token"); | ||
| dispatchRequestWaitsForResponse = false; | ||
| scheduleTheNextDispatch(); | ||
| return; |
There was a problem hiding this comment.
Should we abort all the remaining groups if one fails?
|
|
||
| Runnable postOnExecutor = () -> postGroupJson(group, groups, index, null); | ||
|
|
||
| if (authManager != null && !customerKey.isEmpty()) { |
There was a problem hiding this comment.
Can we reduce the indentation here?
if (authManager == null || customerIsEmpty) {
postOnExecutor.run();
return;
}
// rest of auth
|
@graciecooper Is it possible to introduce a single dispatcher layer for RT and optistream like we did in iOS? or it would require a big refactoring? |
| - Added Federated JWT authentication; OptimoveConfig.Builder.enableAuth(AuthTokenProvider) supplies tokens; the SDK adds X-User-JWT for user-identified Optistream, realtime, Preference Center, Embedded Messaging, Optimobile analytics, and in-app network calls. | ||
| - Added Auth-capable signaling; Outbound SDK requests sent through HttpClient and OptimobileHttpClient include X-Optimove-Auth-Capable: 1 so backends can detect JWT-capable SDK versions. | ||
| - Optistream and realtime paths now group events by customer identity so each request can carry a single JWT. Optimobile analytics drains queued events per stored user id (install/visitor-scoped batches do not attach a user JWT). | ||
|
|
| Map<String, List<OptistreamEvent>> map = new LinkedHashMap<>(); | ||
| for (OptistreamEvent ev : events) { | ||
| String key = userKey(ev); | ||
| map.computeIfAbsent(key, k -> new ArrayList<>()).add(ev); |
There was a problem hiding this comment.
Does it require API 24? I think our minimum is 21?
| } | ||
| httpClient.postJson(realtimeConfigs.getRealtimeGateway(), realtimeGson.toJson(group)) | ||
| .userJwt(token) | ||
| .successListener(jsonResponse -> dispatchGroupAtIndex(groups, index + 1, allForFailure)) |
There was a problem hiding this comment.
Should it be allForFailure or just the one group?
| } | ||
|
|
||
| @Nullable | ||
| public static String blockingJwt(@Nullable AuthManager authManager, @Nullable String userId, long timeoutMs) { |
There was a problem hiding this comment.
In case of failures, should we log?
Description of Changes
Adds Federated JWT authentication to the Android SDK. When
OptimoveConfig.Builder.enableAuth(AuthTokenProvider)is used, the SDK asks the app for a JWT per user context and attaches it asX-User-JWTon user-identified outbound traffic (anonymous / install-scoped traffic omits the JWT where applicable).Key changes
enableAuth(AuthTokenProvider)on OptimoveConfig.BuilderuserIdentifierso the JWT matches the batchUsage
Kotlin
Breaking Changes
Release Checklist
Prepare:
Bump versions in:
Integration tests
T&T Only
Mobile Only
Deferred Deep Links
Combined
Release Procedure
devtomaster