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

Constrain new URLs #556

Open
joepio opened this issue Jan 10, 2023 · 0 comments · May be fixed by #505
Open

Constrain new URLs #556

joepio opened this issue Jan 10, 2023 · 0 comments · May be fixed by #505

Comments

@joepio
Copy link
Member

joepio commented Jan 10, 2023

At this moment, Atomic Server is very liberal in accepting Commits that create new resources. This can lead to weird situations, that should be prevented:

  • Create resources on non-existing drives / subdomains.
  • Create resources on drives owned by someone else. (e.g. john using the mary.com/hi URL, not allowing mary to edit it)
  • Reserve precious paths e.g. example.com/welcome even if you only should access example.com/shared/1259015

This should definitely not be allowed.

How can we deal with this?

Check for append right in path

User creates example.com/topics/1/test/2, we check the resources example.com, example.com/topics, example.com/topics/1 and example.com/topics/1/test. If the user has an append right on any of these resources, we allow minting the URL. If not, the user cannot create the resource.

  • The amount of checks is far higher than before, perhaps making things slower.
  • The parent may still be a different, even completely external resource
  • Might be too much checks, as we're already checking for this right during authorization checks

Check if the parent is in the path of the new URL

User creates example.com/topics/1/test/2, we need to check that the parent of that resource is part of that URL, such as example.com or example.com/topics/1.

If that parent matches, we can simply perform the regular check at the parent.

This would be a far more severe limitation, but would help to keep things organised and predictable.

  • No additional performance cost, checking if parent matches is trivial
  • Users can still create nested resources easily
  • We can still have sub-paths that do not exist, e.g. example.com/topics/1 may not exist, even if /topics/1/test does exist. This can be confusing.
  • Parent-child relations can no longer span across devices. This is probably a good thing in many situations (because: how do you query foreign machines?), but it also severely limits the flexibility of hierarchies.
  • We also need to check if the URL ends with a slash. Otherwise, users might create a resource for atomicdata.dev.example.com with the parent set to atomicdata.dev
  • The default collections URLs aren't allowed now. Should we nest these or make an exception? I think we should nest them.
  • And how about /endpoint resources? Should we nest these, too? Might be a good idea, but has quite a bit of impact.
  • When importing JSON-AD, we might encounter local-ids that do not adhere to this nesting. How do we deal with these? => Throw an error, the JSON-AD creator should consider the parent-child constraints.
  • How do we deal with users having a public home page, yet desire private sub-resources? => User could create /home and make that one public.

Check if the origin of the URL is the same as the primary parent

  • Recursively check the parents, until you arrive at the Drive, then take the URL of the Drive
  • Check if the Drive URL is inside the URL of the new resource
  • This still allows reserving pretty URLs
joepio added a commit that referenced this issue Jan 23, 2023
@joepio joepio linked a pull request Jan 23, 2023 that will close this issue
18 tasks
joepio added a commit that referenced this issue Jan 24, 2023
joepio added a commit that referenced this issue Feb 5, 2023
joepio added a commit that referenced this issue Feb 25, 2023
joepio added a commit that referenced this issue Feb 28, 2023
joepio added a commit that referenced this issue Mar 11, 2023
joepio added a commit that referenced this issue Mar 30, 2023
joepio added a commit that referenced this issue Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant