[
https://issues.apache.org/jira/browse/KNOX-2707?focusedWorklogId=729289&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-729289
]
ASF GitHub Bot logged work on KNOX-2707:
----------------------------------------
Author: ASF GitHub Bot
Created on: 17/Feb/22 21:57
Start Date: 17/Feb/22 21:57
Worklog Time Spent: 10m
Work Description: pzampino commented on a change in pull request #537:
URL: https://github.com/apache/knox/pull/537#discussion_r809173808
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java
##########
@@ -17,12 +17,37 @@
*/
package org.apache.knox.gateway;
+import java.util.Set;
+
import org.apache.knox.gateway.i18n.messages.Message;
import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;
+import org.apache.knox.gateway.plang.Ast;
+import org.apache.knox.gateway.plang.SyntaxException;
@Messages(logger="org.apache.knox.gateway")
public interface IdentityAsserterMessages {
@Message( level = MessageLevel.ERROR, text = "Required subject/identity not
available. Check authentication/federation provider for proper configuration."
)
void subjectNotAvailable();
+
+ @Message( level = MessageLevel.WARN, text = "Virtual group name is missing
after dot character.")
+ void missingVirtualGroupName();
+
+ @Message( level = MessageLevel.WARN, text = "Parse error: {2}. At {0}={1}")
+ void parseError(String key, String script, SyntaxException e);
+
+ @Message( level = MessageLevel.WARN, text = "Invalid result: {2}. Expected
boolean when evaluating: {1}. For virtualGroup: {0}")
Review comment:
My initial feeling about this is that it should be DEBUG rather than
WARN.
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java
##########
@@ -17,12 +17,37 @@
*/
package org.apache.knox.gateway;
+import java.util.Set;
+
import org.apache.knox.gateway.i18n.messages.Message;
import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;
+import org.apache.knox.gateway.plang.Ast;
+import org.apache.knox.gateway.plang.SyntaxException;
@Messages(logger="org.apache.knox.gateway")
public interface IdentityAsserterMessages {
@Message( level = MessageLevel.ERROR, text = "Required subject/identity not
available. Check authentication/federation provider for proper configuration."
)
void subjectNotAvailable();
+
+ @Message( level = MessageLevel.WARN, text = "Virtual group name is missing
after dot character.")
+ void missingVirtualGroupName();
+
+ @Message( level = MessageLevel.WARN, text = "Parse error: {2}. At {0}={1}")
Review comment:
Assuming this is in reference to the mapping expression param values,
perhaps something like "Invalid mapping parameter value: {2}. At {0}={1}"? I
would have to see some examples of the parsing error messages though to really
comment on this message.
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java
##########
@@ -17,12 +17,37 @@
*/
package org.apache.knox.gateway;
+import java.util.Set;
+
import org.apache.knox.gateway.i18n.messages.Message;
import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;
+import org.apache.knox.gateway.plang.Ast;
+import org.apache.knox.gateway.plang.SyntaxException;
@Messages(logger="org.apache.knox.gateway")
public interface IdentityAsserterMessages {
@Message( level = MessageLevel.ERROR, text = "Required subject/identity not
available. Check authentication/federation provider for proper configuration."
)
void subjectNotAvailable();
+
+ @Message( level = MessageLevel.WARN, text = "Virtual group name is missing
after dot character.")
Review comment:
Is this in reference to param names? If so, perhaps something like
"Invalid mapping parameter name: Missing required group name."
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java
##########
@@ -17,12 +17,37 @@
*/
package org.apache.knox.gateway;
+import java.util.Set;
+
import org.apache.knox.gateway.i18n.messages.Message;
import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;
+import org.apache.knox.gateway.plang.Ast;
+import org.apache.knox.gateway.plang.SyntaxException;
@Messages(logger="org.apache.knox.gateway")
public interface IdentityAsserterMessages {
@Message( level = MessageLevel.ERROR, text = "Required subject/identity not
available. Check authentication/federation provider for proper configuration."
)
void subjectNotAvailable();
+
+ @Message( level = MessageLevel.WARN, text = "Virtual group name is missing
after dot character.")
Review comment:
Did we agree that these are not necessarily "virtual" groups? Should be
drop the "virtual" throughout?
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java
##########
@@ -17,12 +17,37 @@
*/
package org.apache.knox.gateway;
+import java.util.Set;
+
import org.apache.knox.gateway.i18n.messages.Message;
import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;
+import org.apache.knox.gateway.plang.Ast;
+import org.apache.knox.gateway.plang.SyntaxException;
@Messages(logger="org.apache.knox.gateway")
public interface IdentityAsserterMessages {
@Message( level = MessageLevel.ERROR, text = "Required subject/identity not
available. Check authentication/federation provider for proper configuration."
)
void subjectNotAvailable();
+
+ @Message( level = MessageLevel.WARN, text = "Virtual group name is missing
after dot character.")
+ void missingVirtualGroupName();
+
+ @Message( level = MessageLevel.WARN, text = "Parse error: {2}. At {0}={1}")
+ void parseError(String key, String script, SyntaxException e);
+
+ @Message( level = MessageLevel.WARN, text = "Invalid result: {2}. Expected
boolean when evaluating: {1}. For virtualGroup: {0}")
+ void invalidResult(String virtualGroupName, Ast ast, Object result);
+
+ @Message( level = MessageLevel.DEBUG, text = "Adding user: {0} to virtual
group: {1} using predicate: {2}")
+ void addingUserToVirtualGroup(String username, String virtualGroupName, Ast
ast);
+
+ @Message( level = MessageLevel.DEBUG, text = "Not adding user: {0} to
virtual group: {1} using predicate: {2}")
+ void notAddingUserToVirtualGroup(String username, String virtualGroupName,
Ast ast);
Review comment:
Do we really need to log this, even at DEBUG level?
##########
File path:
gateway-util-common/src/main/java/org/apache/knox/gateway/plang/Ast.java
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.plang;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class Ast {
Review comment:
What is Ast?
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/IdentityAsserterMessages.java
##########
@@ -17,12 +17,37 @@
*/
package org.apache.knox.gateway;
+import java.util.Set;
+
import org.apache.knox.gateway.i18n.messages.Message;
import org.apache.knox.gateway.i18n.messages.MessageLevel;
import org.apache.knox.gateway.i18n.messages.Messages;
+import org.apache.knox.gateway.plang.Ast;
+import org.apache.knox.gateway.plang.SyntaxException;
@Messages(logger="org.apache.knox.gateway")
public interface IdentityAsserterMessages {
@Message( level = MessageLevel.ERROR, text = "Required subject/identity not
available. Check authentication/federation provider for proper configuration."
)
void subjectNotAvailable();
+
+ @Message( level = MessageLevel.WARN, text = "Virtual group name is missing
after dot character.")
+ void missingVirtualGroupName();
+
+ @Message( level = MessageLevel.WARN, text = "Parse error: {2}. At {0}={1}")
+ void parseError(String key, String script, SyntaxException e);
+
+ @Message( level = MessageLevel.WARN, text = "Invalid result: {2}. Expected
boolean when evaluating: {1}. For virtualGroup: {0}")
+ void invalidResult(String virtualGroupName, Ast ast, Object result);
+
+ @Message( level = MessageLevel.DEBUG, text = "Adding user: {0} to virtual
group: {1} using predicate: {2}")
+ void addingUserToVirtualGroup(String username, String virtualGroupName, Ast
ast);
+
+ @Message( level = MessageLevel.DEBUG, text = "Not adding user: {0} to
virtual group: {1} using predicate: {2}")
+ void notAddingUserToVirtualGroup(String username, String virtualGroupName,
Ast ast);
+
+ @Message( level = MessageLevel.DEBUG, text = "Checking user: {0} (with
groups: {1}) whether to add virtualGroup: {2} using predicate: {3}")
+ void checkingVirtualGroup(String userName, Set<String> userGroups, String
virtualGroupName, Ast ast);
+
+ @Message( level = MessageLevel.INFO, text = "User: {0} (with groups: {1})
added to virtual groups: {2}")
Review comment:
Do we need this since all the user's groups will be enumerated in the
audit log? Well, assuming that these groups DO show up in those log messages.
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/VirtualGroupMapper.java
##########
@@ -0,0 +1,97 @@
+/*
+ * 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.identityasserter.common.filter;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
+
+import org.apache.knox.gateway.IdentityAsserterMessages;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.plang.Arity;
+import org.apache.knox.gateway.plang.Ast;
+import org.apache.knox.gateway.plang.Interpreter;
+
+public class VirtualGroupMapper {
+ private final IdentityAsserterMessages LOG =
MessagesFactory.get(IdentityAsserterMessages.class);
+ private final Map<String, Ast> virtualGroupToPredicateMap;
+
+ public VirtualGroupMapper(Map<String, Ast> virtualGroupToPredicateMap) {
+ this.virtualGroupToPredicateMap = virtualGroupToPredicateMap;
+ }
+
+ /**
+ * @return all virtual groups where the corresponding predicate matches
+ */
+ public Set<String> virtualGroupsOfUser(String username, Set<String>
groups, ServletRequest request) {
Review comment:
I would expect a method name like mapGroups(String, Set<String>,
ServletRequest) since this class is a GroupMapper.
##########
File path:
gateway-util-common/src/main/java/org/apache/knox/gateway/plang/Parser.java
##########
@@ -0,0 +1,109 @@
+/*
+ * 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.plang;
+
+import java.io.IOException;
+import java.io.PushbackReader;
+import java.io.StringReader;
+
+public class Parser {
Review comment:
Should all of this parsing-related code go into the gateway-util-common
module or its own module?
##########
File path:
gateway-provider-identity-assertion-common/src/main/java/org/apache/knox/gateway/identityasserter/common/filter/CommonIdentityAssertionFilter.java
##########
@@ -59,6 +74,55 @@ public void init(FilterConfig filterConfig) throws
ServletException {
throw new ServletException("Unable to load principal mapping table.",
e);
}
}
+ virtualGroupMapper = new
VirtualGroupMapper(loadVirtualGroups(filterConfig));
+ }
+
+ private Map<String, Ast> loadVirtualGroups(FilterConfig filterConfig) {
+ Map<String, Ast> predicateToGroupMapping = new HashMap<>();
+ loadVirtualGroupConfig(filterConfig, predicateToGroupMapping);
+ if (predicateToGroupMapping.isEmpty() && filterConfig.getServletContext()
!= null) {
+ loadVirtualGroupConfig(filterConfig.getServletContext(),
predicateToGroupMapping);
+ }
+ if
(predicateToGroupMapping.keySet().stream().anyMatch(StringUtils::isBlank)) {
+ LOG.missingVirtualGroupName();
+ }
+ return predicateToGroupMapping;
+ }
+
+ private void loadVirtualGroupConfig(FilterConfig config, Map<String, Ast>
result) {
+ for (String paramName :
virtualGroupParameterNames(config.getInitParameterNames())) {
Review comment:
Seems like unnecessarily iterating over the group mapping params twice;
It's not clear how much value is added by the virtualGroupParameterNames()
method.
--
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.
To unsubscribe, e-mail: dev-unsubscr...@knox.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Issue Time Tracking
-------------------
Worklog Id: (was: 729289)
Time Spent: 20m (was: 10m)
> Virtual Group Mapping Provider
> ------------------------------
>
> Key: KNOX-2707
> URL: https://issues.apache.org/jira/browse/KNOX-2707
> Project: Apache Knox
> Issue Type: New Feature
> Reporter: Attila Magyar
> Assignee: Attila Magyar
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Presenting a more flexible way to map principals to groups than the existing
> _group.principal.mapping_ mechanism in {_}CommonIdentityAssertionFilter{_}.
> See the motivations behind this at
> [https://cwiki.apache.org/confluence/display/KNOX/KIP-16+-+Virtual+Groups+in+Apache+Knox]
> Example:
> {code:xml}
> <provider>
> <role>identity-assertion</role>
> <name>Default</name>
> <enabled>true</enabled>
> <param>
> <name>virtual.group.mapping.vgroup1</name>
> <value>(or (username 'tom') (member 'analyst'))</value>
> </param>
> </provider>
> {code}
> We add user 'tom' or any other users who are member of the 'analyst' group to
> virtual group vgroup1.
> General usage:
> {code:java}
> <name>virtual.group.mapping.VIRTUAL-GROUP-NAME</name>
> <value>PREDICATE</value>
> {code}
> If the *PREDICATE* evaluates to true the user is added to
> {*}VIRTUAL-GROUP-NAME{*}.
> There can be any number of virtual group mappings within the provider.
> h2. Language Syntax
> The predicate uses a parenthesized prefix notation language, similar to Lisp.
> * Everything in the language is either an atom or a list
> * A list is written with its elements separated by whitespace, and
> surrounded by parentheses, like (or true false false)
> * Lists can be nested to arbitrary level, like (or true (and false true))
> * An atom is either a boolean (true/false), a string, a number or a symbol
> (which denotes a functions name or a variable name).
> * Strings are single-quoted which makes easier to embed the language into
> XML or JSON.
> * There is a one to one mapping between the textual syntax and the parser
> generated AST. You can always infer the exact AST just by looking at the code
> (homoiconicity).
> From this code the parses generates the following AST:
> h5. Textual code:
> {code:java}
> (or true (and false true))
> {code}
> h5. AST:
> {code:java}
> [or, true, [and, false, true]]
> {code}
> This has exactly the same structure, but everything is converted to internal
> Java representations. Lists are ArrayLists, booleans are java.lang.Booleans,
> etc.
> h2. Evaluation rules
> * A literal atom evaluates to itself ('astring', 123).
> * If an atom is a symbol (like {_}or{_}, {_}username{_}, {_}true{_},
> {_}false{_}) then the atom is looked up in a dictionary.
> * The head of a list is the name of the function we're about to call. The
> rest are the parameters.
> * Before calling the function (1st item of the list) we evaluate the rest of
> the list (recursively).
> {code:java}
> (= 0 (size groups))
> ^ ^ ^^^^^^^^^^^^
> | | |
> func param1 param2
> {code}
> # Get the head of the list (=), see if it's an existing function
> # Evaluate param1, and param2, recusrivly
> # Call the function (=)
> h2. Special forms
> For some expressions the evaluation rule is slightly different. These are
> called special forms. These are the _or_ and the {_}and{_} operators. To
> support short-circuit evaluation, the parameters are not evaluated at the
> call site but by the definition itself.
> {code:java}
> (or true (and true false))
> ^ ^ ^
> | | |
> func param1 param2
> {code}
> First we call the operator (or) then we let the operator decide which part to
> evaluate. In the above example the _or_ will stop evaluating the expression
> after it sees that the first argument is {_}true{_}.
> These few evaluation rules (general + special forms) cover the semantics of
> the whole language.
> h2. Supported functions
> h3. or
> Evaluates true if one or more of its operands is true. Supports short-circuit
> evaluation and variable number of arguments.
> Number of arguments: 1..N
> {code:java}
> (or bool1 bool2 ... boolN)
> {code}
> h5. Example
> {code:java}
> (or true false true)
> {code}
> h3. and
> Evaluates true if all of its operands are true. Supports short-circuit
> evaluation and variable number of arguments.
> Number of arguments: 1..N
> {code:java}
> (and bool1 bool2 ... boolN)
> {code}
> h5. Example
> {code:java}
> (and true false true)
> {code}
> h3. not
> Negates the operand.
> Number of arguments: 1
> {code:java}
> (not aBool)
> {code}
> h5. Example
> {code:java}
> (not true)
> {code}
> h3. =
> Evaluates true if the two operands are equal.
> Number of arguments: 2
> {code:java}
> (= op1 op2)
> {code}
> h5. Example
> {code:java}
> (= 'apple' 'orange')
> {code}
> h3. !=
> Evaluates true if the two operands are not equal.
> Number of arguments: 2
> {code:java}
> (!= op1 op2)
> {code}
> h5. Example
> {code:java}
> (!= 'apple' 'orange')
> {code}
> h3. member
> Evaluates true if the current user is a member of the given group
> Number of arguments: 1
> {code:java}
> (member aString)
> {code}
> h5. Example
> {code:java}
> (member 'analyst')
> {code}
> h3. username
> Evaluates true if the current user has the given username
> Number of arguments: 1
> {code:java}
> (username aString)
> {code}
> h5. Example
> {code:java}
> (username 'admin')
> {code}
> This is a shorter version of (= username 'admin')
> h3. size
> Gets the size of a list
> Number of arguments: 1
> {code:java}
> (size alist)
> {code}
> h5. Example
> {code:java}
> (size groups)
> {code}
> h3. empty
> Evaluates to true if the given list is empty
> Number of arguments: 1
> {code:java}
> (empty alist)
> {code}
> h5. Example
> {code:java}
> (empty groups)
> {code}
> h3. match
> Evaluates true if the given string matches to the given regexp. Or any items
> of the given list matches the given regexp.
> Number of arguments: 2
> {code:java}
> (match aString aRegExpString)
> (match aList aRegExpString)
> {code}
> h5. Example
> {code:java}
> (match username 'tom|sam')
> {code}
> This function can also take a list as a first argument. In this case it will
> return true if the regexp matches to *any of the items* in the list.
> {code:java}
> (match groups 'analyst|scientist')
> {code}
> This returns true if the user is either in the 'analyst' group or in the
> 'scientist' group.
> h3. request-header
> Returns the value of the specified request header as a String. If the given
> key doesn't exist empty string is returned.
> Number of arguments: 1
> {code:java}
> (request-header aString)
> {code}
> h5. Example
> {code:java}
> (request-header 'User-Agent')
> {code}
> h3. request-attribute
> Returns the value of the specified request attribute as a String. If the
> given key doesn't exist empty string is returned.
> Number of arguments: 1
> {code:java}
> (request-attribute aString)
> {code}
> h5. Example
> {code:java}
> (request-attribute 'sourceRequestUrl')
> {code}
> h3. session
> Returns the value of the specified session attribute as a String. If the
> given key doesn't exist empty string is returned.
> Number of arguments: 1
> {code:java}
> (session aString)
> {code}
> h5. Example
> {code:java}
> (session 'subject.userRoles')
> {code}
> h3. lowercase
> Converts the given string to lowercase.
> Number of arguments: 1
> {code:java}
> (lowercase aString)
> {code}
> h5. Example
> {code:java}
> (lowercase 'KNOX')
> {code}
> h3. uppercase
> Converts the given string to uppercase.
> Number of arguments: 1
> {code:java}
> (uppercase aString)
> {code}
> h5. Example
> {code:java}
> (uppercase 'knox')
> {code}
> h2. Constants
> The following constants are populated automatically from the current security
> context.
> h3. username
> The username (principal) of the current user, derived from
> javax.security.auth.Subject.
> h3. groups
> The groups of the current user (LDAP or OS level), derived from
> {_}subject.getPrincipals(GroupPrincipal.class{_}.
> h2. Examples
> {code:java}
> (or
> (and
> (member 'admin')
> (member 'datalake'))
> (or
> (username 'lmccay')
> (username 'pzampino')))
> {code}
> 1. Returns true if the user is either 'lmccay' or 'pzampino'
> 2. Returns true if the user is both in the 'admin' and the 'datalake' group.
> 3. Returns false otherwise.
> A shorter version of this is
> {code:java}
> (or
> (and (member 'admin') (member 'datalake'))
> (match username 'lmccay|pzampino'))
> {code}
> h2. Any group
> If we want to put a user into a virtual group if they are a member of ANY
> groups, we can use either of the following predicates.
> {code:java}
> (!= (size groups) 0)
> {code}
> {code:java}
> (not (empty groups))
> {code}
> {code:java}
> (match groups '.*')
> {code}
> h3. Same group as the username
> If we wan to check if the user is in the same group as their username we can
> use the following predicate
> {code:java}
> (member username)
> {code}
> h2. Implementation notes
> CommonIdentityAssertionFilter parses all the group mapping predicates at
> deployment time and caches the ASTs. In the doFilter we only evaluate the
> ASTs without parsing the text over and over.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
