Merge pull request #391 from jeroenr/master

CORS improvements and clean up

core/src/main/java/org/keycloak/util/CollectionUtil.java

@@ -0,0 +1,26 @@
+package org.keycloak.util;
+
+import java.util.Collection;
+import java.util.Iterator;
+
+/**
+ * @author <a href="mailto:jeroen.rosenberg@gmail.com">Jeroen Rosenberg</a>
+ */
+public class CollectionUtil {
+
+    public static String join(Collection<String> strings) {
+        return join(strings, ", ");
+    }
+
+    public static String join(Collection<String> strings, String separator) {
+        Iterator<String> iter = strings.iterator();
+        StringBuilder sb = new StringBuilder();
+        if(iter.hasNext()){
+            sb.append(iter.next());
+            while(iter.hasNext()){
+                sb.append(separator).append(iter.next());
+            }
+        }
+        return sb.toString();
+    }
+}

services/src/main/java/org/keycloak/services/resources/Cors.java

@@ -1,5 +1,7 @@
 package org.keycloak.services.resources;
 
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
@@ -8,7 +10,7 @@ import javax.ws.rs.core.Response.ResponseBuilder;
 
 import org.jboss.resteasy.spi.HttpRequest;
 import org.keycloak.models.ClientModel;
-import org.keycloak.models.UserModel;
+import org.keycloak.util.CollectionUtil;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -16,20 +18,25 @@ import org.keycloak.models.UserModel;
 public class Cors {
 
     public static final long DEFAULT_MAX_AGE = TimeUnit.HOURS.toSeconds(1);
-    public static final String DEFAULT_ALLOW_METHODS = "GET, OPTIONS";
+    public static final String DEFAULT_ALLOW_METHODS = "GET, HEAD, OPTIONS";
+    public static final String DEFAULT_ALLOW_HEADERS = "Origin, Accept, X-Requested-With, Content-Type, Access-Control-Request-Method, Access-Control-Request-Headers";
 
-    public static final String ORIGIN = "Origin";
+    public static final String ORIGIN_HEADER = "Origin";
+    public static final String AUTHORIZATION_HEADER = "Authorization";
 
     public static final String ACCESS_CONTROL_ALLOW_ORIGIN = "Access-Control-Allow-Origin";
     public static final String ACCESS_CONTROL_ALLOW_METHODS = "Access-Control-Allow-Methods";
     public static final String ACCESS_CONTROL_ALLOW_HEADERS = "Access-Control-Allow-Headers";
+    public static final String ACCESS_CONTROL_EXPOSE_HEADERS = "Access-Control-Expose-Headers";
     public static final String ACCESS_CONTROL_ALLOW_CREDENTIALS = "Access-Control-Allow-Credentials";
     public static final String ACCESS_CONTROL_MAX_AGE = "Access-Control-Max-Age";
 
+
     private HttpRequest request;
     private ResponseBuilder response;
     private Set<String> allowedOrigins;
-    private String[] allowedMethods;
+    private Set<String> allowedMethods;
+    private Set<String> exposedHeaders;
 
     private boolean preflight;
     private boolean auth;
@@ -61,12 +68,17 @@ public class Cors {
     }
 
     public Cors allowedMethods(String... allowedMethods) {
-        this.allowedMethods = allowedMethods;
+        this.allowedMethods = new HashSet<String>(Arrays.asList(allowedMethods));
+        return this;
+    }
+
+    public Cors exposedHeaders(String... exposedHeaders) {
+        this.exposedHeaders = new HashSet<String>(Arrays.asList(exposedHeaders));
         return this;
     }
 
     public Response build() {
-        String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN);
+        String origin = request.getHttpHeaders().getRequestHeaders().getFirst(ORIGIN_HEADER);
         if (origin == null) {
             return response.build();
         }
@@ -78,21 +90,20 @@ public class Cors {
         response.header(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
 
         if (allowedMethods != null) {
-            StringBuilder sb = new StringBuilder();
+            response.header(ACCESS_CONTROL_ALLOW_METHODS, CollectionUtil.join(allowedMethods));
-            for (int i = 0; i < allowedMethods.length; i++) {
-                if (i > 0) {
-                    sb.append(", ");
-                }
-                sb.append(allowedMethods[i]);
-            }
-            response.header(ACCESS_CONTROL_ALLOW_METHODS, sb.toString());
         } else {
             response.header(ACCESS_CONTROL_ALLOW_METHODS, DEFAULT_ALLOW_METHODS);
         }
 
+        if (exposedHeaders != null) {
+            response.header(ACCESS_CONTROL_EXPOSE_HEADERS, CollectionUtil.join(exposedHeaders));
+        }
+
         response.header(ACCESS_CONTROL_ALLOW_CREDENTIALS, Boolean.toString(auth));
         if (auth) {
-            response.header(ACCESS_CONTROL_ALLOW_HEADERS, "Authorization");
+            response.header(ACCESS_CONTROL_ALLOW_HEADERS, String.format("%s, %s", DEFAULT_ALLOW_HEADERS, AUTHORIZATION_HEADER));
+        } else {
+            response.header(ACCESS_CONTROL_ALLOW_HEADERS, DEFAULT_ALLOW_HEADERS);
         }
 
         response.header(ACCESS_CONTROL_MAX_AGE, DEFAULT_MAX_AGE);

services/src/main/java/org/keycloak/services/resources/TokenService.java

@@ -125,8 +125,7 @@ public class TokenService {
     }
 
     public static UriBuilder tokenServiceBaseUrl(UriBuilder baseUriBuilder) {
-        UriBuilder base = baseUriBuilder.path(RealmsResource.class).path(RealmsResource.class, "getTokenService");
+        return baseUriBuilder.path(RealmsResource.class).path(RealmsResource.class, "getTokenService");
-        return base;
     }
 
     public static UriBuilder accessCodeToTokenUrl(UriInfo uriInfo) {
@@ -294,7 +293,7 @@ public class TokenService {
 
         ClientModel client = authorizeClient(authorizationHeader, form, audit);
         String refreshToken = form.getFirst(OAuth2Constants.REFRESH_TOKEN);
-        AccessToken accessToken = null;
+        AccessToken accessToken;
         try {
             accessToken = tokenManager.refreshAccessToken(uriInfo, realm, client, refreshToken, audit);
         } catch (OAuthErrorException e) {
@@ -313,7 +312,7 @@ public class TokenService {
 
         audit.success();
 
-        return Cors.add(request, Response.ok(res, MediaType.APPLICATION_JSON_TYPE)).auth().allowedOrigins(client).allowedMethods("POST").build();
+        return Cors.add(request, Response.ok(res, MediaType.APPLICATION_JSON_TYPE)).auth().allowedOrigins(client).allowedMethods("POST").exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS).build();
     }
 
     @Path("auth/request/login")
@@ -501,7 +500,7 @@ public class TokenService {
             credentials.setValue(formData.getFirst("password"));
 
             boolean passwordUpdateSuccessful;
-            String passwordUpdateError = null;
+            String passwordUpdateError;
             try {
                 passwordUpdateSuccessful = AuthenticationProviderManager.getManager(realm, providerSession).updatePassword(user, formData.getFirst("password"));
                 passwordUpdateError = "Password update failed";
@@ -655,12 +654,12 @@ public class TokenService {
 
         audit.success();
 
-        return Cors.add(request, Response.ok(res)).auth().allowedOrigins(client).allowedMethods("POST").build();
+        return Cors.add(request, Response.ok(res)).auth().allowedOrigins(client).allowedMethods("POST").exposedHeaders(Cors.ACCESS_CONTROL_ALLOW_METHODS).build();
     }
 
     protected ClientModel authorizeClient(String authorizationHeader, MultivaluedMap<String, String> formData, Audit audit) {
-        String client_id = null;
+        String client_id;
-        String clientSecret = null;
+        String clientSecret;
         if (authorizationHeader != null) {
             String[] usernameSecret = BasicAuthHelper.parseHeader(authorizationHeader);
             if (usernameSecret == null) {
@@ -1005,11 +1004,7 @@ public class TokenService {
     }
 
     private boolean checkSsl() {
-        if (realm.isSslNotRequired()) {
+        return realm.isSslNotRequired() || uriInfo.getBaseUri().getScheme().equals("https");
-            return true;
-        }
-
-        return uriInfo.getBaseUri().getScheme().equals("https");
     }
 
 }