From 7d1f73104e123eb5b0ad87eb37bc0eb46b0e65bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Franti=C5=A1ek=20Bu=C4=8D=C3=ADk?= Date: Wed, 27 Jul 2022 08:47:31 +0200 Subject: [PATCH] feat: return error response on noAuthnContext --- .../src/main/webapp/WEB-INF/web-context.xml | 2 +- .../oidc/saml/ExtendedOAuth2Exception.java | 18 ++++ .../saml/PerunSamlAuthenticationProvider.java | 12 ++- .../oidc/saml/PerunSamlProcessingFilter.java | 48 ----------- ...nticationExceptionAuthenticationToken.java | 85 +++++++++++++++++++ .../SamlAuthenticationExceptionPrincipal.java | 37 ++++++++ .../oidc/server/filters/AuthProcFilter.java | 7 ++ .../ics/oidc/server/filters/FiltersUtils.java | 11 ++- .../oidc/web/controllers/LoginController.java | 9 +- .../request/ConnectOAuth2RequestFactory.java | 30 +++++++ .../token/TofuUserApprovalHandler.java | 7 +- .../web/interceptor/UserInfoInterceptor.java | 4 + 12 files changed, 214 insertions(+), 56 deletions(-) create mode 100644 perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/ExtendedOAuth2Exception.java delete mode 100644 perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlProcessingFilter.java create mode 100644 perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionAuthenticationToken.java create mode 100644 perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionPrincipal.java diff --git a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/web-context.xml b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/web-context.xml index c3be0a4b5..5af4aeb10 100644 --- a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/web-context.xml +++ b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/web-context.xml @@ -251,7 +251,7 @@ access="permitAll()"/> - + diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/ExtendedOAuth2Exception.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/ExtendedOAuth2Exception.java new file mode 100644 index 000000000..690a54ea1 --- /dev/null +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/ExtendedOAuth2Exception.java @@ -0,0 +1,18 @@ +package cz.muni.ics.oidc.saml; + +import org.springframework.security.oauth2.common.exceptions.OAuth2Exception; + +public class ExtendedOAuth2Exception extends OAuth2Exception { + + private final String errorCode; + + public ExtendedOAuth2Exception(String errorCode, String msg) { + super(msg); + this.errorCode = errorCode; + } + + @Override + public String getOAuth2ErrorCode() { + return errorCode; + } +} diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlAuthenticationProvider.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlAuthenticationProvider.java index 0e828f847..e05e079c4 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlAuthenticationProvider.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlAuthenticationProvider.java @@ -5,9 +5,10 @@ import java.util.Collection; import java.util.List; import lombok.extern.slf4j.Slf4j; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; -import org.springframework.security.core.userdetails.User; import org.springframework.security.saml.SAMLAuthenticationProvider; import org.springframework.security.saml.SAMLCredential; @@ -27,6 +28,15 @@ public PerunSamlAuthenticationProvider(List adminIds) { } } + @Override + public Authentication authenticate(Authentication authentication) throws AuthenticationException { + try { + return super.authenticate(authentication); + } catch (Exception e) { + return new SamlAuthenticationExceptionAuthenticationToken(e); + } + } + @Override protected Object getPrincipal(SAMLCredential credential, Object userDetail) { PerunUser user = (PerunUser) userDetail; diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlProcessingFilter.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlProcessingFilter.java deleted file mode 100644 index 0cb757248..000000000 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlProcessingFilter.java +++ /dev/null @@ -1,48 +0,0 @@ -package cz.muni.ics.oidc.saml; - -import java.io.IOException; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import lombok.extern.slf4j.Slf4j; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.AuthenticationException; -import org.springframework.security.saml.SAMLProcessingFilter; - -@Slf4j -public class PerunSamlProcessingFilter extends SAMLProcessingFilter { - - @Override - public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) - throws IOException, ServletException { - log.debug("doFilter"); - super.doFilter(req, res, chain); - } - - @Override - protected void successfulAuthentication(HttpServletRequest request, - HttpServletResponse response, FilterChain chain, - Authentication authResult) - throws IOException, ServletException { - log.debug("successful authentication"); - super.successfulAuthentication(request, response, chain, authResult); - } - - @Override - protected void unsuccessfulAuthentication(HttpServletRequest request, - HttpServletResponse response, - AuthenticationException failed) - throws IOException, ServletException { - log.debug("unsuccessfull authentication"); - super.unsuccessfulAuthentication(request, response, failed); - } - - @Override - public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response) throws AuthenticationException { - log.debug("Attempting authentication"); - return super.attemptAuthentication(request, response); - } -} diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionAuthenticationToken.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionAuthenticationToken.java new file mode 100644 index 000000000..fdf8e3046 --- /dev/null +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionAuthenticationToken.java @@ -0,0 +1,85 @@ +/* Copyright 2009 Vladimir Schäfer + * + * 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 + * + * https://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 cz.muni.ics.oidc.saml; + +import java.security.Principal; +import java.util.Collections; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import lombok.extern.slf4j.Slf4j; +import org.opensaml.common.SAMLException; +import org.opensaml.saml2.core.StatusCode; +import org.springframework.security.authentication.AbstractAuthenticationToken; +import org.springframework.security.authentication.InsufficientAuthenticationException; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.oauth2.common.exceptions.OAuth2Exception; +import org.springframework.security.saml.SAMLStatusException; + +@Getter +@ToString +@EqualsAndHashCode(callSuper = true) +@Slf4j +public class SamlAuthenticationExceptionAuthenticationToken extends AbstractAuthenticationToken { + + private static final Principal PRINCIPAL = new SamlAuthenticationExceptionPrincipal(); + public static final SimpleGrantedAuthority ROLE_EXCEPTION = new SimpleGrantedAuthority("ROLE_EXCEPTION"); + private final Exception causeException; + + public SamlAuthenticationExceptionAuthenticationToken(Exception causeException) { + super(Collections.singleton(ROLE_EXCEPTION)); + this.causeException = causeException; + } + + @Override + public Object getCredentials() { + return "EXCEPTION_IN_SAML_AUTHENTICATION"; + } + + @Override + public Object getPrincipal() { + return PRINCIPAL; + } + + @Override + public boolean isAuthenticated() { + return true; + } + + @Override + public void eraseCredentials() { } + + public OAuth2Exception createOAuth2Exception() { + if (causeException != null) { + Throwable t = causeException; + while (t.getCause() != null) { + log.warn("OAuth2 exception from SAML translation: {} - {}", t.getClass().getSimpleName(), t.getMessage()); + t = t.getCause(); + } + if (t instanceof InsufficientAuthenticationException) { + return new ExtendedOAuth2Exception("unmet_authentication_requirements", t.getMessage()); + } + if (t instanceof SAMLStatusException) { + String code = ((SAMLStatusException) t).getStatusCode(); + if (StatusCode.NO_AUTHN_CONTEXT_URI.equalsIgnoreCase(code)) { + return new ExtendedOAuth2Exception("unmet_authentication_requirements", t.getMessage()); + } + } + return new OAuth2Exception(t.getMessage()); + } + //TODO: handle + return new OAuth2Exception(""); + } +} diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionPrincipal.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionPrincipal.java new file mode 100644 index 000000000..6332f0668 --- /dev/null +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlAuthenticationExceptionPrincipal.java @@ -0,0 +1,37 @@ +/* Copyright 2009 Vladimir Schäfer + * + * 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 + * + * https://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 cz.muni.ics.oidc.saml; + +import java.security.Principal; +import java.util.Collections; +import javax.security.auth.Subject; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import org.springframework.security.authentication.AbstractAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; + +public class SamlAuthenticationExceptionPrincipal implements Principal { + + @Override + public String getName() { + return null; + } + + @Override + public boolean implies(Subject subject) { + return Principal.super.implies(subject); + } +} diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilter.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilter.java index 120e6a5c1..7d5a79712 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilter.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFilter.java @@ -1,6 +1,7 @@ package cz.muni.ics.oidc.server.filters; import cz.muni.ics.oidc.exceptions.ConfigurationException; +import cz.muni.ics.oidc.saml.SamlAuthenticationExceptionAuthenticationToken; import cz.muni.ics.oidc.saml.SamlProperties; import java.io.IOException; import java.util.Arrays; @@ -13,6 +14,8 @@ import javax.servlet.http.HttpSession; import lombok.Getter; import lombok.extern.slf4j.Slf4j; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; /** * Abstract class for Perun AuthProc filters. All filters defined and called in the @@ -117,6 +120,10 @@ private boolean skip(HttpServletRequest req) { } log.debug("{} - marking filter as applied", filterName); req.getSession(true).setAttribute(getSessionAppliedParamName(), true); + Authentication a = SecurityContextHolder.getContext().getAuthentication(); + if (a instanceof SamlAuthenticationExceptionAuthenticationToken) { + return true; + } String sub = FiltersUtils.getUserIdentifier(req, samlProperties.getUserIdentifierAttribute()); String clientId = FiltersUtils.getClientId(req); diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/FiltersUtils.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/FiltersUtils.java index adc2cc6c0..2aa29580e 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/FiltersUtils.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/FiltersUtils.java @@ -99,7 +99,6 @@ public static Map createRequestMap(Map paramet * @param clientService service fetching client details * @return extracted client, null if some error occurs */ - @SuppressWarnings("unchecked") public static ClientDetailsEntity extractClientFromRequest(HttpServletRequest request, OAuth2RequestFactory authRequestFactory, ClientDetailsEntityService clientService) @@ -199,11 +198,15 @@ public static PerunUser getPerunUserById(PerunAdapter perunAdapter, SAMLCredenti } public static SAMLCredential getSamlCredential(HttpServletRequest request) { - ExpiringUsernameAuthenticationToken p = (ExpiringUsernameAuthenticationToken) request.getUserPrincipal(); - if (p == null) { + if (request.getUserPrincipal() instanceof ExpiringUsernameAuthenticationToken) { + ExpiringUsernameAuthenticationToken p = (ExpiringUsernameAuthenticationToken) request.getUserPrincipal(); + if (p == null) { + return null; + } + return (SAMLCredential) p.getCredentials(); + } else { return null; } - return (SAMLCredential) p.getCredentials(); } public static String getExtLogin(SAMLCredential credential, String idAttribute) { diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/web/controllers/LoginController.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/web/controllers/LoginController.java index 5437b375e..2485b08a1 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/web/controllers/LoginController.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/web/controllers/LoginController.java @@ -2,6 +2,8 @@ import cz.muni.ics.oidc.server.configurations.PerunOidcConfig; import cz.muni.ics.oidc.web.WebHtmlClasses; +import cz.muni.ics.openid.connect.view.HttpCodeView; +import cz.muni.ics.openid.connect.view.JsonErrorView; import java.util.Map; import javax.servlet.http.HttpServletRequest; import lombok.extern.slf4j.Slf4j; @@ -9,6 +11,7 @@ import org.opensaml.ws.message.encoder.MessageEncodingException; import org.opensaml.xml.util.XMLHelper; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; import org.springframework.security.core.context.SecurityContextHolder; @@ -62,7 +65,11 @@ public String loginFailure(HttpServletRequest req, Map model) { if (exc != null) { String code = exc.getStatusCode(); if (StatusCode.NO_AUTHN_CONTEXT_URI.equalsIgnoreCase(code)) { - model.put(KEY_ERROR_MSG, "login_failure.no_authn_context.msg"); + model.put(HttpCodeView.CODE, HttpStatus.FORBIDDEN); + model.put(JsonErrorView.ERROR, "unmet_authentication_requirements"); + model.put(JsonErrorView.ERROR_MESSAGE, "Cannot log in. MFA has been requested and not performed"); + return JsonErrorView.VIEWNAME; + //model.put(KEY_ERROR_MSG, "login_failure.no_authn_context.msg"); } } } diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/request/ConnectOAuth2RequestFactory.java b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/request/ConnectOAuth2RequestFactory.java index 9de7129fd..a96d63ccb 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/request/ConnectOAuth2RequestFactory.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/request/ConnectOAuth2RequestFactory.java @@ -18,6 +18,8 @@ package cz.muni.ics.openid.connect.request; +import static cz.muni.ics.oidc.saml.SamlAuthenticationExceptionAuthenticationToken.ROLE_EXCEPTION; + import com.google.common.base.Strings; import com.google.gson.JsonElement; import com.google.gson.JsonObject; @@ -37,6 +39,7 @@ import cz.muni.ics.oauth2.model.ClientDetailsEntity; import cz.muni.ics.oauth2.model.PKCEAlgorithm; import cz.muni.ics.oauth2.service.ClientDetailsEntityService; +import cz.muni.ics.oidc.saml.SamlAuthenticationExceptionAuthenticationToken; import java.io.Serializable; import java.text.ParseException; import java.util.Collections; @@ -44,10 +47,15 @@ import java.util.Set; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.common.exceptions.InvalidClientException; import org.springframework.security.oauth2.common.exceptions.OAuth2Exception; import org.springframework.security.oauth2.common.util.OAuth2Utils; import org.springframework.security.oauth2.provider.AuthorizationRequest; +import org.springframework.security.oauth2.provider.ClientDetails; +import org.springframework.security.oauth2.provider.OAuth2Request; +import org.springframework.security.oauth2.provider.TokenRequest; import org.springframework.security.oauth2.provider.request.DefaultOAuth2RequestFactory; import org.springframework.stereotype.Component; @@ -151,6 +159,28 @@ public AuthorizationRequest createAuthorizationRequest(Map input return request; } + @Override + public OAuth2Request createOAuth2Request(AuthorizationRequest request) { + if (request.getAuthorities() != null && request.getAuthorities().contains(ROLE_EXCEPTION)) { + Authentication a = SecurityContextHolder.getContext().getAuthentication(); + if (a instanceof SamlAuthenticationExceptionAuthenticationToken) { + throw ((SamlAuthenticationExceptionAuthenticationToken) a).createOAuth2Exception(); + } + } + return super.createOAuth2Request(request); + } + + @Override + public TokenRequest createTokenRequest(AuthorizationRequest authorizationRequest, String grantType) { + if (authorizationRequest.getAuthorities() != null && authorizationRequest.getAuthorities().contains(ROLE_EXCEPTION)) { + Authentication a = SecurityContextHolder.getContext().getAuthentication(); + if (a instanceof SamlAuthenticationExceptionAuthenticationToken) { + throw ((SamlAuthenticationExceptionAuthenticationToken) a).createOAuth2Exception(); + } + } + return super.createTokenRequest(authorizationRequest, grantType); + } + /** * * @param jwtString diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/token/TofuUserApprovalHandler.java b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/token/TofuUserApprovalHandler.java index e700d1e12..b1d52e90e 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/token/TofuUserApprovalHandler.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/token/TofuUserApprovalHandler.java @@ -21,6 +21,7 @@ import com.google.common.base.Strings; import com.google.common.collect.Sets; import cz.muni.ics.oauth2.service.SystemScopeService; +import cz.muni.ics.oidc.saml.SamlAuthenticationExceptionAuthenticationToken; import cz.muni.ics.openid.connect.model.ApprovedSite; import cz.muni.ics.openid.connect.model.WhitelistedSite; import cz.muni.ics.openid.connect.request.ConnectRequestParameters; @@ -37,6 +38,7 @@ import javax.servlet.http.HttpSession; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.core.Authentication; +import org.springframework.security.oauth2.common.exceptions.OAuth2Exception; import org.springframework.security.oauth2.provider.AuthorizationRequest; import org.springframework.security.oauth2.provider.ClientDetails; import org.springframework.security.oauth2.provider.ClientDetailsService; @@ -114,7 +116,10 @@ public boolean isApproved(AuthorizationRequest authorizationRequest, Authenticat */ @Override public AuthorizationRequest checkForPreApproval(AuthorizationRequest authorizationRequest, Authentication userAuthentication) { - + if (userAuthentication instanceof SamlAuthenticationExceptionAuthenticationToken) { + SamlAuthenticationExceptionAuthenticationToken exc = (SamlAuthenticationExceptionAuthenticationToken) userAuthentication; + throw exc.createOAuth2Exception(); + } //First, check database to see if the user identified by the userAuthentication has stored an approval decision String userId = userAuthentication.getName(); diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/interceptor/UserInfoInterceptor.java b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/interceptor/UserInfoInterceptor.java index 9cfc4a88c..2cd77617e 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/interceptor/UserInfoInterceptor.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/interceptor/UserInfoInterceptor.java @@ -31,6 +31,7 @@ import cz.muni.ics.oauth2.model.SamlAuthenticationDetails; import cz.muni.ics.oauth2.model.SystemScope; import cz.muni.ics.oidc.models.PerunUser; +import cz.muni.ics.oidc.saml.SamlAuthenticationExceptionAuthenticationToken; import cz.muni.ics.oidc.server.configurations.PerunOidcConfig; import cz.muni.ics.openid.connect.model.OIDCAuthenticationToken; import cz.muni.ics.openid.connect.model.UserInfo; @@ -73,6 +74,9 @@ public class UserInfoInterceptor extends HandlerInterceptorAdapter { @Override public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { Authentication auth = SecurityContextHolder.getContext().getAuthentication(); + if (auth instanceof SamlAuthenticationExceptionAuthenticationToken) { + return true; + } if (auth != null){ request.setAttribute("userAuthorities", gson.toJson(auth.getAuthorities()));