back ported prompt behavior to 1.1, closes #810, addresses #667

openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright 2014 The MITRE Corporation
+ * Copyright 2015 The MITRE Corporation
  *   and the MIT Kerberos and Internet Trust Consortium
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
@@ -13,14 +13,13 @@
  * 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.mitre.openid.connect.web;
 
 import java.lang.reflect.Type;
-import java.security.Principal;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -31,10 +30,13 @@ import org.mitre.openid.connect.service.UserInfoService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
+import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.web.servlet.ModelAndView;
 import org.springframework.web.servlet.handler.HandlerInterceptorAdapter;
 import org.springframework.web.servlet.view.RedirectView;
+import org.springframework.web.servlet.view.UrlBasedViewResolver;
 
+import com.google.common.base.Strings;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonElement;
@@ -69,19 +71,36 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter {
 			// or if there's already a userInfo object in there
 
 			// TODO: this is a patch to get around a potential information leak from #492
-			if (!(modelAndView.getView() instanceof RedirectView)) {
 
-				// get our principal from the security context
+			if (modelAndView.getView() instanceof RedirectView) {
-				Principal p = request.getUserPrincipal();
+				// don't add them
+			} else {
+				if (Strings.isNullOrEmpty(modelAndView.getViewName())) {
+					// add them
+					injectUserInfo(modelAndView);
+				} else {
+					if (modelAndView.getViewName().startsWith(UrlBasedViewResolver.FORWARD_URL_PREFIX) || 
+							modelAndView.getViewName().startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) {
+						// don't add them
+					} else {
+						// add them
+						injectUserInfo(modelAndView);
+					}
+				}
+			}
+		}
+	}
 
-				if (p instanceof Authentication && !modelAndView.getModel().containsKey("userAuthorities")){
+	private void injectUserInfo(ModelAndView modelAndView) {
-					Authentication auth = (Authentication)p;
+		Authentication auth = SecurityContextHolder.getContext().getAuthentication();
+
+		if (auth instanceof Authentication && !modelAndView.getModel().containsKey("userAuthorities")){
 			modelAndView.addObject("userAuthorities", gson.toJson(auth.getAuthorities()));
 		}
 
-				if (p instanceof OIDCAuthenticationToken) {
+		if (auth instanceof OIDCAuthenticationToken) {
 			// if they're logging into this server from a remote OIDC server, pass through their user info
-					OIDCAuthenticationToken oidc = (OIDCAuthenticationToken) p;
+			OIDCAuthenticationToken oidc = (OIDCAuthenticationToken) auth;
 			if (oidc.getUserInfo() != null) {
 				modelAndView.addObject("userInfo", oidc.getUserInfo());
 				modelAndView.addObject("userInfoJson", oidc.getUserInfo().toJson());
@@ -91,10 +110,10 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter {
 			}
 		} else {
 			// don't bother checking if we don't have a principal or a userInfoService to work with
-					if (p != null && p.getName() != null && userInfoService != null) { 
+			if (auth != null && auth.getName() != null && userInfoService != null) {
 
 				// try to look up a user based on the principal's name
-						UserInfo user = userInfoService.getByUsername(p.getName());
+				UserInfo user = userInfoService.getByUsername(auth.getName());
 
 				// if we have one, inject it so views can use it
 				if (user != null) {
@@ -104,11 +123,5 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter {
 			}
 		}
 	}
-		}
-
-	}
-
-
-
 
 }

openid-connect-server/src/main/java/org/mitre/oauth2/web/OAuthConfirmationController.java

@@ -19,6 +19,7 @@
  */
 package org.mitre.oauth2.web;
 
+import java.net.URISyntaxException;
 import java.security.Principal;
 import java.util.Date;
 import java.util.HashMap;
@@ -27,6 +28,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.http.client.utils.URIBuilder;
 import org.mitre.oauth2.model.ClientDetailsEntity;
 import org.mitre.oauth2.model.SystemScope;
 import org.mitre.oauth2.service.ClientDetailsEntityService;
@@ -43,10 +45,12 @@ import org.springframework.http.HttpStatus;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.oauth2.common.exceptions.OAuth2Exception;
 import org.springframework.security.oauth2.provider.AuthorizationRequest;
+import org.springframework.security.oauth2.provider.endpoint.RedirectResolver;
 import org.springframework.stereotype.Controller;
 import org.springframework.web.bind.annotation.ModelAttribute;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.SessionAttributes;
+import org.springframework.web.servlet.mvc.support.RedirectAttributes;
 
 import com.google.common.base.Joiner;
 import com.google.common.base.Splitter;
@@ -77,6 +81,9 @@ public class OAuthConfirmationController {
 	@Autowired
 	private StatsService statsService;
 	
+	@Autowired
+	private RedirectResolver redirectResolver;
+
 	private static Logger logger = LoggerFactory.getLogger(OAuthConfirmationController.class);
 
 	public OAuthConfirmationController() {
@@ -90,23 +97,10 @@ public class OAuthConfirmationController {
 	@PreAuthorize("hasRole('ROLE_USER')")
 	@RequestMapping("/oauth/confirm_access")
 	public String confimAccess(Map<String, Object> model, @ModelAttribute("authorizationRequest") AuthorizationRequest authRequest,
-			Principal p) {
+			Principal p, RedirectAttributes ra) {
 
 		// Check the "prompt" parameter to see if we need to do special processing
 
-		String prompt = (String)authRequest.getExtensions().get("prompt");
-		List<String> prompts = Splitter.on(" ").splitToList(Strings.nullToEmpty(prompt));
-		if (prompts.contains("none")) {
-			// we're not supposed to prompt, so "return an error"
-			logger.info("Client requested no prompt, returning 403 from confirmation endpoint");
-			model.put("code", HttpStatus.FORBIDDEN);
-			return HttpCodeView.VIEWNAME;
-		}
-		
-		if (prompts.contains("consent")) {
-			model.put("consent", true);
-		}
-
 		//AuthorizationRequest clientAuth = (AuthorizationRequest) model.remove("authorizationRequest");
 
 		ClientDetailsEntity client = null;
@@ -123,6 +117,34 @@ public class OAuthConfirmationController {
 			return HttpCodeView.VIEWNAME;
 		}
 
+		String prompt = (String)authRequest.getExtensions().get("prompt");
+		List<String> prompts = Splitter.on(" ").splitToList(Strings.nullToEmpty(prompt));
+		if (prompts.contains("none")) {
+			// if we've got a redirect URI then we'll send it 
+			
+			String url = redirectResolver.resolveRedirect(authRequest.getRedirectUri(), client);
+			
+			try {
+				URIBuilder uriBuilder = new URIBuilder(url);
+			
+				uriBuilder.addParameter("error", "interaction_required");
+				if (!Strings.isNullOrEmpty(authRequest.getState())) {
+					uriBuilder.addParameter("state", authRequest.getState()); // copy the state parameter if one was given
+				}
+
+				return "redirect:" + uriBuilder.toString();
+				
+			} catch (URISyntaxException e) {
+				logger.error("Can't build redirect URI for prompt=none, sending error instead", e);
+				model.put("code", HttpStatus.FORBIDDEN);
+				return HttpCodeView.VIEWNAME;
+			}
+		}
+		
+		if (prompts.contains("consent")) {
+			model.put("consent", true);
+		}
+		
 		if (client == null) {
 			logger.error("confirmAccess: could not find client " + authRequest.getClientId());
 			model.put("code", HttpStatus.NOT_FOUND);

openid-connect-server/src/main/java/org/mitre/openid/connect/filter/PromptFilter.java

@@ -20,6 +20,7 @@
 package org.mitre.openid.connect.filter;
 
 import java.io.IOException;
+import java.net.URISyntaxException;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
@@ -33,6 +34,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
+import org.apache.http.client.utils.URIBuilder;
 import org.mitre.oauth2.model.ClientDetailsEntity;
 import org.mitre.oauth2.service.ClientDetailsEntityService;
 import org.mitre.openid.connect.web.AuthenticationTimeStamper;
@@ -44,6 +46,7 @@ import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.security.oauth2.common.exceptions.InvalidClientException;
 import org.springframework.security.oauth2.provider.AuthorizationRequest;
 import org.springframework.security.oauth2.provider.OAuth2RequestFactory;
+import org.springframework.security.oauth2.provider.endpoint.RedirectResolver;
 import org.springframework.stereotype.Component;
 import org.springframework.web.filter.GenericFilterBean;
 
@@ -68,6 +71,9 @@ public class PromptFilter extends GenericFilterBean {
 	@Autowired
 	private ClientDetailsEntityService clientService;
 
+	@Autowired
+	private RedirectResolver redirectResolver;
+
 	/**
 	 * 
 	 */
@@ -102,17 +108,41 @@ public class PromptFilter extends GenericFilterBean {
 			List<String> prompts = Splitter.on(" ").splitToList(Strings.nullToEmpty(prompt));
 
 			if (prompts.contains("none")) {
-				logger.info("Client requested no prompt");
 				// see if the user's logged in
 				Authentication auth = SecurityContextHolder.getContext().getAuthentication();
 
 				if (auth != null) {
 					// user's been logged in already (by session management)
-					// we're OK, continue without prompting
+					// we're OK, continue without prompting (in case the user already has a saved state)
 					chain.doFilter(req, res);
 				} else {
+					logger.info("Client requested no prompt");
 					// user hasn't been logged in, we need to "return an error"
-					logger.info("User not logged in, no prompt requested, returning 403 from filter");
+					if (client != null && authRequest.getRedirectUri() != null) {
+						
+						// if we've got a redirect URI then we'll send it 
+						
+						String url = redirectResolver.resolveRedirect(authRequest.getRedirectUri(), client);
+						
+						try {
+							URIBuilder uriBuilder = new URIBuilder(url);
+						
+							uriBuilder.addParameter("error", "login_required");
+							if (!Strings.isNullOrEmpty(authRequest.getState())) {
+								uriBuilder.addParameter("state", authRequest.getState()); // copy the state parameter if one was given
+							}
+							
+							response.sendRedirect(uriBuilder.toString());
+							return;
+							
+						} catch (URISyntaxException e) {
+							logger.error("Can't build redirect URI for prompt=none, sending error instead", e);
+							response.sendError(HttpServletResponse.SC_FORBIDDEN, "Access Denied");
+							return;
+						}
+					}
+					
+					logger.error("Can't build redirect URI for prompt=none, sending error instead");
 					response.sendError(HttpServletResponse.SC_FORBIDDEN, "Access Denied");
 					return;
 				}