keycloak-uncached

KEYCLOAK-3237 Change OIDC adapters to use scope=openid as

7/14/2016 2:31:07 PM

Details

diff --git a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java
index cf51bdf..25969a6 100755
--- a/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java
+++ b/adapters/oidc/adapter-core/src/main/java/org/keycloak/adapters/OAuthRequestAuthenticator.java
@@ -34,6 +34,7 @@ import org.keycloak.representations.AccessTokenResponse;
 import org.keycloak.representations.IDToken;
 import org.keycloak.common.util.KeycloakUriBuilder;
 import org.keycloak.common.util.UriUtils;
+import org.keycloak.util.TokenUtil;
 
 import java.io.IOException;
 import java.util.concurrent.atomic.AtomicLong;
@@ -171,9 +172,9 @@ public class OAuthRequestAuthenticator {
         if (idpHint != null && idpHint.length() > 0) {
             redirectUriBuilder.queryParam(AdapterConstants.KC_IDP_HINT,idpHint);
         }
-        if (scope != null && scope.length() > 0) {
-            redirectUriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
-        }
+
+        scope = TokenUtil.attachOIDCScope(scope);
+        redirectUriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
 
         return redirectUriBuilder.build().toString();
     }
diff --git a/adapters/oidc/jaxrs-oauth-client/src/main/java/org/keycloak/jaxrs/JaxrsOAuthClient.java b/adapters/oidc/jaxrs-oauth-client/src/main/java/org/keycloak/jaxrs/JaxrsOAuthClient.java
index cd2599e..ae305e2 100755
--- a/adapters/oidc/jaxrs-oauth-client/src/main/java/org/keycloak/jaxrs/JaxrsOAuthClient.java
+++ b/adapters/oidc/jaxrs-oauth-client/src/main/java/org/keycloak/jaxrs/JaxrsOAuthClient.java
@@ -20,6 +20,7 @@ package org.keycloak.jaxrs;
 import org.keycloak.AbstractOAuthClient;
 import org.keycloak.OAuth2Constants;
 import org.keycloak.representations.AccessTokenResponse;
+import org.keycloak.util.TokenUtil;
 
 import javax.ws.rs.BadRequestException;
 import javax.ws.rs.InternalServerErrorException;
@@ -85,14 +86,13 @@ public class JaxrsOAuthClient extends AbstractOAuthClient {
     }
     public Response redirect(UriInfo uriInfo, String redirectUri) {
         String state = getStateCode();
+        String scopeParam = TokenUtil.attachOIDCScope(scope);
 
         UriBuilder uriBuilder = UriBuilder.fromUri(authUrl)
                 .queryParam(OAuth2Constants.CLIENT_ID, clientId)
                 .queryParam(OAuth2Constants.REDIRECT_URI, redirectUri)
-                .queryParam(OAuth2Constants.STATE, state);
-        if (scope != null) {
-            uriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
-        }
+                .queryParam(OAuth2Constants.STATE, state)
+                .queryParam(OAuth2Constants.SCOPE, scopeParam);
 
         URI url = uriBuilder.build();
 
diff --git a/adapters/oidc/js/src/main/resources/keycloak.js b/adapters/oidc/js/src/main/resources/keycloak.js
index 2dca960..a4d24fe 100755
--- a/adapters/oidc/js/src/main/resources/keycloak.js
+++ b/adapters/oidc/js/src/main/resources/keycloak.js
@@ -208,6 +208,8 @@
                 action = 'registrations';
             }
 
+            var scope = (options && options.scope) ? "openid " + options.scope : "openid";
+
             var url = getRealmUrl()
                 + '/protocol/openid-connect/' + action
                 + '?client_id=' + encodeURIComponent(kc.clientId)
@@ -215,7 +217,8 @@
                 + '&state=' + encodeURIComponent(state)
                 + '&nonce=' + encodeURIComponent(nonce)
                 + '&response_mode=' + encodeURIComponent(kc.responseMode)
-                + '&response_type=' + encodeURIComponent(kc.responseType);
+                + '&response_type=' + encodeURIComponent(kc.responseType)
+                + '&scope=' + encodeURIComponent(scope);
 
             if (options && options.prompt) {
                 url += '&prompt=' + encodeURIComponent(options.prompt);
@@ -229,10 +232,6 @@
                 url += '&kc_idp_hint=' + encodeURIComponent(options.idpHint);
             }
 
-            if (options && options.scope) {
-                url += '&scope=' + encodeURIComponent(options.scope);
-            }
-
             if (options && options.locale) {
                 url += '&ui_locales=' + encodeURIComponent(options.locale);
             }
