Skip to content

Commit 0953557

Browse files
rosalyntanrosalyntan
committed
Address review comments.
1 parent 2c65bca commit 0953557

File tree

8 files changed

+65
-52
lines changed

8 files changed

+65
-52
lines changed

‎appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/ExchangePlayIntegrityTokenRequest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
* Client-side model of the ExchangePlayIntegrityTokenRequest payload from the Firebase App Check
2424
* Token Exchange API.
2525
*/
26-
public class ExchangePlayIntegrityTokenRequest {
26+
class ExchangePlayIntegrityTokenRequest {
2727

2828
@VisibleForTesting static final String PLAY_INTEGRITY_TOKEN_KEY = "playIntegrityToken";
2929

‎appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/GeneratePlayIntegrityChallengeRequest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
* Client-side model of the GeneratePlayIntegrityChallengeRequest payload from the Firebase App
2222
* Check Token Exchange API.
2323
*/
24-
public class GeneratePlayIntegrityChallengeRequest {
24+
class GeneratePlayIntegrityChallengeRequest {
2525

2626
public GeneratePlayIntegrityChallengeRequest() {}
2727

‎appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/GeneratePlayIntegrityChallengeResponse.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,19 @@
1515
package com.google.firebase.appcheck.playintegrity.internal;
1616

1717
import static com.google.android.gms.common.internal.Preconditions.checkNotNull;
18+
import static com.google.android.gms.common.util.Strings.emptyToNull;
1819

1920
import androidx.annotation.NonNull;
2021
import androidx.annotation.VisibleForTesting;
22+
import com.google.firebase.FirebaseException;
2123
import org.json.JSONException;
2224
import org.json.JSONObject;
2325

2426
/**
2527
* Client-side model of the GeneratePlayIntegrityChallengeResponse payload from the Firebase App
2628
* Check Token Exchange API.
2729
*/
28-
public class GeneratePlayIntegrityChallengeResponse {
30+
class GeneratePlayIntegrityChallengeResponse {
2931

3032
@VisibleForTesting static final String CHALLENGE_KEY = "challenge";
3133
@VisibleForTesting static final String TIME_TO_LIVE_KEY = "ttl";
@@ -35,10 +37,13 @@ public class GeneratePlayIntegrityChallengeResponse {
3537

3638
@NonNull
3739
public static GeneratePlayIntegrityChallengeResponse fromJsonString(@NonNull String jsonString)
38-
throws JSONException {
40+
throws FirebaseException, JSONException {
3941
JSONObject jsonObject = new JSONObject(jsonString);
40-
String challenge = jsonObject.optString(CHALLENGE_KEY, null);
41-
String timeToLive = jsonObject.optString(TIME_TO_LIVE_KEY, null);
42+
String challenge = emptyToNull(jsonObject.optString(CHALLENGE_KEY));
43+
String timeToLive = emptyToNull(jsonObject.optString(TIME_TO_LIVE_KEY));
44+
if (challenge == null || timeToLive == null) {
45+
throw new FirebaseException("Unexpected server response.");
46+
}
4247
return new GeneratePlayIntegrityChallengeResponse(challenge, timeToLive);
4348
}
4449

‎appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/PlayIntegrityAppCheckProvider.java

Lines changed: 27 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import androidx.annotation.NonNull;
1818
import androidx.annotation.VisibleForTesting;
19-
import com.google.android.gms.tasks.Continuation;
2019
import com.google.android.gms.tasks.Task;
2120
import com.google.android.gms.tasks.Tasks;
2221
import com.google.android.play.core.integrity.IntegrityManager;
@@ -26,7 +25,6 @@
2625
import com.google.firebase.FirebaseApp;
2726
import com.google.firebase.appcheck.AppCheckProvider;
2827
import com.google.firebase.appcheck.AppCheckToken;
29-
import com.google.firebase.appcheck.internal.AppCheckTokenResponse;
3028
import com.google.firebase.appcheck.internal.DefaultAppCheckToken;
3129
import com.google.firebase.appcheck.internal.NetworkClient;
3230
import com.google.firebase.appcheck.internal.RetryManager;
@@ -71,34 +69,28 @@ public PlayIntegrityAppCheckProvider(@NonNull FirebaseApp firebaseApp) {
7169
public Task<AppCheckToken> getToken() {
7270
return getPlayIntegrityAttestation()
7371
.continueWithTask(
74-
new Continuation<IntegrityTokenResponse, Task<AppCheckTokenResponse>>() {
75-
@Override
76-
public Task<AppCheckTokenResponse> then(@NonNull Task<IntegrityTokenResponse> task) {
77-
if (task.isSuccessful()) {
78-
ExchangePlayIntegrityTokenRequest request =
79-
new ExchangePlayIntegrityTokenRequest(task.getResult().token());
80-
return Tasks.call(
81-
backgroundExecutor,
82-
() ->
83-
networkClient.exchangeAttestationForAppCheckToken(
84-
request.toJsonString().getBytes(UTF_8),
85-
NetworkClient.PLAY_INTEGRITY,
86-
retryManager));
87-
}
88-
return Tasks.forException(task.getException());
72+
task -> {
73+
if (task.isSuccessful()) {
74+
ExchangePlayIntegrityTokenRequest request =
75+
new ExchangePlayIntegrityTokenRequest(task.getResult().token());
76+
return Tasks.call(
77+
backgroundExecutor,
78+
() ->
79+
networkClient.exchangeAttestationForAppCheckToken(
80+
request.toJsonString().getBytes(UTF_8),
81+
NetworkClient.PLAY_INTEGRITY,
82+
retryManager));
8983
}
84+
return Tasks.forException(task.getException());
9085
})
9186
.continueWithTask(
92-
new Continuation<AppCheckTokenResponse, Task<AppCheckToken>>() {
93-
@Override
94-
public Task<AppCheckToken> then(@NonNull Task<AppCheckTokenResponse> task) {
95-
if (task.isSuccessful()) {
96-
return Tasks.forResult(
97-
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
98-
}
99-
// TODO: Surface more error details.
100-
return Tasks.forException(task.getException());
87+
task -> {
88+
if (task.isSuccessful()) {
89+
return Tasks.forResult(
90+
DefaultAppCheckToken.constructFromAppCheckTokenResponse(task.getResult()));
10191
}
92+
// TODO: Surface more error details.
93+
return Tasks.forException(task.getException());
10294
});
10395
}
10496

@@ -114,20 +106,16 @@ private Task<IntegrityTokenResponse> getPlayIntegrityAttestation() {
114106
networkClient.generatePlayIntegrityChallenge(
115107
generateChallengeRequest.toJsonString().getBytes(UTF_8), retryManager)));
116108
return generateChallengeTask.continueWithTask(
117-
new Continuation<GeneratePlayIntegrityChallengeResponse, Task<IntegrityTokenResponse>>() {
118-
@Override
119-
public Task<IntegrityTokenResponse> then(
120-
@NonNull Task<GeneratePlayIntegrityChallengeResponse> task) {
121-
if (task.isSuccessful()) {
122-
return integrityManager.requestIntegrityToken(
123-
IntegrityTokenRequest.builder()
124-
.setCloudProjectNumber(Long.parseLong(projectNumber))
125-
.setNonce(task.getResult().getChallenge())
126-
.build());
127-
}
128-
// TODO: Surface more error details.
129-
return Tasks.forException(task.getException());
109+
task -> {
110+
if (task.isSuccessful()) {
111+
return integrityManager.requestIntegrityToken(
112+
IntegrityTokenRequest.builder()
113+
.setCloudProjectNumber(Long.parseLong(projectNumber))
114+
.setNonce(task.getResult().getChallenge())
115+
.build());
130116
}
117+
// TODO: Surface more error details.
118+
return Tasks.forException(task.getException());
131119
});
132120
}
133121
}

‎appcheck/firebase-appcheck-playintegrity/src/main/java/com/google/firebase/appcheck/playintegrity/internal/package-info.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
/** @hide */
16+
package com.google.firebase.appcheck.playintegrity.internal;

‎appcheck/firebase-appcheck-playintegrity/src/test/java/com/google/firebase/appcheck/playintegrity/internal/GeneratePlayIntegrityChallengeResponseTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.truth.Truth.assertThat;
1818
import static org.junit.Assert.assertThrows;
1919

20+
import com.google.firebase.FirebaseException;
2021
import org.json.JSONObject;
2122
import org.junit.Test;
2223
import org.junit.runner.RunWith;
@@ -48,7 +49,7 @@ public void fromJsonString_nullChallenge_throwsException() throws Exception {
4849
jsonObject.put(GeneratePlayIntegrityChallengeResponse.TIME_TO_LIVE_KEY, TIME_TO_LIVE);
4950

5051
assertThrows(
51-
NullPointerException.class,
52+
FirebaseException.class,
5253
() -> GeneratePlayIntegrityChallengeResponse.fromJsonString(jsonObject.toString()));
5354
}
5455

@@ -58,7 +59,7 @@ public void fromJsonString_nullTimeToLive_throwsException() throws Exception {
5859
jsonObject.put(GeneratePlayIntegrityChallengeResponse.CHALLENGE_KEY, CHALLENGE);
5960

6061
assertThrows(
61-
NullPointerException.class,
62+
FirebaseException.class,
6263
() -> GeneratePlayIntegrityChallengeResponse.fromJsonString(jsonObject.toString()));
6364
}
6465
}

‎appcheck/firebase-appcheck/src/main/java/com/google/firebase/appcheck/internal/AppCheckTokenResponse.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import androidx.annotation.NonNull;
2121
import androidx.annotation.VisibleForTesting;
22+
import com.google.firebase.FirebaseException;
2223
import org.json.JSONException;
2324
import org.json.JSONObject;
2425

@@ -35,10 +36,13 @@ public class AppCheckTokenResponse {
3536

3637
@NonNull
3738
public static AppCheckTokenResponse fromJsonString(@NonNull String jsonString)
38-
throws JSONException {
39+
throws FirebaseException, JSONException {
3940
JSONObject jsonObject = new JSONObject(jsonString);
4041
String token = emptyToNull(jsonObject.optString(TOKEN_KEY));
4142
String timeToLive = emptyToNull(jsonObject.optString(TIME_TO_LIVE_KEY));
43+
if (token == null || timeToLive == null) {
44+
throw new FirebaseException("Unexpected server response.");
45+
}
4246
return new AppCheckTokenResponse(token, timeToLive);
4347
}
4448

‎appcheck/firebase-appcheck/src/test/java/com/google/firebase/appcheck/internal/AppCheckTokenResponseTest.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static com.google.common.truth.Truth.assertThat;
1818
import static org.junit.Assert.assertThrows;
1919

20+
import com.google.firebase.FirebaseException;
2021
import org.json.JSONObject;
2122
import org.junit.Test;
2223
import org.junit.runner.RunWith;
@@ -49,8 +50,7 @@ public void fromJsonString_nullToken_throwsException() throws Exception {
4950
jsonObject.put(AppCheckTokenResponse.TIME_TO_LIVE_KEY, TIME_TO_LIVE);
5051

5152
assertThrows(
52-
NullPointerException.class,
53-
() -> AppCheckTokenResponse.fromJsonString(jsonObject.toString()));
53+
FirebaseException.class, () -> AppCheckTokenResponse.fromJsonString(jsonObject.toString()));
5454
}
5555

5656
@Test
@@ -59,7 +59,6 @@ public void fromJsonString_nullTimeToLive_throwsException() throws Exception {
5959
jsonObject.put(AppCheckTokenResponse.TOKEN_KEY, APP_CHECK_TOKEN);
6060

6161
assertThrows(
62-
NullPointerException.class,
63-
() -> AppCheckTokenResponse.fromJsonString(jsonObject.toString()));
62+
FirebaseException.class, () -> AppCheckTokenResponse.fromJsonString(jsonObject.toString()));
6463
}
6564
}

0 commit comments

Comments
 (0)