KEYCLOAK-3316 Remove the IDToken if scope=openid is not used

services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java

@@ -250,11 +250,16 @@ public class TokenManager {
         validation.clientSession.setTimestamp(currentTime);
         validation.userSession.setLastSessionRefresh(currentTime);
 
-        AccessTokenResponse res = responseBuilder(realm, authorizedClient, event, session, validation.userSession, validation.clientSession)
+        AccessTokenResponseBuilder responseBuilder = responseBuilder(realm, authorizedClient, event, session, validation.userSession, validation.clientSession)
                 .accessToken(validation.newToken)
-                .generateIDToken()
+                .generateRefreshToken();
-                .generateRefreshToken()
+
-                .build();
+        String scopeParam = validation.clientSession.getNote(OAuth2Constants.SCOPE);
+        if (TokenUtil.isOIDCRequest(scopeParam)) {
+            responseBuilder.generateIDToken();
+        }
+
+        AccessTokenResponse res = responseBuilder.build();
 
         return new RefreshResult(res, TokenUtil.TOKEN_TYPE_OFFLINE.equals(refreshToken.getType()));
     }

services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java

@@ -53,6 +53,7 @@ import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.managers.RealmManager;
 import org.keycloak.services.resources.Cors;
 import org.keycloak.sessions.AuthenticationSessionModel;
+import org.keycloak.util.TokenUtil;
 
 import javax.ws.rs.OPTIONS;
 import javax.ws.rs.POST;
