From c0db96df7d9a27f48bbca8c67e7c25f36189454f Mon Sep 17 00:00:00 2001 From: Dominik Frantisek Bucik <bucik@ics.muni.cz> Date: Thu, 27 Jan 2022 11:38:14 +0100 Subject: [PATCH] =?UTF-8?q?refactor:=20=F0=9F=92=A1=20Refactored=20device?= =?UTF-8?q?=20code=20auth?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../webapp/WEB-INF/views/approveDevice.jsp | 2 +- .../webapp/WEB-INF/views/requestUserCode.jsp | 2 +- .../WEB-INF/views/themedApproveDevice.jsp | 2 +- .../WEB-INF/views/themedRequestUserCode.jsp | 2 +- .../src/main/webapp/WEB-INF/web-context.xml | 6 +++-- .../oauth2/web/endpoint/DeviceEndpoint.java | 9 ++++--- .../saml/SamlInvalidateSessionFilter.java | 27 +++---------------- .../filters/AuthProcFiltersContainer.java | 10 +++---- .../server/filters/PerunFilterConstants.java | 3 +-- .../web/endpoint/AuthorizationEndpoint.java | 2 -- .../web/endpoint/UserDeviceEndpoint.java | 22 +++++++++++++++ 11 files changed, 44 insertions(+), 43 deletions(-) create mode 100644 perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/UserDeviceEndpoint.java diff --git a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/approveDevice.jsp b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/approveDevice.jsp index 4a5462ba5..dbcdc1108 100644 --- a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/approveDevice.jsp +++ b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/approveDevice.jsp @@ -37,7 +37,7 @@ </h1> <form name="confirmationForm" - action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }device/approved" method="post"> + action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }auth/device/approved" method="post"> <div class="row"> <div class="span5 offset1 well-small" style="text-align: left"> diff --git a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/requestUserCode.jsp b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/requestUserCode.jsp index df4dd18e2..94d1eaae7 100644 --- a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/requestUserCode.jsp +++ b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/requestUserCode.jsp @@ -39,7 +39,7 @@ </c:if> - <form action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }device/code" method="POST"> + <form action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }auth/device" method="POST"> <div class="row-fluid"> <div class="span12"> diff --git a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedApproveDevice.jsp b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedApproveDevice.jsp index 80b75a830..329c74fb6 100644 --- a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedApproveDevice.jsp +++ b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedApproveDevice.jsp @@ -33,7 +33,7 @@ <div id="content"> <c:remove scope="session" var="SPRING_SECURITY_LAST_EXCEPTION" /> <form name="confirmationForm" - action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }device/approved" method="post"> + action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }auth/device/approved" method="post"> <p> <c:if test="${not empty client.policyUri}"> <spring:message code="device_approve_privacy"/>${" "}<a target='_blank' href='${fn:escapeXml(client.policyUri)}'><em>${fn:escapeXml(client.clientName)}</em></a> diff --git a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedRequestUserCode.jsp b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedRequestUserCode.jsp index b3130f9f8..9b3ab9611 100644 --- a/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedRequestUserCode.jsp +++ b/perun-oidc-server-webapp/src/main/webapp/WEB-INF/views/themedRequestUserCode.jsp @@ -52,7 +52,7 @@ </c:choose> <form name="confirmationForm" class="mt-2" method="POST" - action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }device/code"> + action="${ config.issuer }${ config.issuer.endsWith('/') ? '' : '/' }auth/device"> <div class="row-fluid"> <div class="span12"> <div> 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 39d6253a2..d71d6200b 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 @@ -56,6 +56,7 @@ <mvc:exclude-mapping path="/#{T(cz.muni.ics.openid.connect.web.controller.GuiController).API_URL}/**" /> <mvc:exclude-mapping path="#{T(cz.muni.ics.oauth2.web.endpoint.DeviceEndpoint).ENDPOINT_URL}**" /> <mvc:exclude-mapping path="#{T(cz.muni.ics.oauth2.web.endpoint.DeviceEndpoint).REQUEST_USER_CODE_URL}**" /> + <mvc:exclude-mapping path="#{T(cz.muni.ics.oauth2.web.endpoint.DeviceEndpoint).REQUEST_USER_CODE_INIT_URL}**" /> <mvc:exclude-mapping path="#{T(cz.muni.ics.oauth2.web.endpoint.DeviceEndpoint).DEVICE_APPROVED_URL}**" /> <mvc:exclude-mapping path="/#{T(cz.muni.ics.oauth2.web.endpoint.IntrospectionEndpoint).URL}**" /> <mvc:exclude-mapping path="/#{T(cz.muni.ics.oauth2.web.endpoint.RevocationEndpoint).URL}**" /> @@ -83,10 +84,10 @@ <mvc:interceptor> <!-- Exclude APIs and other machine-facing endpoints from these interceptors --> <mvc:mapping path="/**" /> + <mvc:exclude-mapping path="/token**"/> + <mvc:exclude-mapping path="/resources/**" /> <mvc:exclude-mapping path="/#{T(cz.muni.ics.openid.connect.web.endpoint.JWKSetPublishingEndpoint).URL}**" /> <mvc:exclude-mapping path="/#{T(cz.muni.ics.discovery.web.DiscoveryEndpoint).WELL_KNOWN_URL}/**" /> - <mvc:exclude-mapping path="/resources/**" /> - <mvc:exclude-mapping path="/token**"/> <mvc:exclude-mapping path="/#{T(cz.muni.ics.openid.connect.web.endpoint.DynamicClientRegistrationEndpoint).URL}/**" /> <mvc:exclude-mapping path="/#{T(cz.muni.ics.openid.connect.web.endpoint.ProtectedResourceRegistrationEndpoint).URL}/**" /> <mvc:exclude-mapping path="/#{T(cz.muni.ics.openid.connect.web.endpoint.UserInfoEndpoint).URL}**" /> @@ -241,6 +242,7 @@ authentication-manager-ref="authenticationManager"> <security:csrf disabled="true"/> <security:intercept-url pattern="/authorize" access="permitAll()"/> + <security:intercept-url pattern="/device" access="permitAll()"/> <security:intercept-url pattern="/saml/**" access="permitAll()"/> <security:intercept-url pattern="/logout" access="permitAll()"/> <security:intercept-url pattern="#{T(cz.muni.ics.oidc.web.controllers.LogoutController).MAPPING_SUCCESS}" access="permitAll()"/> diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oauth2/web/endpoint/DeviceEndpoint.java b/perun-oidc-server/src/main/java/cz/muni/ics/oauth2/web/endpoint/DeviceEndpoint.java index 019505f54..27036e3c7 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oauth2/web/endpoint/DeviceEndpoint.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oauth2/web/endpoint/DeviceEndpoint.java @@ -120,9 +120,10 @@ public class DeviceEndpoint { // other public static final String DEFAULT = "default"; public static final String ENDPOINT_URL = "/devicecode"; - public static final String REQUEST_USER_CODE_URL = "/device/code"; - public static final String CHECK_USER_CODE_URL = "/device/checkcode"; - public static final String DEVICE_APPROVED_URL = "/device/approved"; + public static final String REQUEST_USER_CODE_INIT_URL = "/device"; + public static final String REQUEST_USER_CODE_URL = "/auth/device"; + public static final String CHECK_USER_CODE_URL = "/auth/device/authorize"; + public static final String DEVICE_APPROVED_URL = "/auth/device/approved"; private final ClientDetailsEntityService clientService; private final SystemScopeService scopeService; @@ -184,7 +185,7 @@ public class DeviceEndpoint { if (StringUtils.hasText(acrValues)) { uriParams.put(ACR_VALUES, acrValues); } - String uriBase = perunOidcConfig.getConfigBean().getIssuer(false) + REQUEST_USER_CODE_URL; + String uriBase = perunOidcConfig.getConfigBean().getIssuer(false) + REQUEST_USER_CODE_INIT_URL; response.put(VERIFICATION_URI, constructVerificationURI(uriBase, uriParams)); if (perunOidcConfig.getConfigBean().isAllowCompleteDeviceCodeUri()) { diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlInvalidateSessionFilter.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlInvalidateSessionFilter.java index df523801b..caa993771 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlInvalidateSessionFilter.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/SamlInvalidateSessionFilter.java @@ -1,9 +1,5 @@ package cz.muni.ics.oidc.saml; -import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.AUTHORIZE_REQ_PATTERN; -import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.DEVICE_APPROVE_REQ_PATTERN; -import static org.springframework.http.HttpHeaders.REFERER; - import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -26,14 +22,10 @@ import org.springframework.web.filter.GenericFilterBean; @Slf4j public class SamlInvalidateSessionFilter extends GenericFilterBean { - private static final RequestMatcher AUTHORIZE_MATCHER = new AntPathRequestMatcher(AUTHORIZE_REQ_PATTERN); - private static final RequestMatcher AUTHORIZE_ALL_MATCHER = new AntPathRequestMatcher(AUTHORIZE_REQ_PATTERN + "/**"); - private static final RequestMatcher DEVICE_CODE_MATCHER = new AntPathRequestMatcher(DEVICE_APPROVE_REQ_PATTERN); - private static final RequestMatcher DEVICE_CODE_ALL_MATCHER = new AntPathRequestMatcher(DEVICE_APPROVE_REQ_PATTERN + "/**"); private static final RequestMatcher MATCHER = new OrRequestMatcher( - Arrays.asList(AUTHORIZE_MATCHER, AUTHORIZE_ALL_MATCHER, DEVICE_CODE_MATCHER, DEVICE_CODE_ALL_MATCHER)); - - public static final RequestMatcher MATCH = new AntPathRequestMatcher("/authorize"); + new AntPathRequestMatcher("/authorize"), + new AntPathRequestMatcher("/device") + ); private final SecurityContextLogoutHandler contextLogoutHandler; private final List<String> internalReferrers = new ArrayList<>(); @@ -73,24 +65,13 @@ public class SamlInvalidateSessionFilter extends GenericFilterBean { { HttpServletRequest req = (HttpServletRequest) request; HttpServletResponse res = (HttpServletResponse) response; - if (MATCH.matches(req)) { + if (MATCHER.matches(req)) { log.debug("INV_SESS - invalidate"); contextLogoutHandler.logout(req, res, null); } else { log.debug("INV_SESS - skipping"); } chain.doFilter(req, res); - -// HttpServletRequest req = (HttpServletRequest) request; -// HttpServletResponse res = (HttpServletResponse) response; -// if (MATCHER.matches(req)) { -// String referer = req.getHeader(REFERER); -// if (!isInternalReferer(referer)) { -// log.debug("Got external referer, clear session to reauthenticate"); -// contextLogoutHandler.logout(req, res, null); -// } -// } -// chain.doFilter(req, res); } private boolean isInternalReferer(String referer) { diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFiltersContainer.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFiltersContainer.java index 2faea8d35..eaa6ed5b7 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFiltersContainer.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/AuthProcFiltersContainer.java @@ -1,7 +1,7 @@ package cz.muni.ics.oidc.server.filters; import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.AUTHORIZE_REQ_PATTERN; -import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.DEVICE_CHECK_CODE_REQ_PATTERN; +import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.DEVICE_APPROVE_REQ_PATTERN; import cz.muni.ics.oauth2.model.ClientDetailsEntity; import cz.muni.ics.oauth2.service.ClientDetailsEntityService; @@ -16,7 +16,6 @@ import java.util.List; import java.util.Properties; import javax.annotation.PostConstruct; import javax.servlet.FilterChain; -import javax.servlet.GenericFilter; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; @@ -42,8 +41,8 @@ public class AuthProcFiltersContainer extends GenericFilterBean { private static final RequestMatcher AUTHORIZE_MATCHER = new AntPathRequestMatcher(AUTHORIZE_REQ_PATTERN); private static final RequestMatcher AUTHORIZE_ALL_MATCHER = new AntPathRequestMatcher(AUTHORIZE_REQ_PATTERN + "/**"); - private static final RequestMatcher DEVICE_CODE_MATCHER = new AntPathRequestMatcher(DEVICE_CHECK_CODE_REQ_PATTERN); - private static final RequestMatcher DEVICE_CODE_ALL_MATCHER = new AntPathRequestMatcher(DEVICE_CHECK_CODE_REQ_PATTERN + "/**"); + private static final RequestMatcher DEVICE_CODE_MATCHER = new AntPathRequestMatcher(DEVICE_APPROVE_REQ_PATTERN); + private static final RequestMatcher DEVICE_CODE_ALL_MATCHER = new AntPathRequestMatcher(DEVICE_APPROVE_REQ_PATTERN + "/**"); private static final RequestMatcher MATCHER = new OrRequestMatcher( Arrays.asList(AUTHORIZE_MATCHER, AUTHORIZE_ALL_MATCHER, DEVICE_CODE_MATCHER, DEVICE_CODE_ALL_MATCHER)); @@ -79,8 +78,7 @@ public class AuthProcFiltersContainer extends GenericFilterBean { HttpServletRequest req = (HttpServletRequest) servletRequest; HttpServletResponse res = (HttpServletResponse) servletResponse; if (!MATCHER.matches(req)) { - log.debug("Custom filters have been skipped, did not match '{}' nor '{}' request", AUTHORIZE_MATCHER, - AUTHORIZE_REQ_PATTERN); + log.debug("Custom filters have been skipped, did not match authorization nor device req URL"); } else { List<AuthProcFilter> filters = perunFiltersContext.getFilters(); if (filters != null && !filters.isEmpty()) { diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/PerunFilterConstants.java b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/PerunFilterConstants.java index 64e494956..d623a97ee 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/PerunFilterConstants.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/oidc/server/filters/PerunFilterConstants.java @@ -12,8 +12,7 @@ import java.util.Map; public class PerunFilterConstants { public static final String AUTHORIZE_REQ_PATTERN = "/auth/authorize"; - public static final String DEVICE_APPROVE_REQ_PATTERN = "/device/code"; - public static final String DEVICE_CHECK_CODE_REQ_PATTERN = "/device/checkcode"; + public static final String DEVICE_APPROVE_REQ_PATTERN = "/auth/device/authorize"; public static final String PARAM_CLIENT_ID = "client_id"; public static final String PARAM_SCOPE = "scope"; diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/AuthorizationEndpoint.java b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/AuthorizationEndpoint.java index f2d6022bf..050b07052 100644 --- a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/AuthorizationEndpoint.java +++ b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/AuthorizationEndpoint.java @@ -19,6 +19,4 @@ public class AuthorizationEndpoint { return view; } - //TODO: handle also device endpoint - } diff --git a/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/UserDeviceEndpoint.java b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/UserDeviceEndpoint.java new file mode 100644 index 000000000..81f20091c --- /dev/null +++ b/perun-oidc-server/src/main/java/cz/muni/ics/openid/connect/web/endpoint/UserDeviceEndpoint.java @@ -0,0 +1,22 @@ +package cz.muni.ics.openid.connect.web.endpoint; + +import javax.servlet.http.HttpServletRequest; +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Controller; +import org.springframework.util.StringUtils; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.servlet.view.RedirectView; + +@Controller +@Slf4j +public class UserDeviceEndpoint { + + @RequestMapping(value = "/device") + public RedirectView authorize(HttpServletRequest req) { + String redirect = "/auth/device" + (StringUtils.hasText(req.getQueryString()) ? '?' + req.getQueryString() : ""); + RedirectView view = new RedirectView(redirect); + view.setContextRelative(true); + log.debug("DEVICE_ENDPOINT: Redirecting to: {}", view); + return view; + } +}