cs3org / OCM-API

OpenCloudMesh API
38 stars 11 forks source link

Explicitly mark invite flow attributes as required #143

Closed MahdiBaghbani closed 1 week ago

MahdiBaghbani commented 3 weeks ago

This PR makes the name and email fields in the invite flow as optional and everything else is required.

Fixes #142 Fixes #117

glpatcern commented 3 weeks ago

Hi there, interesting that this was left unspecified, despite the implicit assumption was that everything is required - and this is how Reva implemented it

IMHO it makes sense to have everything required, after all this flow implements a business card exchange. Also the fact that Reva currently provides the only implementation of this flow should be weighted in.

MahdiBaghbani commented 3 weeks ago

Reva implemented it

I think this one is for the invite flow Reva Protobuf

Vendors are guaranteed to provide the fields that are related to them, eg: userID or providerDomain

If everything is required then the vendors should supply placeholder (fake) strings for the non-essential values that the user didn't (or refused) to provide at the vendor itself, like email or name.

I'm in favor of both cases (explicitly mark as required or mark as optional).

MahdiBaghbani commented 1 week ago

This is the diff on git cli:

OCM-API on  email-field-invite-flow [?]
❯ git diff HEAD..origin/develop
diff --git a/IETF-RFC.md b/IETF-RFC.md
index 538ed4b..d5b6214 100644
--- a/IETF-RFC.md
+++ b/IETF-RFC.md
@@ -137,11 +137,11 @@ Whereas the precise syntax of the Invite Message and the Invite Acceptance Gestu
 * to the `/invited-accepted` path in the Invite Sender OCM Server's OCM API
 * using `application/json` as the `Content-Type` HTTP request header
 * its request body containing a JSON document representing an object with the following string fields:
-  * REQUIRED: `recipientProvider` - FQDN of the Invite Receiver OCM Server
-  * REQUIRED: `token` - the Invite Token. The Invite Sender OCM Server SHOULD recall which Invite Sender OCM Address this token was linked to
-  * REQUIRED: `userID` - the Invite Receiver's identifier at their OCM Server
-  * REQUIRED: `email` - non-normative / informational; an email address for the Invite Receiver. Not necessarily at the same FQDN as their OCM Server
-  * REQUIRED: `name` - human-readable name of the Invite Receiver, as a suggestion for display in the Invite Sender's address book
+  * `recipientProvider` - FQDN of the Invite Receiver OCM Server
+  * `token` - the Invite Token. The Invite Sender OCM Server SHOULD recall which Invite Sender OCM Address this token was linked to
+  * `userID` - the Invite Receiver's identifier at their OCM Server
+  * `email` - non-normative / informational; an email address for the Invite Receiver. Not necessarily at the same FQDN as their OCM Server
+  * `name` - human-readable name of the Invite Receiver, as a suggestion for display in the Invite Sender's address book
 * using TLS
 * using [httpsig](https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures-12)

@@ -157,9 +157,9 @@ The Invite Acceptance Response SHOULD be a HTTP response:
 * in response to the Invite Acceptance Request
 * using `application/json` as the `Content-Type` HTTP response header
 * its response body containing a JSON document representing an object with the following string fields:
-  * REQUIRED: `userID` - the Invite Sender's identifier at their OCM Server
-  * REQUIRED: `email` - non-normative / informational; an email address for the Invite Sender. Not necessarily at the same FQDN as their OCM Server
-  * REQUIRED: `name` - human-readable name of the Invite Sender, as a suggestion for display in the Invite Receiver's address book
+  * `userID` - the Invite Sender's identifier at their OCM Server
+  * `email` - non-normative / informational; an email address for the Invite Sender. Not necessarily at the same FQDN as their OCM Server
+  * `name` - human-readable name of the Invite Sender, as a suggestion for display in the Invite Receiver's address book

 A 200 response status means the Invitation Acceptance Request was successful.
 A 400 response status means the Invitation Token is invalid or does not exist.
diff --git a/spec.yaml b/spec.yaml
index 6c0887c..cb4bae4 100644
--- a/spec.yaml
+++ b/spec.yaml
@@ -692,53 +692,45 @@ definitions:
         providerId: "51dc30ddc473d43a6011e9ebba6ca770"
   AcceptedInvite:
     type: object
-    required:
-      - recipientProvider
-      - token
-      - userID
-      - email
-      - name
-    properties:
-      recipientProvider:
-        type: string
-        format: fqdn
-        description: FQDN of the receiver OCM service.
-        example: receiver.org
-      token:
-        type: string
-        description: Token received in the invite
-        example: xyz
-      userID:
-        type: string
-        description: Unique ID to identify the Invite Receiver at their OCM Server.
-        example: 51dc30ddc473d43a6011e9ebba6ca770
-      email:
-        type: string
-        description: Email address of the Invite Receiver.
-        example: richard@gmail.com
-      name:
-        type: string
-        description: Name of the Invite Receiver.
-        example: Richard Feynman
+    allOf:
+      - properties:
+          recipientProvider:
+            type: string
+            format: fqdn
+            description: FQDN of the receiver OCM service.
+            example: receiver.org
+          token:
+            type: string
+            description: Token received in the invite
+            example: xyz
+          userID:
+            type: string
+            description: Unique ID to identify the Invite Receiver at their OCM Server.
+            example: 51dc30ddc473d43a6011e9ebba6ca770
+          email:
+            type: string
+            description: Email address of the Invite Receiver.
+            example: richard@gmail.com
+          name:
+            type: string
+            description: Name of the Invite Receiver.
+            example: Richard Feynman
   AcceptedInviteResponse:
     type: object
-    required:
-      - userID
-      - email
-      - name
-    properties:
-      userID:
-        type: string
-        description: Unique ID to identify the Invite Sender at their OCM Server.
-        example: 9302
-      email:
-        type: string
-        description: Email ID of the Invite Sender.
-        example: john@sender.org
-      name:
-        type: string
-        description: Name of the Invite Sender.
-        example: John Doe
+    allOf:
+      - properties:
+          userID:
+            type: string
+            description: Unique ID to identify the Invite Sender at their OCM Server.
+            example: 9302
+          email:
+            type: string
+            description: Email ID of the Invite Sender.
+            example: john@sender.org
+          name:
+            type: string
+            description: Name of the Invite Sender.
+            example: John Doe
   TokenRequest:
     type: object
     allOf:
(END)
-      userID:
-        type: string
-        description: Unique ID to identify the Invite Sender at their OCM Server.
-        example: 9302
-      email:
-        type: string
-        description: Email ID of the Invite Sender.
-        example: john@sender.org
-      name:
-        type: string
-        description: Name of the Invite Sender.
-        example: John Doe
+    allOf:
+      - properties:
+          userID:
+            type: string
+            description: Unique ID to identify the Invite Sender at their OCM Server.
+            example: 9302
+          email:
+            type: string
+            description: Email ID of the Invite Sender.
+            example: john@sender.org
+          name:
+            type: string
+            description: Name of the Invite Sender.
+            example: John Doe
   TokenRequest:
     type: object
     allOf:
michielbdejong commented 1 week ago

@MahdiBaghbani and after a git fetch?

MahdiBaghbani commented 1 week ago

@michielbdejong is still the same.

This problem in GitHub has been discussed here: https://github.com/orgs/community/discussions/16351

If I create a new pull request this is what GitHub will show us: Screenshot 2024-10-10 182017

glpatcern commented 1 week ago

So the new PR would look correct, as far as I can see. Maybe just go ahead and create it!