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

Allow to customize the redirect URI / return to URL #279

Merged
merged 6 commits into from
Jan 29, 2020

Conversation

lbalmaceda
Copy link
Contributor

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

@lbalmaceda lbalmaceda added CH: Added small Small review labels Jan 28, 2020
@lbalmaceda lbalmaceda added this to the v1-Next milestone Jan 28, 2020
@lbalmaceda lbalmaceda requested a review from a team January 28, 2020 21:25
@@ -43,7 +43,7 @@
*
* @return the callback Uri.
*/
public static String getCallbackUri(@NonNull String scheme, @NonNull String packageName, @NonNull String domain) {
Copy link
Contributor Author

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 👍

Copy link
Contributor

@Widcket Widcket left a 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.

@@ -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.
Copy link
Contributor

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'.
Copy link
Contributor

@Widcket Widcket Jan 29, 2020

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.

Widcket
Widcket previously approved these changes Jan 29, 2020
Copy link
Contributor

@Widcket Widcket left a 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.

Copy link
Contributor

@Widcket Widcket left a 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 <[email protected]>
Co-Authored-By: Rita Zerrizuela <[email protected]>
Copy link
Contributor

@Widcket Widcket left a comment

Choose a reason for hiding this comment

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

LGTM

@lbalmaceda lbalmaceda merged commit af5decd into master Jan 29, 2020
@lbalmaceda lbalmaceda changed the title Allow to customize the redirect uri / return to url Allow to customize the redirect URI / return to URL Jan 29, 2020
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.21.0 Jan 29, 2020
@lbalmaceda lbalmaceda deleted the cust-redirect-uri branch January 20, 2021 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CH: Added small Small review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants