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.

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

Policy backend API spec #2607

Merged
merged 7 commits into from
Nov 23, 2023
Merged

Conversation

woutslakhorst
Copy link
Member

@woutslakhorst woutslakhorst commented Nov 15, 2023

closes #2606

The specification is required to generate a client for the Nuts node and for policy backends to generate the server API.

  • Client code generation

Copy link
Member

@gerardsn gerardsn left a comment

Choose a reason for hiding this comment

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

Also see comments in issue.


Will user information also be passed to the backend? If so, what field parameter do they belong in?

@@ -0,0 +1,133 @@
openapi: 3.1.0
Copy link
Member

@gerardsn gerardsn Nov 15, 2023

Choose a reason for hiding this comment

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

read the docs

We use version 3.0.y of the OAS.

Not sure if it matters

Copy link
Member Author

Choose a reason for hiding this comment

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

no

- request_method
- presentation_submission
- vps
properties:
Copy link
Member

Choose a reason for hiding this comment

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

missing the input_desciptor_constraint_id_map from the introspection endpoint? Could probably use a simpler name here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then the entire id to value mapping would have to be standardised. I think it's too early for a decision on that. With the VPs included, a backend can make a decision.

Copy link
Member

Choose a reason for hiding this comment

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

I was under the impression that we would introduce this mapping so other parties do not have to deal with PEX/VC/VP and just check the string value they need extracted from the VC/VP. (makes it easier to start)

Do you still want to add this? If not (or undecided) we should probably remove the map from the introspection endpoint too for now

Copy link
Member Author

Choose a reason for hiding this comment

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

That'll be an incremental change. We can then also process feedback in the same change.

Comment on lines +93 to +95
- request_method
- presentation_submission
- vps
Copy link
Member

Choose a reason for hiding this comment

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

These are optional? Most backends probably can't handle PEX/VC/VP

Copy link
Member Author

Choose a reason for hiding this comment

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

The first versions might just have to, we'll have to experiment a bit to see what's useful.

"200":
description: |
DID has been found and the scope is supported.
If the scope is supported but no presentation definition is required, the response will be 200 with a presentation definition without any input descriptors.
Copy link
Member

Choose a reason for hiding this comment

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

what would the use case for this be?

Copy link
Member Author

Choose a reason for hiding this comment

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

public service, like a directory service

@@ -175,10 +175,10 @@ func TestHTTPClient_Authorized(t *testing.T) {
handler := http2.Handler{StatusCode: http.StatusOK}
_, client := testServerAndClient(t, &handler)

response, err := client.Authorized(ctx, "http://::1:1", request)
response, err := client.Authorized(ctx, "http://test.test", request)
Copy link
Member

Choose a reason for hiding this comment

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

bad test, uses external resources and assumes test.test won't be available?

Better use the test server and an invalid (local) port?

Copy link
Member Author

Choose a reason for hiding this comment

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

from wiki

.test is a reserved top-level domain intended for usage in software testing. It is guaranteed to never be registered into the Internet.

an ipv6 address won't work on our CI. A .local takes a second to (not) resolve.

- request_method
- presentation_submission
- vps
properties:
Copy link
Member

Choose a reason for hiding this comment

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

I was under the impression that we would introduce this mapping so other parties do not have to deal with PEX/VC/VP and just check the string value they need extracted from the VC/VP. (makes it easier to start)

Do you still want to add this? If not (or undecided) we should probably remove the map from the introspection endpoint too for now

policy/api/v1/client/client.go Outdated Show resolved Hide resolved
policy/api/v1/client/client.go Outdated Show resolved Hide resolved
policy/api/v1/client/client.go Outdated Show resolved Hide resolved
@woutslakhorst woutslakhorst force-pushed the feature/2606/pex_backend_interface branch from 422e868 to 124df4f Compare November 23, 2023 09:25
@woutslakhorst woutslakhorst merged commit 9f20395 into master Nov 23, 2023
8 of 9 checks passed
@woutslakhorst woutslakhorst deleted the feature/2606/pex_backend_interface branch November 23, 2023 09:32
reinkrul pushed a commit that referenced this pull request Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Design scope to presentation definition and request authorization API interface
3 participants