@@ -340,10 +341,16 @@ public class TokenEndpoint {
 
         AccessToken token = tokenManager.createClientAccessToken(session, parseResult.getCode().getRequestedRoles(), realm, client, user, userSession, clientSession);
 
-        AccessTokenResponse res = tokenManager.responseBuilder(realm, client, event, session, userSession, clientSession)
+        TokenManager.AccessTokenResponseBuilder responseBuilder = tokenManager.responseBuilder(realm, client, event, session, userSession, clientSession)
                 .accessToken(token)
-                .generateIDToken()
+                .generateRefreshToken();
-                .generateRefreshToken().build();
+
+        String scopeParam = clientSession.getNote(OAuth2Constants.SCOPE);
+        if (TokenUtil.isOIDCRequest(scopeParam)) {
+            responseBuilder.generateIDToken();
+        }
+
+        AccessTokenResponse res = responseBuilder.build();
 
         event.success();
 
@@ -450,11 +457,16 @@ public class TokenEndpoint {
         UserSessionModel userSession = processor.getUserSession();
         updateUserSessionFromClientAuth(userSession);
 
-        AccessTokenResponse res = tokenManager.responseBuilder(realm, client, event, session, userSession, clientSession)
+        TokenManager.AccessTokenResponseBuilder responseBuilder = tokenManager.responseBuilder(realm, client, event, session, userSession, clientSession)
                 .generateAccessToken()
-                .generateRefreshToken()
+                .generateRefreshToken();
-                .generateIDToken()
+
-                .build();
+        String scopeParam = clientSession.getNote(OAuth2Constants.SCOPE);
+        if (TokenUtil.isOIDCRequest(scopeParam)) {
+            responseBuilder.generateIDToken();
+        }
+
+        AccessTokenResponse res = responseBuilder.build();
 
 
         event.success();
@@ -515,11 +527,16 @@ public class TokenEndpoint {
 
         updateUserSessionFromClientAuth(userSession);
 
-        AccessTokenResponse res = tokenManager.responseBuilder(realm, client, event, session, userSession, clientSession)
+        TokenManager.AccessTokenResponseBuilder responseBuilder = tokenManager.responseBuilder(realm, client, event, session, userSession, clientSession)
                 .generateAccessToken()
-                .generateRefreshToken()
+                .generateRefreshToken();
-                .generateIDToken()
+
-                .build();
+        String scopeParam = clientSession.getNote(OAuth2Constants.SCOPE);
+        if (TokenUtil.isOIDCRequest(scopeParam)) {
+            responseBuilder.generateIDToken();
+        }
+
+        AccessTokenResponse res = responseBuilder.build();
 
         event.success();
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java

@@ -94,6 +94,8 @@ import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsername;
 import static org.keycloak.testsuite.admin.ApiUtil.findUserByUsernameId;
 import static org.keycloak.testsuite.util.OAuthClient.AUTH_SERVER_ROOT;
 import static org.keycloak.testsuite.util.ProtocolMapperUtil.createRoleNameMapper;
+
+import org.keycloak.util.TokenUtil;
 import org.openqa.selenium.By;
 
 /**
@@ -994,7 +996,8 @@ public class AccessTokenTest extends AbstractKeycloakTest {
         Form form = new Form();
         form.param(OAuth2Constants.GRANT_TYPE, OAuth2Constants.PASSWORD)
                 .param("username", username)
-                .param("password", password);
+                .param("password", password)
+                .param(OAuth2Constants.SCOPE, OAuth2Constants.SCOPE_OPENID);
         return grantTarget.request()
                 .header(HttpHeaders.AUTHORIZATION, header)
                 .post(Entity.form(form));

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java

@@ -24,6 +24,7 @@ import org.junit.Test;
 import org.keycloak.admin.client.resource.ClientResource;
 import org.keycloak.admin.client.resource.ProtocolMappersResource;
 import org.keycloak.admin.client.resource.UserResource;
+import org.keycloak.common.util.UriUtils;
 import org.keycloak.protocol.oidc.OIDCLoginProtocol;
 import org.keycloak.protocol.oidc.mappers.AddressMapper;
 import org.keycloak.representations.AccessToken;
@@ -148,7 +149,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         }
 
         {
-            OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "test-user@localhost", "password");
+            OAuthClient.AccessTokenResponse response = browserLogin("password", "test-user@localhost", "password");
 
             IDToken idToken = oauth.verifyIDToken(response.getIdToken());
             assertNotNull(idToken.getAddress());
@@ -197,6 +198,8 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
             assertTrue(accessToken.getRealmAccess().getRoles().contains("realm-user"));
             Assert.assertFalse(accessToken.getResourceAccess("test-app").getRoles().contains("customer-user"));
             assertTrue(accessToken.getResourceAccess("app").getRoles().contains("hardcoded"));
+
+            oauth.openLogout();
         }
 
         // undo mappers
@@ -224,13 +227,15 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
 
 
         {
-            OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "test-user@localhost", "password");
+            OAuthClient.AccessTokenResponse response = browserLogin("password", "test-user@localhost", "password");
             IDToken idToken = oauth.verifyIDToken(response.getIdToken());
             assertNull(idToken.getAddress());
             assertNull(idToken.getOtherClaims().get("home_phone"));
             assertNull(idToken.getOtherClaims().get("hard"));
             assertNull(idToken.getOtherClaims().get("nested"));
             assertNull(idToken.getOtherClaims().get("department"));
+
+            oauth.openLogout();
         }
 
 
@@ -248,7 +253,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         protocolMappers.createMapper(Arrays.asList(realmMapper, clientMapper));
 
         // Login user
-        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "test-user@localhost", "password");
+        OAuthClient.AccessTokenResponse response = browserLogin("password", "test-user@localhost", "password");
         IDToken idToken = oauth.verifyIDToken(response.getIdToken());
 
         // Verify attribute is filled
@@ -282,7 +287,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         protocolMappers.createMapper(Arrays.asList(realmMapper, clientMapper));
 
         // Login user
-        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "test-user@localhost", "password");
+        OAuthClient.AccessTokenResponse response = browserLogin("password", "test-user@localhost", "password");
         IDToken idToken = oauth.verifyIDToken(response.getIdToken());
 
         // Verify attribute is filled
@@ -316,7 +321,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         protocolMappers.createMapper(Arrays.asList(realmMapper, clientMapper));
 
         // Login user
-        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "rich.roles@redhat.com", "password");
+        OAuthClient.AccessTokenResponse response = browserLogin("password", "rich.roles@redhat.com", "password");
         IDToken idToken = oauth.verifyIDToken(response.getIdToken());
 
         // Verify attribute is filled
@@ -354,9 +359,16 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         // Login user
         ClientManager.realm(adminClient.realm("test")).clientId(clientId).directAccessGrant(true);
         oauth.clientId(clientId);
-        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("secret", "rich.roles@redhat.com", "password");
+
+        String oldRedirectUri = oauth.getRedirectUri();
+        oauth.redirectUri(UriUtils.getOrigin(oldRedirectUri) + "/test-app-authz");
+
+        OAuthClient.AccessTokenResponse response = browserLogin("secret", "rich.roles@redhat.com", "password");
         IDToken idToken = oauth.verifyIDToken(response.getIdToken());
 
+        // revert redirect_uri
+        oauth.redirectUri(oldRedirectUri);
+
         // Verify attribute is filled
         Map<String, Object> roleMappings = (Map<String, Object>)idToken.getOtherClaims().get("roles-custom");
         Assert.assertThat(roleMappings.keySet(), containsInAnyOrder("realm", clientId));
@@ -387,7 +399,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         // Login user
         ClientManager.realm(adminClient.realm("test")).clientId(clientId).directAccessGrant(true);
         oauth.clientId(clientId);
-        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "rich.roles@redhat.com", "password");
+        OAuthClient.AccessTokenResponse response = browserLogin("password", "rich.roles@redhat.com", "password");
         IDToken idToken = oauth.verifyIDToken(response.getIdToken());
 
         // Verify attribute is filled
@@ -419,7 +431,7 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         // Login user
         ClientManager.realm(adminClient.realm("test")).clientId(clientId).directAccessGrant(true);
         oauth.clientId(clientId);
-        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "rich.roles@redhat.com", "password");
+        OAuthClient.AccessTokenResponse response = browserLogin("password", "rich.roles@redhat.com", "password");
         IDToken idToken = oauth.verifyIDToken(response.getIdToken());
 
         // Verify attribute is filled
@@ -468,4 +480,9 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest {
         return rep;
     }
 
+    private OAuthClient.AccessTokenResponse browserLogin(String clientSecret, String username, String password) {
+        OAuthClient.AuthorizationEndpointResponse authzEndpointResponse = oauth.doLogin(username, password);
+        return oauth.doAccessTokenRequest(authzEndpointResponse.getCode(), clientSecret);
+    }
+
 }

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/OIDCWellKnownProviderTest.java

@@ -121,7 +121,8 @@ public class OIDCWellKnownProviderTest extends AbstractKeycloakTest {
 
     @Test
     public void testIssuerMatches() throws Exception {
-        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "test-user@localhost", "password");
+        OAuthClient.AuthorizationEndpointResponse authzResp = oauth.doLogin("test-user@localhost", "password");
+        OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(authzResp.getCode(), "password");
         Assert.assertEquals(200, response.getStatusCode());
         IDToken idToken = oauth.verifyIDToken(response.getIdToken());
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oidc/ScopeParameterTest.java

@@ -0,0 +1,142 @@
+/*
+ * Copyright 2016 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.testsuite.oidc;
+
+import java.util.List;
+
+import javax.ws.rs.core.UriBuilder;
+
+import org.jboss.arquillian.graphene.page.Page;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.keycloak.OAuth2Constants;
+import org.keycloak.events.Details;
+import org.keycloak.representations.AccessToken;
+import org.keycloak.representations.idm.EventRepresentation;
+import org.keycloak.representations.idm.RealmRepresentation;
+import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
+import org.keycloak.testsuite.ActionURIUtils;
+import org.keycloak.testsuite.Assert;
+import org.keycloak.testsuite.AssertEvents;
+import org.keycloak.testsuite.admin.AbstractAdminTest;
+import org.keycloak.testsuite.pages.AccountUpdateProfilePage;
+import org.keycloak.testsuite.pages.AppPage;
+import org.keycloak.testsuite.pages.ErrorPage;
+import org.keycloak.testsuite.pages.LoginPage;
+import org.keycloak.testsuite.pages.OAuthGrantPage;
+import org.keycloak.testsuite.util.ClientManager;
+import org.keycloak.testsuite.util.OAuthClient;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class ScopeParameterTest extends AbstractTestRealmKeycloakTest {
+
+    @Rule
+    public AssertEvents events = new AssertEvents(this);
+
+    @Page
+    protected AppPage appPage;
+
+    @Page
+    protected LoginPage loginPage;
+
+    @Page
+    protected AccountUpdateProfilePage profilePage;
+
+    @Page
+    protected OAuthGrantPage grantPage;
+
+    @Page
+    protected ErrorPage errorPage;
+
+
+    @Override
+    public void configureTestRealm(RealmRepresentation testRealm) {
+    }
+
+    @Before
+    public void clientConfiguration() {
+        ClientManager.realm(adminClient.realm("test")).clientId("test-app").directAccessGrant(true);
+        /*
+         * Configure the default client ID. Seems like OAuthClient is keeping the state of clientID
+         * For example: If some test case configure oauth.clientId("sample-public-client"), other tests
+         * will faile and the clientID will always be "sample-public-client
+         * @see AccessTokenTest#testAuthorizationNegotiateHeaderIgnored()
+         */
+        oauth.clientId("test-app");
+        oauth.maxAge(null);
+    }
+
+    @Override
+    public void addTestRealms(List<RealmRepresentation> testRealms) {
+        RealmRepresentation realm = AbstractAdminTest.loadJson(getClass().getResourceAsStream("/testrealm.json"), RealmRepresentation.class);
+        testRealms.add(realm);
+    }
+
+
+    // If scope=openid is missing, IDToken won't be present
+    @Test
+    public void testMissingScopeOpenid() {
+        String loginFormUrl = oauth.getLoginFormUrl();
+        loginFormUrl = ActionURIUtils.removeQueryParamFromURI(loginFormUrl, OAuth2Constants.SCOPE);
+
+        driver.navigate().to(loginFormUrl);
+        oauth.fillLoginForm("test-user@localhost", "password");
+        EventRepresentation loginEvent = events.expectLogin().assertEvent();
+
+        String code = new OAuthClient.AuthorizationEndpointResponse(oauth).getCode();
+        OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, "password");
+
+        // IDToken is not there
+        Assert.assertEquals(200, response.getStatusCode());
+        Assert.assertNull(response.getIdToken());
+        Assert.assertNotNull(response.getRefreshToken());
+
+        AccessToken token = oauth.verifyToken(response.getAccessToken());
+        Assert.assertEquals(token.getSubject(), loginEvent.getUserId());
+
+        // Refresh and assert idToken still not present
+        response = oauth.doRefreshTokenRequest(response.getRefreshToken(), "password");
+        Assert.assertEquals(200, response.getStatusCode());
+        Assert.assertNull(response.getIdToken());
+
+        token = oauth.verifyToken(response.getAccessToken());
+        Assert.assertEquals(token.getSubject(), loginEvent.getUserId());
+    }
+
+
+    // If scope=openid is missing, IDToken won't be present
+    @Test
+    public void testMissingScopeOpenidInResourceOwnerPasswordCredentialRequest() throws Exception {
+        OAuthClient.AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", "test-user@localhost", "password");
+
+        assertEquals(200, response.getStatusCode());
+
+        // idToken not present
+        Assert.assertNull(response.getIdToken());
+
+        Assert.assertNotNull(response.getRefreshToken());
+        AccessToken accessToken = oauth.verifyToken(response.getAccessToken());
+        Assert.assertEquals(accessToken.getPreferredUsername(), "test-user@localhost");
+
+    }
+}