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

Refactor api token based login to UAA, fix refresh token logic #824

Merged
merged 3 commits into from
Oct 9, 2024
Merged

Refactor api token based login to UAA, fix refresh token logic #824

merged 3 commits into from
Oct 9, 2024

Conversation

vuil
Copy link
Contributor

@vuil vuil commented Oct 8, 2024 •
edited
Loading

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Updated unit tests

Manual testing

Prior to fix, artificially roll back expiration time then run
> ./bin/tanzu_before_fix context get-token tspmapitoken
will trigger interactive login....

[i] Opening the browser window to complete the login
Log in by visiting this link:

    https://xxxxhostauth/oauth/authorize?client_id=tp_cli_app&code_challenge=3rgxYwz2lRK9ZK9kwhdJW85MW5cyutFdN54AZ-drtk0&code_challenge_method=S256&redirect_uri=http%3A%2F%2F127.0.0.1%3A60475%2Fcallback&response_type=code&state=c97c3bca05ff7145bad12013ccdbba91

After fix, the commands performs token refresh successfully
> ./bin/tanzu context get-token tspmapitoken
{"kind":"ExecCredential","apiVersion":"client.authent....

Tested login and context create with TANZU_API_TOKEN

 TANZU_API_TOKEN=3bf6c287c61243fcb826280b08dad345-r ./bin/tanzu context create tspmapitoken --type tanzu --endpoint https://xxxhost

 TANZU_API_TOKEN=3bf6c287c61243fcb826280b08dad345-r ./bin/tanzu login --endpoint https://xxxhost

Release note

Fix issue leading to premature triggering of interactive login.

Additional information

Special notes for your reviewer

@vuil
Refresh token with custom TLS config
31d6637 
Token refresh needs to account for custom CA cert and cert validation
flag. Factors out the TLSConfig customization using the context.Context
to be used for that well.

Signed-off-by: Vui Lam <[email protected]>
@vuil vuil force-pushed the uaa-api-token-refactor branch from 63170d0 to 7e2f929 Compare October 8, 2024 19:38
marckhouzam
marckhouzam approved these changes Oct 8, 2024
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

LGTM
Just one question

pkg/auth/common/login_handler.go Outdated Show resolved Hide resolved
@vuil vuil marked this pull request as ready for review October 9, 2024 00:15
@vuil vuil requested a review from a team as a code owner October 9, 2024 00:15
@vuil
Simplify auth with api token against uaa
6baaf00 
Refactored login handler so that we can leverage it to perform the token
refresh using provided API token.

Also:
- ensures that the API token based login updates CLI Context with
the refresh token obtained from UAA.
- do not interactively login on expired refresh token for api-token type
  tokens.

Signed-off-by: Vui Lam <[email protected]>
@vuil vuil force-pushed the uaa-api-token-refactor branch from 90e1f38 to 8cb8003 Compare October 9, 2024 00:20
@vuil vuil force-pushed the uaa-api-token-refactor branch from 8cb8003 to ca10d43 Compare October 9, 2024 00:23
prkalle
prkalle reviewed Oct 9, 2024
View reviewed changes
go.mod Outdated Show resolved Hide resolved
prkalle
prkalle approved these changes Oct 9, 2024
View reviewed changes
Copy link
Contributor

@prkalle prkalle left a comment

Choose a reason for hiding this comment

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

Nice!
LGTM. Thanks!

@vuil vuil merged commit d33d4b7 into vmware-tanzu:main Oct 9, 2024
7 checks passed
@marckhouzam marckhouzam added this to the v1.5.1 milestone Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prkalle prkalle prkalle approved these changes

@marckhouzam marckhouzam marckhouzam approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.5.1
Development

Successfully merging this pull request may close these issues.
3 participants
@vuil @marckhouzam @prkalle