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.

Sign up for GitHub

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

[Userpools] Fix network calls to run in background when specified fixes #668 #702

Merged
merged 1 commit into from
Feb 15, 2019
Merged

[Userpools] Fix network calls to run in background when specified fixes #668 #702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
213 changes: 114 additions & 99 deletions ...der/src/main/java/com/amazonaws/mobileconnectors/cognitoidentityprovider/CognitoUser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import android.content.SharedPreferences;
import android.os.Handler;

import com.amazonaws.internal.ReturningRunnable;
import com.amazonaws.mobileconnectors.cognitoidentityprovider.continuations.AuthenticationContinuation;
import com.amazonaws.mobileconnectors.cognitoidentityprovider.continuations.AuthenticationDetails;
import com.amazonaws.mobileconnectors.cognitoidentityprovider.continuations.ChallengeContinuation;
Expand Down Expand Up @@ -335,13 +336,13 @@ public void confirmSignUp(String confirmationCode,
* contentions
*/
private void confirmSignUpInternal(String confirmationCode, boolean forcedAliasCreation) {
final ConfirmSignUpRequest confirmUserRegistrationRequest = new ConfirmSignUpRequest();
confirmUserRegistrationRequest.setClientId(clientId);
confirmUserRegistrationRequest.setSecretHash(secretHash);
confirmUserRegistrationRequest.setUsername(userId);
confirmUserRegistrationRequest.setConfirmationCode(confirmationCode);
confirmUserRegistrationRequest.setForceAliasCreation(forcedAliasCreation);
confirmUserRegistrationRequest.setUserContextData(getUserContextData());
final ConfirmSignUpRequest confirmUserRegistrationRequest = new ConfirmSignUpRequest()
.withClientId(clientId)
.withSecretHash(secretHash)
.withUsername(userId)
.withConfirmationCode(confirmationCode)
.withForceAliasCreation(forcedAliasCreation)
.withUserContextData(getUserContextData());
final String pinpointEndpointId = pool.getPinpointEndpointId();
if (pinpointEndpointId != null) {
final AnalyticsMetadataType amd = new AnalyticsMetadataType();
Expand Down Expand Up @@ -416,10 +417,11 @@ public void resendConfirmationCode(final VerificationHandler callback) {
* Internal method to request registration code resend.
*/
private ResendConfirmationCodeResult resendConfirmationCodeInternal() {
final ResendConfirmationCodeRequest resendConfirmationCodeRequest = new ResendConfirmationCodeRequest();
resendConfirmationCodeRequest.setUsername(userId);
resendConfirmationCodeRequest.setClientId(clientId);
resendConfirmationCodeRequest.setSecretHash(secretHash);
final ResendConfirmationCodeRequest resendConfirmationCodeRequest =
new ResendConfirmationCodeRequest()
.withUsername(userId)
.withClientId(clientId)
.withSecretHash(secretHash);
final String pinpointEndpointId = pool.getPinpointEndpointId();
resendConfirmationCodeRequest.setUserContextData(getUserContextData());
if (pinpointEndpointId != null) {
Expand Down Expand Up @@ -753,6 +755,9 @@ public void getSession(final AuthenticationHandler callback) {
}

/**
* Note: Please use {@link #getSession(AuthenticationHandler)} or
* {@link #getSessionInBackground(AuthenticationHandler)} instead.
*
* Initiates user authentication through the generic auth flow (also called
* as Enhanced or Custom authentication). This is the first step in user
* authentication. The response to this step from the service will contain
Expand All @@ -767,6 +772,26 @@ public void getSession(final AuthenticationHandler callback) {
*/
public Runnable initiateUserAuthentication(final AuthenticationDetails authenticationDetails,
final AuthenticationHandler callback, final boolean runInBackground) {
final Runnable task = _initiateUserAuthentication(authenticationDetails, callback, runInBackground);
if (runInBackground) {
return new Runnable() {
@Override
public void run() {
new Thread(new Runnable() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very hard to understand what is going on here. Do you have any test cases that cover these changes?

E.g. the changes in this commit makes getSessionInBackground call AuthenticationHandler#onFailure() on background thread once user enters (incorrect) username/password and continueTask() has been called. onSuccess() is still called from main thread however. I would have expected the callbacks to run on main thread, or at least be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a code fix that will be addressing the issue you have pointed out, the release is pending.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally, this client calls the onSuccess and onFailure cases on the same thread that foo() is called. Except when the fooInBackground() variant is used, that is when the callbacks are made on the main thread's looper.

@Override
public void run() {
task.run();
}
}).start();
}
};
} else {
return task;
}
}

Runnable _initiateUserAuthentication(final AuthenticationDetails authenticationDetails,
final AuthenticationHandler callback, final boolean runInBackground) {
if (CognitoServiceConstants.CHLG_TYPE_USER_PASSWORD_VERIFIER.equals(
authenticationDetails.getAuthenticationType())) {
return startWithUserSrpAuth(authenticationDetails, callback, runInBackground);
Expand Down Expand Up @@ -2316,58 +2341,48 @@ public void run() {
*/
private Runnable startWithUserSrpAuth(final AuthenticationDetails authenticationDetails,
final AuthenticationHandler callback, final boolean runInBackground) {
final AuthenticationHelper authenticationHelper = new AuthenticationHelper(
pool.getUserPoolId());
final InitiateAuthRequest initiateAuthRequest = initiateUserSrpAuthRequest(
authenticationDetails, authenticationHelper);
try {
final InitiateAuthResult initiateAuthResult = cognitoIdentityProviderClient
.initiateAuth(initiateAuthRequest);
updateInternalUsername(initiateAuthResult.getChallengeParameters());
if (CognitoServiceConstants.CHLG_TYPE_USER_PASSWORD_VERIFIER
.equals(initiateAuthResult.getChallengeName())) {
if (authenticationDetails.getPassword() != null) {
final RespondToAuthChallengeRequest challengeRequest = userSrpAuthRequest(
initiateAuthResult.getChallengeParameters(),
authenticationDetails.getPassword(),
initiateAuthResult.getChallengeName(),
initiateAuthResult.getSession(),
authenticationHelper
);
return respondToChallenge(challengeRequest, callback, runInBackground);
}
}
return handleChallenge(initiateAuthResult, authenticationDetails, callback, runInBackground);
} catch (final ResourceNotFoundException rna) {
final CognitoUser cognitoUser = this;
if (rna.getMessage().contains("Device")) {
CognitoDeviceHelper.clearCachedDevice(usernameInternal, pool.getUserPoolId(),
context);
return new Runnable() {
@Override
public void run() {
return new Runnable() {
@Override
public void run() {
final AuthenticationHelper authenticationHelper = new AuthenticationHelper(
pool.getUserPoolId());
final InitiateAuthRequest initiateAuthRequest = initiateUserSrpAuthRequest(
authenticationDetails, authenticationHelper);
try {
final InitiateAuthResult initiateAuthResult = cognitoIdentityProviderClient
.initiateAuth(initiateAuthRequest);
updateInternalUsername(initiateAuthResult.getChallengeParameters());
if (CognitoServiceConstants.CHLG_TYPE_USER_PASSWORD_VERIFIER
.equals(initiateAuthResult.getChallengeName())) {
if (authenticationDetails.getPassword() != null) {
final RespondToAuthChallengeRequest challengeRequest = userSrpAuthRequest(
initiateAuthResult.getChallengeParameters(),
authenticationDetails.getPassword(),
initiateAuthResult.getChallengeName(),
initiateAuthResult.getSession(),
authenticationHelper
);
respondToChallenge(challengeRequest, callback, runInBackground).run();
}
}
handleChallenge(initiateAuthResult, authenticationDetails, callback, runInBackground).run();
} catch (final ResourceNotFoundException rna) {
final CognitoUser cognitoUser = CognitoUser.this;
if (rna.getMessage().contains("Device")) {
CognitoDeviceHelper.clearCachedDevice(usernameInternal, pool.getUserPoolId(),
context);
final AuthenticationContinuation authenticationContinuation = new AuthenticationContinuation(
cognitoUser, context, runInBackground, callback);
callback.getAuthenticationDetails(authenticationContinuation,
cognitoUser.getUserId());
}
};
} else {
return new Runnable() {
@Override
public void run() {
} else {
callback.onFailure(rna);
}
};
}
} catch (final Exception e) {
return new Runnable() {
@Override
public void run() {
} catch (final Exception e) {
callback.onFailure(e);
}
};
}
}
};
}

/**
Expand All @@ -2383,38 +2398,38 @@ public void run() {
*/
private Runnable startWithCustomAuth(final AuthenticationDetails authenticationDetails,
final AuthenticationHandler callback, final boolean runInBackground) {
try {
final AuthenticationHelper authenticationHelper = new AuthenticationHelper(this.getUserPoolId());
final InitiateAuthRequest initiateAuthRequest = initiateCustomAuthRequest(
authenticationDetails,
authenticationHelper);
final InitiateAuthResult initiateAuthResult = cognitoIdentityProviderClient
.initiateAuth(initiateAuthRequest);
updateInternalUsername(initiateAuthResult.getChallengeParameters());
if (CognitoServiceConstants.CHLG_TYPE_USER_PASSWORD_VERIFIER
.equals(initiateAuthResult.getChallengeName())) {
if (authenticationDetails.getPassword() == null) {
throw new IllegalStateException("Failed to find password in " +
"authentication details to response to PASSWORD_VERIFIER challenge");
}
final RespondToAuthChallengeRequest challengeRequest = userSrpAuthRequest(
initiateAuthResult.getChallengeParameters(),
authenticationDetails.getPassword(),
initiateAuthResult.getChallengeName(),
initiateAuthResult.getSession(),
authenticationHelper
);
return respondToChallenge(challengeRequest, callback, runInBackground);
}
return handleChallenge(initiateAuthResult, authenticationDetails, callback, runInBackground);
} catch (final Exception e) {
return new Runnable() {
@Override
public void run() {
return new Runnable() {
@Override
public void run() {
try {
final AuthenticationHelper authenticationHelper = new AuthenticationHelper(CognitoUser.this.getUserPoolId());
final InitiateAuthRequest initiateAuthRequest = initiateCustomAuthRequest(
authenticationDetails,
authenticationHelper);
final InitiateAuthResult initiateAuthResult = cognitoIdentityProviderClient
.initiateAuth(initiateAuthRequest);
updateInternalUsername(initiateAuthResult.getChallengeParameters());
if (CognitoServiceConstants.CHLG_TYPE_USER_PASSWORD_VERIFIER
.equals(initiateAuthResult.getChallengeName())) {
if (authenticationDetails.getPassword() == null) {
throw new IllegalStateException("Failed to find password in " +
"authentication details to response to PASSWORD_VERIFIER challenge");
}
final RespondToAuthChallengeRequest challengeRequest = userSrpAuthRequest(
initiateAuthResult.getChallengeParameters(),
authenticationDetails.getPassword(),
initiateAuthResult.getChallengeName(),
initiateAuthResult.getSession(),
authenticationHelper
);
respondToChallenge(challengeRequest, callback, runInBackground).run();
}
handleChallenge(initiateAuthResult, authenticationDetails, callback, runInBackground).run();
} catch (final Exception e) {
callback.onFailure(e);
}
};
}
}
};
}

/**
Expand Down Expand Up @@ -2605,22 +2620,22 @@ public void run() {
*/
private Runnable startWithUserPasswordAuth(final AuthenticationDetails authenticationDetails,
final AuthenticationHandler callback, final boolean runInBackground) {
try {
final InitiateAuthRequest initiateAuthRequest = initiateUserPasswordAuthRequest(
authenticationDetails);
final InitiateAuthResult initiateAuthResult = cognitoIdentityProviderClient
.initiateAuth(initiateAuthRequest);
this.usernameInternal = initiateAuthResult.getChallengeParameters()
.get(CognitoServiceConstants.CHLG_PARAM_USER_ID_FOR_SRP);
return handleChallenge(initiateAuthResult, authenticationDetails, callback, runInBackground);
} catch (final Exception e) {
return new Runnable() {
@Override
public void run() {
return new Runnable() {
@Override
public void run() {
try {
final InitiateAuthRequest initiateAuthRequest = initiateUserPasswordAuthRequest(
authenticationDetails);
final InitiateAuthResult initiateAuthResult = cognitoIdentityProviderClient
.initiateAuth(initiateAuthRequest);
CognitoUser.this.usernameInternal = initiateAuthResult.getChallengeParameters()
.get(CognitoServiceConstants.CHLG_PARAM_USER_ID_FOR_SRP);
handleChallenge(initiateAuthResult, authenticationDetails, callback, runInBackground).run();
} catch (final Exception e) {
callback.onFailure(e);
}
};
}
}
};
}

/**
Expand Down
16 changes: 8 additions & 8 deletions ...src/main/java/com/amazonaws/mobileconnectors/cognitoidentityprovider/CognitoUserPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,14 +416,14 @@ private SignUpResult signUpInternal(String userId, String password,
secretHash = CognitoSecretHash.getSecretHash(userId, clientId, clientSecret);

// Create User registration request
final SignUpRequest signUpUserRequest = new SignUpRequest();
signUpUserRequest.setUsername(userId);
signUpUserRequest.setPassword(password);
signUpUserRequest.setClientId(clientId);
signUpUserRequest.setSecretHash(secretHash);
signUpUserRequest.setUserAttributes(userAttributes.getAttributesList());
signUpUserRequest.setValidationData(validationDataList);
signUpUserRequest.setUserContextData(getUserContextData(userId));
final SignUpRequest signUpUserRequest = new SignUpRequest()
.withUsername(userId)
.withPassword(password)
.withClientId(clientId)
.withSecretHash(secretHash)
.withUserAttributes(userAttributes.getAttributesList())
.withValidationData(validationDataList)
.withUserContextData(getUserContextData(userId));
String ppEndpoint = getPinpointEndpointId();
if (ppEndpoint != null) {
AnalyticsMetadataType amd = new AnalyticsMetadataType();
Expand Down