Skip to content
This repository was archived by the owner on Sep 28, 2022. It is now read-only.

Commit bfb1433

Browse files
author
Dominik František Bučík
authored
Merge pull request #206 from dBucik/refactor_filters
refactor: Refactoring AuthProcFilters
2 parents 77d8100 + 677943d commit bfb1433

32 files changed

+662
-861
lines changed

perun-oidc-server-webapp/src/main/webapp/WEB-INF/web-context.xml

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@
252252
<security:intercept-url pattern="#{T(cz.muni.ics.oidc.web.controllers.LoginController).MAPPING_FAILURE}"
253253
access="permitAll()"/>
254254
<security:intercept-url pattern="/**" access="hasRole('ROLE_USER')"/>
255-
<security:custom-filter ref="mdcMuFilter" before="FIRST"/>
255+
<security:custom-filter ref="mdcFilter" before="FIRST"/>
256256
<security:custom-filter ref="metadataGeneratorFilter" before="CHANNEL_FILTER"/>
257257
<security:custom-filter ref="clearSessionFilter" after="CHANNEL_FILTER"/>
258258
<security:custom-filter ref="samlFilter" before="CSRF_FILTER"/>
@@ -337,8 +337,6 @@
337337
<property name="order" value="1" />
338338
</bean>
339339

340-
<bean id="mdcMuFilter" class="cz.muni.ics.oidc.server.filters.impl.MultiMDCFilter"/>
341-
342340
<!-- SAML -->
343341
<bean id="clearSessionFilter" class="cz.muni.ics.oidc.saml.SamlInvalidateSessionFilter">
344342
<constructor-arg name="contextLogoutHandler" ref="logoutHandler"/>
@@ -362,7 +360,7 @@
362360

363361
<bean id="successLogoutHandler" class="cz.muni.ics.oidc.saml.PerunOidcLogoutSuccessHandler">
364362
<property name="defaultTargetUrl" value="#{T(cz.muni.ics.oidc.web.controllers.LogoutController).MAPPING_SUCCESS}"/>
365-
<property name="targetUrlParameter" value="#{T(cz.muni.ics.oidc.server.filters.PerunFilterConstants).PARAM_TARGET}"/>
363+
<property name="targetUrlParameter" value="#{T(cz.muni.ics.oidc.server.filters.AuthProcFilterConstants).PARAM_TARGET}"/>
366364
</bean>
367365

368366
<bean id="logoutHandler" class="org.springframework.security.web.authentication.logout.SecurityContextLogoutHandler">

perun-oidc-server/src/main/java/cz/muni/ics/mdc/RemoteAddressMDCFilter.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ public class RemoteAddressMDCFilter {
2020
"REMOTE_ADDR"
2121
};
2222

23-
private static final String REMOTE_ADDR = "remoteAddr";
23+
private static final String REMOTE_ADDRESS = "remoteAddr";
2424

2525
public void doFilter(ServletRequest servletRequest) {
26-
MDC.put(REMOTE_ADDR, getRemoteAddr((HttpServletRequest) servletRequest));
26+
MDC.put(REMOTE_ADDRESS, getRemoteAddr((HttpServletRequest) servletRequest));
2727
}
2828

2929
private String getRemoteAddr(HttpServletRequest request) {
@@ -37,7 +37,7 @@ private String getRemoteAddr(HttpServletRequest request) {
3737
return ipList.split(",")[0];
3838
}
3939
}
40-
return "";
40+
return "-";
4141
}
4242

4343
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package cz.muni.ics.oidc;
2+
3+
public interface PerunConstants {
4+
5+
String REGISTRAR_TARGET_NEW = "targetnew";
6+
String REGISTRAR_TARGET_EXISTING = "targetexisting";
7+
String REGISTRAR_TARGET_EXTENDED = "targetextended";
8+
9+
String REGISTRAR_PARAM_VO = "vo";
10+
11+
}

perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunHTTPRedirectDeflateEncoder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import org.opensaml.xml.util.Pair;
88
import org.springframework.util.StringUtils;
99

