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

Change REST API to not work with encoded subjects #708

Closed
zepatrik opened this issue Sep 14, 2021 · 2 comments · Fixed by #729
Closed

Change REST API to not work with encoded subjects #708

zepatrik opened this issue Sep 14, 2021 · 2 comments · Fixed by #729
Labels
breaking change Changes behavior in a breaking manner. bug Something is not working. help wanted We are looking for help on this one. rfc A request for comments to discuss and share ideas. upstream Issue is caused by an upstream dependency.
Milestone

Comments

@zepatrik
Copy link
Member

Describe the bug

The REST handler returns and expects subjects encoded. This disallows to put some characters into the subject when using REST, but it works over gRPC. The APIs should however be compatible and work with the same inputs/outputs.
This is a follow up of #661.

Reproducing the bug

Steps to reproduce the behavior:

			rts := []*relationtuple.InternalRelationTuple{
				{
					Namespace: nspace.Name,
					Object:    "group:B",
					Relation:  "member",
					Subject: &relationtuple.SubjectSet{
						Namespace: nspace.Name,
						Object:    "group:A",
						Relation:  "member",
					},
				},
				{
					Namespace: nspace.Name,
					Object:    "@all",
					Relation:  "member",
					Subject: &relationtuple.SubjectID{ID: "this:will#be interpreted:as a@subject set"},
				},
			}

work as expected over gRPC, but give 400 and 404 over REST.

Expected behavior

Parity between the API interfaces.

Breaking change

We will have to change the schema of the JSON data:

{
  "$schema": "../../../.schema/relation_tuple.schema.json",
  "namespace": "videos",
  "object": "/cats/1.mp4",
  "relation": "owner",
  "subject": "videos:/cats#owner"
}

will become

{
  "$schema": "../../../.schema/relation_tuple.schema.json",
  "namespace": "videos",
  "object": "/cats/1.mp4",
  "relation": "owner",
  "subject": {
    "namespace": "videos",
    "object": "/cats",
    "relation": "owner"
  }
}
@zepatrik zepatrik added bug Something is not working. help wanted We are looking for help on this one. breaking change Changes behavior in a breaking manner. rfc A request for comments to discuss and share ideas. labels Sep 14, 2021
@zepatrik zepatrik added this to the v0.7.0 milestone Sep 16, 2021
@zepatrik
Copy link
Member Author

zepatrik commented Sep 16, 2021

Outdated proposal

Proposal for the new relationtuple schema:

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "additionalProperties": false,
  "required": ["namespace", "relation", "object", "subject"],
  "properties": {
    "$schema": {
      "type": "string",
      "format": "uri-reference",
      "description": "Add this to allow defining the schema, useful for IDE integration"
    },
    "namespace": {
      "type": "string",
      "description": "The namespace of the object and relation in this tuple."
    },
    "relation": {
      "type": "string",
      "description": "The relation of the object and subject."
    },
    "object": {
      "type": "string",
      "description": "The object affected by this relation."
    },
    "subject": {
      "oneOf": [
        {
          "type": "object",
          "description": "The subject set affected by this relation.",
          "properties": {
            "namespace": {
              "type": "string",
              "description": "The namespace of the object and relation in this subject set."
            },
            "relation": {
              "type": "string",
              "description": "The relation of this subject set."
            },
            "object": {
              "type": "string",
              "description": "The object referenced in this subject set."
            }
          },
          "additionalProperties": false,
          "required": ["namespace", "relation", "object"]
        },
        {
          "type": "string",
          "description": "The subject ID affected by this relation."
        }
      ]
    }
  }
}

with the changes

     "subject": {
       "oneOf": [
         {
-          "type": "string",
-          "pattern": "^.*:.*#.*$",
-          "description": "The subject set affected by this relation. Uses the encoding of \"<namespace>:<object>#<relation>\"."
+          "type": "object",
+          "description": "The subject set affected by this relation.",
+          "properties": {
+            "namespace": {
+              "type": "string",
+              "description": "The namespace of the object and relation in this subject set."
+            },
+            "relation": {
+              "type": "string",
+              "description": "The relation of this subject set."
+            },
+            "object": {
+              "type": "string",
+              "description": "The object referenced in this subject set."
+            }
+          },
+          "additionalProperties": false,
+          "required": ["namespace", "relation", "object"]
         },
         {
           "type": "string",
-          "description": "The subject affected by this relation. Use \"<namespace>:<object>#<relation>\" to describe a subject set.",
-          "not": {
-            "pattern": "^.*:.*#.*$"
-          }
+          "description": "The subject ID affected by this relation."
         }
       ]
     }

@zepatrik
Copy link
Member Author

As per discussion with @aeneasr, this will become

{
  "$schema": "http://json-schema.org/draft-07/schema#",
  "type": "object",
  "additionalProperties": false,
  "allOf": [
    {
      "required": [
        "namespace",
        "relation",
        "object"
      ],
      "properties": {
        "$schema": {
          "type": "string",
          "format": "uri-reference",
          "description": "Add this to allow defining the schema, useful for IDE integration"
        },
        "namespace": {
          "type": "string",
          "description": "The namespace of the object and relation in this tuple."
        },
        "object": {
          "type": "string",
          "description": "The object affected by this relation."
        },
        "relation": {
          "type": "string",
          "description": "The relation of the object and subject."
        }
      }
    },
    {
      "oneOf": [
        {
          "required": [
            "subject_id"
          ],
          "properties": {
            "subject_id": {
              "type": "string",
              "description": "The subject ID affected by this relation."
            }
          }
        },
        {
          "required": [
            "subject_set"
          ],
          "properties": {
            "subject_set": {
              "type": "object",
              "description": "The subject set affected by this relation.",
              "properties": {
                "namespace": {
                  "type": "string",
                  "description": "The namespace of the object and relation in this subject set."
                },
                "object": {
                  "type": "string",
                  "description": "The object referenced in this subject set."
                },
                "relation": {
                  "type": "string",
                  "description": "The relation of this subject set."
                }
              },
              "additionalProperties": false,
              "required": [
                "namespace",
                "relation",
                "object"
              ]
            }
          }
        }
      ]
    }
  ]
}

@zepatrik zepatrik added the upstream Issue is caused by an upstream dependency. label Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes behavior in a breaking manner. bug Something is not working. help wanted We are looking for help on this one. rfc A request for comments to discuss and share ideas. upstream Issue is caused by an upstream dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant