SecureIdentityAlliance / osia

Open Standard set of APIs for interoperability of identity management building blocks.
https://osia.readthedocs.io/en/latest/
Other
20 stars 20 forks source link

Enrollment technical interfaces #15

Closed christiaanps closed 4 years ago

olivier-heurtier-idemia commented 4 years ago

Hi @christiaanps I reviewed the functional description and added a few comments inline. Globally I'm fine with it. Do you think it would be possible to add in all service an optional "finalize" flag, as discussed during the workshops? Thanks

olivier-heurtier-idemia commented 4 years ago

Hi @christiaanps I reviewed your last changes and the YAML. See my comments above plus those 2 general comments:

Thanks for your contribution, result is excellent.

christiaanps commented 4 years ago

Hi @olivier-heurtier-idemia

I have fixed the PR. Also I have added the mimetype and added the Annexes.

Thank you for the good review!

olivier-heurtier-idemia commented 4 years ago

After discussing internally in IDEMIA, we think that it would be interesting to add a DeleteEnrollment. It is not very complex and can allow a simple & proper management of resources for those that do not trust completely a GC mechanism.

olivier-heurtier-idemia commented 4 years ago

For consistency with the other API (PR & ABIS mostly), could you pluralize the resource name in the path (/v1/enrollment -> /v1/enrollments) This seems to be the usual convention (See https://opensource.zalando.com/restful-api-guidelines/#134 or the Google calendar API for example).

christiaanps commented 4 years ago

For consistency with the other API (PR & ABIS mostly), could you pluralize the resource name in the path (/v1/enrollment -> /v1/enrollments) This seems to be the usual convention (See https://opensource.zalando.com/restful-api-guidelines/#134 or the Google calendar API for example).

I already have v1-enrollments to find enrollments. How would you like me to change this path? Should I add /v1/enrollments/find?

olivier-heurtier-idemia commented 4 years ago

For consistency with the other API (PR & ABIS mostly), could you pluralize the resource name in the path (/v1/enrollment -> /v1/enrollments) This seems to be the usual convention (See https://opensource.zalando.com/restful-api-guidelines/#134 or the Google calendar API for example).

I already have v1-enrollments to find enrollments. How would you like me to change this path? Should I add /v1/enrollments/find?

Hi @christiaanps Since there is no conflict on the paths you can just add a 's' to all the path and keep '/v1/enrollments' for the find enrollments service.

christiaanps commented 4 years ago

For consistency with the other API (PR & ABIS mostly), could you pluralize the resource name in the path (/v1/enrollment -> /v1/enrollments) This seems to be the usual convention (See https://opensource.zalando.com/restful-api-guidelines/#134 or the Google calendar API for example).

I already have v1-enrollments to find enrollments. How would you like me to change this path? Should I add /v1/enrollments/find?

Hi @christiaanps Since there is no conflict on the paths you can just add a 's' to all the path and keep '/v1/enrollments' for the find enrollments service.

I have updated the PR