From 5624c12232238721b7f0e6599c688dcd8c52e349 Mon Sep 17 00:00:00 2001 From: Justin Richer Date: Wed, 27 May 2015 12:12:01 -0400 Subject: [PATCH] back ported prompt behavior to 1.1, closes #810, addresses #667 --- .../connect/web/UserInfoInterceptor.java | 87 +++++++++++-------- .../web/OAuthConfirmationController.java | 50 ++++++++--- .../openid/connect/filter/PromptFilter.java | 36 +++++++- 3 files changed, 119 insertions(+), 54 deletions(-) diff --git a/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java b/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java index cf1bc7514..f0e55ffc4 100644 --- a/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java +++ b/openid-connect-common/src/main/java/org/mitre/openid/connect/web/UserInfoInterceptor.java @@ -1,26 +1,25 @@ /******************************************************************************* - * 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"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * 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,46 +71,57 @@ 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 - Principal p = request.getUserPrincipal(); - - if (p instanceof Authentication && !modelAndView.getModel().containsKey("userAuthorities")){ - Authentication auth = (Authentication)p; - modelAndView.addObject("userAuthorities", gson.toJson(auth.getAuthorities())); - } - - if (p instanceof OIDCAuthenticationToken) { - // if they're logging into this server from a remote OIDC server, pass through their user info - OIDCAuthenticationToken oidc = (OIDCAuthenticationToken) p; - if (oidc.getUserInfo() != null) { - modelAndView.addObject("userInfo", oidc.getUserInfo()); - modelAndView.addObject("userInfoJson", oidc.getUserInfo().toJson()); - } else { - modelAndView.addObject("userInfo", null); - modelAndView.addObject("userInfoJson", "null"); - } + if (modelAndView.getView() instanceof RedirectView) { + // don't add them + } else { + if (Strings.isNullOrEmpty(modelAndView.getViewName())) { + // add them + injectUserInfo(modelAndView); } 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) { - - // try to look up a user based on the principal's name - UserInfo user = userInfoService.getByUsername(p.getName()); - - // if we have one, inject it so views can use it - if (user != null) { - modelAndView.addObject("userInfo", user); - modelAndView.addObject("userInfoJson", user.toJson()); - } + if (modelAndView.getViewName().startsWith(UrlBasedViewResolver.FORWARD_URL_PREFIX) || + modelAndView.getViewName().startsWith(UrlBasedViewResolver.REDIRECT_URL_PREFIX)) { + // don't add them + } else { + // add them + injectUserInfo(modelAndView); } } } } - } + private void injectUserInfo(ModelAndView modelAndView) { + Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + if (auth instanceof Authentication && !modelAndView.getModel().containsKey("userAuthorities")){ + modelAndView.addObject("userAuthorities", gson.toJson(auth.getAuthorities())); + } + if (auth instanceof OIDCAuthenticationToken) { + // if they're logging into this server from a remote OIDC server, pass through their user info + OIDCAuthenticationToken oidc = (OIDCAuthenticationToken) auth; + if (oidc.getUserInfo() != null) { + modelAndView.addObject("userInfo", oidc.getUserInfo()); + modelAndView.addObject("userInfoJson", oidc.getUserInfo().toJson()); + } else { + modelAndView.addObject("userInfo", null); + modelAndView.addObject("userInfoJson", "null"); + } + } else { + // don't bother checking if we don't have a principal or a userInfoService to work with + if (auth != null && auth.getName() != null && userInfoService != null) { + + // try to look up a user based on the principal's name + UserInfo user = userInfoService.getByUsername(auth.getName()); + + // if we have one, inject it so views can use it + if (user != null) { + modelAndView.addObject("userInfo", user); + modelAndView.addObject("userInfoJson", user.toJson()); + } + } + } + } } diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/web/OAuthConfirmationController.java b/openid-connect-server/src/main/java/org/mitre/oauth2/web/OAuthConfirmationController.java index 499466d6a..d8b2da89e 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/web/OAuthConfirmationController.java +++ b/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; @@ -76,6 +80,9 @@ public class OAuthConfirmationController { @Autowired private StatsService statsService; + + @Autowired + private RedirectResolver redirectResolver; private static Logger logger = LoggerFactory.getLogger(OAuthConfirmationController.class); @@ -90,23 +97,10 @@ public class OAuthConfirmationController { @PreAuthorize("hasRole('ROLE_USER')") @RequestMapping("/oauth/confirm_access") public String confimAccess(Map 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 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 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); diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/filter/PromptFilter.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/filter/PromptFilter.java index 23da6f8aa..4843caba6 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/filter/PromptFilter.java +++ b/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 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; }