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

Use regular expressions to validate tailscale hostname #2714 #2719

Conversation

FroggyFlox
Copy link
Member

Fixes #2714
@Hooverdan96, @phillxnet: ready for review.

Following up on @Hooverdan96's suggestion to leverage regular expressions to better validate the hostname key in the Tailscale service config.

The current mechanism uses a hard-coded list of characters to exclude them from a custom tailscale hostname if present. This is rather ugly and does not scale to all the characters (such as unicode) that need to be excluded.

Switch to using regular expressions to exclude only characters from the hostname that do not belong to that regular expression.

All tests still pass...

buildvm155:/opt/rockstor/src/rockstor # poetry run django-admin test
Creating test database for alias 'default'...
Creating test database for alias 'smart_manager'...
System check identified no issues (0 silenced).
......................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 278 tests in 15.039s

OK

... and I was able to configure and turn ON the Tailscale service with a custom hostname being taken in effect (as seen on the Tailscale machines dashboard).

The current mechanism uses a hard-coded list of characters to exclude
them from a custom tailscale hostname if present. This is rather ugly
and does not scale to all the characters (such as unicode) that need to
be excluded.

Switch to using regular expressions to exclude only characters from the
hostname that do not belong to that regular expression.
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@FroggyFlox Thanks for this follow-up. Much appreciated.

RPM built OK with this included so all tests passed via %check scriptlet. And your addition to the test data confirms the added sensitivity and move to inclusion rather than list of chars to exclude.

Web-UI test irrelevant as we already have your prior equivalent character inclusion via the following form validator:

$.validator.addMethod('isAlphanumericWithHyphens', function(value, element) {
var regExp = new RegExp(/^[A-Za-z0-9-]+$/);
return this.optional(element) || regExp.test(value);
}, 'The hostname can only contain alphanumeric characters or hyphens.')

@FroggyFlox
Copy link
Member Author

Thanks a lot for the review and RPM build testing, @phillxnet.
Given your approval and passing tests, I'll go ahead and merge this.
@Hooverdan96, let me know if you see something needing validation that I missed and I'll create a dedicated PR. I did not think about the unicode characters so thank you very much for bringing it up in your previous review!

@FroggyFlox FroggyFlox merged commit 7405231 into rockstor:testing Oct 21, 2023
@FroggyFlox FroggyFlox deleted the 2714_Tailscale-consider-regex-filtering-for-hostname-validation branch October 21, 2023 15:14
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 this pull request may close these issues.

[Tailscale]: consider regex filtering for hostname validation/adjustments
2 participants