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

Custom domains improvements #2203

Open
mktcode opened this issue Apr 17, 2022 · 7 comments
Open

Custom domains improvements #2203

mktcode opened this issue Apr 17, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@mktcode
Copy link
Contributor

mktcode commented Apr 17, 2022

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.

@mktcode mktcode added the enhancement New feature or request label Apr 17, 2022
@samuveth
Copy link
Contributor

Yes! Bye bye snapshot-spaces repo :)

@mktcode
Copy link
Contributor Author

mktcode commented Apr 17, 2022

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
Copy link

hjjlxm commented Apr 18, 2022

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. #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
Copy link
Member

That make lot of sense, we can do this

@mktcode
Copy link
Contributor Author

mktcode commented Apr 23, 2022

TODO:

Hub:

  • add domain column (unique)
  • write script to migrate existing domain settings
    • use domains from domains.json, remove existing db values
  • adjust saving/editing/graphql accordingly
  • implement CNAME check (internal function and API endpoint)

UI:

  • change logic to rely on DB info instead of JSON file
    • requires some work on useApp.ts and router.ts
  • check CNAME on settings page
  • add CNAME info on custom domains

@zzuziak
Copy link
Contributor

zzuziak commented Feb 7, 2023

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

@mktcode
Copy link
Contributor Author

mktcode commented Feb 8, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants