AppAuth-Android icon indicating copy to clipboard operation
AppAuth-Android copied to clipboard

result code not sent in AuthorizationManagementActivity.sendResult()

Open rwst opened this issue 3 years ago • 1 comments

  • [X] I am using the latest release (master/0.11.1)
  • [X] I searched for existing GitHub issues
  • [X] I read the documentation
  • [X] I verified the client configuration matches the information in the identity provider (or I am using dynamic client registration)
  • [X] I am either using a custom URI scheme or https with App Links for client redirect.
  • [ ] I can reproduce the issue in the demo app (optional)

Configuration

  • Version: 0.11.1
  • Integration: (native(Java/Kotlin), Xamarin, ReactNative, etc)
  • Identity provider: (Google, Okta, Gluu, Auth0, KeyCloack, etc)

Issue Description

The code for AuthorizationManagementActivity.sendResult() was added in 7aa9bf3e to merge approaches for sending results to PendingIntents. This is the relevant part of the diff:

git diff e9c6f5a4 7aa9bf3e
@@ -262,16 +266,7 @@ public class AuthorizationManagementActivity extends Activity {
         }
         responseData.setData(responseUri);
 
-        if (mCompleteIntent != null) {
-            Logger.debug("Authorization complete - invoking completion intent");
-            try {
-                mCompleteIntent.send(this, 0, responseData);
-            } catch (CanceledException ex) {
-                Logger.error("Failed to send completion intent", ex);
-            }
-        } else {
-            setResult(RESULT_OK, responseData);
-        }
+        sendResult(mCompleteIntent, responseData, RESULT_OK);
     }
 
     private void handleAuthorizationCanceled() {
@@ -280,16 +275,8 @@ public class AuthorizationManagementActivity extends Activity {
                 AuthorizationException.GeneralErrors.USER_CANCELED_AUTH_FLOW,
                 null)
                 .toIntent();
-        if (mCancelIntent != null) {
-            try {
-                mCancelIntent.send(this, 0, cancelData);
-            } catch (CanceledException ex) {
-                Logger.error("Failed to send cancel intent", ex);
-            }
-        } else {
-            setResult(RESULT_CANCELED, cancelData);
-            Logger.debug("No cancel intent set - will return to previous activity");
-        }
+
+        sendResult(mCancelIntent, cancelData, RESULT_CANCELED);
     }
 
     private void extractState(Bundle state) {
@@ -301,33 +288,48 @@ public class AuthorizationManagementActivity extends Activity {
 
         mAuthIntent = state.getParcelable(KEY_AUTH_INTENT);
         mAuthorizationStarted = state.getBoolean(KEY_AUTHORIZATION_STARTED, false);
+        mCompleteIntent = state.getParcelable(KEY_COMPLETE_INTENT);
+        mCancelIntent = state.getParcelable(KEY_CANCEL_INTENT);
         try {
             String authRequestJson = state.getString(KEY_AUTH_REQUEST, null);
             mAuthRequest = authRequestJson != null
-                    ? AuthorizationRequest.jsonDeserialize(authRequestJson)
+                    ? AuthorizationManagementUtil.requestFrom(authRequestJson)
                     : null;
         } catch (JSONException ex) {
-            throw new IllegalStateException("Unable to deserialize authorization request", ex);
+            sendResult(
+                    mCancelIntent,
+                    AuthorizationRequestErrors.INVALID_REQUEST.toIntent(),
+                    RESULT_CANCELED);
+        }
+    }
+
+    private void sendResult(PendingIntent callback, Intent cancelData, int resultCode) {
+        if (callback != null) {
+            try {
+                callback.send(this, 0, cancelData);
+            } catch (CanceledException e) {
+                Logger.error("Failed to send cancel intent", e);
+            }
+        } else {
+            setResult(resultCode, cancelData);
         }
-        mCompleteIntent = state.getParcelable(KEY_COMPLETE_INTENT);
-        mCancelIntent = state.getParcelable(KEY_CANCEL_INTENT);
     }

As you can see the code was refactored with the intention of removing duplicate code. However, the resulting sendResult() loses the resultCode parameter (sending a 0 in both cases) and has the other parameter named cancelData, although the function handles both completion and cancel scenarios.

A small patch fixing these issues will be submitted.

rwst avatar Mar 11 '22 09:03 rwst

diff --git a/library/java/net/openid/appauth/AuthorizationManagementActivity.java b/library/java/net/openid/appauth/AuthorizationManagementActivity.java
index 4084287..2b4435f 100644
--- a/library/java/net/openid/appauth/AuthorizationManagementActivity.java
+++ b/library/java/net/openid/appauth/AuthorizationManagementActivity.java
@@ -327,15 +327,15 @@ public class AuthorizationManagementActivity extends AppCompatActivity {
         }
     }
 
-    private void sendResult(PendingIntent callback, Intent cancelData, int resultCode) {
+    private void sendResult(PendingIntent callback, Intent data, int resultCode) {
         if (callback != null) {
             try {
-                callback.send(this, 0, cancelData);
+                callback.send(this, resultCode, data);
             } catch (CanceledException e) {
-                Logger.error("Failed to send cancel intent", e);
+                Logger.error("Failed to send intent", e);
             }
         } else {
-            setResult(resultCode, cancelData);
+            setResult(resultCode, data);
         }
     }

rwst avatar Mar 11 '22 09:03 rwst