Skip to content
This repository has been archived by the owner on Jul 24, 2023. It is now read-only.

Remove usage of response intents from retrieve BBQ flow #132

Merged
merged 2 commits into from
Oct 23, 2017
Merged

Remove usage of response intents from retrieve BBQ flow #132

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d77ad83
Remove usage of response intents from retrieve BBQ flow
Oct 19, 2017
965eba1
Respond to Stan's comments
Oct 20, 2017
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
43 changes: 27 additions & 16 deletions api/java/org/openyolo/api/internal/CredentialRetrieveActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import android.app.Activity;
import android.content.Context;
import android.content.Intent;
import android.os.BadParcelableException;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.util.Log;
import android.view.WindowManager.LayoutParams;
Expand All @@ -36,7 +36,7 @@
import org.openyolo.protocol.CredentialRetrieveResult;
import org.openyolo.protocol.Protobufs;
import org.openyolo.protocol.Protobufs.CredentialRetrieveBbqResponse;
import org.openyolo.protocol.internal.IntentUtil;
import org.openyolo.protocol.ProtocolConstants;

/**
* An invisible Activity that forwards a given {@link CredentialRetrieveRequest} based on the
Expand Down Expand Up @@ -79,7 +79,7 @@ public void onCreate(@Nullable Bundle savedInstanceState) {
.queryFor(
CREDENTIAL_DATA_TYPE,
request.toProtocolBuffer(),
new CredentialRetrieveQueryCallback());
new CredentialRetrieveQueryCallback(request));
}

@Override
Expand All @@ -90,6 +90,13 @@ protected void onDestroy() {


private class CredentialRetrieveQueryCallback implements QueryCallback {

private CredentialRetrieveRequest mRetrieveRequest;

CredentialRetrieveQueryCallback(@NonNull CredentialRetrieveRequest retrieveRequest) {
mRetrieveRequest = retrieveRequest;
}

@Override
public void onResponse(long queryId, List<QueryResponse> queryResponses) {
ArrayList<Intent> retrieveIntents = new ArrayList<>();
Expand All @@ -106,23 +113,27 @@ public void onResponse(long queryId, List<QueryResponse> queryResponses) {

protoResponses.put(queryResponse.responderPackage, response);

if (response.getRetrieveIntent().isEmpty()) {
continue;
}

Intent retrieveIntent;
try {
retrieveIntent =
IntentUtil.fromBytes(response.getRetrieveIntent().toByteArray());
} catch (BadParcelableException ex) {
Log.w(LOG_TAG, "Failed to parse intent from bytes");
// a provider indicates that it has credentials via the credentials_available field
// on the response proto. Prior 0.3.0, the provider would directly return an intent.
// For backwards compatibility this is currently inspected as a fallback for the
// absence of the credentials_available field, but the Intent itself is no longer
// used.
// TODO: remove backwards compatibility with retrieve_intent after 0.3.0
if (!response.getCredentialsAvailable()
&& !response.getRetrieveIntent().isEmpty()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

&& !response.getRetrieveIntent().isEmpty()
=> && response.getRetrieveIntent().isEmpty(), no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed as the code has been reordered to fit your request for using the old intent as a fallback.

continue;
}

if (!queryResponse.responderPackage.equals(
retrieveIntent.getComponent().getPackageName())) {
Log.w(LOG_TAG, "Package mismatch between provider and retrieve intent");
continue;
Intent retrieveIntent = new Intent(ProtocolConstants.RETRIEVE_CREDENTIAL_ACTION);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are deprecating retrieve intent, but here it will be just ignored. So we are breaking the compatibility if a provider don't have the action org.openyolo.credential.retrieve, as it was not require before (and it's my case).

I think we should have both, and prefer the action org.openyolo.credential.retrieve if the provider did set getCredentialsAvailable().

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, we'll support the old intent for 0.3.0 but please do add support for direct invocation of the retrieve flow ASAP. It's actually required in the spec:

http://openid.net/specs/openyolo-android-ID1.html#retrieving-credentials

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes you're right, I've made the change on my side. It will be live in ~ 2 weeks.

retrieveIntent.setPackage(queryResponse.responderPackage);
retrieveIntent.putExtra(
ProtocolConstants.EXTRA_RETRIEVE_REQUEST,
mRetrieveRequest.toProtocolBuffer().toByteArray());

if (!response.getPreloadedData().isEmpty()) {
retrieveIntent.putExtra(ProtocolConstants.EXTRA_RETRIEVE_PRELOADED_DATA,
response.getPreloadedData().toByteArray());
}

retrieveIntents.add(retrieveIntent);
Expand Down
18 changes: 6 additions & 12 deletions ...rs/barbican/java/org/openyolo/demoprovider/barbican/provider/CredentialQueryReceiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package org.openyolo.demoprovider.barbican.provider;

import android.content.Context;
import android.content.Intent;
import android.support.annotation.NonNull;
import android.util.Log;
import com.google.bbq.Protobufs.BroadcastQuery;
Expand All @@ -28,7 +27,6 @@
import org.openyolo.protocol.AuthenticationDomain;
import org.openyolo.protocol.CredentialRetrieveRequest;
import org.openyolo.protocol.Protobufs.CredentialRetrieveBbqResponse;
import org.openyolo.protocol.internal.IntentUtil;
import org.openyolo.spi.BaseCredentialQueryReceiver;

/**
Expand Down Expand Up @@ -85,15 +83,11 @@ protected void processCredentialRequest(
+ query.getRequestingApp() + ": "
+ credentialsFound);

if (credentialsFound) {
Intent retrieveIntent = RetrieveCredentialActivity.createIntent(context, request);
CredentialRetrieveBbqResponse response =
CredentialRetrieveBbqResponse.newBuilder()
.setRetrieveIntent(IntentUtil.toByteString(retrieveIntent))
.build();
responseBytes = response.toByteArray();
}

responseSender.sendResponse(query, responseBytes);
responseSender.sendResponse(
query,
CredentialRetrieveBbqResponse.newBuilder()
.setCredentialsAvailable(credentialsFound)
.build()
.toByteArray());
}
}
4 changes: 1 addition & 3 deletions demoproviders/trapdoor/java/org/openyolo/demoprovider/trapdoor/CredentialQueryReceiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@
import android.util.Log;
import com.google.bbq.Protobufs.BroadcastQuery;
import com.google.bbq.QueryResponseSender;
import com.google.protobuf.ByteString;
import java.util.Set;
import org.openyolo.protocol.AuthenticationDomain;
import org.openyolo.protocol.AuthenticationMethods;
import org.openyolo.protocol.CredentialRetrieveRequest;
import org.openyolo.protocol.Protobufs.CredentialRetrieveBbqResponse;
import org.openyolo.protocol.internal.IntentUtil;
import org.openyolo.spi.BaseCredentialQueryReceiver;

/**
Expand Down Expand Up @@ -67,7 +65,7 @@ protected void processCredentialRequest(

Intent retrieveIntent = RetrieveActivity.createIntent(applicationContext);
CredentialRetrieveBbqResponse response = CredentialRetrieveBbqResponse.newBuilder()
.setRetrieveIntent(ByteString.copyFrom(IntentUtil.toBytes(retrieveIntent)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

So Intent retrieveIntent = RetrieveActivity.createIntent(applicationContext); is not used anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd ignore the trapdoor code for now, I'm actually going to delete this provider as it's not a good example any more (it treats retrieve() like hint()).

.setCredentialsAvailable(true)
.build();

Log.i(LOG_TAG, "Accepting credential request for claimed caller: "
Expand Down
9 changes: 9 additions & 0 deletions protocol/java/org/openyolo/protocol/ProtocolConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ public final class ProtocolConstants {
*/
public static final String EXTRA_RETRIEVE_RESULT = "org.openyolo.credential.retrieve.result";

/**
* The extra value key used to carry preloaded data as part of a credential retrieve Intent.
* This data is supplied by the credential provider during the broadcast phase of the retrieve
* flow, and sent back verbatim. The provider _should not_ implicitly trust this data, as it
* is possible for the client to tamper with it.
*/
public static final String EXTRA_RETRIEVE_PRELOADED_DATA =
"org.openyolo.credential.retrieve.preloaded_data";

/**
* The extra key value used to carry a hint request.
*/
Expand Down
216 changes: 0 additions & 216 deletions protocol/java/org/openyolo/protocol/RetrieveBbqResponse.java
View file
Open in desktop

This file was deleted.

Loading