feature: custom domains using Fairings #596

igalic · 2019-05-24T13:52:13Z

This Pull Request addresses #94; replaces #447;
We start by adding custom_domain to the blogs table; and a custom type Host(String)

A blog with custom_domain has special routes with higher priority, but administrative routes will remain on the instance host.

We'll use Fairings to extract the Host header, to then specially route them.

Domain administration is left to the user (and/or admin)
(but we might be able to provide a check on whether it's already correctly configured)

TODO

codecov · 2019-05-24T17:28:16Z

Codecov Report

Merging #596 into master will increase coverage by 3.54%.
The diff coverage is 32.14%.

@@            Coverage Diff             @@
##           master     #596      +/-   ##
==========================================
+ Coverage   30.96%   34.51%   +3.54%     
==========================================
  Files          66       67       +1     
  Lines        7822     7916      +94     
  Branches     1881     1894      +13     
==========================================
+ Hits         2422     2732     +310     
+ Misses       4700     4421     -279     
- Partials      700      763      +63

src/main.rs

plume-models/src/blogs.rs

codecov · 2019-05-28T15:57:58Z

Codecov Report

Merging #596 into master will decrease coverage by 0.47%.
The diff coverage is 16.73%.

@@            Coverage Diff             @@
##           master     #596      +/-   ##
==========================================
- Coverage   35.33%   34.86%   -0.48%     
==========================================
  Files          68       68              
  Lines        7915     8149     +234     
  Branches     1894     1946      +52     
==========================================
+ Hits         2797     2841      +44     
- Misses       4341     4529     +188     
- Partials      777      779       +2

igalic · 2019-05-29T13:49:05Z

so: I'm a bit at an impasse or crossroads with this patch, and it all kinda depends on how far i wanna push it.

For now, i've added a 404 handler, which redirects to the main-site, which means, actually, everything works now 😹 (by redirecting to the main site)

We could present the complete illusion that this custom domain, is the real domain, and all there is is this one blog.

To do this, I would have to adapt the URLs ap_url, inbox_url and outbox_url on blog creation (or update) and i would have to modify the templates to use the correct URL scheme.
Alternatively we create an URL builder which we resolves the correct URLs.

Currently, there are a lot of places in which URLs are built ad-hoc.
In some cases, this is okay, because it's just pointing to a rocket route, but in many cases it feels painful, because we're doing manual labour that a function could remove.

the compute_box() function for instance, feels quite pointless. There is no computation happening at all, just concatenation! compute_box() itself calls ap_url() which only prepends https://

I would like to propose a Universal URL builder… or rather, a plumey-versal URL builder, which can be used from any part of plume's code to construct a correct plume URL.

trinity-1686a · 2019-05-29T22:17:47Z

To do this, I would have to adapt the URLs ap_url, inbox_url and outbox_url on blog creation (or update) and i would have to modify the templates to use the correct URL scheme.

I think it would be best to still handle most of (~all) ap stuff on the main domain, having an id (as requested in #296) for the activity that goes to the main domain, with a redirect to the correct path when queried (so either the main domain with better name than the id, or custom one when applicable). Also with some webfinger trick it should be possible to have more than one handle for a blog, so there could be the one from the main domain, that shouldn't change because it would break a lot of federation stuff, and an alias based on the custom domain. All this would be in the spirit of not breaking things if the custom domain for a blog changes

I would like to propose a Universal URL builder… or rather, a plumey-versal URL builder, which can be used from any part of plume's code to construct a correct plume URL.

that would be a good idea for everything that need to integrate the domain in addition to a path. It could be a small macro that call the rocket one, and prepand the domain name, either from local_instance if none is given, or the one given (which would be a custom domain)

igalic · 2019-05-30T16:38:00Z

okay, so, at this point, i'm about as done as i can be, without touching the templates or refactoring the URL building.

migrations/sqlite/2019-05-22-124153_add_custom_domains/up.sql

elegaanz

I gave it a try, and it seems to work quite well so far! I left a few comment about the code itself.

plume-models/src/blogs.rs

src/routes/blogs.rs

src/routes/posts.rs

src/routes/blogs.rs

igalic

validation?

src/routes/blogs.rs

we can use this to handle rocket Requests (from_request) but we still have to figure out how to automatically assign the type to custom_domain: Optional<Host>, and public_domain: Host.

we now use DieselNewType and Shrinkwrap to automatically derive all the things we need.

thanks a lot to @fdb-hiroshima and @BaptisteGelez for helping with the code.

this rewrites the URL to /custom_domain/<url> (we have no route handlers for this path yet) Lots of help from @fdb-hiroshima & @BaptisteGelez in dealing with the borrow checker — thank you 💜️

follow #572 and cache the list of custom domains.

thanks, again, @fdb-hiroshima for helping with this code, when i got stuck in lifetime-hecc.

there is still some issue with `None`, and an error making no sense at some place

also please a bit clippy

Co-Authored-By: fdb-hiroshima <[email protected]>

The actual DNS check still needs to be written

Co-Authored-By: fdb-hiroshima <[email protected]>

src/routes/blogs.rs

this doesn't work (yet?), because i don't know how to store something mutable in State<>

trinity-1686a

👀

src/routes/blogs.rs

trinity-1686a · 2019-08-15T17:44:15Z

src/main.rs

@@ -196,6 +197,7 @@ Then try to restart Plume
        }
    });

