Skip to content

Commit

Permalink
KNOX-3014 - Fix a bug where unauthenticated path configured in shiro …
Browse files Browse the repository at this point in the history
…provider throw exception (#879)

* KNOX-3014 - Fix a bug where unauthenticated path configured in shiro provider throw exception

* Formatting changes

* Adding /knoxtoken/api/v1/jwks.json and v1 will be depreciated

* Check at Knox level if the request is configured to be anonymous usign shiro configs. Add unit tests for better test coverage.
  • Loading branch information
moresandeep authored Mar 13, 2024
1 parent c098afa commit 84999b8
Show file tree
Hide file tree
Showing 6 changed files with 292 additions and 59 deletions.
9 changes: 9 additions & 0 deletions gateway-provider-security-shiro/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
<groupId>org.apache.knox</groupId>
<artifactId>gateway-util-common</artifactId>
</dependency>
<dependency>
<groupId>org.apache.knox</groupId>
<artifactId>gateway-util-urltemplate</artifactId>
</dependency>

<dependency>
<groupId>org.jboss.shrinkwrap</groupId>
Expand Down Expand Up @@ -87,6 +91,11 @@
<artifactId>commons-io</artifactId>
</dependency>

<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>

<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you 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.apache.knox.gateway;

import org.apache.knox.gateway.i18n.messages.Message;
import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;

@Messages(logger="org.apache.knox.gateway")
public interface ShiroMessages {
@Message(level = MessageLevel.INFO, text = "Request {0} matches unauthenticated path configured in topology, letting it through" )
void unauthenticatedPathBypass(String uri);

@Message( level = MessageLevel.WARN, text = "Invalid URL pattern for rule: {0}" )
void invalidURLPattern(String rule);
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

public class ShiroDeploymentContributor extends ProviderDeploymentContributorBase {

Expand All @@ -43,8 +44,10 @@ public class ShiroDeploymentContributor extends ProviderDeploymentContributorBas
private static final String SESSION_TIMEOUT = "sessionTimeout";
private static final String REMEMBER_ME = "rememberme";
private static final String SHRIO_CONFIG_FILE_NAME = "shiro.ini";
private static final String SHIRO_URL_CONFIG = "urls";
private static final int DEFAULT_SESSION_TIMEOUT = 30; // 30min


@Override
public String getRole() {
return "authentication";
Expand Down Expand Up @@ -132,6 +135,17 @@ public void contributeFilter( DeploymentContext context, Provider provider,

resource.addFilter().name( getName() ).role(
getRole() ).impl( SHIRO_FILTER_CLASSNAME ).params( params );

/* Collect all shiro `urls.` params and them as filter params to POST_FILTER_CLASSNAME */
Map<String, String> shiroURLs = providerParams.entrySet().stream().filter(e -> e.getKey().startsWith(SHIRO_URL_CONFIG)).collect(
Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));

for(final Map.Entry<String, String> m: shiroURLs.entrySet()) {
params.add( resource.createFilterParam()
.name( m.getKey() )
.value( m.getValue() ) );
}

resource.addFilter().name( "Post" + getName() ).role(
getRole() ).impl( POST_FILTER_CLASSNAME ).params( params );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,8 @@
*/
package org.apache.knox.gateway.filter;

import java.io.IOException;
import java.security.Principal;
import java.security.PrivilegedExceptionAction;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.Callable;

import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;

import org.apache.commons.lang3.StringUtils;
import org.apache.knox.gateway.ShiroMessages;
import org.apache.knox.gateway.audit.api.Action;
import org.apache.knox.gateway.audit.api.ActionOutcome;
import org.apache.knox.gateway.audit.api.AuditContext;
Expand All @@ -40,20 +27,76 @@
import org.apache.knox.gateway.audit.api.Auditor;
import org.apache.knox.gateway.audit.api.ResourceType;
import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
import org.apache.knox.gateway.i18n.messages.MessagesFactory;
import org.apache.knox.gateway.security.GroupPrincipal;
import org.apache.knox.gateway.security.PrimaryPrincipal;
import org.apache.knox.gateway.util.urltemplate.Matcher;
import org.apache.knox.gateway.util.urltemplate.Parser;
import org.apache.knox.gateway.util.urltemplate.Template;
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.subject.Subject;

import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import java.io.IOException;
import java.net.URISyntaxException;
import java.security.Principal;
import java.security.PrivilegedExceptionAction;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;

public class ShiroSubjectIdentityAdapter implements Filter {
private static final ShiroMessages LOG = MessagesFactory.get(ShiroMessages.class);
private static final String SUBJECT_USER_GROUPS = "subject.userGroups";
private static AuditService auditService = AuditServiceFactory.getAuditService();
private static Auditor auditor = auditService.getAuditor(
AuditConstants.DEFAULT_AUDITOR_NAME, AuditConstants.KNOX_SERVICE_NAME,
AuditConstants.KNOX_COMPONENT_NAME );
private static final String SHIRO_URL_CONFIG = "urls";
/* Map of shiro url configs */
private static Map<String,String> urls = new HashMap<>();

/* List of URLs with anon authentication */
private static List<Matcher> anonUrls = new ArrayList<>();

@Override
public void init( FilterConfig filterConfig ) throws ServletException {
/* Create a shiro urls config map */
final Enumeration<String> params = filterConfig.getInitParameterNames();
while (params.hasMoreElements()) {
String param = params.nextElement();
if (StringUtils.startsWithIgnoreCase(param, SHIRO_URL_CONFIG)) {
String value = filterConfig.getInitParameter(param);
final String pathParam = param.substring(param.indexOf('.') + 1);
urls.put(pathParam, value);
if("anon".equalsIgnoreCase(value)) {
final Template urlPatternTemplate;
final Matcher urlMatcher = new Matcher();
try {
urlPatternTemplate = Parser.parseTemplate(pathParam);
urlMatcher.add(urlPatternTemplate, pathParam);
anonUrls.add(urlMatcher);
} catch (URISyntaxException e) {
LOG.invalidURLPattern(param);
throw new ServletException(e);
}

}

}
}
}

@Override
Expand Down Expand Up @@ -96,56 +139,110 @@ public Void run() throws Exception {
};
Subject shiroSubject = SecurityUtils.getSubject();

/**
* For cases when we want anonymous authentication to urls in shiro.
* This is when do not want authentication for jwks endpoints using
* shiro.
*/
if (shiroSubject == null || shiroSubject.getPrincipal() == null) {
throw new IllegalStateException("Unable to determine authenticated user from Shiro, please check that your Knox Shiro configuration is correct");
}

final String principal = shiroSubject.getPrincipal().toString();
Set<Principal> principals = new HashSet<>();
Principal p = new PrimaryPrincipal(principal);
principals.add(p);
AuditContext context = auditService.getContext();
context.setUsername( principal );
auditService.attachContext(context);
String sourceUri = (String)request.getAttribute( AbstractGatewayFilter.SOURCE_REQUEST_CONTEXT_URL_ATTRIBUTE_NAME );
auditor.audit( Action.AUTHENTICATION , sourceUri, ResourceType.URI, ActionOutcome.SUCCESS );

Set<String> userGroups;
// map ldap groups saved in session to Java Subject GroupPrincipal(s)
if (SecurityUtils.getSubject().getSession().getAttribute(SUBJECT_USER_GROUPS) != null) {
userGroups = (Set<String>)SecurityUtils.getSubject().getSession().getAttribute(SUBJECT_USER_GROUPS);
} else { // KnoxLdapRealm case
if( shiroSubject.getPrincipal() instanceof String ) {
userGroups = new HashSet<>(shiroSubject.getPrincipals().asSet());
userGroups.remove(principal);
} else { // KnoxPamRealm case
Set<Principal> shiroPrincipals = new HashSet<>(shiroSubject.getPrincipals().asSet());
userGroups = new HashSet<>(); // Here we are creating a new UserGroup
// so we don't need to remove a Principal
// In the case of LDAP the userGroup may have already Principal
// added to the list of groups, so it is not needed.
for( Principal shiroPrincipal: shiroPrincipals ) {
userGroups.add(shiroPrincipal.toString() );
}
if(!isRequestPathInShiroConfig(((HttpServletRequest)request))) {
throw new IllegalStateException("Unable to determine authenticated user from Shiro, please check that your Knox Shiro configuration is correct");
}

LOG.unauthenticatedPathBypass(
((HttpServletRequest) request).getRequestURI());
final String principal = "anonymous";
javax.security.auth.Subject subject = new javax.security.auth.Subject();
subject.getPrincipals().add(new PrimaryPrincipal(principal));
AuditContext context = auditService.getContext();
context.setUsername(principal);
auditService.attachContext(context);
javax.security.auth.Subject.doAs(subject, action);
} else {

final String principal = shiroSubject.getPrincipal().toString();
Set<Principal> principals = new HashSet<>();
Principal p = new PrimaryPrincipal(principal);
principals.add(p);
AuditContext context = auditService.getContext();
context.setUsername(principal);
auditService.attachContext(context);
String sourceUri = (String) request.getAttribute(
AbstractGatewayFilter.SOURCE_REQUEST_CONTEXT_URL_ATTRIBUTE_NAME);
auditor.audit(Action.AUTHENTICATION, sourceUri, ResourceType.URI,
ActionOutcome.SUCCESS);

Set<String> userGroups;
// map ldap groups saved in session to Java Subject GroupPrincipal(s)
if (SecurityUtils.getSubject().getSession()
.getAttribute(SUBJECT_USER_GROUPS) != null) {
userGroups = (Set<String>) SecurityUtils.getSubject().getSession()
.getAttribute(SUBJECT_USER_GROUPS);
} else { // KnoxLdapRealm case
if (shiroSubject.getPrincipal() instanceof String) {
userGroups = new HashSet<>(shiroSubject.getPrincipals().asSet());
userGroups.remove(principal);
} else { // KnoxPamRealm case
Set<Principal> shiroPrincipals = new HashSet<>(
shiroSubject.getPrincipals().asSet());
userGroups = new HashSet<>(); // Here we are creating a new UserGroup
// so we don't need to remove a Principal
// In the case of LDAP the userGroup may have already Principal
// added to the list of groups, so it is not needed.
for (Principal shiroPrincipal : shiroPrincipals) {
userGroups.add(shiroPrincipal.toString());
}
}
}
for (String userGroup : userGroups) {
Principal gp = new GroupPrincipal(userGroup);
principals.add(gp);
}
auditor.audit(Action.AUTHENTICATION, sourceUri, ResourceType.URI,
ActionOutcome.SUCCESS, "Groups: " + userGroups);

// The newly constructed Sets check whether this Subject has been set read-only
// before permitting subsequent modifications. The newly created Sets also prevent
// illegal modifications by ensuring that callers have sufficient permissions.
//
// To modify the Principals Set, the caller must have AuthPermission("modifyPrincipals").
// To modify the public credential Set, the caller must have AuthPermission("modifyPublicCredentials").
// To modify the private credential Set, the caller must have AuthPermission("modifyPrivateCredentials").
javax.security.auth.Subject subject = new javax.security.auth.Subject(
true, principals, Collections.emptySet(), Collections.emptySet());
javax.security.auth.Subject.doAs(subject, action);
}
for (String userGroup : userGroups) {
Principal gp = new GroupPrincipal(userGroup);
principals.add(gp);
}
auditor.audit( Action.AUTHENTICATION , sourceUri, ResourceType.URI, ActionOutcome.SUCCESS, "Groups: " + userGroups );

// The newly constructed Sets check whether this Subject has been set read-only
// before permitting subsequent modifications. The newly created Sets also prevent
// illegal modifications by ensuring that callers have sufficient permissions.
//
// To modify the Principals Set, the caller must have AuthPermission("modifyPrincipals").
// To modify the public credential Set, the caller must have AuthPermission("modifyPublicCredentials").
// To modify the private credential Set, the caller must have AuthPermission("modifyPrivateCredentials").
javax.security.auth.Subject subject = new javax.security.auth.Subject(true, principals, Collections.emptySet(), Collections.emptySet());
javax.security.auth.Subject.doAs( subject, action );

return null;
}
}

/**
* A helper function that checks whether the request path is defined in shiro
* config, specifically under the urls section with anon authentication. e.g.
* <param>
* <name>urls./knoxtoken/api/v1/jwks.json</name>
* <value>anon</value>
* </param>
*
* @param request
* @return true if request has anon auth.
* @throws URISyntaxException
*/
private static boolean isRequestPathInShiroConfig(
final HttpServletRequest request) throws URISyntaxException {
boolean isPathInConfig = false;
final String requestContextPath = StringUtils.startsWith(
request.getPathInfo(), "/") ?
request.getPathInfo() :
"/" + request.getPathInfo();
final Template requestUrlTemplate = Parser.parseLiteral(requestContextPath);
for (final Matcher m : anonUrls) {
if (m.match(requestUrlTemplate) != null) {
isPathInConfig = true;
}
}
return isPathInConfig;
}
}
8 changes: 8 additions & 0 deletions gateway-release/home/conf/topologies/sandbox.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@
<name>main.ldapRealm.contextFactory.authenticationMechanism</name>
<value>simple</value>
</param>
<param>
<name>urls./knoxtoken/api/v2/jwks.json</name>
<value>anon</value>
</param>
<param>
<name>urls./knoxtoken/api/v1/jwks.json</name>
<value>anon</value>
</param>
<param>
<name>urls./**</name>
<value>authcBasic</value>
Expand Down
Loading

0 comments on commit 84999b8

Please sign in to comment.