10-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.AARC_IDP_HINT;
10+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.AARC_IDP_HINT;
1111

1212
public class PerunHTTPRedirectDeflateEncoder extends HTTPRedirectDeflateEncoder {
1313

perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunOidcLogoutSuccessHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package cz.muni.ics.oidc.saml;
22

3-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PARAM_POST_LOGOUT_REDIRECT_URI;
4-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PARAM_STATE;
3+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_POST_LOGOUT_REDIRECT_URI;
4+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_STATE;
55

66
import java.io.UnsupportedEncodingException;
77
import java.net.URLDecoder;

perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlEntryPoint.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
package cz.muni.ics.oidc.saml;
22

3-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.AARC_IDP_HINT;
4-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.CLIENT_ID_PREFIX;
5-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.EFILTER_PREFIX;
6-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.FILTER_PREFIX;
7-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.IDP_ENTITY_ID_PREFIX;
8-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PARAM_CLIENT_ID;
9-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PARAM_PROMPT;
10-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.REFEDS_MFA;
3+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.AARC_IDP_HINT;
4+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.CLIENT_ID_PREFIX;
5+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.EFILTER_PREFIX;
6+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.FILTER_PREFIX;
7+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.IDP_ENTITY_ID_PREFIX;
8+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_CLIENT_ID;
9+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_PROMPT;
10+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.REFEDS_MFA;
1111

1212
import cz.muni.ics.oidc.models.Facility;
1313
import cz.muni.ics.oidc.models.PerunAttributeValue;
1414
import cz.muni.ics.oidc.server.adapters.PerunAdapter;
1515
import cz.muni.ics.oidc.server.configurations.FacilityAttrsConfig;
1616
import cz.muni.ics.oidc.server.configurations.PerunOidcConfig;
17-
import cz.muni.ics.oidc.server.filters.PerunFilterConstants;
17+
import cz.muni.ics.oidc.server.filters.AuthProcFilterConstants;
1818
import java.io.IOException;
1919
import java.util.ArrayList;
2020
import java.util.Arrays;
@@ -160,12 +160,12 @@ private void processPrompt(HttpServletRequest request, WebSSOProfileOptions opti
160160
}
161161

162162
private void processAcrValues(HttpServletRequest request, WebSSOProfileOptions options) {
163-
String acrValues = request.getParameter(PerunFilterConstants.PARAM_ACR_VALUES);
163+
String acrValues = request.getParameter(AuthProcFilterConstants.PARAM_ACR_VALUES);
164164
log.debug("Processing acr_values parameter: {}", acrValues);
165165
List<String> acrs = convertAcrValuesToList(acrValues);
166166

167167
if (!hasAcrForcingIdp(acrs)) {
168-
String clientId = request.getParameter(PerunFilterConstants.PARAM_CLIENT_ID);
168+
String clientId = request.getParameter(AuthProcFilterConstants.PARAM_CLIENT_ID);
169169
String idpFilter = extractIdpFilterForRp(clientId);
170170
if (idpFilter != null) {
171171
log.debug("Added IdP filter as SAML AuthnContextClassRef ({})", idpFilter);

perun-oidc-server/src/main/java/cz/muni/ics/oidc/saml/PerunSamlUtils.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
package cz.muni.ics.oidc.saml;
22

3-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PARAM_ACR_VALUES;
4-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PARAM_FORCE_AUTHN;
5-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PARAM_PROMPT;
6-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PROMPT_LOGIN;
7-
import static cz.muni.ics.oidc.server.filters.PerunFilterConstants.PROMPT_SELECT_ACCOUNT;
3+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_ACR_VALUES;
4+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_FORCE_AUTHN;
5+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PARAM_PROMPT;
6+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PROMPT_LOGIN;
7+
import static cz.muni.ics.oidc.server.filters.AuthProcFilterConstants.PROMPT_SELECT_ACCOUNT;
88

9-
import cz.muni.ics.oidc.server.filters.PerunFilterConstants;
9+
import cz.muni.ics.oidc.server.filters.AuthProcFilterConstants;
1010
import javax.servlet.ServletRequest;
1111
import lombok.extern.slf4j.Slf4j;
1212
import org.springframework.util.StringUtils;
@@ -32,7 +32,7 @@ public static boolean needsReAuthByForceAuthn(ServletRequest request) {
3232
public static boolean needsReAuthByMfa(ServletRequest request) {
3333
String acrValues = request.getParameter(PARAM_ACR_VALUES);
3434
boolean res = StringUtils.hasText(acrValues)
35-
&& acrValues.contains(PerunFilterConstants.REFEDS_MFA);
35+
&& acrValues.contains(AuthProcFilterConstants.REFEDS_MFA);
3636
log.debug("requires reAuth by MFA acr - {}", res);
3737
return res;
3838
}
Lines changed: 101 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,95 @@
11
package cz.muni.ics.oidc.server.filters;
22

3+
import cz.muni.ics.oidc.exceptions.ConfigurationException;
4+
import cz.muni.ics.oidc.saml.SamlProperties;
35
import java.io.IOException;
4-
import java.security.Principal;
56
import java.util.Arrays;
7+
import java.util.Collections;
68
import java.util.HashSet;
9+
import java.util.List;
710
import java.util.Set;
811
import javax.servlet.http.HttpServletRequest;
912
import javax.servlet.http.HttpServletResponse;
1013
import javax.servlet.http.HttpSession;
14+
import lombok.Getter;
1115
import lombok.extern.slf4j.Slf4j;
1216

1317
/**
14-
* Abstract class for Perun filters. All filters called in CallPerunFiltersFilter has to extend this.
15-
*
16-
* Configuration of filter names:
17-
* <ul>
18-
* <li><b>filter.names</b> - comma separated list of names of the request filters</li>
19-
* </ul>
18+
* Abstract class for Perun AuthProc filters. All filters defined and called in the
19+
* {@link cz.muni.ics.oidc.server.filters.AuthProcFiltersContainer} instance have to extend this base class.
2020
*
2121
* Configuration of filter (replace [name] part with the name defined for the filter):
2222
* <ul>
2323
* <li><b>filter.[name].class</b> - Class the filter instantiates</li>
24-
* <li><b>filter.[name].subs</b> - comma separated list of sub values for which execution of filter will be skipped
25-
* if user's SUB is in the list</li>
26-
* <li><b>filter.[name].clientIds</b> - comma separated list of client_id values for which execution of filter
27-
* will be skipped if client_id is in the list</li>
24+
* <li><b>filter.[name].skip_for_users</b> - comma separated list of users for whom the execution of the filter
25+
* will be skipped if the users' SUB matches any value in the list</li>
26+
* <li><b>filter.[name].skip_for_clients</b> - comma separated list of clients for which the execution of the filter
27+
* will be skipped if the CLIENT_ID matches any value in the list</li>
28+
* <li><b>filter.[name].execute_for_users</b> - comma separated list of users for whom the filter will be executed
29+
* if the users' SUB matches any value in the list</li>
30+
* <li><b>filter.[name].execute_for_clients</b> - comma separated list of clients for whom the filter will be executed
31+
* if the CLIENT_ID matches any value in the list</li>
2832
* </ul>
33+
* <i>NOTE: if none of the SKIP/EXECUTE conditions is specified (or the lists are empty), filter is run for all users
34+
* and all clients</i>
2935
*
3036
* @see cz.muni.ics.oidc.server.filters.impl package for specific filters and their configuration
3137
*
3238
* @author Dominik Baranek <[email protected]>
3339
* @author Dominik Frantisek Bucik <[email protected]>
3440
*/
3541
@Slf4j
42+
@Getter
3643
public abstract class AuthProcFilter {
3744

45+
public static final String APPLIED = "APPLIED_";
46+
3847
private static final String DELIMITER = ",";
39-
private static final String CLIENT_IDS = "clientIds";
48+
private static final String EXECUTE = "execute";
49+
private static final String EXECUTE_FOR_CLIENTS = "execute_for_clients";
50+
private static final String EXECUTE_FOR_USERS = "execute_for_users";
51+
private static final String SKIP_FOR_CLIENTS = "skip_for_clients";
52+
private static final String SKIP_FOR_USERS = "skip_for_users";
4053
private static final String SUBS = "subs";
54+
private static final String CLIENT_IDS = "clientIds";
4155

4256
private final String filterName;
43-
private Set<String> clientIds = new HashSet<>();
44-
private Set<String> subs = new HashSet<>();
45-
46-
public AuthProcFilter(AuthProcFilterParams params) {
47-
filterName = params.getFilterName();
48-
49-
if (params.hasProperty(CLIENT_IDS)) {
50-
this.clientIds = new HashSet<>(Arrays.asList(params.getProperty(CLIENT_IDS).split(DELIMITER)));
57+
private final Set<String> executeForClients = new HashSet<>();
58+
private final Set<String> executeForUsers = new HashSet<>();
59+
private final Set<String> skipForClients = new HashSet<>();
60+
private final Set<String> skipForUsers = new HashSet<>();
61+
62+
private final SamlProperties samlProperties;
63+
64+
public AuthProcFilter(AuthProcFilterInitContext ctx) throws ConfigurationException {
65+
filterName = ctx.getFilterName();
66+
this.samlProperties = ctx.getBeanUtil().getBean(SamlProperties.class);
67+
initializeExecutionRulesLists(ctx);
68+
69+
if (!Collections.disjoint(executeForClients, skipForClients)) {
70+
throw new ConfigurationException("Filter '" + filterName + "' is configured to be run and skipped for the same client");
71+
} else if (!Collections.disjoint(executeForUsers, skipForUsers)) {
72+
throw new ConfigurationException("Filter '" + filterName + "' is configured to be run and skipped for the same user");
5173
}
5274

53-
if (params.hasProperty(SUBS)) {
54-
this.subs = new HashSet<>(Arrays.asList(params.getProperty(SUBS).split(DELIMITER)));
75+
log.info("{} - filter initialized", filterName);
76+
if (!skipForUsers.isEmpty()) {
77+
log.info("{} - skip execution for users with SUB in: '{}'", filterName, skipForUsers);
78+
}
79+
if (!skipForClients.isEmpty()) {
80+
log.info("{} - skip execution for clients with CLIENT_ID in: '{}'", filterName, skipForClients);
81+
}
82+
if (!executeForUsers.isEmpty()) {
83+
log.info("{} - execute for users with SUB in: '{}'", filterName, executeForUsers);
84+
}
85+
if (!executeForClients.isEmpty()) {
86+
log.info("{} - execute for clients with CLIENT_ID in: '{}'", filterName, executeForClients);
5587
}
56-
57-
log.debug("{} - filter initialized", filterName);
58-
log.debug("{} - skip execution for users with SUB in: {}", filterName, subs);
59-
log.debug("{} - skip execution for clients with CLIENT_ID in: {}", filterName, clientIds);
6088
}
6189

62-
protected abstract String getSessionAppliedParamName();
90+
protected String getSessionAppliedParamName() {
91+
return APPLIED + getClass().getSimpleName() + '_' + getFilterName();
92+
}
6393

6494
/**
6595
* In this method is done whole logic of filer
@@ -69,10 +99,10 @@ public AuthProcFilter(AuthProcFilterParams params) {
6999
* @return boolean if filter was successfully done
70100
* @throws IOException this exception could be thrown because of failed or interrupted I/O operation
71101
*/
72-
protected abstract boolean process(HttpServletRequest request, HttpServletResponse response, FilterParams params)
102+
protected abstract boolean process(HttpServletRequest request, HttpServletResponse response, AuthProcFilterCommonVars params)
73103
throws IOException;
74104

75-
public boolean doFilter(HttpServletRequest req, HttpServletResponse res, FilterParams params) throws IOException {
105+
public boolean doFilter(HttpServletRequest req, HttpServletResponse res, AuthProcFilterCommonVars params) throws IOException {
76106
if (!skip(req)) {
77107
log.trace("{} - executing filter", filterName);
78108
return process(req, res, params);
@@ -81,14 +111,18 @@ public boolean doFilter(HttpServletRequest req, HttpServletResponse res, FilterP
81111
}
82112
}
83113

84-
private boolean skip(HttpServletRequest request) {
85-
if (hasBeenApplied(request.getSession(true))) {
114+
private boolean skip(HttpServletRequest req) {
115+
if (hasBeenApplied(req.getSession(true))) {
86116
return true;
87117
}
88118
log.debug("{} - marking filter as applied", filterName);
89-
request.getSession(true).setAttribute(getSessionAppliedParamName(), true);
90-
return skipForSub(request.getUserPrincipal())
91-
|| skipForClientId(request.getParameter(PerunFilterConstants.PARAM_CLIENT_ID));
119+
req.getSession(true).setAttribute(getSessionAppliedParamName(), true);
120+
String sub = FiltersUtils.getUserIdentifier(req, samlProperties.getUserIdentifierAttribute());
121+
String clientId = FiltersUtils.getClientId(req);
122+
123+
boolean explicitExecution = executeForSub(sub) || executeForClientId(clientId);
124+
boolean explicitSkip = skipForClientId(clientId) || skipForSub(sub);
125+
return !explicitExecution && explicitSkip;
92126
}
93127

94128
private boolean hasBeenApplied(HttpSession sess) {
@@ -100,21 +134,45 @@ private boolean hasBeenApplied(HttpSession sess) {
100134
return false;
101135
}
102136

103-
private boolean skipForSub(Principal p) {
104-
String sub = (p != null) ? p.getName() : null;
105-
if (sub != null && subs.contains(sub)) {
106-
log.debug("{} - skip filter execution: matched one of the ignored SUBS ({})", filterName, sub);
107-
return true;
108-
}
109-
return false;
137+
private boolean executeForSub(String sub) {
138+
return checkRule(sub, executeForUsers, "{} - execute filter: matched one of the explicit SUBS ({})");
139+
}
140+
141+
private boolean executeForClientId(String clientId) {
142+
return checkRule(clientId, executeForClients, "{} - execute filter: matched one of the explicit CLIENT_IDS ({})");
143+
}
144+
145+
private boolean skipForSub(String sub) {
146+
return checkRule(sub, skipForUsers, "{} - skip filter execution: matched one of the ignored SUBS ({})");
110147
}
111148

112149
private boolean skipForClientId(String clientId) {
113-
if (clientId != null && clientIds.contains(clientId)){
114-
log.debug("{} - skip filter execution: matched one of the ignored CLIENT_IDS ({})", filterName, clientId);
150+
return checkRule(clientId, skipForClients, "{} - skip filter execution: matched one of the ignored CLIENT_IDS ({})");
151+
}
152+
153+
private boolean checkRule(String param, Set<String> ruleSet, String logMsg) {
154+
if (param != null && ruleSet.contains(param)){
155+
log.debug(logMsg, filterName, param);
115156
return true;
116157
}
117158
return false;
118159
}
119160

161+
private void initializeExecutionRulesLists(AuthProcFilterInitContext ctx) {
162+
initializeExecutionRuleList(ctx, EXECUTE_FOR_CLIENTS, executeForClients);
163+
initializeExecutionRuleList(ctx, SKIP_FOR_CLIENTS, skipForClients);
164+
initializeExecutionRuleList(ctx, CLIENT_IDS, skipForClients);
165+
166+
initializeExecutionRuleList(ctx, EXECUTE_FOR_USERS, executeForUsers);
167+
initializeExecutionRuleList(ctx, SKIP_FOR_USERS, skipForUsers);
168+
initializeExecutionRuleList(ctx, SUBS, skipForUsers);
169+
}
170+
171+
private void initializeExecutionRuleList(AuthProcFilterInitContext ctx, String property, Set<String> list) {
172+
if (ctx.hasProperty(property)) {
173+
String value = ctx.getProperty(property, "");
174+
list.addAll(Arrays.asList(value.split(DELIMITER)));
175+
}
176+
}
177+
120178
}

0 commit comments

Comments
 (0)