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 a new device registration route #121

Merged
merged 10 commits into from
Nov 17, 2023

Conversation

adrum
Copy link
Contributor

@adrum adrum commented Nov 13, 2023

This PR adds a new route to the UI that is focused only on registering a new device. It should be a route that would only be accessible when enrolling a new device. Below is a NGINX example of how to handle headscale's register endpoint and redirect it to this new endpoint:

    location /register/nodekey {
        rewrite ^/register/(.*)$ /web/register.html?nodekey=$1 redirect;
    }

Now, instead of seeing headscale's default endpoint, it would be greeted with our familiar CreateDevice component with the device key prepopulated. The form only requires a user assignment and a submission. After successful submission, it redirects to the devices.html route.

It would be even better if we could specify this pattern in headscale itself, but I think we could accomplish the same thing at the reverse proxy layer.

@routerino
Copy link
Contributor

routerino commented Nov 13, 2023

I don't really get the purpose of this pull request. It's relying on information that requires an API key so from a security perspective there doesn't appear to be any benefit. It's also not adding any functionality that doesn't already exist on the devices page. I'm not a fan of adding complexity to the UI without a measurable benefit.

Couldn't we just add the ?nodekey population to the devices endpoint directly?

@adrum
Copy link
Contributor Author

adrum commented Nov 13, 2023

This PR accomplishes a way to pre-populate the device registration form with the nodekey from a URL.

One pain point I experienced when enrolling AppleTVs was it displayed a QR code with a URL to the basic white screen with text containing the cli command to finish node registration. It was quite cumbersome on mobile to do this otherwise.

I would've preferred it just take to me the UI where I can approve the device.

It would be better if there was a way for headscale to be configured to send users to a different url/path, such as one provided by one of the UI projects like this. That would avoid the redirect complexity this adds.

If it makes more sense to get that feature integrated in headscale first, we can close this for now and revisit this later.

@adrum
Copy link
Contributor Author

adrum commented Nov 13, 2023

I think adding the query string parameter to devices.html is much simpler. I'll make that change and see what you think.

(I reread your original message and had a different interpretation the second time around 😅)

@adrum
Copy link
Contributor Author

adrum commented Nov 14, 2023

This has been simplified. What do you think?

Updated NGINX, just in case others come across this:

    location /register/nodekey {
        rewrite ^/register/(.*)$ /web/devices.html?nodekey=$1 redirect;
    }

@routerino
Copy link
Contributor

routerino commented Nov 16, 2023

Gave it a test and had a look, looks good to me. Expand out !!newDeviceKey as the !! does not make sense at a glance.

create a /documentation/route_queries.md that explains what a uri query is, a heading for devices and explains the parameter and I'll merge it.

@adrum
Copy link
Contributor Author

adrum commented Nov 17, 2023

@routerino I complete the two requests. Let me know what you think of the documentation.

@routerino
Copy link
Contributor

Looks good to me.

@routerino routerino merged commit 1f73a7b into gurucomputing:master Nov 17, 2023
@adrum adrum deleted the feature/register-device branch November 18, 2023 20:41
routerino pushed a commit that referenced this pull request Feb 24, 2024
* added a new device registration form

* removed unused variables

* remove old register html

* bind new device key from url

* remove redirect

* clear nodekey query string param after successuful addition

* expand bool flipping

* added documention on nodekey

* remove null check

* tweak wording
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.

None yet

2 participants