-
Notifications
You must be signed in to change notification settings - Fork 141
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
Allow to customize the redirect URI / return to URL #279
Conversation
@@ -43,7 +43,7 @@ | |||
* | |||
* @return the callback Uri. | |||
*/ | |||
public static String getCallbackUri(@NonNull String scheme, @NonNull String packageName, @NonNull String domain) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class is package private already 👍
auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.java
Outdated
Show resolved
Hide resolved
auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.java
Outdated
Show resolved
Hide resolved
auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.java
Outdated
Show resolved
Hide resolved
auth0/src/test/java/com/auth0/android/provider/WebAuthProviderTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review the naming.
auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.java
Outdated
Show resolved
Hide resolved
auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.java
Outdated
Show resolved
Hide resolved
@@ -255,6 +272,19 @@ public Builder withScheme(@NonNull String scheme) { | |||
return this; | |||
} | |||
|
|||
/** | |||
* Specify a custom Redirect Uri to use to invoke the app on redirection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. There are 2 instances in this doc comment.
@@ -241,9 +258,9 @@ public Builder withAudience(@NonNull String audience) { | |||
} | |||
|
|||
/** | |||
* Specify a custom Scheme to use on the Callback Uri. Default scheme is 'https'. | |||
* Specify a custom Scheme to use on the Redirect Uri. Default scheme is 'https'. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use URI in the doc comments, for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be a good idea to follow the same criteria in all the doc comments. Not a blocker.
auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were a couple of instances left unchanged.
Co-Authored-By: Rita Zerrizuela <zeta@widcket.com>
Co-Authored-By: Rita Zerrizuela <zeta@widcket.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
Adds methods to the
WebAuthProvider
builders in order to customize the URL used when the backend succeeds authenticating/logging out and needs to redirect back to the app.References
A similar PR existed previously #268.
Testing
This change adds unit test coverage
This change adds integration test coverage
This change has been tested on the latest version of the platform/language or why not
Checklist
I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors