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

Add subdomain support #1537

Merged
merged 3 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 129 additions & 0 deletions spec/lucky/subdomain_spec.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
require "../spec_helper"

include ContextHelper

abstract class BaseAction < Lucky::Action
include Lucky::Subdomain
accepted_formats [:html], default: :html
end

class Simple::Index < BaseAction
register_subdomain

get "/simple" do
plain_text subdomain
end
end

class Specific::Index < BaseAction
register_subdomain "foo"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work for multiple subdomain too? Like "staging.dashboard.foo"

Copy link
Member Author

@matthewmcgarvey matthewmcgarvey Jul 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My current job uses "multiple subdomains" as well because we have a tenant subdomain and the environment subdomain so in staging a url might look like tenant1.staging.example.com.

This subdomain support really focuses on 1 subdomain. Without any special handling, the subdomain for my example would be tenant1.staging and it would be up to me to add special logic to parse it further.

Another way to handle that would be to configure the tld_length to be 2 only in staging so that the subdomain would become tenant1 but I think that might be a sort of hack and of course only works if I don't care about that second subdomain part.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, yeah, that's what my sites do too. staging.tenant1.example.com. That's fine though. We just would document that on the site saying if you use deep nested subdomains, then you'll need to do your own split and parse.


get "/specific" do
plain_text subdomain
end
end

class Regex::Index < BaseAction
register_subdomain /www\d/

get "/regex" do
plain_text subdomain
end
end

class Multiple::Index < BaseAction
register_subdomain ["test", "staging", /(prod|production)/]

get "/multiple" do
plain_text subdomain
end
end

describe Lucky::Subdomain do
it "handles general subdomain expectation" do
request = build_request(host: "foo.example.com")
response = Simple::Index.new(build_context(request), params).call
response.body.should eq "foo"
end

it "raises error if subdomain missing" do
request = build_request(host: "example.com")
expect_raises(Lucky::InvalidSubdomainError) do
Simple::Index.new(build_context(request), params).call
end
end

it "handles specific subdomain expectation" do
request = build_request(host: "foo.example.com")
response = Specific::Index.new(build_context(request), params).call
response.body.should eq "foo"
end

it "raises error if subdomain does not match specific" do
request = build_request(host: "admin.example.com")
expect_raises(Lucky::InvalidSubdomainError) do
Specific::Index.new(build_context(request), params).call
end
end

it "handles regex subdomain expectation" do
request = build_request(host: "www4.example.com")
response = Regex::Index.new(build_context(request), params).call
response.body.should eq "www4"
end

it "raises error if subdomain does not match regex" do
request = build_request(host: "4www.example.com")
expect_raises(Lucky::InvalidSubdomainError) do
Regex::Index.new(build_context(request), params).call
end
end

it "handles multiple options for expectation" do
request = build_request(host: "test.example.com")
response = Multiple::Index.new(build_context(request), params).call
response.body.should eq "test"

request = build_request(host: "staging.example.com")
response = Multiple::Index.new(build_context(request), params).call
response.body.should eq "staging"

request = build_request(host: "prod.example.com")
response = Multiple::Index.new(build_context(request), params).call
response.body.should eq "prod"

request = build_request(host: "production.example.com")
response = Multiple::Index.new(build_context(request), params).call
response.body.should eq "production"
end

it "raises error if subdomain does not match any expectations" do
request = build_request(host: "development.example.com")
expect_raises(Lucky::InvalidSubdomainError) do
Multiple::Index.new(build_context(request), params).call
end
end

it "has configuration for urls with larger tld length" do
Lucky::Subdomain.temp_config(tld_length: 2) do
request = build_request(host: "foo.example.co.uk")
response = Simple::Index.new(build_context(request), params).call
response.body.should eq "foo"
end
end

it "will fail if using ip address" do
request = build_request(host: "development.127.0.0.1:3000")
expect_raises(Lucky::InvalidSubdomainError) do
Simple::Index.new(build_context(request), params).call
end
end

it "will not fail if using localhost and port with tld length set to 0" do
Lucky::Subdomain.temp_config(tld_length: 0) do
request = build_request(host: "foo.locahost:3000")
response = Simple::Index.new(build_context(request), params).call
response.body.should eq "foo"
end
end
end
5 changes: 3 additions & 2 deletions spec/support/context_helper.cr
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ module ContextHelper
method = "GET",
body = "",
content_type = "",
fixed_length : Bool = false
fixed_length : Bool = false,
host = "example.com"
) : HTTP::Request
headers = HTTP::Headers.new
headers.add("Content-Type", content_type)
headers.add("Host", "example.com")
headers.add("Host", host)
if fixed_length
body = HTTP::FixedLengthContent.new(IO::Memory.new(body), body.size)
end
Expand Down
3 changes: 3 additions & 0 deletions src/lucky/errors.cr
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,7 @@ module Lucky
MESSAGE
end
end

class InvalidSubdomainError < Error
end
end
71 changes: 71 additions & 0 deletions src/lucky/subdomain.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
module Lucky::Subdomain
# Taken from https://github.com/rails/rails/blob/afc6abb674b51717dac39ea4d9e2252d7e40d060/actionpack/lib/action_dispatch/http/url.rb#L8
IP_HOST_REGEXP = /\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/

Habitat.create do
# tld_length is the number of Top Level Domain segments separated by periods
# the default is 1 because most domains end in ".com" or ".org"
# The tld_length should be changed to 2 when you have a ".co.uk" domain for example
# It can also be changed to 0 for local development so that you can use `tenant.localhost:3000`
setting tld_length : Int32 = 1
end

alias Matcher = String | Regex | Bool | Array(String | Regex) | Array(String) | Array(Regex)

# Sets up a subdomain requirement for an action
#
# ```
# register_subdomain # subdomain required but can be anything
# register_subdomain "admin" # subdomain required and must equal "admin"
# register_subdomain /(dev|qa|prod)/ # subdomain required and must match regex
# register_subdomain ["tenant1", "tenant2", /tenant\d/] # subdomain required and must match one of the items in the array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love all the different options. Thinking about how I currently use subdomains, I only need a subdomain on my routes when I'm in staging. In production, we don't use subdomains. Is that possible to do? I'm thinking when you do qa.luckyframework.org, that would only be on the QA environment, and not in production.. So we need to be able to say register_subdomain "qa" if LuckyEnv.qa? or whatever...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pushed an update to better clarify. The macro is now called require_subdomain.

In this situation you don't actually require a subdomain, you just do have a subdomain in certain scenarios. In those situations you wouldn't use require_subdomain but you can still check if a subdomain was provided by calling subdomain?. Calling subdomain will raise a compile time error though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. That gives us a nice escape hatch then too.

# ```
macro register_subdomain(matcher = true)
before _match_subdomain

private def subdomain : String
_fetch_subdomain.not_nil!
end

private def _match_subdomain
_match_subdomain({{ matcher }})
end
end

def subdomain : String
{% raise "No subdomain available without calling `register_subdomain` first." %}
matthewmcgarvey marked this conversation as resolved.
Show resolved Hide resolved
end

private def _fetch_subdomain : String?
host = request.hostname
return if host.nil? || IP_HOST_REGEXP.matches?(host)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to use Socket::IPAddress. This will handle IPV6 and IPV4 and it looks like it does not use a Regex so I think it might be faster.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try to look for something in the stdlib and didn't find anything. I didn't see this class. Unfortunately the docs for it say this:

IPAddress won't resolve domains, including localhost. If you must resolve an IP, or don't know whether a String contains an IP or a domain name, you should use Addrinfo.resolve instead.

And it looks like Addrinfo.resolve won't answer the question of whether or not the host uses an ip address.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point. You might looks at this to check if this is an IP. But it is not as nice as having using all stdlib tooling to fetch the sub domain.

https://crystal-lang.org/api/1.0.0/Socket.html#ip?(string:String)-class-method
https://play.crystal-lang.org/#/r/bicq

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @wontruefree. This one may be a little tricky since IPV6 would get passed through, but since Addrinfo.resolve works on normal hosts, we may get a false positive. This Socket::IPAddress looks like it may work https://crystal-lang.org/api/1.0.0/Socket/IPAddress.html

If you pass in "localhost", it'll raise an exception. The only thing I don't like is that the IP would actually be the edge case here which means we're raising an exception and rescuing 90% of the time which is probably a performance penalty 😝 ...

Maybe this is a "fine enough for now" scenario, and we just open a separate issue to figure out a better way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you follow the rails link I left on the constant, the regex is exactly how Rails handles subdomains. It doesn't extract a subdomain if it matches the regex. I guess I'm not seeing how this wouldn't work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like rails uses that second regex https://github.com/rails/rails/blob/afc6abb674b51717dac39ea4d9e2252d7e40d060/actionpack/lib/action_dispatch/http/url.rb#L9 which handles IPV6

/(^[^:]+:\/\/)?(\[[^\]]+\]|[^:]+)(?::(\d+$))?/ =~ "fe80::aede:48ff:fe00:1122"

It seems they're only using it when building the host url though. So I think we can just ignore it for now. I've never actually seen anyone access a site using IPV6. Maybe API calls for hackers trying to find vulnerabilities? I think this is a "fine enough for now" scenario though.


parts = host.split('.')
parts.pop(settings.tld_length + 1)

parts.empty? ? nil : parts.join(".")
end

private def _match_subdomain(matcher : Matcher)
expected = [matcher].flatten.compact
return continue if expected.empty?

actual = _fetch_subdomain
result = expected.any? do |expected_subdomain|
case expected_subdomain
when true
actual.present?
when Symbol
actual.to_s == expected_subdomain.to_s
else
expected_subdomain === actual
end
end

if result
continue
else
raise InvalidSubdomainError.new
matthewmcgarvey marked this conversation as resolved.
Show resolved Hide resolved
end
end
end