diff --git a/adapters/oidc/servlet-oauth-client/src/main/java/org/keycloak/servlet/ServletOAuthClient.java b/adapters/oidc/servlet-oauth-client/src/main/java/org/keycloak/servlet/ServletOAuthClient.java
index f3dbcdc..64099e6 100755
--- a/adapters/oidc/servlet-oauth-client/src/main/java/org/keycloak/servlet/ServletOAuthClient.java
+++ b/adapters/oidc/servlet-oauth-client/src/main/java/org/keycloak/servlet/ServletOAuthClient.java
@@ -30,6 +30,7 @@ import org.keycloak.jose.jws.JWSInputException;
 import org.keycloak.representations.AccessTokenResponse;
 import org.keycloak.representations.IDToken;
 import org.keycloak.common.util.KeycloakUriBuilder;
+import org.keycloak.util.TokenUtil;
 
 import javax.security.cert.X509Certificate;
 import javax.servlet.http.Cookie;
@@ -91,15 +92,15 @@ public class ServletOAuthClient extends KeycloakDeploymentDelegateOAuthClient {
         String state = getStateCode();
         KeycloakDeployment resolvedDeployment = resolveDeployment(getDeployment(), request);
         String authUrl = resolvedDeployment.getAuthUrl().clone().build().toString();
+        String scopeParam = TokenUtil.attachOIDCScope(scope);
 
         KeycloakUriBuilder uriBuilder =  KeycloakUriBuilder.fromUri(authUrl)
                 .queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE)
                 .queryParam(OAuth2Constants.CLIENT_ID, getClientId())
                 .queryParam(OAuth2Constants.REDIRECT_URI, redirectUri)
-                .queryParam(OAuth2Constants.STATE, state);
-        if (scope != null) {
-            uriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
-        }
+                .queryParam(OAuth2Constants.STATE, state)
+                .queryParam(OAuth2Constants.SCOPE, scopeParam);
+
         URI url = uriBuilder.build();
 
         String stateCookiePath = this.stateCookiePath;
