[DC] Add support for digital credential issuance [chromium/src : main]

2 views
Skip to first unread message

Helen Qin (Gerrit)

unread,
Nov 8, 2024, 6:15:50 PM11/8/24
to Mohamed Amir Yosef, AyeAye, Tricium, Christian Biesinger, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Kaan Icer, Luna Lu, devtools...@chromium.org, devtools-re...@chromium.org, feature-co...@chromium.org, asvitki...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Mohamed Amir Yosef

Helen Qin added 4 comments

Patchset-level comments
File-level comment, Patchset 4 (Latest):
Helen Qin . resolved

Thanks!

File content/public/android/java/src/org/chromium/content/browser/webid/IdentityCredentialsDelegate.java
Line 132, Patchset 4 (Latest): // android.credentials.GetCredentialException requires API level 34
Helen Qin . unresolved

Is this still relevant? `android.credentials.GetCredentialException` shouldn't be used anywhere.

Line 138, Patchset 4 (Latest): var response = IntentHelper.extractGetCredentialResponse(code, data);
Helen Qin . unresolved

Now you need to retrieve either a CreateCredentialResponse or a CreateCredentialException from the result data. And here's example of how to do it:


```
// Imports
import androidx.credentials.provider.PendingIntentHandler
import androidx.credentials.CreateCredentialResponse
androidx.credentials.exceptions.CreateCredentialException

// ...

 val providerData = data?.getParcelable<Intent>("androidx.identitycredentials.BUNDLE_KEY_PROVIDER_DATA")
 
val response = PendingIntentHandler.retrieveCreateCredentialResponse(providerData)
if (response == null) {
    val exception = data.retrieveGetCredentialException()
    // Handle exception
} else {
  // Handle response
  val responseJson = response.data.getString("androidx.credentials.BUNDLE_KEY_RESPONSE_JSON")
  // This `responseJson` is the wallet generated json response
}
```
Line 147, Patchset 4 (Latest): if (e instanceof GetCredentialException
Helen Qin . unresolved

this should be `androidx.credentials.exceptions.CreateCredentialException`

https://developer.android.com/reference/kotlin/androidx/credentials/exceptions/CreateCredentialException

Open in Gerrit

Related details

Attention is currently required from:
  • Mohamed Amir Yosef
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I4aae7252c9fe673a8fbebd7202644b13c148a4f6
Gerrit-Change-Number: 6006016
Gerrit-PatchSet: 4
Gerrit-Owner: Mohamed Amir Yosef <ma...@chromium.org>
Gerrit-CC: Christian Biesinger <cbies...@chromium.org>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-CC: Helen Qin <hele...@google.com>
Gerrit-CC: Kaan Icer <ic...@chromium.org>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Luna Lu <loon...@chromium.org>
Gerrit-Attention: Mohamed Amir Yosef <ma...@chromium.org>
Gerrit-Comment-Date: Fri, 08 Nov 2024 23:15:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Mohamed Amir Yosef (Gerrit)

unread,
Nov 8, 2024, 7:10:16 PM11/8/24
to Helen Qin, AyeAye, Tricium, Christian Biesinger, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Kaan Icer, Luna Lu, devtools...@chromium.org, devtools-re...@chromium.org, feature-co...@chromium.org, asvitki...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Helen Qin

Mohamed Amir Yosef added 4 comments

File content/public/android/java/src/org/chromium/content/browser/webid/IdentityCredentialsDelegate.java
Line 132, Patchset 4 (Latest): // android.credentials.GetCredentialException requires API level 34
Helen Qin . resolved

Is this still relevant? `android.credentials.GetCredentialException` shouldn't be used anywhere.

Mohamed Amir Yosef

Done

File third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5
Line 247, Patchset 4 (Latest): name: "DigitalCredentialsCreate",
Mohamed Amir Yosef . unresolved

not needed.

File third_party/blink/renderer/platform/runtime_enabled_features.json5
Line 4873, Patchset 4 (Latest): origin_trial_feature_name: "WebIdentityDigitalCredentialsIssuance",
origin_trial_os: ["android"],
origin_trial_allows_third_party: true,
Mohamed Amir Yosef . unresolved

Not needed.

Line 4876, Patchset 4 (Latest): depends_on: ["FedCm"],
Mohamed Amir Yosef . unresolved

not needed

Open in Gerrit

Related details

Attention is currently required from:
  • Helen Qin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I4aae7252c9fe673a8fbebd7202644b13c148a4f6
Gerrit-Change-Number: 6006016
Gerrit-PatchSet: 4
Gerrit-Owner: Mohamed Amir Yosef <ma...@chromium.org>
Gerrit-CC: Christian Biesinger <cbies...@chromium.org>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-CC: Helen Qin <hele...@google.com>
Gerrit-CC: Kaan Icer <ic...@chromium.org>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Luna Lu <loon...@chromium.org>
Gerrit-Attention: Helen Qin <hele...@google.com>
Gerrit-Comment-Date: Sat, 09 Nov 2024 00:10:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Helen Qin <hele...@google.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

Mohamed Amir Yosef (Gerrit)

unread,
Nov 11, 2024, 5:36:52 AM11/11/24
to Helen Qin, AyeAye, Tricium, Christian Biesinger, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Kaan Icer, Luna Lu, devtools...@chromium.org, devtools-re...@chromium.org, feature-co...@chromium.org, asvitki...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Helen Qin

Mohamed Amir Yosef added 5 comments

File content/public/android/java/src/org/chromium/content/browser/webid/IdentityCredentialsDelegate.java
Line 138, Patchset 4 (Latest): var response = IntentHelper.extractGetCredentialResponse(code, data);
Helen Qin . resolved

Now you need to retrieve either a CreateCredentialResponse or a CreateCredentialException from the result data. And here's example of how to do it:


```
// Imports
import androidx.credentials.provider.PendingIntentHandler
import androidx.credentials.CreateCredentialResponse
androidx.credentials.exceptions.CreateCredentialException

// ...

 val providerData = data?.getParcelable<Intent>("androidx.identitycredentials.BUNDLE_KEY_PROVIDER_DATA")
 
val response = PendingIntentHandler.retrieveCreateCredentialResponse(providerData)
if (response == null) {
    val exception = data.retrieveGetCredentialException()
    // Handle exception
} else {
  // Handle response
  val responseJson = response.data.getString("androidx.credentials.BUNDLE_KEY_RESPONSE_JSON")
  // This `responseJson` is the wallet generated json response
}
```
Mohamed Amir Yosef

Done

Line 147, Patchset 4 (Latest): if (e instanceof GetCredentialException
Helen Qin . resolved

this should be `androidx.credentials.exceptions.CreateCredentialException`

https://developer.android.com/reference/kotlin/androidx/credentials/exceptions/CreateCredentialException

Mohamed Amir Yosef

Done

File third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5
Line 247, Patchset 4 (Latest): name: "DigitalCredentialsCreate",
Mohamed Amir Yosef . resolved

not needed.

Mohamed Amir Yosef

Done

File third_party/blink/renderer/platform/runtime_enabled_features.json5
Line 4873, Patchset 4 (Latest): origin_trial_feature_name: "WebIdentityDigitalCredentialsIssuance",
origin_trial_os: ["android"],
origin_trial_allows_third_party: true,
Mohamed Amir Yosef . resolved

Not needed.

Mohamed Amir Yosef

Done

Line 4876, Patchset 4 (Latest): depends_on: ["FedCm"],
Mohamed Amir Yosef . resolved

not needed

Mohamed Amir Yosef

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Helen Qin
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I4aae7252c9fe673a8fbebd7202644b13c148a4f6
Gerrit-Change-Number: 6006016
Gerrit-PatchSet: 4
Gerrit-Owner: Mohamed Amir Yosef <ma...@chromium.org>
Gerrit-CC: Christian Biesinger <cbies...@chromium.org>
Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
Gerrit-CC: Helen Qin <hele...@google.com>
Gerrit-CC: Kaan Icer <ic...@chromium.org>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Luna Lu <loon...@chromium.org>
Gerrit-Attention: Helen Qin <hele...@google.com>
Gerrit-Comment-Date: Mon, 11 Nov 2024 10:36:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Mohamed Amir Yosef <ma...@chromium.org>
Comment-In-Reply-To: Helen Qin <hele...@google.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

Helen Qin (Gerrit)

unread,
Nov 12, 2024, 11:53:17 PM11/12/24
to Mohamed Amir Yosef, AyeAye, Tricium, Christian Biesinger, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Kaan Icer, Luna Lu, devtools...@chromium.org, devtools-re...@chromium.org, feature-co...@chromium.org, asvitki...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
Attention needed from Mohamed Amir Yosef

Helen Qin added 2 comments

File content/public/android/java/src/org/chromium/content/browser/webid/IdentityCredentialsDelegate.java
Line 144, Patchset 6 (Latest): data.getParcelable(
Helen Qin . unresolved

I don't think you can unconditionally use this API as it is only [available on Android 33+](https://developer.android.com/reference/android/os/Bundle#getParcelable(java.lang.String,%20java.lang.Class%3CT%3E)). It can cause issue when deployed to a user device that's running a lower android version.

I would suggest that the lint suppression on NewApi and properly resolve such an error. Usually what people do is:
```
if (Build.VERSION.SDK_INT >= 33) { 
  // Use the new api
  data.getParcelable(IntentHelper.BUNDLE_KEY_PROVIDER_DATA, Intent.class);
} else {
  // Use the old api
  data.getParcelable<Intent>(IntentHelper.BUNDLE_KEY_PROVIDER_DATA);
}
```
Line 152, Patchset 6 (Latest): PendingIntentHandler.retrieveCreateCredentialException(
Helen Qin . unresolved

This could also be null, please make sure that's handled correctly.

In that case, it means neither a response nor an exception was propagated back, which is really unexpected. If possible, that should be logged somehow. Anyway, we should gracefully fail it on the user side (maybe just pass back a default exception)

Open in Gerrit

Related details

Attention is currently required from:
  • Mohamed Amir Yosef
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I4aae7252c9fe673a8fbebd7202644b13c148a4f6
    Gerrit-Change-Number: 6006016
    Gerrit-PatchSet: 6
    Gerrit-Owner: Mohamed Amir Yosef <ma...@chromium.org>
    Gerrit-CC: Christian Biesinger <cbies...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Helen Qin <hele...@google.com>
    Gerrit-CC: Kaan Icer <ic...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Luna Lu <loon...@chromium.org>
    Gerrit-Attention: Mohamed Amir Yosef <ma...@chromium.org>
    Gerrit-Comment-Date: Wed, 13 Nov 2024 04:53:08 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Helen Qin (Gerrit)

    unread,
    Nov 13, 2024, 8:00:38 PM11/13/24
    to Mohamed Amir Yosef, AyeAye, Tricium, Christian Biesinger, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Kaan Icer, Luna Lu, devtools...@chromium.org, devtools-re...@chromium.org, feature-co...@chromium.org, asvitki...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org
    Attention needed from Mohamed Amir Yosef

    Helen Qin added 1 comment

    File content/public/android/java/src/org/chromium/content/browser/webid/IdentityCredentialsDelegate.java
    Line 152, Patchset 6: PendingIntentHandler.retrieveCreateCredentialException(
    Helen Qin . unresolved

    This could also be null, please make sure that's handled correctly.

    In that case, it means neither a response nor an exception was propagated back, which is really unexpected. If possible, that should be logged somehow. Anyway, we should gracefully fail it on the user side (maybe just pass back a default exception)

    Helen Qin

    Oh, I meant a default DC exception, e.g. user cancellation error. What does the developer get if a raw `Exception` is passed back?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mohamed Amir Yosef
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I4aae7252c9fe673a8fbebd7202644b13c148a4f6
    Gerrit-Change-Number: 6006016
    Gerrit-PatchSet: 7
    Gerrit-Owner: Mohamed Amir Yosef <ma...@chromium.org>
    Gerrit-CC: Christian Biesinger <cbies...@chromium.org>
    Gerrit-CC: Chromium Metrics Reviews <chromium-met...@google.com>
    Gerrit-CC: Helen Qin <hele...@google.com>
    Gerrit-CC: Kaan Icer <ic...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Luna Lu <loon...@chromium.org>
    Gerrit-Attention: Mohamed Amir Yosef <ma...@chromium.org>
    Gerrit-Comment-Date: Thu, 14 Nov 2024 01:00:27 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Helen Qin <hele...@google.com>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mohamed Amir Yosef (Gerrit)

    unread,
    Jan 10, 2025, 11:59:45 AMJan 10
    to Helen Qin, AyeAye, Tricium, Christian Biesinger, Chromium Metrics Reviews, chromium...@chromium.org, Kentaro Hara, Kaan Icer, Luna Lu, devtools...@chromium.org, devtools-re...@chromium.org, feature-co...@chromium.org, asvitki...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, npm+...@chromium.org, yigu+...@chromium.org

    Mohamed Amir Yosef abandoned this change

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: abandon
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I4aae7252c9fe673a8fbebd7202644b13c148a4f6
    Gerrit-Change-Number: 6006016
    Gerrit-PatchSet: 9
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages