OIDC: Expire machines/nodes after token expiry

[evenh]

• read the CONTRIBUTING guidelines
• raised a GitHub issue or discussed it on the projects chat beforehand
• added unit tests
• added integration tests
• updated documentation if needed
• updated CHANGELOG.md

This PR seems to work (at least headscale mockoidc and Azure AD) for my use case.

Fixes #935.

[evenh]

Please test @nikking @FStelzer and see if this PR solves your use case as well.

[kradalby]

This looks reasonable, lets wait for feedback from other users so we have some testers.

Ideally we would have a integration test case as well, it shouldnt be too problematic now that you can add a config to the Scenario:

1. Set a really low update interval
2. Set a really short expiry
3. Add node, verify it works
4. Wait for expiry
5. Verify it doesnt work

[FStelzer]

Thanks for working on this.
I think the expiry would need to be set per machine. Totally fine to have a global default, but as soon as you have normal users as well as subnet routers you might want a different expiry (or none at all).
Also I'm having a hard time wrapping my head around to what is the purpose/effect of this condition in the diff "machine.Expiry.After(*machine.LastSuccessfulUpdate)"?
Not sure if the update can be written after the expire happened but I think you might be able get a race here if it is and circumvent the expiry, no?

Also we might need to differentiate between expiring inactive/offline clients and a forceful expiry after a certain time.
For example in our case i have internal (employed) users, a few subnet routers and a couple of freelancers.
The routers should never expire, the internal employees should reauthenticate if they want offline for a bit (or at least every week for example) and the freelances i would like to auth every day. At the moment I can easily handle this via the cli.

[evenh]

@reynico What would that property be used for? As far as I understand every OIDC JWT token has an expiry set.

---

@FStelzer: This PR addresses only users/machines which explicitly logs in via OIDC. I think configuring different token lifetimes for employees/freelances should be done in your auth solution (Azure AD, Okta, etc…).

As for the diff: I've observed that if only checking for isExpired(), the function will return true every time after a given machine has expired, e.g. expiring the machine every time the function runs.
By comparing a node's expiry timestamp and the last successful update/sync with Headscale, this statement will only be true until the next state sync. Thoughts and improvements around this logic is much appreciated 😅

[FStelzer]

@FStelzer: This PR addresses only users/machines which explicitly logs in via OIDC. I think configuring different token lifetimes for employees/freelances should be done in your auth solution (Azure AD, Okta, etc…).

Ok, that sounds perfect. I think the other comment regarding the config let me think this would be something static/configured. Configuring this via OIDC is perfect. It needs some documentation though. There is no seperate OIDC docs page afaik. So maybe add a comment mentioning this field and its use in the config-example.yaml? Or start an OIDC docs page :)

As for the diff: I've observed that if only checking for isExpired(), the function will return true every time after a given machine has expired, e.g. expiring the machine every time the function runs. By comparing a node's expiry timestamp and the last successful update/sync with Headscale, this statement will only be true until the next state sync. Thoughts and improvements around this logic is much appreciated sweat_smile

Thanks for the clarification. That makes sense. Probably not much to improve on this logic. Maybe put the condition in it's own if (...) {continue} with a comment just for readability.

[juanfont]

Or start an OIDC docs page :)

I fully agree with this. We need a dedicated doc for OIDC. It is not rocket science, but it is not trivial to set up only based on the config example...