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

API support for fetching session token #802

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

API support for fetching session token #802

wants to merge 3 commits into from

Conversation

pmathew92
Copy link
Contributor

@pmathew92 pmathew92 commented Feb 20, 2025

Changes

This PR adds support to fetch a session token, in exchange for a refresh token, to be used for SSO

Testing

  • This change adds unit test coverage

  • This change has been tested on the latest version of the platform/language or why not

Checklist

@pmathew92 pmathew92 requested a review from a team as a code owner February 20, 2025 08:26
@@ -29,7 +30,9 @@ public abstract class BaseCredentialsManager internal constructor(

@Throws(CredentialsManagerException::class)
public abstract fun saveCredentials(credentials: Credentials)
public abstract fun saveSsoCredentials(ssoCredentials: SSOCredentials)
Copy link
Contributor

@Widcket Widcket Feb 20, 2025

Choose a reason for hiding this comment

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

I'm not sure this method is needed. We're already providing a method for getting fresh SSOCredentials, which automaticaly saves the refresh token.

Copy link
Contributor

Choose a reason for hiding this comment

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

The same functionality can also be achieved by simply retrieving the credentials, replacing the refresh token, and saving them back.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the case we have it for internal code reuse, then we should not have it be public.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this method for the scenario where we get a new refresh token when calling the API directly from the AuthenticationAPI class. The user would have to call this to replace the existing one else the following getSsoCredentials call will fail.

Mockito.`when`(storage.retrieveString("com.auth0.refresh_token"))
.thenReturn("refresh-token")
manager.saveSsoCredentials(ssoCredentials)
verify(storage).store("com.auth0.refresh_token", "refresh_token")
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the difference between this test and the previous one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous one checks for scenario when the existing refresh_token is the same as the one received in the session token credentials and doesn't save it. This one checks for when we have separate refresh_token than the existing one and save it

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