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

Jump to bottom

feature/SPRIND-89 #177

Merged
merged 29 commits into from
Jan 24, 2025
Merged

feature/SPRIND-89 #177

merged 29 commits into from
Jan 24, 2025

Conversation

BtencateSphereon
Copy link
Contributor

No description provided.

Brummos added 19 commits January 9, 2025 13:20
@Brummos
feat: added support for first party applications
9c273b9
@Brummos
Merge branch 'develop' into feature/SPRIND-89
f3b89fc 
# Conflicts:
#	packages/issuer-rest/lib/oid4vci-api-functions.ts
@Brummos
chore: cleanup
5a0dba4
@Brummos
chore: cleanup
bf1217f
@Brummos
chore: typo fix
ce3ef0b
@Brummos
chore: specifically check for true in isAuthorizationChallengeEndpoin…
2b4c069 
…tEnabled as it is an optional endpoint
@Brummos
chore: added acquiring authorization challenge authorization code to …
acf2cdc 
…vci client using the rest api
@Brummos
chore: cleanup
58af8fe
@Brummos
chore: make opts optional as client id is default from the client
b97ff27
@Brummos
chore: use authorization_code for acquiring the access token
3a426b5
@Brummos
chore: cleanup
1825b96
@Brummos
chore: refactor authorization challenge code error handling
e8dca63
@Brummos
chore: small fixes
3e2c8d7
@Brummos
chore: fixes
084d916
@Brummos
chore: added tests
ba107cc
@Brummos
chore: added tests
1eef34b
@Brummos
chore: added tests
c6c5df2
@Brummos
chore: fixes
a8f712d
@Brummos
chore: fix test
4a74802
@Sphereon-Opensource Sphereon-Opensource deleted a comment from codecov bot Jan 15, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 15, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 74.69880% with 42 lines in your changes missing coverage. Please review.

Project coverage is 48.98%. Comparing base (44db119) to head (147da96).
Report is 31 commits behind head on develop.

Files with missing lines Patch % Lines
packages/client/lib/OpenID4VCIClient.ts 66.66% 8 Missing ⚠️
packages/client/lib/OpenID4VCIClientV1_0_11.ts 57.89% 8 Missing ⚠️
...s/issuer/lib/builder/IssuerMetadataBuilderV1_13.ts 0.00% 6 Missing ⚠️
packages/client/lib/AuthorizationCodeClient.ts 82.75% 5 Missing ⚠️
packages/client/lib/OpenID4VCIClientV1_0_13.ts 84.21% 3 Missing ⚠️
.../lib/builder/AuthorizationServerMetadataBuilder.ts 0.00% 3 Missing ⚠️
packages/client/lib/MetadataClientV1_0_11.ts 60.00% 2 Missing ⚠️
packages/client/lib/MetadataClientV1_0_13.ts 60.00% 2 Missing ⚠️
packages/issuer-rest/lib/OID4VCIServer.ts 77.77% 2 Missing ⚠️
packages/issuer-rest/lib/oid4vci-api-functions.ts 95.00% 2 Missing ⚠️
... and 1 more
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #177      +/-   ##
===========================================
+ Coverage    48.11%   48.98%   +0.86%     
===========================================
  Files           75       75              
  Lines         5175     5316     +141     
  Branches      1800     1804       +4     
===========================================
+ Hits          2490     2604     +114     
- Misses        2682     2709      +27     
  Partials         3        3
Flag Coverage Δ
unittest 48.98% <74.69%> (+0.86%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BtencateSphereon BtencateSphereon marked this pull request as ready for review January 15, 2025 22:08
ronal2do
ronal2do approved these changes Jan 16, 2025
View reviewed changes
Copy link

@ronal2do ronal2do left a comment

Choose a reason for hiding this comment

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

Looks great, I'm just missing unit tests

nklomp
nklomp requested changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@nklomp nklomp left a comment

Choose a reason for hiding this comment

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

Some remarks.

What is the deal with isFirstParty? And why do we put that in the AuthResponse? That is unlikely in the spec. Where is that param being used?

packages/client/lib/MetadataClient.ts Outdated Show resolved Hide resolved
packages/client/lib/MetadataClient.ts Outdated Show resolved Hide resolved
packages/client/lib/MetadataClientV1_0_11.ts Outdated Show resolved Hide resolved
packages/client/lib/MetadataClientV1_0_13.ts Outdated Show resolved Hide resolved
packages/client/lib/__tests__/OpenID4VCIClient.spec.ts Show resolved Hide resolved
packages/issuer-rest/lib/OID4VCIServer.ts Outdated Show resolved Hide resolved
packages/issuer-rest/lib/oid4vci-api-functions.ts Outdated Show resolved Hide resolved
packages/siop-oid4vp/lib/authorization-response/Payload.ts Show resolved Hide resolved
@nklomp
Copy link
Contributor

nklomp commented Jan 24, 2025

There are some conflicts it seems. REST LGTM

nklomp
nklomp approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@nklomp nklomp left a comment

Choose a reason for hiding this comment

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

LGTM after conflicts will be resolved

@Brummos
Merge branch 'develop' into feature/SPRIND-89
147da96 
# Conflicts:
#	packages/siop-oid4vp/lib/authorization-response/types.ts
#	packages/siop-oid4vp/lib/op/OP.ts
#	packages/siop-oid4vp/lib/schemas/AuthorizationResponseOpts.schema.ts
@BtencateSphereon BtencateSphereon merged commit b24bc4c into develop Jan 24, 2025
2 checks passed
@BtencateSphereon BtencateSphereon deleted the feature/SPRIND-89 branch January 24, 2025 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nklomp nklomp nklomp approved these changes

@ronal2do ronal2do ronal2do approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@BtencateSphereon @nklomp @ronal2do @Brummos @sanderPostma