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

Crypto exception raised on save after refresh could be retried ? #661

Closed
5 tasks done
bennycao opened this issue May 24, 2023 · 4 comments
Closed
5 tasks done

Crypto exception raised on save after refresh could be retried ? #661

bennycao opened this issue May 24, 2023 · 4 comments
Labels
feature request A feature has been asked for or suggested by the community

Comments

@bennycao
Copy link
Contributor

Checklist

  • I have looked into the Readme, Examples, and FAQ and have not found a suitable solution or answer.
  • I have looked into the API documentation and have not found a suitable solution or answer.
  • I have searched the issues and have not found a suitable solution or answer.
  • I have searched the Auth0 Community forums and have not found a suitable solution or answer.
  • I agree to the terms within the Auth0 Code of Conduct.

Describe the problem you'd like to have solved

When saving credentials with SecureCredentialsManager a CryptoException could be thrown.
This could happen in the scenario of saving credentials after token refresh here https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt#L565.

Would it be possible to retry and save the credentials as described here https://github.com/auth0/Auth0.Android/blob/main/auth0/src/main/java/com/auth0/android/authentication/storage/SecureCredentialsManager.kt#L190 ? For this given scenario ? This would prevent user being signed out, as there is no way to resave the credentials on the client side.

Describe the ideal solution

Retry to save credentials in a token refresh scenario where CryptoException is thrown

Alternatives and current workarounds

No response

Additional context

No response

@bennycao bennycao added the feature request A feature has been asked for or suggested by the community label May 24, 2023
@poovamraj
Copy link
Contributor

@bennycao I guess this isn't an ideal solution we can adopt. It is best to leave it to the users to decide how to handle such exceptions. Is there any specific scenario that is stopping you from achieving this?

@bennycao
Copy link
Contributor Author

@bennycao I guess this isn't an ideal solution we can adopt. It is best to leave it to the users to decide how to handle such exceptions. Is there any specific scenario that is stopping you from achieving this?

@poovamraj this is in the scenario where it is a token refresh, which means we the client do not control or know about the refreshed credentials

@poovamraj
Copy link
Contributor

How about we rather return the fetched credentials as part of the exception and allow the developer to handle it from there? The issue here comes out of the fresh credential not being exposed.

Although we have to wonder if passing the credentials with sensitive information through credentials is ideal as there will be loggers catching and printing this 🤔

@poovamraj
Copy link
Contributor

Hi @bennycao, we have the feature you requested in this PR - #666

We will check this internally to ensure masking mechanism for the Credentials is enough. We can continue the discussion in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A feature has been asked for or suggested by the community
Projects
None yet
Development

No branches or pull requests

2 participants