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

Fix default preauthkey expiration #237

Merged
merged 2 commits into from
Nov 26, 2021

Conversation

cure
Copy link
Contributor

@cure cure commented Nov 26, 2021

When creating a preauthkey, the default expiration was passed through as a nil value, instead of the default value (24h). This resulted in the preauthkey being created with expiration key '0001-01-01 00:00:00', which meant the key would not work, because it was already expired.

This commit applies the default expiration time (24h) when a preauthkey is created without a specific expiration. It also updates an integration test to make sure this bug does not reoccur.

@kradalby
Copy link
Collaborator

This one's on me for not commenting better! And also not removing the default text.

It was added intentionally, however that was only to preserve the current behaviour of the cli at the time, which was infinite keys if there was no expiration set. IIRC.

Anyways, I'm very happy for this to go in, and while we are at it, how about 1h or 30m default expiration? 24h seems a bit extreme as a default?

@kradalby
Copy link
Collaborator

Ah, and as you pointed out, the expiry check in the other end was probably incorrect. Thanks for catching this.

kradalby
kradalby previously approved these changes Nov 26, 2021
a nil value, instead of the default value (1h). This resulted in the
preauthkey being created with expiration key '0001-01-01 00:00:00',
which meant the key would not work, because it was already expired.

This commit applies the default expiration time (1h) when a preauthkey
is created without a specific expiration. It also updates an integration
test to make sure this bug does not reoccur.
@cure cure force-pushed the preauthkeys-fix-default-expiration branch from b4d89b0 to c7f3e06 Compare November 26, 2021 15:04
@cure
Copy link
Contributor Author

cure commented Nov 26, 2021

Sure! One hour default sounds good, I've rebased and pushed the corresponding change.

jccit added a commit to jccit/headscale that referenced this pull request Nov 26, 2021
@cure cure merged commit d944aa6 into juanfont:main Nov 26, 2021
@cure cure deleted the preauthkeys-fix-default-expiration branch November 26, 2021 16:09
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.

2 participants