janhoy commented on code in PR #1791:
URL: https://github.com/apache/solr/pull/1791#discussion_r1296859919


##########
solr/webapp/web/js/angular/controllers/login.js:
##########
@@ -60,92 +60,169 @@ solrAdminApp.controller('LoginController',
             var hp = AuthenticationService.decodeHashParams(hash);
             var expectedState = sessionStorage.getItem("auth.stateRandom") + 
"_" + sessionStorage.getItem("auth.location");
             sessionStorage.setItem("auth.state", "error");
-            if (hp['access_token'] && hp['token_type'] && hp['state']) {
-              // Validate state
-              if (hp['state'] !== expectedState) {
-                $scope.error = "Problem with auth callback";
-                console.log("Expected state param " + expectedState + " but 
got " + hp['state']);
-                errorText += "Invalid values in state parameter. ";
-              }
-              // Validate token type
-              if (hp['token_type'].toLowerCase() !== "bearer") {
-                console.log("Expected token_type param 'bearer', but got " + 
hp['token_type']);
-                errorText += "Invalid values in token_type parameter. ";
-              }
-              // Unpack ID token and validate nonce, get username
-              if (hp['id_token']) {
-                var idToken = hp['id_token'].split(".");
-                if (idToken.length === 3) {
-                  var payload = 
AuthenticationService.decodeJwtPart(idToken[1]);
-                  if (!payload['nonce'] || payload['nonce'] !== 
sessionStorage.getItem("auth.nonce")) {
-                    errorText += "Invalid 'nonce' value, possible attack 
detected. Please log in again. ";
-                  }
+            $scope.authData = AuthenticationService.getAuthDataHeader();
+            if (!validateState(hp['state'], expectedState)) {
+              $scope.error = "Problems with OpenID callback";
+              $scope.errorDescription = errorText;
+              $scope.http401 = "true";
+              sessionStorage.setItem("auth.state", "error");
+            }
+            else {
+              // for backward compatibility default flow to 'implicit'
+              var flow = $scope.authData ? 
$scope.authData['authorization_flow'] : "implicit";
+              console.log("Callback: authorization_flow : " +flow);
+              var isCodePKCE = flow == 'code_pkce';
+              if (isCodePKCE) {
+                // code flow with PKCE
+                console.debug("Callback. Using code flow with PKCE");
+                var code = hp['code'];
+                var tokenEndpoint = $scope.authData['tokenEndpoint'];
+                // concurrent Solr API calls will trigger 401 and erase 
session's "auth.realm" in app.js
+                // save it before it's erased
+                var authRealm = sessionStorage.getItem("auth.realm");
+
+                var data = {
+                  'grant_type': 'authorization_code',
+                  'code': code,
+                  'redirect_uri': $window.location.href.split('#')[0],
+                  'scope': "openid " + $scope.authData['scope'],
+                  'code_verifier': sessionStorage.getItem('codeVerifier'),
+                  "client_id": $scope.authData['client_id']
+                };
 
-                  if (errorText === "") {
-                    sessionStorage.setItem("auth.username", payload['sub']);
-                    sessionStorage.setItem("auth.header", "Bearer " + 
hp['access_token']);
-                    sessionStorage.removeItem("auth.statusText");
-                    sessionStorage.removeItem("auth.stateRandom");
-                    sessionStorage.removeItem("auth.wwwAuthHeader");
-                    console.log("User " + payload['sub'] + " is logged in");
-                    var redirectTo = sessionStorage.getItem("auth.location");
-                    console.log("Redirecting to stored location " + 
redirectTo);
-                    sessionStorage.setItem("auth.state", "authenticated");
-                    sessionStorage.removeItem("http401");
-                    $location.path(redirectTo).hash("");
+                console.debug(`Callback. Got code: ${code} \nCalling token 
endpoint:: ${tokenEndpoint} `);
+                AuthenticationService.getOAuthTokens(tokenEndpoint, 
data).then(function(response) {

Review Comment:
   Here's a patch that sets CSP dynamically. Verified the headers get correctly 
set, but not yet spun it up with true JWT plugin, will be able to do that next 
week.
   
   ```patch
   Subject: [PATCH] Update io.netty:* to v4.1.96.Final (#1813)
   
   (cherry picked from commit b0d3085b83994b1636e455f95e6858aab7039e29)
   ---
   Index: solr/server/etc/jetty.xml
   IDEA additional info:
   Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
   <+>UTF-8
   ===================================================================
   diff --git a/solr/server/etc/jetty.xml b/solr/server/etc/jetty.xml
   --- a/solr/server/etc/jetty.xml      (revision 
2fb8f9e3734eb9572c701a7e1a1a9834a94f4a38)
   +++ b/solr/server/etc/jetty.xml      (date 1692202681699)
   @@ -90,6 +90,7 @@
          <Set name="originalPathAttribute">requestedPath</Set>
    
          <!-- security-related headers -->
   +<!-- TODO: SOLR-16896 - csp is now set from Java backend
          <Call name="addRule">
            <Arg>
              <New class="org.eclipse.jetty.rewrite.handler.HeaderPatternRule">
   @@ -99,6 +100,7 @@
              </New>
            </Arg>
          </Call>
   +-->
          <Call name="addRule">
            <Arg>
              <New class="org.eclipse.jetty.rewrite.handler.HeaderPatternRule">
   Index: solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
   IDEA additional info:
   Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
   <+>UTF-8
   ===================================================================
   diff --git 
a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java 
b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java
   --- a/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java     
(revision 2fb8f9e3734eb9572c701a7e1a1a9834a94f4a38)
   +++ b/solr/core/src/java/org/apache/solr/servlet/LoadAdminUiServlet.java     
(date 1692260572052)
   @@ -16,11 +16,13 @@
     */
    package org.apache.solr.servlet;
    
   +import com.google.common.net.HttpHeaders;
    import java.io.IOException;
    import java.io.InputStream;
    import java.io.OutputStreamWriter;
    import java.io.Writer;
    import java.nio.charset.StandardCharsets;
   +import java.util.stream.Collectors;
    import javax.servlet.http.HttpServletRequest;
    import javax.servlet.http.HttpServletResponse;
    import org.apache.commons.io.output.CloseShieldOutputStream;
   @@ -37,6 +39,8 @@
      // check system properties for whether or not admin UI is disabled, 
default is false
      private static final boolean disabled =
          Boolean.parseBoolean(System.getProperty("disableAdminUI", "false"));
   +  // Any system property that starts with this prefix will be added to the 
CSP connect-src
   +  public static final String SYSPROP_CSP_HOSTS_PREFIX = "solr.ui.csp.hosts";
    
      @Override
      public void doGet(HttpServletRequest _request, HttpServletResponse 
_response) throws IOException {
   @@ -60,6 +64,12 @@
          if (in != null && cores != null) {
            response.setCharacterEncoding("UTF-8");
            response.setContentType("text/html");
   +        String connectSrc = generateCspConnectSrc();
   +        response.setHeader(
   +            HttpHeaders.CONTENT_SECURITY_POLICY,
   +            "default-src 'none'; base-uri 'none'; connect-src "
   +                + connectSrc
   +                + "; form-action 'self'; font-src 'self'; frame-ancestors 
'none'; img-src 'self' data:; media-src 'self'; style-src 'self' 
'unsafe-inline'; script-src 'self'; worker-src 'self';");
    
            // We have to close this to flush OutputStreamWriter buffer
            try (Writer out =
   @@ -76,4 +86,18 @@
          }
        }
      }
   +
   +  /**
   +   * Iterate all system properties with prefix {@link 
#SYSPROP_CSP_HOSTS_PREFIX} and concatenate
   +   * them into a space separated string that can be use in CSP
   +   */
   +  private String generateCspConnectSrc() {
   +    var props =
   +        System.getProperties().keySet().stream()
   +            .filter(k -> k instanceof String && ((String) 
k).startsWith(SYSPROP_CSP_HOSTS_PREFIX))
   +            .map(key -> System.getProperty((String) key))
   +            .collect(Collectors.toList());
   +    props.add("'self'");
   +    return String.join(" ", props);
   +  }
    }
   Index: 
solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java
   IDEA additional info:
   Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
   <+>UTF-8
   ===================================================================
   diff --git 
a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java
 
b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java
   --- 
a/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java
 (revision 2fb8f9e3734eb9572c701a7e1a1a9834a94f4a38)
   +++ 
b/solr/modules/jwt-auth/src/java/org/apache/solr/security/jwt/JWTAuthPlugin.java
 (date 1692260035611)
   @@ -19,6 +19,7 @@
    import java.io.ByteArrayInputStream;
    import java.io.IOException;
    import java.lang.invoke.MethodHandles;
   +import java.net.URI;
    import java.nio.charset.StandardCharsets;
    import java.nio.file.Files;
    import java.nio.file.Path;
   @@ -63,6 +64,7 @@
    import org.apache.solr.security.ConfigEditablePlugin;
    import 
org.apache.solr.security.jwt.JWTAuthPlugin.JWTAuthenticationResponse.AuthCode;
    import org.apache.solr.security.jwt.api.ModifyJWTAuthPluginConfigAPI;
   +import org.apache.solr.servlet.LoadAdminUiServlet;
    import org.apache.solr.util.CryptoKeys;
    import org.eclipse.jetty.client.api.Request;
    import org.jose4j.jwa.AlgorithmConstraints;
   @@ -282,10 +284,25 @@
        }
    
        initConsumer();
   +    registerTokenEndpointForCsp();
    
        lastInitTime = Instant.now();
      }
    
   +  /** Record Issuer URL as a system property so it can be picked up and 
sent to Admin UI as CSP */
   +  private void registerTokenEndpointForCsp() {
   +    final String syspropName = LoadAdminUiServlet.SYSPROP_CSP_HOSTS_PREFIX 
+ ".jwt";
   +    String url = !issuerConfigs.isEmpty() ? 
getPrimaryIssuer().getTokenEndpoint() : null;
   +    if (url != null) {
   +      URI uri = URI.create(url);
   +      System.setProperty(
   +          syspropName,
   +          String.format(Locale.ROOT, "%s://%s:%s", uri.getScheme(), 
uri.getHost(), uri.getPort()));
   +    } else {
   +      System.clearProperty(syspropName);
   +    }
   +  }
   +
      /**
       * Given a configuration object of a file name or list of file names, 
read X509 certificates from
       * each file
   ```
   
   The only problem with this is that now the CSP header is only set when 
loading the admin UI, not for all the API endpoints as it used to be. Is there 
a way to have Jetty add the header only if the servlet has not added it already?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to