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.

Sign up for GitHub

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

Jump to bottom

Set OpenID Connect Expiry #1191

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Set OpenID Connect Expiry #1191

merged 1 commit into from
Jan 31, 2023

Conversation

kradalby
Copy link
Collaborator

@kradalby kradalby commented Jan 31, 2023 •
edited
Loading

This commit adds a default OpenID Connect expiry to 180d to align with Tailscale SaaS (previously infinite or based on token expiry).

In addition, it adds an option use the expiry time from the Token sent by the OpenID provider. This will typically cause really short expiry and you should only turn on this option if you know what you are desiring.

This fixes #1176.

@kradalby kradalby marked this pull request as ready for review January 31, 2023 11:50
@kradalby kradalby requested a review from juanfont as a code owner January 31, 2023 11:50
@kradalby kradalby mentioned this pull request Jan 31, 2023
Closed
@Hacksawfred3232
Copy link

Hacksawfred3232 commented Jan 31, 2023 •
edited
Loading

Tested the commit out, it seemed to work okay bar one glitch.
It seemed that viper or whatever handles the time management bugged out when I tried setting the expiry to 7d for 7 days. Providing 7 days in hours (168h) seemed to have to did the trick. This issue may seem to be reflected in the tests as well?

@kradalby
Copy link
Collaborator Author

@Hacksawfred3232 your right! I usually use the prometheus parser which supports days, I will fix that.

@joshuataylor
Copy link
Contributor

joshuataylor commented Jan 31, 2023 •
edited
Loading

I couldn't figure out how to see this via the CLI in Linux, so I tested this with a Mac client, and by default it was pretty short (Google), using the main branch.

With this PR I tested a few different times using the config option, and each worked! 🎉

image

@kradalby @evenh
Set OpenID Connect Expiry
07940ec 
This commit adds a default OpenID Connect expiry to 180d to align with
Tailscale SaaS (previously infinite or based on token expiry).

In addition, it adds an option use the expiry time from the Token sent
by the OpenID provider. This will typically cause really short expiry
and you should only turn on this option if you know what you are
desiring.

This fixes juanfont#1176.

Co-authored-by: Even Holthe <even.holthe@bekk.no>
Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
@kradalby
Copy link
Collaborator Author

Tested the commit out, it seemed to work okay bar one glitch.

This should be fixed now

juanfont
juanfont approved these changes Jan 31, 2023
View reviewed changes
Copy link
Owner

@juanfont juanfont left a comment

Choose a reason for hiding this comment

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

Cool!

@kradalby kradalby merged commit da48cf6 into juanfont:main Jan 31, 2023
@kradalby kradalby deleted the node-expiry branch January 31, 2023 17:55
@reynico
Copy link
Contributor

reynico commented Feb 2, 2023

Hi Kristoffer, do you plan to do a release including this change anytime soon?

Thanks!

@kradalby
Copy link
Collaborator Author

kradalby commented Feb 3, 2023

@reynico we aim to do it today, just depends on getting in #1195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juanfont juanfont juanfont approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OIDC configure expiry time
5 participants
@kradalby @Hacksawfred3232 @joshuataylor @reynico @juanfont