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 refresh token support for logins #258

Merged
merged 21 commits into from
May 8, 2024
Merged

Add refresh token support for logins #258

merged 21 commits into from
May 8, 2024

Conversation

tminusplus
Copy link
Contributor

@tminusplus tminusplus commented Sep 26, 2023

What was changed

This adds refresh tokens to tcld by switching to the golang.org/x/oauth2 which supports device codes and refresh tokens.

Other changes in this PR:

  • Noticed that other tests were reading the actual tcld config folder. And would panic if it does not exist. So added NewTestApp which sets the config dir to a temporary directory + other convenience settings.
  • Removed LoginService to reduce coupling in code, so that connection.go can be tested without invoking a login.
  • Removed mocks around LoginService, to ensure the tcld code is exercised in tests.
  • Added making the config directory, if it doesn't exist, in feature.go to avoid nil panics in testing.
  • Removed the Before code in the NewLoginCommand, as nothing was done with the loaded login config.
  • Removed the use of os.RemoveAll, to ensure a bug does not remove directories on a user's computer when the intent was to remove a single file.

Why?

Allows us to shorten token access expiration times.

Checklist

  • Tested logging in and making a request.
  • Tested logging in on an older tcld, updating, and then making a request.
  • Tested logging in on an older tcld, updating, re-logging in, and verified the config is in the new format.
  • Tested waiting for the access token to expire, and then making a request to get a new refresh token.
  • Added unit tests around refresh token support and the use of the golang.org/x/oauth2 library.
  • Tested login flows with refresh tokens disabled server-side.

@tminusplus tminusplus changed the title [draft] Add refresh token support for logins Add refresh token support for logins Oct 26, 2023
@tminusplus tminusplus marked this pull request as ready for review October 26, 2023 23:36
app/login.go Outdated Show resolved Hide resolved
app/connection_test.go Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
Copy link
Contributor

@shakeelrao shakeelrao left a comment

Choose a reason for hiding this comment

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

🚀

@tminusplus tminusplus merged commit d558b08 into main May 8, 2024
4 checks passed
@tminusplus tminusplus deleted the tszucs/refresh branch May 8, 2024 23:25
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.

4 participants