+    let mut valid_domains: HashMap<&str, Instant> = HashMap::new();


I'd be in favor of using the newtype design pattern for this, so that it make more sense what it is, even at the type level.
I don't know where are the &str coming from, but you might want to use String instead, or you'll have some difficulties with lifetime

aye. i haven't gotten that far yet, because i haven't implemented the other side of this

trinity-1686a

👀

src/routes/blogs.rs

thanks to @fdb-hiroshima for the review

src/routes/blogs.rs

n.b.: no validation yet

thanks, @fdb-hiroshima Co-Authored-By: fdb-hiroshima <[email protected]>

elegaanz · 2019-08-21T09:15:24Z

src/routes/blogs.rs

@@ -351,6 +351,10 @@ pub fn edit(name: String, rockets: PlumeRocket) -> Result<Ructe, ErrorPage> {
            .clone()
            .expect("blogs::edit: User was None while it shouldn't");
        let medias = Media::for_user(conn, user.id).expect("Couldn't list media");
+        let custom_domain = match blog.custom_domain {
+            Some(ref c) => c.to_string(),
+            _ => String::from(""),


It is a quite personal opinion, but I think blog.custom_domain.map(ToString::to_string).unwrap_or_default() is more explicit (but, yes, it is only my opinion, maybe it is not).

another question might be: why were you able to use Option<String> for theme in EditForm but i have to use String here?

igalic requested review from elegaanz and trinity-1686a and removed request for elegaanz May 24, 2019 13:52

trinity-1686a reviewed May 25, 2019

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

trinity-1686a reviewed May 25, 2019

View reviewed changes

plume-models/src/blogs.rs Outdated Show resolved Hide resolved

trinity-1686a reviewed May 27, 2019

View reviewed changes

plume-models/src/blogs.rs Outdated Show resolved Hide resolved

plume-models/src/blogs.rs Outdated Show resolved Hide resolved

plume-models/src/blogs.rs Outdated Show resolved Hide resolved

elegaanz reviewed May 28, 2019

View reviewed changes

plume-models/src/blogs.rs Show resolved Hide resolved

rfwatson reviewed Jun 2, 2019

View reviewed changes

migrations/sqlite/2019-05-22-124153_add_custom_domains/up.sql Show resolved Hide resolved

elegaanz reviewed Jun 15, 2019

View reviewed changes

plume-models/src/blogs.rs Outdated Show resolved Hide resolved

src/routes/blogs.rs Outdated Show resolved Hide resolved

elegaanz reviewed Jul 12, 2019

View reviewed changes

src/routes/posts.rs Show resolved Hide resolved

elegaanz reviewed Jul 12, 2019

View reviewed changes

src/routes/posts.rs Outdated Show resolved Hide resolved

trinity-1686a reviewed Jul 12, 2019

View reviewed changes

src/routes/posts.rs Outdated Show resolved Hide resolved

trinity-1686a reviewed Jul 31, 2019

View reviewed changes

src/routes/blogs.rs Outdated Show resolved Hide resolved

igalic commented Jul 31, 2019

View reviewed changes

src/routes/blogs.rs Show resolved Hide resolved

trinity-1686a reviewed Aug 1, 2019

View reviewed changes

src/routes/blogs.rs Outdated Show resolved Hide resolved

igalic added 10 commits August 14, 2019 23:51

add custom_domain (default to NULL) to schema

6bcc4f0

add Host(String) wrapper type

92bbeeb

we can use this to handle rocket Requests (from_request) but we still have to figure out how to automatically assign the type to custom_domain: Optional<Host>, and public_domain: Host.

implement Host more completely by doing less

65ae51b

we now use DieselNewType and Shrinkwrap to automatically derive all the things we need.

list custom_domains

351c01f

thanks a lot to @fdb-hiroshima and @BaptisteGelez for helping with the code.

create & attach an AdHoc Fairing for dealing with Custom Domains

3a4c2f2

this rewrites the URL to /custom_domain/<url> (we have no route handlers for this path yet) Lots of help from @fdb-hiroshima & @BaptisteGelez in dealing with the borrow checker — thank you 💜️

appease clippy

2746e08

cache custom_domains list

e6747de

follow #572 and cache the list of custom domains.

normalize URLs before setting them

b09b51c

move impl closer to mother struct

f94b0c7

CustomDomainFairing must not match /static/ and /api/

0645f7e

thanks, again, @fdb-hiroshima for helping with this code, when i got stuck in lifetime-hecc.

igalic and others added 9 commits August 14, 2019 23:51

fix syntax: we're now down to type errors

6cd8bd8

make url! work better

c5f6b88

there is still some issue with `None`, and an error making no sense at some place

finally fix url!

b172a80

also please a bit clippy

start modifying /blogs/new to add custom_domain to the form

2dedcdb

Strings have to be cloned

fdc7da0

fix clippy warning

6027012

Co-Authored-By: fdb-hiroshima <[email protected]>

Basic domain validation

da6757c

The actual DNS check still needs to be written

review by @fdb-hiroshima

ff0c82e

Co-Authored-By: fdb-hiroshima <[email protected]>

start exploring some ideas for domain validation

bf1673d

trinity-1686a reviewed Aug 14, 2019

View reviewed changes

src/routes/blogs.rs Outdated Show resolved Hide resolved

implement domain validation using rocket::State

ed30284

this doesn't work (yet?), because i don't know how to store something mutable in State<>

trinity-1686a reviewed Aug 15, 2019

View reviewed changes

trinity-1686a reviewed Aug 19, 2019

View reviewed changes

src/routes/blogs.rs Outdated Show resolved Hide resolved

src/routes/blogs.rs Show resolved Hide resolved

src/routes/blogs.rs Outdated Show resolved Hide resolved

igalic added 3 commits August 19, 2019 21:19

correctly mutex-lock and modify valid_domains

444a467

thanks to @fdb-hiroshima for the review

one step closer to validating custom domains!

6072351

validate custom domain!

5e46922

trinity-1686a reviewed Aug 20, 2019

View reviewed changes

src/routes/blogs.rs Outdated Show resolved Hide resolved

igalic and others added 10 commits August 20, 2019 11:18

do not panic if reqwest does not work

38ece9b

fix getter router for domain_validation!

f67ce93

also mount domain_validation into /custom_domains/

df47cdd

Allow editing blogs with custom_domain

064dd79

n.b.: no validation yet

use Custom status code

1694698

thanks, @fdb-hiroshima Co-Authored-By: fdb-hiroshima <[email protected]>

use Status::* as @fdb-hiroshima suggested

768f126

when building links, skip both, /custom_domain/ and <custom_domain>

df7adaf

fix mount point for blog post routing function

f395c10

restore df7adaf ← after fixing mount-point; fix query() parsing

d242e6d

Allow updating of custom_domain

3c4abcf

elegaanz reviewed Aug 21, 2019

View reviewed changes

marek-lach added the C: Feature This is a new feature label Aug 30, 2019

elegaanz changed the base branch from master to main July 5, 2020 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: custom domains using Fairings #596

feature: custom domains using Fairings #596

igalic commented May 24, 2019 •

edited

Loading

codecov bot commented May 24, 2019 •

edited

Loading

codecov bot commented May 28, 2019 •

edited

Loading

igalic commented May 29, 2019

trinity-1686a commented May 29, 2019

igalic commented May 30, 2019

elegaanz left a comment

igalic left a comment

trinity-1686a left a comment

trinity-1686a Aug 15, 2019 •

edited

Loading

igalic Aug 19, 2019

trinity-1686a left a comment

elegaanz Aug 21, 2019

igalic Aug 21, 2019

feature: custom domains using Fairings #596

Are you sure you want to change the base?

feature: custom domains using Fairings #596

Conversation

igalic commented May 24, 2019 • edited Loading

TODO

codecov bot commented May 24, 2019 • edited Loading

Codecov Report

codecov bot commented May 28, 2019 • edited Loading

Codecov Report

igalic commented May 29, 2019

trinity-1686a commented May 29, 2019

igalic commented May 30, 2019

elegaanz left a comment

Choose a reason for hiding this comment

igalic left a comment

Choose a reason for hiding this comment

trinity-1686a left a comment

Choose a reason for hiding this comment

trinity-1686a Aug 15, 2019 • edited Loading

Choose a reason for hiding this comment

igalic Aug 19, 2019

Choose a reason for hiding this comment

trinity-1686a left a comment

Choose a reason for hiding this comment

elegaanz Aug 21, 2019

Choose a reason for hiding this comment

igalic Aug 21, 2019

Choose a reason for hiding this comment

igalic commented May 24, 2019 •

edited

Loading

codecov bot commented May 24, 2019 •

edited

Loading

codecov bot commented May 28, 2019 •

edited

Loading

trinity-1686a Aug 15, 2019 •

edited

Loading