diff --git a/core/src/main/java/org/keycloak/OAuth2Constants.java b/core/src/main/java/org/keycloak/OAuth2Constants.java
index 75a2277..6c00831 100644
--- a/core/src/main/java/org/keycloak/OAuth2Constants.java
+++ b/core/src/main/java/org/keycloak/OAuth2Constants.java
@@ -62,6 +62,15 @@ public interface OAuth2Constants {
     // http://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
     String OFFLINE_ACCESS = "offline_access";
 
+    // http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+    String SCOPE_OPENID = "openid";
+
+    // http://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims
+    String SCOPE_PROFILE = "profile";
+    String SCOPE_EMAIL = "email";
+    String SCOPE_ADDRESS = "address";
+    String SCOPE_PHONE = "phone";
+
     String UI_LOCALES_PARAM = "ui_locales";
 
 
diff --git a/core/src/main/java/org/keycloak/util/TokenUtil.java b/core/src/main/java/org/keycloak/util/TokenUtil.java
index eb28943..6168019 100644
--- a/core/src/main/java/org/keycloak/util/TokenUtil.java
+++ b/core/src/main/java/org/keycloak/util/TokenUtil.java
@@ -38,14 +38,30 @@ public class TokenUtil {
     public static final String TOKEN_TYPE_OFFLINE = "Offline";
 
 
+    public static String attachOIDCScope(String scopeParam) {
+        if (scopeParam == null || scopeParam.isEmpty()) {
+            return OAuth2Constants.SCOPE_OPENID;
+        } else {
+            return OAuth2Constants.SCOPE_OPENID + " " + scopeParam;
+        }
+    }
+
+    public static boolean isOIDCRequest(String scopeParam) {
+        return hasScope(scopeParam, OAuth2Constants.SCOPE_OPENID);
+    }
+
     public static boolean isOfflineTokenRequested(String scopeParam) {
-        if (scopeParam == null) {
+        return hasScope(scopeParam, OAuth2Constants.OFFLINE_ACCESS);
+    }
+
+    public static boolean hasScope(String scopeParam, String targetScope) {
+        if (scopeParam == null || targetScope == null) {
             return false;
         }
 
         String[] scopes = scopeParam.split(" ");
         for (String scope : scopes) {
-            if (OAuth2Constants.OFFLINE_ACCESS.equals(scope)) {
+            if (targetScope.equals(scope)) {
                 return true;
             }
         }
diff --git a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java
index 2b895f1..6a83c91 100755
--- a/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java
+++ b/services/src/main/java/org/keycloak/protocol/oidc/endpoints/AuthorizationEndpoint.java
@@ -53,6 +53,7 @@ import org.keycloak.services.managers.ClientSessionCode;
 import org.keycloak.services.messages.Messages;
 import org.keycloak.services.resources.LoginActionsService;
 import org.keycloak.services.util.CacheControlUtil;
+import org.keycloak.util.TokenUtil;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -98,6 +99,7 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
         KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.PROMPT_PARAM);
         KNOWN_REQ_PARAMS.add(AdapterConstants.KC_IDP_HINT);
         KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.NONCE_PARAM);
+        KNOWN_REQ_PARAMS.add(OIDCLoginProtocol.MAX_AGE_PARAM);
     }
 
     private enum Action {
@@ -156,6 +158,10 @@ public class AuthorizationEndpoint extends AuthorizationEndpointBase {
             return errorResponse;
         }
 
+        if (!TokenUtil.isOIDCRequest(scope)) {
+            logger.oidcScopeMissing();
+        }
+
         createClientSession();
         // So back button doesn't work
         CacheControlUtil.noBackButtonCacheControlHeader();
diff --git a/services/src/main/java/org/keycloak/services/resources/AbstractSecuredLocalService.java b/services/src/main/java/org/keycloak/services/resources/AbstractSecuredLocalService.java
index 547424c..2a8c0bb 100755
--- a/services/src/main/java/org/keycloak/services/resources/AbstractSecuredLocalService.java
+++ b/services/src/main/java/org/keycloak/services/resources/AbstractSecuredLocalService.java
@@ -34,6 +34,7 @@ import org.keycloak.services.managers.Auth;
 import org.keycloak.services.managers.AuthenticationManager;
 import org.keycloak.services.util.CookieHelper;
 import org.keycloak.common.util.UriUtils;
+import org.keycloak.util.TokenUtil;
 
 import javax.ws.rs.GET;
 import javax.ws.rs.Path;
@@ -237,15 +238,14 @@ public abstract class AbstractSecuredLocalService {
 
         public Response redirect(UriInfo uriInfo, String redirectUri) {
             String state = getStateCode();
+            String scopeParam = TokenUtil.attachOIDCScope(scope);
 
             UriBuilder uriBuilder = UriBuilder.fromUri(authUrl)
                     .queryParam(OAuth2Constants.CLIENT_ID, clientId)
                     .queryParam(OAuth2Constants.REDIRECT_URI, redirectUri)
                     .queryParam(OAuth2Constants.STATE, state)
-                    .queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE);
-            if (scope != null) {
-                uriBuilder.queryParam(OAuth2Constants.SCOPE, scope);
-            }
+                    .queryParam(OAuth2Constants.RESPONSE_TYPE, OAuth2Constants.CODE)
+                    .queryParam(OAuth2Constants.SCOPE, scopeParam);
 
             URI url = uriBuilder.build();
 
diff --git a/services/src/main/java/org/keycloak/services/ServicesLogger.java b/services/src/main/java/org/keycloak/services/ServicesLogger.java
index fb71a2e..272ad6b 100644
--- a/services/src/main/java/org/keycloak/services/ServicesLogger.java
+++ b/services/src/main/java/org/keycloak/services/ServicesLogger.java
@@ -32,6 +32,8 @@ import static org.jboss.logging.Logger.Level.ERROR;
 import static org.jboss.logging.Logger.Level.FATAL;
 import static org.jboss.logging.Logger.Level.INFO;
 import static org.jboss.logging.Logger.Level.WARN;
+
+import org.jboss.logging.annotations.Once;
 import org.keycloak.email.EmailException;
 import org.keycloak.events.EventListenerProvider;
 import org.keycloak.models.ModelDuplicateException;
@@ -402,4 +404,9 @@ public interface ServicesLogger extends BasicLogger {
     @LogMessage(level = ERROR)
     @Message(id=90, value="Failed to close ProviderSession")
     void failedToCloseProviderSession(@Cause Throwable t);
+
+    @LogMessage(level = WARN)
+    @Message(id=91, value="Request is missing scope 'openid' so it's not treated as OIDC, but just pure OAuth2 request. This can have impact in future versions (eg. removed IDToken from the Token Response)")
+    @Once
+    void oidcScopeMissing();
 }
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java
index 7ccd8d7..12c1f30 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/OAuthClient.java
@@ -39,6 +39,7 @@ import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.RefreshToken;
 import org.keycloak.util.BasicAuthHelper;
 import org.keycloak.common.util.PemUtils;
+import org.keycloak.util.TokenUtil;
 import org.openqa.selenium.By;
 import org.openqa.selenium.WebDriver;
 
@@ -442,9 +443,10 @@ public class OAuthClient {
         if(uiLocales != null){
             b.queryParam(OAuth2Constants.UI_LOCALES_PARAM, uiLocales);
         }
-        if (scope != null) {
-            b.queryParam(OAuth2Constants.SCOPE, scope);
-        }
+
+        String scopeParam = TokenUtil.attachOIDCScope(scope);
+        b.queryParam(OAuth2Constants.SCOPE, scopeParam);
+
         return b.build(realm).toString();
     }
 
diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java
index d066eed..8c7b290 100644
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java
@@ -523,9 +523,10 @@ public class OAuthClient {
         if(uiLocales != null){
             b.queryParam(OAuth2Constants.UI_LOCALES_PARAM, uiLocales);
         }
-        if (scope != null) {
-            b.queryParam(OAuth2Constants.SCOPE, scope);
-        }
+
+        String scopeParam = TokenUtil.attachOIDCScope(scope);
+        b.queryParam(OAuth2Constants.SCOPE, scopeParam);
+
         if (maxAge != null) {
             b.queryParam(OIDCLoginProtocol.MAX_AGE_PARAM, maxAge);
         }