[
https://issues.apache.org/jira/browse/KNOX-2393?focusedWorklogId=448009&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-448009
]
ASF GitHub Bot logged work on KNOX-2393:
----------------------------------------
Author: ASF GitHub Bot
Created on: 18/Jun/20 20:05
Start Date: 18/Jun/20 20:05
Worklog Time Spent: 10m
Work Description: pzampino commented on a change in pull request #349:
URL: https://github.com/apache/knox/pull/349#discussion_r442471941
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java
##########
@@ -92,6 +100,18 @@ public void init( FilterConfig filterConfig ) throws
ServletException {
publicKey = CertificateUtils.parseRSAPublicKey(verificationPEM);
}
+ /* get unauthenticated paths list */
+ String unAuthPathString =
filterConfig.getInitParameter(SSO_UNAUTHENTICATED_PATHS_PARAM);
+ /* if no list specified use default value */
+ if (StringUtils.isBlank(unAuthPathString)) {
+ unAuthPathString = DEFAULT_SSO_UNAUTHENTICATED_PATHS_PARAM;
+ }
+
+ final StringTokenizer st = new StringTokenizer(unAuthPathString, ";");
Review comment:
Could we support ',' in addition to ';' as a delimiter here? I've
personally been burned by using the incorrect delimiter in some provide config
property values.
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/JWTMessages.java
##########
@@ -55,4 +55,7 @@
@Message( level = MessageLevel.DEBUG, text = "Audience claim has been
validated." )
void jwtAudienceValidated();
+
+ @Message( level = MessageLevel.INFO, text = "Path {0} is configured as
unauthenticated path, letting the request {1} through" )
+ void unAuthenticatePathBypass(String path, String uri);
Review comment:
nit: unauthenticatedPathBypass (unauthenticated is a whole word)
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java
##########
@@ -121,13 +141,25 @@ public void doFilter(ServletRequest request,
ServletResponse response, FilterCha
List<Cookie> ssoCookies = CookieUtils.getCookiesForName(req, cookieName);
if (ssoCookies.isEmpty()) {
+ /* check for unauthenticated paths to bypass */
+ for (final String path : unAuthenticatedPaths) {
+ if (req.getRequestURI().contains(path)) {
+ /* This path is configured as an unauthenticated path let the
request through */
+ final Subject sub = new Subject();
+ sub.getPrincipals().add(new PrimaryPrincipal("anonymous"));
+ LOGGER.unAuthenticatePathBypass(path, req.getRequestURI());
+ continueWithEstablishedSecurityContext(sub, req, res, chain);
+ }
+ }
+
if (req.getMethod().equals("OPTIONS")) {
// CORS preflight requests to determine allowed origins and related
config
// must be able to continue without being redirected
Subject sub = new Subject();
sub.getPrincipals().add(new PrimaryPrincipal("anonymous"));
continueWithEstablishedSecurityContext(sub, req, res, chain);
- } else {
+ }
+ else {
Review comment:
Any good reason to have bumped this else onto its own line?
##########
File path:
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java
##########
@@ -121,13 +141,25 @@ public void doFilter(ServletRequest request,
ServletResponse response, FilterCha
List<Cookie> ssoCookies = CookieUtils.getCookiesForName(req, cookieName);
if (ssoCookies.isEmpty()) {
+ /* check for unauthenticated paths to bypass */
+ for (final String path : unAuthenticatedPaths) {
+ if (req.getRequestURI().contains(path)) {
+ /* This path is configured as an unauthenticated path let the
request through */
+ final Subject sub = new Subject();
+ sub.getPrincipals().add(new PrimaryPrincipal("anonymous"));
+ LOGGER.unAuthenticatePathBypass(path, req.getRequestURI());
+ continueWithEstablishedSecurityContext(sub, req, res, chain);
+ }
+ }
+
if (req.getMethod().equals("OPTIONS")) {
Review comment:
Safer to reverse this to "OPTIONS".equals(req.getMethod()) since it will
handle null from getMethod().
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 448009)
Time Spent: 20m (was: 10m)
> Add a configurable list of paths that SSOCookieProvider can ignore
> ------------------------------------------------------------------
>
> Key: KNOX-2393
> URL: https://issues.apache.org/jira/browse/KNOX-2393
> Project: Apache Knox
> Issue Type: Bug
> Components: KnoxSSO
> Reporter: Sandeep More
> Assignee: Sandeep More
> Priority: Major
> Fix For: 1.4.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> There are some cases where browser sends automatic GET requests [1] (e.g.
> favicon.ico) that can interfere with KnoxSSO flow depending on the timing of
> the requests and cause SSO page to land on favicon icon.
> This could be achieved by adding a list of path for SSO to ignore using a
> property {{gateway.knox.sso.unauthenticated.path.list}}
> e.g.
> {code:xml}
> <provider>
> <role>federation</role>
> <name>SSOCookieProvider</name>
> <enabled>true</enabled>
> <param>
> <name>sso.authentication.provider.url</name>
> <value>/gateway/knoxsso/api/v1/websso</value>
> </param>
> <param>
> <name>gateway.knox.sso.unauthenticated.path.list</name>
> <value>favicon.ico;test;unsafepath</value>
> </param>
> </provider>
> {code}
> [1] [https://bugs.chromium.org/p/chromium/issues/detail?id=39402]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
