gurucomputing / headscale-ui

A web frontend for the headscale Tailscale-compatible coordination server
BSD 3-Clause "New" or "Revised" License
1.79k stars 128 forks source link

Add a new device registration route #121

Closed adrum closed 1 year ago

adrum commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

routerino commented 1 year ago

Looks good to me.