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

Add renew authentication endpoint #51

Merged
merged 4 commits into from
Apr 27, 2017
Merged

Add renew authentication endpoint #51

merged 4 commits into from
Apr 27, 2017

Conversation

lbalmaceda
Copy link
Contributor

@lbalmaceda lbalmaceda commented Apr 3, 2017

Renew the authentication using a refresh_token.
Closes #48

@lbalmaceda lbalmaceda added this to the v1-Next milestone Apr 3, 2017
@lbalmaceda lbalmaceda requested a review from hzalaz April 3, 2017 17:07
@lbalmaceda lbalmaceda requested a review from nikolaseu April 26, 2017 18:55
README.md Outdated
@@ -209,6 +209,24 @@ try {
}
```

### Renew Authentication

Creates a new request to renew the authentication and get fresh new credentials using a valid 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.

can I create something "old" ? xD just kidding, but the term "(re)new" is repeated several times, maybe we can remove the first

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to specify like in ### Request Token for Audience - /oauth/token that this is the "new" /oauth/token refresh token?

@@ -35,6 +36,7 @@
private final String clientId;
private final String clientSecret;
private final String baseUrl;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need this 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.

Not really, was used by the AuthorizeUrlBuilder but now that I see we can replace that parameter with the HttpUrl. Let me fix it. 👍

@@ -54,6 +56,7 @@ public AuthAPI(String domain, String clientId, String clientSecret) {
if (baseUrl == null) {
throw new IllegalArgumentException("The domain had an invalid format and couldn't be parsed as an URL.");
}
this.httpUrl = HttpUrl.parse(baseUrl);
Copy link
Contributor

Choose a reason for hiding this comment

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

createBaseUrl already uses HttpUrl.parse, so maybe make that return that httpUrl directly?
then baseUrl (if really required) can be obtained from baseUrl = httpUrl.toString()

@nikolaseu
Copy link
Contributor

Looks good

@lbalmaceda lbalmaceda merged commit 9f6af57 into master Apr 27, 2017
@lbalmaceda lbalmaceda deleted the add-refresh-token branch May 5, 2017 17:01
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.1.0 May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants