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 method to revoke a refresh_token #86

Merged
merged 2 commits into from
Apr 27, 2017
Merged

Conversation

lbalmaceda
Copy link
Contributor

No description provided.

@lbalmaceda lbalmaceda requested a review from hzalaz April 3, 2017 15:19
@lbalmaceda lbalmaceda added this to the v1-Next milestone Apr 3, 2017
@lbalmaceda lbalmaceda force-pushed the revoke-refresh-token branch from 59970b8 to 6c34390 Compare April 3, 2017 15:38
@lbalmaceda lbalmaceda requested a review from nikolaseu April 26, 2017 13:58
@lbalmaceda lbalmaceda force-pushed the revoke-refresh-token branch from 6c34390 to a3e1d0c Compare April 26, 2017 15:16
Copy link
Contributor

@nikolaseu nikolaseu left a comment

Choose a reason for hiding this comment

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

If it's not that much trouble I would like to make the change in tests, the one with willReturnSuccessfulEmptyBody()

* @return a request to configure and start
*/
@SuppressWarnings("WeakerAccess")
public ParameterizableRequest<Void, AuthenticationException> revokeToken(@NonNull String refreshToken) {
Copy link
Contributor

Choose a reason for hiding this comment

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

return a plain request, nobody should change anything anyway

@Test
public void shouldRevokeTokenSync() throws Exception {
Auth0 auth0 = new Auth0(CLIENT_ID, mockAPI.getDomain(), mockAPI.getDomain());
auth0.setOIDCConformant(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

why this test sets this flag and the async one not?

Copy link
Contributor Author

@lbalmaceda lbalmaceda Apr 27, 2017

Choose a reason for hiding this comment

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

I think I did this to prove that the flag wasn't considered here. Anyway, the method only works for OIDC clients so I'm considering throwing if the method is called with this flag disabled. What do you think @nikolaseu ? and which exception should I throw, IllegalStateException?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Disregard my comment.. The oidc flag won't make a change on this endpoint since it should be callable anyway..

@@ -106,6 +106,12 @@ public AuthenticationAPI willReturnSuccessfulSignUp() {
return this;
}

public AuthenticationAPI willReturnSuccessfulEmptyBody() {
String json = "{}";
server.enqueue(responseWithJSON(json, 200));
Copy link
Contributor

Choose a reason for hiding this comment

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

from the docs:

If the request is valid, the refresh token is revoked and the response is HTTP 200, with an empty response body

empty response body is not the same as an empty object in the body.
just to be sure that when the test passes there are no issues when deserializing the response, let's make this return an actual empty body.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

@lbalmaceda lbalmaceda merged commit 6744d01 into master Apr 27, 2017
@lbalmaceda lbalmaceda deleted the revoke-refresh-token branch April 27, 2017 20:37
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.8.0 Apr 27, 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