search

snapshot-labs / snapshot

Interface for Snapshot. Join us on Discord http://discord.snapshot.org
https://snapshot.org
MIT License
9.22k stars 1.22k forks source link

Custom domains improvements #2203

Open mktcode opened 2 years ago

mktcode commented 2 years ago

Currently we need to update the snapshot-spaces submodule whenever we want to add a new custom domain, leading to a PR for that submodule repo and then the UI repo. I think this list of domains should be considered configuration and not code and therefore does not necessarily belong in a repository.

We already store the custom domains in the space settings in the DB. The json file is redundant and only exists as a simple solution to a problem that could be solved more elegantly (hopefully).

  1. The custom domain value in the DB is not enforced to be unique. Therefore multiple spaces can enter the same domain. When snapshot is now accessed through this domain, we wouldn't have a way to tell which space to load/show.
  2. The custom domain is part of the settings json data and not an indexed column of the spaces table. Therefore it is not easily possible to do a quick lookup when the UI loads.
  3. Another problem is that we recently changed the required DNS setting for custom domains (CNAME=cname.snapshot.org) and now need a way to update/inform 657 spaces.

I would like to suggest a solution that solves all of this, while also improving the UX by removing the need for manually updating a whitelist. I want to put some effort into this because I think the custom domain feature (after expressing some criticism :P) is actually a critical and important feature, not just a "nice to have". In combination with skins it's a first, easy to implement level of customization for DAOs to make themselves and their community feel more "at home". (The second level of customization would be re-usable UI components plus snapshot.js for completely custom integrations.) I would like this to be as simple and automated as possible, especially when we're not talking about 6k spaces anymore but a million.

  1. Domains must be a table column and unique.

To avoid accessing json fields in DB queries I would like to move the domain to it's own column, so we can do fast lookups. For the (currently) 657 spaces with a custom domain, this migration can easily be done by a script, accompanied by a few changes in the hub/graphql api. This would require some work but the changes are rather trivial and the domain field in the settings json is (as far as I can tell) not used at all anyway.

  1. Domains must already have the correct CNAME record set, when updating the space settings.

To prevent spaces from squatting a domain they don't own, by adding it in their space settings, blocking it for anyone else, we can check the CNAME record first and allow only domains that already point to cname.snapshot.org. (If this can also be a wildcard domain, spaces could be required to use <spaceid>.cname.snapshot.org.) This can easily done with node.js in the hub. A legitimate space would configure the domain, update the space settings and that's it.

  1. Information in the UI

By providing the CNAME check as an endpoint in the hub, the UI can give early feedback on the settings page and show an info box for admins, on all existing custom domains that have not yet updated their CNAME record.

With this setup we would not need to whitelist domains anymore and we can safely let spaces configure their domain DNS and update the settings and that's it. Less work for everyone and it's not even that much work required to implement it. In a week from now it could all be done, especially because I already did some work to find some unknowns. Given the long-term benefit I think that's more than reasonable.

samuveth commented 2 years ago

Yes! Bye bye snapshot-spaces repo :)

mktcode commented 2 years ago

Well, that would not even be necessary. It also hosts categories, verified spaces, skins,... But yea I'm definitely for moving that stuff over to the UI repo and get rid of the submodule. I don't see any benefits in having it.

hjjlxm commented 2 years ago

I share your view and admittedly it's a small but a pain point as we got much incoming from users who activated their domain but lead to the homepage of snapshot rather than their space destination. https://github.com/snapshot-labs/snapshot/issues/2205 @mktcode

At present the domain field in the settings seems serve little purpose besides all work done in the domain.json and DNS config.

Your improvements: Configure DNS → put it in the domain field of settings → Save, That's it!

On UX side: No longer need an activation and manually whitelist On snapshot's side: Changes of domain no longer need extra PRs to make it effect.

It will definitely worth the effort!

bonustrack commented 2 years ago

That make lot of sense, we can do this

mktcode commented 2 years ago

TODO:

Hub:

UI:

zzuziak commented 1 year ago

Hey @mktcode, @bonustrack, shall we proceed with this implementation?

mktcode commented 1 year ago

I could work on this again early next month I think but anyone shall feel free. I'm currently just keeping the hub PR branch in sync with master.