Skip to content

Commit 2e485be

Browse files
jiuyangzhaojiuyangzhao
committed
Address code review feedback.
1 parent 8a1ccb0 commit 2e485be

File tree

17 files changed

+66
-55
lines changed

17 files changed

+66
-55
lines changed

‎examples/android/src/main/java/com/dropbox/core/examples/android/DropboxActivity.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import android.content.SharedPreferences;
44
import android.support.v7.app.AppCompatActivity;
55

6-
import android.util.Log;
76
import com.dropbox.core.android.Auth;
87
import com.dropbox.core.json.JsonReadException;
98
import com.dropbox.core.oauth.DbxCredential;
@@ -15,7 +14,8 @@
1514
*/
1615
public abstract class DropboxActivity extends AppCompatActivity {
1716

18-
public final static boolean USE_SLT = false; //If USE_SLT is set to true, our Android example will use our beta feature Short Live Token.
17+
private final static boolean USE_SLT = false; //If USE_SLT is set to true, our Android example
18+
// will use our beta feature Short Live Token.
1919

2020
@Override
2121
protected void onResume() {
@@ -84,4 +84,12 @@ protected boolean hasToken() {
8484
return accessToken != null;
8585
}
8686
}
87+
88+
public static void startOAuth2Authentication(Context context) {
89+
if (USE_SLT) {
90+
Auth.startOAuth2PKCE(context, getString(R.string.app_key), DbxRequestConfigFactory.getRequestConfig());
91+
} else {
92+
Auth.startOAuth2Authentication(context, getString(R.string.app_key));
93+
}
94+
}
8795
}

‎examples/android/src/main/java/com/dropbox/core/examples/android/UserActivity.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,7 @@ protected void onCreate(Bundle savedInstanceState) {
3232
loginButton.setOnClickListener(new View.OnClickListener() {
3333
@Override
3434
public void onClick(View v) {
35-
if (DropboxActivity.USE_SLT) {
36-
Auth.startOAuth2PKCE(UserActivity.this, getString(R.string.app_key), DbxRequestConfigFactory.getRequestConfig());
37-
} else {
38-
Auth.startOAuth2Authentication(UserActivity.this, getString(R.string.app_key));
39-
}
35+
DropboxActivity.startOAuth2Authentication(UserActivity.this);
4036
}
4137
});
4238

‎src/main/java/com/dropbox/core/DbxAppInfo.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public class DbxAppInfo extends Dumpable {
2525

2626
/**
2727
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless
28-
* you are early access partner of this feature. The function signature is subjected to change
28+
* you are early access partner of this feature. The function signature is subject to change
2929
* in next minor version release.
3030
*
3131
* DbxAppInfo without secret. Should only be used in PKCE flow.

‎src/main/java/com/dropbox/core/DbxAuthFinish.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public DbxAuthFinish(String accessToken, String userId, String accountId, String
4242
/**
4343
*
4444
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
45-
* early access partner of this feature. The function signature is subjected to change
45+
* early access partner of this feature. The function signature is subject to change
4646
* in next minor version release.
4747
*
4848
* @param accessToken OAuth access token.
@@ -62,7 +62,7 @@ public DbxAuthFinish(String accessToken, Long expiresIn, String refreshToken, St
6262
/**
6363
*
6464
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
65-
* early access partner of this feature. The function signature is subjected to change
65+
* early access partner of this feature. The function signature is subject to change
6666
* in next minor version release.
6767
*
6868
* @param accessToken OAuth access token.
@@ -104,7 +104,7 @@ public String getAccessToken() {
104104
/**
105105
*
106106
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
107-
* early access partner of this feature. The function signature is subjected to change
107+
* early access partner of this feature. The function signature is subject to change
108108
* in next minor version release.
109109
*
110110
* Returns the time when {@link DbxAuthFinish#accessToken} expires in millisecond. If null then
@@ -121,7 +121,7 @@ public Long getExpiresAt() {
121121

122122
/**
123123
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
124-
* early access partner of this feature. The function signature is subjected to change
124+
* early access partner of this feature. The function signature is subject to change
125125
* in next minor version release.
126126
*
127127
* Returns an <em>refresh token</em> which can be used to obtain new
@@ -167,7 +167,7 @@ public String getTeamId() {
167167
/**
168168
*
169169
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
170-
* early access partner of this feature. The function signature is subjected to change
170+
* early access partner of this feature. The function signature is subject to change
171171
* in next minor version release.
172172
*
173173
* Return the <em>scopes</em> of current OAuth flow. Each scope correspond to a group of

‎src/main/java/com/dropbox/core/DbxAuthInfo.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public DbxAuthInfo(String accessToken, DbxHost host) {
3131

3232
/**
3333
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
34-
* early access partner of this feature. The function signature is subjected to change
34+
* early access partner of this feature. The function signature is subject to change
3535
* in next minor version release.
3636
*
3737
* Creates a new instance with the given parameters.
@@ -62,7 +62,7 @@ public String getAccessToken() {
6262

6363
/**
6464
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
65-
* early access partner of this feature. The function signature is subjected to change
65+
* early access partner of this feature. The function signature is subject to change
6666
* in next minor version release.
6767
*
6868
* Return the millisecond when accessToken is going to expire.
@@ -75,7 +75,7 @@ public Long getExpiresAt() {
7575

7676
/**
7777
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
78-
* early access partner of this feature. The function signature is subjected to change
78+
* early access partner of this feature. The function signature is subject to change
7979
* in next minor version release.
8080
*
8181
* Return the refresh token which can be used to obtain new access token.

‎src/main/java/com/dropbox/core/DbxPKCEManager.java‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.dropbox.core.util.LangUtil;
55
import com.dropbox.core.v2.DbxRawClientV2;
66

7+
import java.io.IOException;
78
import java.io.Serializable;
89
import java.io.UnsupportedEncodingException;
910
import java.security.MessageDigest;
@@ -18,14 +19,13 @@
1819
* This class should be lib/jar private. We make it public so that Android related code can use it.
1920
*
2021
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
21-
* early access partner of this feature. The function signature is subjected to change
22+
* early access partner of this feature. The function signature is subject to change
2223
* in next minor version release.
2324
*
2425
* This class does code verifier and code challenge generation in Proof Key for Code Exchange(PKCE).
2526
* @see <a href="https://tools.ietf.org/html/rfc7636">https://tools.ietf.org/html/rfc7636</a>
2627
*/
27-
public class DbxPKCEManager implements Serializable {
28-
public static final long serialVersionUID = 0;
28+
public class DbxPKCEManager {
2929
public static final String CODE_CHALLENGE_METHODS = "S256";
3030
public static final int CODE_VERIFIER_SIZE = 128;
3131

@@ -43,7 +43,12 @@ public class DbxPKCEManager implements Serializable {
4343
*/
4444
public DbxPKCEManager() {
4545
this.codeVerifier = generateCodeVerifier();
46-
this.codeChallenge = generateCodeChallenge();
46+
this.codeChallenge = generateCodeChallenge(this.codeVerifier);
47+
}
48+
49+
public DbxPKCEManager(String codeVerifier) {
50+
this.codeVerifier = codeVerifier;
51+
this.codeChallenge = generateCodeChallenge(this.codeVerifier);
4752
}
4853

4954
String generateCodeVerifier() {
@@ -56,10 +61,6 @@ String generateCodeVerifier() {
5661
return sb.toString();
5762
}
5863

59-
String generateCodeChallenge() {
60-
return generateCodeChallenge(this.codeVerifier);
61-
}
62-
6364
static String generateCodeChallenge(String codeVerifier) {
6465
try {
6566
MessageDigest digest = MessageDigest.getInstance("SHA-256");
@@ -88,7 +89,7 @@ public String getCodeChallenge() {
8889

8990
/**
9091
* Make oauth2/token request to exchange code for oauth2 access token. Client secret is not
91-
* requried.
92+
* required.
9293
* @param requestConfig Default attributes to use for each request.
9394
* @param oauth2Code OAuth2 code defined in OAuth2 code flow.
9495
* @param appKey Client Key

‎src/main/java/com/dropbox/core/DbxPKCEWebAuth.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
/**
1717
*
1818
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
19-
* early access partner of this feature. The function signature is subjected to change
19+
* early access partner of this feature. The function signature is subject to change
2020
* in next minor version release.
2121
*
2222
* This class does the OAuth2 "authorization code" flow with Proof Key for Code Exchange(PKCE).

‎src/main/java/com/dropbox/core/DbxWebAuth.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ public Builder withDisableSignup(Boolean disableSignup) {
877877

878878
/**
879879
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
880-
* early access partner of this feature. The function signature is subjected to change
880+
* early access partner of this feature. The function signature is subject to change
881881
* in next minor version release.
882882
*
883883
* Whether or not to include refresh token in {@link DbxAuthFinish}
@@ -901,7 +901,7 @@ public Builder withTokenAccessType(TokenAccessType tokenAccessType) {
901901
/**
902902
*
903903
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
904-
* early access partner of this feature. The function signature is subjected to change
904+
* early access partner of this feature. The function signature is subject to change
905905
* in next minor version release.
906906
*
907907
* @param scope A list of scope returned by Dropbox server. Each scope correspond to a group of

‎src/main/java/com/dropbox/core/android/Auth.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public static void startOAuth2Authentication(Context context, String appKey, Str
3434

3535
/**
3636
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
37-
* early access partner of this feature. The function signature is subjected to change
37+
* early access partner of this feature. The function signature is subject to change
3838
* in next minor version release.
3939
*
4040
* @see Auth#startOAuth2PKCE(Context, String, DbxRequestConfig, DbxHost)
@@ -46,11 +46,11 @@ public static void startOAuth2PKCE(Context context, String appKey,
4646

4747
/**
4848
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
49-
* early access partner of this feature. The function signature is subjected to change
49+
* early access partner of this feature. The function signature is subject to change
5050
* in next minor version release.
5151
*
5252
* Starts the Dropbox OAuth process by launching the Dropbox official app (AKA DAuth) or web
53-
* browser if dropbox official app is not available. In browser flow, normally user need to
53+
* browser if dropbox official app is not available. In browser flow, normally user needs to
5454
* sign in.
5555
* @param context the {@link Context} to use to launch the
5656
* * Dropbox authentication activity. This will typically be an
@@ -164,7 +164,7 @@ public static String getUid() {
164164

165165
/**
166166
* <b>Beta</b>: This feature is not available to all developers. Please do NOT use it unless you are
167-
* early access partner of this feature. The function signature is subjected to change
167+
* early access partner of this feature. The function signature is subject to change
168168
* in next minor version release.
169169
*
170170
* @return The result after

‎src/main/java/com/dropbox/core/android/AuthActivity.java‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ public class AuthActivity extends Activity {
143143
private static final String SIS_KEY_AUTH_STATE_NONCE = "SIS_KEY_AUTH_STATE_NONCE";
144144

145145
// saved instance PKCE manger key
146-
private static final String SIS_KEY_PKCE_MANAGER = "SIS_KEY_PKCE_MANAGER";
146+
private static final String SIS_KEY_PKCE_CODE_VERIFIER = "SIS_KEY_PKCE_CODE_VERIFIER";
147147
/**
148148
* Provider of the local security needs of an AuthActivity.
149149
*
@@ -228,7 +228,8 @@ static void setAuthParams(String appKey, String desiredUid,
228228
}
229229

230230
/**
231-
* Set static authentication parameters
231+
* Set static authentication parameters. If both host and webHost are provided, we take use
232+
* host as source of truth.
232233
*/
233234
static void setAuthParams(String appKey, String desiredUid,
234235
String[] alreadyAuthedUids, String sessionId, String webHost,
@@ -291,12 +292,17 @@ public static Intent makeIntent(Context context, String appKey, String webHost,
291292
*/
292293
public static Intent makeIntent(Context context, String appKey, String desiredUid, String[] alreadyAuthedUids,
293294
String sessionId, String webHost, String apiType) {
294-
if (appKey == null) throw new IllegalArgumentException("'appKey' can't be null");
295+
if (appKey == null) {
296+
throw new IllegalArgumentException("'appKey' can't be null");
297+
}
295298
setAuthParams(appKey, desiredUid, alreadyAuthedUids, sessionId, webHost, apiType, null,
296299
null, null);
297300
return new Intent(context, AuthActivity.class);
298301
}
299302

303+
/**
304+
* If both host and webHost are provided, we take use host as source of truth.
305+
*/
300306
static Intent makeIntent(
301307
Context context, String appKey, String desiredUid, String[] alreadyAuthedUids,
302308
String sessionId, String webHost, String apiType, TokenAccessType tokenAccessType,
@@ -430,7 +436,7 @@ protected void onCreate(Bundle savedInstanceState) {
430436
mPKCEManager = new DbxPKCEManager();
431437
} else {
432438
mAuthStateNonce = savedInstanceState.getString(SIS_KEY_AUTH_STATE_NONCE);
433-
mPKCEManager = (DbxPKCEManager) savedInstanceState.getSerializable(SIS_KEY_PKCE_MANAGER);
439+
mPKCEManager = new DbxPKCEManager(savedInstanceState.getString(SIS_KEY_PKCE_CODE_VERIFIER));
434440
}
435441

436442
setTheme(android.R.style.Theme_Translucent_NoTitleBar);
@@ -442,7 +448,7 @@ protected void onCreate(Bundle savedInstanceState) {
442448
protected void onSaveInstanceState(Bundle outState) {
443449
super.onSaveInstanceState(outState);
444450
outState.putString(SIS_KEY_AUTH_STATE_NONCE, mAuthStateNonce);
445-
outState.putSerializable(SIS_KEY_PKCE_MANAGER, mPKCEManager);
451+
outState.putString(SIS_KEY_PKCE_CODE_VERIFIER, mPKCEManager.getCodeVerifier());
446452
}
447453

448454
/**
@@ -604,14 +610,16 @@ protected void onNewIntent(Intent intent) {
604610
newResult.putExtra(EXTRA_UID, uid);
605611
} else if (token.equals(TokenType.OAUTH2CODE.toString())) {
606612
// code flow with PKCE
607-
TokenRequest tokenRequest = new TokenRequest(secret);
613+
TokenRequestAsyncTask tokenRequest = new TokenRequestAsyncTask(secret);
608614
try {
609615
DbxAuthFinish dbxAuthFinish = tokenRequest.execute().get();
610616

611617
if (dbxAuthFinish == null) {
612618
newResult = null;
613619
} else {
614620
newResult = new Intent();
621+
// access_token and access_secret are OAuth1 concept. In OAuth2 we only
622+
// have access token. So I put both of them to be the same.
615623
newResult.putExtra(EXTRA_ACCESS_TOKEN, dbxAuthFinish.getAccessToken());
616624
newResult.putExtra(EXTRA_ACCESS_SECRET, dbxAuthFinish.getAccessToken());
617625
newResult.putExtra(EXTRA_REFRESH_TOKEN, dbxAuthFinish.getRefreshToken());
@@ -677,7 +685,7 @@ private String createStateNonce() {
677685
}
678686

679687
private String createPKCEStateNonce() {
680-
return String.format("oauth2code:%s:%s:%s",
688+
return String.format(Locale.US, "oauth2code:%s:%s:%s",
681689
mPKCEManager.getCodeChallenge(),
682690
DbxPKCEManager.CODE_CHALLENGE_METHODS,
683691
mTokenAccessType.toString());
@@ -709,18 +717,18 @@ public String toString() {
709717
}
710718
}
711719

712-
private class TokenRequest extends AsyncTask<Void, Void, DbxAuthFinish> {
720+
private class TokenRequestAsyncTask extends AsyncTask<Void, Void, DbxAuthFinish> {
713721
private final String code;
714722

715-
private TokenRequest(String code) {
723+
private TokenRequestAsyncTask(String code) {
716724
this.code = code;
717725
}
718726

719727

720728
@Override
721729
protected DbxAuthFinish doInBackground(Void... p) {
722730
try {
723-
return mPKCEManager.makeTokenRequest(mRequestConfig, code, mAppKey, null,mHost);
731+
return mPKCEManager.makeTokenRequest(mRequestConfig, code, mAppKey, null, mHost);
724732
} catch (DbxException e) {
725733
Log.e(TAG, "Token Request Failed: " + e.getMessage());
726734
return null;

0 commit comments

Comments
 (0)