From baa89debd9582d3f4245cc14a8c6cdbaa227d347 Mon Sep 17 00:00:00 2001
From: Konstantinos Georgilakis <cgeorgilakis@grnet.gr>
Date: Mon, 18 Jul 2022 11:01:22 +0300
Subject: [PATCH] Correct isValidScope method of TokenManager for Dynamic
 scopes

Closes #13158
---
 .../keycloak/protocol/oidc/TokenManager.java   |  7 ++++---
 .../testsuite/forms/FlowOverrideTest.java      | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java
index 3c6a286876..1cf98ecd51 100755
--- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java
+++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java
@@ -650,9 +650,6 @@ public class TokenManager {
         if (scopes == null) {
             return true;
         }
-        if (authorizationRequestContext.getAuthorizationDetailEntries() == null || authorizationRequestContext.getAuthorizationDetailEntries().isEmpty()) {
-            return false;
-        }
         Collection<String> requestedScopes = TokenManager.parseScopeParameter(scopes).collect(Collectors.toSet());
         Set<String> rarScopes = authorizationRequestContext.getAuthorizationDetailEntries()
                 .stream()
@@ -664,6 +661,10 @@ public class TokenManager {
             requestedScopes.remove(OAuth2Constants.SCOPE_OPENID);
         }
 
+        if ((authorizationRequestContext.getAuthorizationDetailEntries() == null || authorizationRequestContext.getAuthorizationDetailEntries().isEmpty()) && requestedScopes.size()>0) {
+            return false;
+        }
+
         if (logger.isTraceEnabled()) {
             logger.tracef("Rar scopes to validate requested scopes against: %1s", String.join(" ", rarScopes));
             logger.tracef("Requested scopes: %1s", String.join(" ", requestedScopes));
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java
index e033c65c4f..49f81a8300 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/FlowOverrideTest.java
@@ -26,6 +26,7 @@ import org.keycloak.OAuth2Constants;
 import org.keycloak.admin.client.resource.ClientsResource;
 import org.keycloak.authentication.authenticators.browser.UsernamePasswordFormFactory;
 import org.keycloak.authentication.authenticators.challenge.BasicAuthOTPAuthenticatorFactory;
+import org.keycloak.common.Profile;
 import org.keycloak.events.Details;
 import org.keycloak.models.AuthenticationExecutionModel;
 import org.keycloak.models.AuthenticationFlowBindings;
@@ -41,6 +42,7 @@ import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
 import org.keycloak.testsuite.AssertEvents;
 import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
+import org.keycloak.testsuite.arquillian.annotation.EnableFeature;
 import org.keycloak.testsuite.arquillian.annotation.UncaughtServerErrorExpected;
 import org.keycloak.testsuite.authentication.PushButtonAuthenticatorFactory;
 import org.keycloak.testsuite.pages.AppPage;
@@ -264,6 +266,14 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest {
         events.expectLogin().client("test-app-flow").detail(Details.USERNAME, "test-user@localhost").assertEvent();
     }
 
+    // TODO remove this once DYNAMIC_SCOPES feature is enabled by default
+    @Test
+    @EnableFeature(value = Profile.Feature.DYNAMIC_SCOPES, skipRestart = true)
+    public void testWithClientBrowserOverrideWithDynamicScope() throws Exception {
+        // Just use existing test with DYNAMIC_SCOPES feature enabled as it was failing with DYNAMIC_SCOPES
+        testWithClientBrowserOverride();
+    }
+
     @Test
     public void testNoOverrideBrowser() throws Exception {
         String clientId = "test-app";
@@ -531,6 +541,14 @@ public class FlowOverrideTest extends AbstractTestRealmKeycloakTest {
         events.clear();
     }
 
+    // TODO remove this once DYNAMIC_SCOPES feature is enabled by default
+    @Test
+    @EnableFeature(value = Profile.Feature.DYNAMIC_SCOPES, skipRestart = true)
+    public void testClientOverrideFlowUsingBrowserHttpChallengeWithDynamicScope() {
+        // Just use existing test with DYNAMIC_SCOPES feature enabled as it was failing with DYNAMIC_SCOPES
+        testClientOverrideFlowUsingBrowserHttpChallenge();
+    }
+
     @Test
     public void testRestInterface() throws Exception {
         ClientsResource clients = adminClient.realm("test").clients();