[KEYCLOAK-14344] Cannot revoke offline access for an app if the app doesn't require consent

This commit is contained in:
Douglas Palmer 2020-06-24 14:05:49 -07:00 committed by Stan Silvert
parent c163fce46e
commit 5e44bb781b
5 changed files with 54 additions and 21 deletions

View file

@ -39,6 +39,7 @@ import org.keycloak.representations.account.ConsentScopeRepresentation;
import org.keycloak.representations.account.UserRepresentation;
import org.keycloak.services.ErrorResponse;
import org.keycloak.services.managers.Auth;
import org.keycloak.services.managers.UserSessionManager;
import org.keycloak.services.messages.Messages;
import org.keycloak.services.resources.Cors;
import org.keycloak.services.resources.account.resources.ResourcesService;
@ -372,6 +373,7 @@ public class AccountRestService {
}
session.users().revokeConsentForClient(realm, user.getId(), client.getId());
new UserSessionManager(session).revokeOfflineToken(user, client);
event.success();
return Cors.add(request, Response.noContent()).build();

View file

@ -1147,4 +1147,35 @@ public class AccountRestServiceTest extends AbstractRestServiceTest {
.asResponse();
assertEquals(403, response.getStatus());
}
//KEYCLOAK-14344
@Test
public void revokeOfflineAccess() throws Exception {
oauth.scope(OAuth2Constants.OFFLINE_ACCESS);
oauth.clientId("offline-client");
OAuthClient.AccessTokenResponse offlineTokenResponse = oauth.doGrantAccessTokenRequest("secret1", "view-applications-access", "password");
Assert.assertNull(offlineTokenResponse.getErrorDescription());
TokenUtil token = new TokenUtil("view-applications-access", "password");
SimpleHttp.Response response = SimpleHttp
.doDelete(getAccountUrl("applications/offline-client/consent"), httpClient)
.header("Accept", "application/json")
.auth(token.getToken())
.asResponse();
assertEquals(204, response.getStatus());
List<ClientRepresentation> applications = SimpleHttp
.doGet(getAccountUrl("applications"), httpClient)
.header("Accept", "application/json")
.auth(token.getToken())
.asJson(new TypeReference<List<ClientRepresentation>>() {
});
assertFalse(applications.isEmpty());
Map<String, ClientRepresentation> apps = applications.stream().collect(Collectors.toMap(x -> x.getClientId(), x -> x));
Assert.assertThat(apps.keySet(), containsInAnyOrder("offline-client", "always-display-client"));
assertClientRep(apps.get("offline-client"), "Offline Client", null, false, true, false, null, offlineClientAppUri);
}
}

View file

@ -88,8 +88,8 @@ public class ApplicationsTest extends BaseAccountPageTest {
assertFalse(applications.isEmpty());
Map<String, ApplicationsPage.ClientRepresentation> apps = applications.stream().collect(Collectors.toMap(x -> x.getClientId(), x -> x));
assertThat(apps.keySet(), containsInAnyOrder("always-display-client", "account-console"));
assertClientRep(apps.get("account-console"), "Account Console", false, true, "/realms/test/account/", false);
assertClientRep(apps.get("always-display-client"), "Always Display Client", false, false, "https://localhost:8543/auth/realms/master/app/always-display-client", false);
assertClientRep(apps.get("account-console"), "Account Console", false, true, getAuthServerRoot() + "realms/test/account/", false);
assertClientRep(apps.get("always-display-client"), "Always Display Client", false, false, getAuthServerRoot() + "realms/master/app/always-display-client", false);
}
@Test
@ -99,24 +99,24 @@ public class ApplicationsTest extends BaseAccountPageTest {
assertFalse(applications.isEmpty());
Map<String, ApplicationsPage.ClientRepresentation> apps = applications.stream().collect(Collectors.toMap(x -> x.getClientId(), x -> x));
assertThat(apps.keySet(), containsInAnyOrder("always-display-client", "account-console"));
assertClientRep(apps.get("account-console"), "Account Console", false, true, "/realms/test/account/", true);
assertClientRep(apps.get("always-display-client"), "Always Display Client", false, false, "https://localhost:8543/auth/realms/master/app/always-display-client", false);
assertClientRep(apps.get("account-console"), "Account Console", false, true, getAuthServerRoot() + "realms/test/account/", true);
assertClientRep(apps.get("always-display-client"), "Always Display Client", false, false, getAuthServerRoot() + "realms/master/app/always-display-client", false);
applicationsPage.toggleApplicationDetails("account-console");
applications = applicationsPage.getApplications();
assertFalse(applications.isEmpty());
apps = applications.stream().collect(Collectors.toMap(x -> x.getClientId(), x -> x));
assertThat(apps.keySet(), containsInAnyOrder("always-display-client", "account-console"));
assertClientRep(apps.get("account-console"), "Account Console", false, true, "/realms/test/account/", false);
assertClientRep(apps.get("always-display-client"), "Always Display Client", false, false, "https://localhost:8543/auth/realms/master/app/always-display-client", false);
assertClientRep(apps.get("account-console"), "Account Console", false, true, getAuthServerRoot() + "realms/test/account/", false);
assertClientRep(apps.get("always-display-client"), "Always Display Client", false, false, getAuthServerRoot() + "realms/master/app/always-display-client", false);
}
private void assertClientRep(ApplicationsPage.ClientRepresentation clientRep, String name, boolean userConsentRequired, boolean inUse, String baseUrl, boolean applicationDetailsVisible) {
private void assertClientRep(ApplicationsPage.ClientRepresentation clientRep, String name, boolean userConsentRequired, boolean inUse, String effectiveUrl, boolean applicationDetailsVisible) {
assertNotNull(clientRep);
assertEquals(name, clientRep.getClientName());
assertEquals(userConsentRequired, clientRep.isUserConsentRequired());
assertEquals(inUse, clientRep.isInUse());
assertEquals(baseUrl, clientRep.getBaseUrl());
assertEquals(effectiveUrl, clientRep.getEffectiveUrl());
assertEquals(applicationDetailsVisible, clientRep.isApplicationDetailsVisible());
}

View file

@ -58,9 +58,9 @@ public class ApplicationsPage extends AbstractLoggedInPage {
String clientName = UIUtils.getTextFromElement(app.findElement(By.xpath("//div[@id='application-name-" + clientId + "']")));
boolean userConsentRequired = !UIUtils.getTextFromElement(app.findElement(By.xpath("//div[@id='application-internal-" + clientId + "']"))).equals("Internal");
boolean inUse = UIUtils.getTextFromElement(app.findElement(By.xpath("//div[@id='application-status-" + clientId + "']"))).equals("In use");
String baseURL = UIUtils.getTextFromElement(app.findElement(By.xpath("//div[@id='application-baseurl-" + clientId + "']")));
String effectiveURL = UIUtils.getTextFromElement(app.findElement(By.xpath("//div[@id='application-effectiveurl-" + clientId + "']")));
boolean applicationDetailsVisible = app.findElement(By.xpath("//section[@id='application-expandable-" + clientId + "']")).isDisplayed();
return new ClientRepresentation(clientId, clientName, userConsentRequired, inUse, baseURL, applicationDetailsVisible);
return new ClientRepresentation(clientId, clientName, userConsentRequired, inUse, effectiveURL, applicationDetailsVisible);
}
public class ClientRepresentation {
@ -68,15 +68,15 @@ public class ApplicationsPage extends AbstractLoggedInPage {
private final String clientName;
private final boolean userConsentRequired;
private final boolean inUse;
private final String baseUrl;
private final String effectiveUrl;
private final boolean applicationDetailsVisible;
public ClientRepresentation(String clientId, String clientName, boolean userConsentRequired, boolean inUse, String baseUrl, boolean applicationDetailsVisible) {
public ClientRepresentation(String clientId, String clientName, boolean userConsentRequired, boolean inUse, String effectiveUrl, boolean applicationDetailsVisible) {
this.clientId = clientId;
this.clientName = clientName;
this.userConsentRequired = userConsentRequired;
this.inUse = inUse;
this.baseUrl = baseUrl;
this.effectiveUrl = effectiveUrl;
this.applicationDetailsVisible = applicationDetailsVisible;
}
@ -96,8 +96,8 @@ public class ApplicationsPage extends AbstractLoggedInPage {
return inUse;
}
public String getBaseUrl() {
return baseUrl;
public String getEffectiveUrl() {
return effectiveUrl;
}
public boolean isApplicationDetailsVisible() {

View file

@ -188,9 +188,9 @@ export class ApplicationsPage extends React.Component<ApplicationsPageProps, App
}
</div>
</Grid>
{(application.consent || application.offlineAccess) &&
<Grid gutter='sm'>
<hr />
{application.consent &&
<GridItem>
<React.Fragment>
<ContinueCancelModal
@ -203,9 +203,9 @@ export class ApplicationsPage extends React.Component<ApplicationsPageProps, App
/>
</React.Fragment>
</GridItem>
}
<GridItem><InfoAltIcon /> {Msg.localize('infoMessage')}</GridItem>
</Grid>
}
</DataListContent>
</DataListItem>
)