Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed currentServerDate calculation to use powerAuthSDK timeService instead #139

Merged
merged 4 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/Changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## 1.10.0
- Removed currentServerTime() method [(#139)](https://github.com/wultra/mtoken-sdk-android/pull/139)
- Improved operations handling [(#138)](https://github.com/wultra/mtoken-sdk-android/pull/138)
- Added Huawei notification support [(#136)](https://github.com/wultra/mtoken-sdk-android/pull/136)

## 1.9.0 (Jan 25, 2024)

- Added possibility for custom reject reason [(#130)](https://github.com/wultra/mtoken-sdk-android/pull/130)
Expand Down
46 changes: 46 additions & 0 deletions docs/Migration-1.10.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Migration from 1.9.x to 1.10.x

This guide contains instructions for migration from Wultra Mobile Token SDK for Android version `1.9.x` to version `1.10.x`.

## 1. Current Server Date

### Removed Functionality

The following calculated property was removed from the `IOperationsService` interface:

```kotlin
fun currentServerDate(): ZonedDateTime?
```

The `currentServerDate()` method provided a calculated property based on the difference between the phone's date and the date on the server. However, it had limitations and could be incorrect under certain circumstances, such as when the user decided to change the system time during the runtime of the application.

### Replace with

The new time synchronization directly from `PowerAuthSDK.timeSynchronizationService` is more precise and reliable.

Here is the updated test method for reference:

```kotlin
/** currentServerDate was removed in favour of PowerAuthSDK timeSynchronizationService */
@Test
fun testServerTime() {
var currentTime: ZonedDateTime? = null
val timeService = pa.timeSynchronizationService
if (timeService.isTimeSynchronized) {
val instant = Instant.ofEpochMilli(timeService.currentTime)
val zoneId = ZoneId.systemDefault()
currentTime = ZonedDateTime.ofInstant(instant, zoneId)
}
Assert.assertNotNull(currentTime)
}
```

## 2. Operations handling

API unified with iOS implementation. Instead of replacing list of operations every time `getOperation()` is called OperationsRegister is implemented to keep last fetched operations list and update it when necessary.


### Removed Functionality
`fun operationsLoaded(operations: List<UserOperation>)`
### Replace with
`fun operationsChanged(operations: List<UserOperation>, removed: List<UserOperation>, added: List<UserOperation>)`
1 change: 1 addition & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Remarks:
If you need to upgrade the Wultra Mobile Token SDK for Android to a newer version, you can check the following migration guides:

- [Migration from version `1.4.x` to `1.5.x`](Migration-1.5.md)
- [Migration from version `1.9.x` to `1.10.x`](Migration-1.10.md)


<!-- begin remove -->
Expand Down
14 changes: 7 additions & 7 deletions docs/SDK-Integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Requirements

- `minSdkVersion 19` (Android 4.4 KitKat)
- `minSdkVersion 21` (Android 5.0 Lollipop)
- [PowerAuth Mobile SDK](https://github.com/wultra/powerauth-mobile-sdk) needs to be available in your project.

## Gradle
Expand All @@ -21,11 +21,11 @@ implementation "com.wultra.android.powerauth:powerauth-sdk:1.x.y"

## PowerAuth Compatibility

| WMT SDK | PowerAuth SDK |
|-------------------|---|
| `1.0.x` - `1.2.x` | `1.0.x` - `1.5.x` |
| `1.3.x` - `1.4.x` | `1.6.x` |
| `1.5.x` - `1.7.x` | `1.7.x` |
| `1.8.x` - `1.9.x` | `1.8.x` |
| WMT SDK | PowerAuth SDK |
|--------------------|---|
| `1.0.x` - `1.2.x` | `1.0.x` - `1.5.x` |
| `1.3.x` - `1.4.x` | `1.6.x` |
| `1.5.x` - `1.7.x` | `1.7.x` |
| `1.8.x` - `1.10.x` | `1.8.x` |


32 changes: 11 additions & 21 deletions library/src/androidTest/java/IntegrationTests.kt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import com.wultra.android.powerauth.networking.error.ApiError
import io.getlime.security.powerauth.sdk.PowerAuthAuthentication
import io.getlime.security.powerauth.sdk.PowerAuthSDK
import org.junit.*
import org.threeten.bp.Instant
import org.threeten.bp.ZoneId
import org.threeten.bp.ZonedDateTime
import java.util.concurrent.CompletableFuture
import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -78,30 +80,18 @@ class IntegrationTests {
Assert.assertNotNull(oplist)
}

/** currentServerDate was removed in favour of PowerAuthSDK timeSynchronizationService */
@Test
fun testServerTime() {
val future = CompletableFuture<ZonedDateTime>()
Assert.assertNull(ops.currentServerDate())
ops.getOperations { result ->
result
.onSuccess {
future.complete(ops.currentServerDate())
}
.onFailure {
future.completeExceptionally(it)
}
}
val date = future.get(20, TimeUnit.SECONDS)
Assert.assertNotNull(date)

// Sometimes the CI or the emulator on the CI are behind with time because emulator boot takes some time.
// To verify that the time makes sense (the diff is not like hours or days) we accept 10 minus window.
val maxDiffSeconds = 60 * 10
var currentTime: ZonedDateTime? = null

val secDiff = kotlin.math.abs(date.toEpochSecond() - ZonedDateTime.now().toEpochSecond())
// If the difference between the server and the device is more than the limit, there is something wrong with the server
// or there is a bug. Both cases need a fix.
Assert.assertTrue("Difference is $secDiff seconds, but max $maxDiffSeconds seconds is allowed", secDiff < maxDiffSeconds)
val timeService = pa.timeSynchronizationService
if (timeService.isTimeSynchronized) {
val instant = Instant.ofEpochMilli(timeService.currentTime)
val zoneId = ZoneId.systemDefault()
currentTime = ZonedDateTime.ofInstant(instant, zoneId)
}
Assert.assertNotNull(currentTime)
}

// 1FA test are temporally disabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import com.wultra.android.mtokensdk.api.operation.model.QROperation
import com.wultra.android.mtokensdk.api.operation.model.UserOperation
import com.wultra.android.powerauth.networking.OkHttpBuilderInterceptor
import io.getlime.security.powerauth.sdk.PowerAuthAuthentication
import org.threeten.bp.ZonedDateTime

/**
* Service for operations handling.
Expand Down Expand Up @@ -57,20 +56,6 @@ interface IOperationsService {
*/
val lastFetchResult: Result<List<UserOperation>>?

/**
* Current server date
*
* This is calculated property based on the difference between phone date
* and date on the server.
*
* This property is available after the first successful operation list request.
* It might be nil if the server doesn't provide such a feature.
*
* Note that this value might be incorrect when the user decide to
* change the system time during the runtime of the application.
*/
fun currentServerDate(): ZonedDateTime?

/**
* If operations are loading.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ import com.wultra.android.powerauth.networking.tokens.IPowerAuthTokenProvider
import io.getlime.security.powerauth.sdk.PowerAuthAuthentication
import io.getlime.security.powerauth.sdk.PowerAuthSDK
import okhttp3.OkHttpClient
import org.threeten.bp.Instant
import org.threeten.bp.ZoneId
import org.threeten.bp.ZonedDateTime
import org.threeten.bp.temporal.ChronoUnit
import java.util.*
import kotlin.math.abs

/**
* Convenience factory method to create an IOperationsService instance
Expand Down Expand Up @@ -133,9 +133,6 @@ class OperationsService: IOperationsService {
// Mutex
private val mutex = Object()

// Difference in milliseconds between server and phone time
private var serverDateShiftInMilliSeconds: Long? = null

/**
* Constructs OperationService
*
Expand All @@ -152,7 +149,16 @@ class OperationsService: IOperationsService {
this.operationApi = OperationApi(httpClient, baseURL, appContext, powerAuthSDK, tokenProvider, userAgent, gsonBuilder)
}

override fun currentServerDate() = serverDateShiftInMilliSeconds?.let { ZonedDateTime.now().plus(it, ChronoUnit.MILLIS) }
private fun currentDate(): ZonedDateTime = run {
val timeService = powerAuthSDK.timeSynchronizationService
if (timeService.isTimeSynchronized) {
val currentTimeInstant = Instant.ofEpochMilli(timeService.currentTime)
val defaultTimeZoneId = ZoneId.systemDefault()
return ZonedDateTime.ofInstant(currentTimeInstant, defaultTimeZoneId)
} else {
return ZonedDateTime.now()
}
}

override fun isLoadingOperations() = synchronized(mutex) { tasks.isNotEmpty() }

Expand All @@ -163,10 +169,8 @@ class OperationsService: IOperationsService {
if (startLoading) {
// Notify start loading
listener?.operationsLoading(true)
val dateStarted = ZonedDateTime.now()
operationApi.list(object : IApiCallResponseListener<OperationListResponse> {
override fun onSuccess(result: OperationListResponse) {
processServerTime(result, dateStarted)
processOperationsListResult(Result.success(result.responseObject))
}
override fun onFailure(error: ApiError) {
Expand Down Expand Up @@ -200,43 +204,6 @@ class OperationsService: IOperationsService {
}
}

private fun processServerTime(response: OperationListResponse, requestStarted: ZonedDateTime) {

// server does not support this feature
if (response.currentTimestamp == null) {
return
}

val now = ZonedDateTime.now()
val requestDelayMilliseconds = now.toInstant().toEpochMilli() - requestStarted.toInstant().toEpochMilli()

// We're adding half of the time that the request took to compensate for the network delay
val serverTime = response.currentTimestamp.plus((requestDelayMilliseconds / 2), ChronoUnit.MILLIS)

// Already calculated server time
val currentServerDate = currentServerDate()

// If this is not a first calculation, do some adjustments
if (currentServerDate != null) {

// Difference between already calculated server time and the new server time
val timeChangeMilliseconds = abs((currentServerDate.toInstant().toEpochMilli() - serverTime.toInstant().toEpochMilli()))

// If the change is under the limit, we ignore the new value to avoid unnecessary changes that might be due to network delay.
if (timeChangeMilliseconds < MIN_SERVER_TIME_CHANGE_MS) {
return
}

// Reject small change if the network connection took long time
// This is to avoid volatility of the value
if (requestDelayMilliseconds > SERVER_TIME_DELAY_THRESHOLD_MS && timeChangeMilliseconds < FORCED_SERVER_TIME_CHANGE_MS) {
return
}
}

serverDateShiftInMilliSeconds = serverTime.toInstant().toEpochMilli() - now.toInstant().toEpochMilli()
}

override fun getHistory(authentication: PowerAuthAuthentication, callback: (result: Result<List<OperationHistoryEntry>>) -> Unit) {
operationApi.history(
authentication,
Expand All @@ -254,7 +221,7 @@ class OperationsService: IOperationsService {

override fun authorizeOperation(operation: IOperation, authentication: PowerAuthAuthentication, callback: (result: Result<Unit>) -> Unit) {

val currentDate = currentServerDate() ?: ZonedDateTime.now()
val currentDate = currentDate()
val authorizeRequest = AuthorizeRequest(AuthorizeRequestObject(operation, currentDate))
operationApi.authorize(
authorizeRequest,
Expand Down
Loading