-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Update OID4VCI implementation #52
Conversation
Is this still for review despite the unsuccessful tests? |
Yes please! That's one interop test, I'll delete that later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of those TODOs seem quite significant, maybe add some of them as issues?
vclib-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/IssuerMetadata.kt
Show resolved
Hide resolved
vclib-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/OidcUserInfo.kt
Show resolved
Hide resolved
...b-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/SupportedAlgorithmsContainer.kt
Outdated
Show resolved
Hide resolved
...d/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/SupportedCredentialFormatDefinition.kt
Show resolved
Hide resolved
vclib-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/CredentialIssuer.kt
Outdated
Show resolved
Hide resolved
vclib-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/CredentialIssuer.kt
Show resolved
Hide resolved
vclib-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/WalletService.kt
Show resolved
Hide resolved
vclib-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidc/IdTokenType.kt
Show resolved
Hide resolved
subjectSyntaxTypesSupported = arrayOf(URN_TYPE_JWK_THUMBPRINT, PREFIX_DID_KEY), | ||
idTokenTypesSupported = arrayOf(IdTokenType.SUBJECT_SIGNED), | ||
responseTypesSupported = listOf(ID_TOKEN), | ||
scopesSupported = listOf(SCOPE_OPENID), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we're polishing: shouldn't all of these be sorted (to fix order) sets (to eliminate duplicates, since they make no sense) and not lists for good measure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with sets, but they don't need to be sorted
* Must contain an entry form [IssuerMetadata.authorizationServers]. | ||
*/ | ||
@SerialName("locations") | ||
val locations: Collection<String>? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again: do we want a set here?
*/ | ||
@SerialName("credential_identifiers") | ||
val credentialIdentifiers: Collection<String>? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set?
vclib-openid/src/commonMain/kotlin/at/asitplus/wallet/lib/oidvci/CredentialFormatEnum.kt
Show resolved
Hide resolved
*/ | ||
@SerialName("order") | ||
val order: Array<String>? = null, | ||
val order: Collection<String>? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set
@SerialName("display") | ||
val display: Collection<DisplayProperties>? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set
*/ | ||
@SerialName("type") | ||
val types: Collection<String>? = null, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set
* List of attributes that shall be requested explicitly (selective disclosure), | ||
* or `null` to make no restrictions | ||
*/ | ||
val requestedAttributes: List<String>? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set
import kotlinx.datetime.Clock | ||
import kotlin.random.Random | ||
|
||
/** | ||
* Client service to retrieve credentials using | ||
* [OpenID for Verifiable Credential Issuance](https://openid.net/specs/openid-4-verifiable-credential-issuance-1_0.html). | ||
* Implemented from Draft `openid-4-verifiable-credential-issuance-1_0-11`, 2023-02-03. | ||
*/ | ||
class WalletService( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might want to add an optional clock parameter here
I have no idea why the iOS Test Run fails ... why is the test task "skipped", @JesusMcCloud? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know It's not directly related to this PR, but RelyingPartyMetadata still uses Lists. I think we want set semantics there too. Change at your own discretion, as this file remained untouched by this MR.
Update our implementation of OpenID for Verifiable Credenital Issuance to latest draft (13, from 2024-02-08).
Far fetched goal is interoperability with EUDIW reference implementations.
The interface for actualliy issuing credentials (
Issuer.issueCredential()
) now needs some rework, but that should be done in a separate PR.Fixes #47