Merge pull request #1704 from weseek/imprv/use-lucene-syntax

Imprv/use lucene syntax

package.json

@@ -109,6 +109,7 @@
     "i18next-node-fs-backend": "^2.1.0",
     "i18next-sprintf-postprocessor": "^0.2.2",
     "is-iso-date": "^0.0.1",
+    "lucene-query-parser": "^1.2.0",
     "md5": "^2.2.1",
     "method-override": "^3.0.0",
     "migrate-mongo": "^7.0.1",

src/server/service/passport.js

@@ -1,5 +1,6 @@
 const debug = require('debug')('growi:service:PassportService');
 const urljoin = require('url-join');
+const luceneQueryParser = require('lucene-query-parser');
 const passport = require('passport');
 const LocalStrategy = require('passport-local').Strategy;
 const LdapStrategy = require('passport-ldapauth');
@@ -674,73 +675,67 @@ class PassportService {
       return true;
     }
 
-    const expr = this.parseABLCRule(rule);
-    if (expr == null) {
+    // parse with lucene-query-parser
+    // see https://github.com/thoward/lucene-query-parser.js/wiki
+    const luceneRule = luceneQueryParser.parse(rule);
+    if (luceneRule == null) {
       return false;
     }
-    debug({ 'Parsed Rule': JSON.stringify(expr, null, 2) });
+    debug({ 'Parsed Rule': JSON.stringify(luceneRule, null, 2) });
 
     const attributes = this.extractAttributesFromSAMLResponse(response);
     debug({ 'Extracted Attributes': JSON.stringify(attributes, null, 2) });
 
-    let evaluatedExpr = false;
-    for (const orOp of expr) {
-      let evaluatedOrOp = true;
-      for (const andOp of orOp) {
-        if (attributes[andOp[0]] == null) {
-          evaluatedOrOp = false;
-          break;
-        }
-        evaluatedOrOp = evaluatedOrOp && attributes[andOp[0]].includes(andOp[1]);
-      }
-      evaluatedExpr = evaluatedExpr || evaluatedOrOp;
+    return this.evaluateRuleForSamlAttributes(attributes, luceneRule);
+  }
+
+  /**
+   * Evaluate whether the specified rule is satisfied under the specified attributes
+   *
+   * @param {object} attributes results by extractAttributesFromSAMLResponse
+   * @param {object} luceneRule Expression Tree Structure generated by lucene-query-parser
+   * @see https://github.com/thoward/lucene-query-parser.js/wiki
+   */
+  evaluateRuleForSamlAttributes(attributes, luceneRule) {
+    const { left, right, operator } = luceneRule;
+
+    // when combined rules
+    if (right != null) {
+      return this.evaluateCombinedRulesForSamlAttributes(attributes, left, right, operator);
+    }
+    if (left != null) {
+      return this.evaluateRuleForSamlAttributes(attributes, left);
     }
 
-    return evaluatedExpr;
+    const { field, term } = luceneRule;
+    if (field === '<implicit>') {
+      return attributes[term] != null;
+    }
+    return attributes[field].includes(term);
   }
 
   /**
-   * Parse a rule string for the attribute-based login control
+   * Evaluate whether the specified two rules are satisfied under the specified attributes
    *
-   * The syntax rules are as follows.
-   * <attr> and <value> are any characters except "|", "&", "=".
-   *
-   * ## Syntax
-   *    <expr>   ::= <or_op> | <or_op> "|" <expr>
-   *    <or_op>  ::= <and_op> | <and_op> "&" <or_op>
-   *    <and_op> ::= <attr> "=" <value>
-   *
-   * ## Example
-   *  In:  "Department = A | Department = B & Position = Leader"
-   *  Out:
-   *    [
-   *      [
-   *        ["Department", "A"]
-   *      ],
-   *      [
-   *        ["Department","B"],
-   *        ["Position","Leader"]
-   *      ]
-   *    ]
-   *
-   *   In:  Invalid syntax string like a "This is a & bad & rule string."
-   *   Out: null
+   * @param {object} attributes results by extractAttributesFromSAMLResponse
+   * @param {object} luceneRuleLeft Expression Tree Structure generated by lucene-query-parser
+   * @param {object} luceneRuleRight Expression Tree Structure generated by lucene-query-parser
+   * @param {string} luceneOperator operator string expression
+   * @see https://github.com/thoward/lucene-query-parser.js/wiki
    */
-  parseABLCRule(rule) {
-    let expr = rule.split('|');
-    expr = expr.map(orOp => orOp.trim().split('&'));
-    expr = expr.map(orOp => orOp.map(andOp => andOp.trim().split('=')));
-    expr = expr.map(orOp => orOp.map(andOp => andOp.map(v => v.trim())));
-    for (const orOp of expr) {
-      for (const andOp of orOp) {
-        if (andOp.length !== 2) {
-          return null;
-        }
-      }
+  evaluateCombinedRulesForSamlAttributes(attributes, luceneRuleLeft, luceneRuleRight, luceneOperator) {
+    if (luceneOperator === 'OR') {
+      return this.evaluateRuleForSamlAttributes(attributes, luceneRuleLeft) || this.evaluateRuleForSamlAttributes(attributes, luceneRuleRight);
+    }
+    if (luceneOperator === 'AND') {
+      return this.evaluateRuleForSamlAttributes(attributes, luceneRuleLeft) && this.evaluateRuleForSamlAttributes(attributes, luceneRuleRight);
+    }
+    if (luceneOperator === 'NOT') {
+      return this.evaluateRuleForSamlAttributes(attributes, luceneRuleLeft) && !this.evaluateRuleForSamlAttributes(attributes, luceneRuleRight);
     }
-    return expr;
-  }
 
+    throw new Error(`Unsupported operator: ${luceneOperator}`);
+  }
 
   /**
    * Extract attributes from a SAML response

src/test/service/passport.test.js

@@ -24,17 +24,23 @@ describe('PassportService test', () => {
 
     /* eslint-disable indent */
     describe.each`
-      conditionId | departments   | positions     | ruleStr                                                                       | expected
-      ${1}        | ${['A']}      | ${[]}         | ${'Department = A | Department = B & Position = Leader'}                      | ${true}
-      ${2}        | ${['B']}      | ${['Leader']} | ${'Department = A | Department = B & Position = Leader'}                      | ${true}
-      ${3}        | ${['A', 'C']} | ${['Leader']} | ${'Department = A | Department = B & Position = Leader'}                      | ${true}
-      ${4}        | ${['B', 'C']} | ${['Leader']} | ${'Department = A | Department = B & Position = Leader'}                      | ${true}
-      ${5}        | ${[]}         | ${[]}         | ${'Department = A | Department = B & Position = Leader'}                      | ${false}
-      ${6}        | ${['C']}      | ${['Leader']} | ${'Department = A | Department = B & Position = Leader'}                      | ${false}
-      ${7}        | ${['A']}      | ${['Leader']} | ${'Department = A & Position = Leader | Department = B & Position = Leader'}  | ${true}
-      ${8}        | ${['B']}      | ${['Leader']} | ${'Department = A & Position = Leader | Department = B & Position = Leader'}  | ${true}
-      ${9}        | ${['C']}      | ${['Leader']} | ${'Department = A & Position = Leader | Department = B & Position = Leader'}  | ${false}
-      ${9}        | ${['A', 'B']} | ${['']}       | ${'Department = A & Position = Leader | Department = B & Position = Leader'}  | ${false}
+      conditionId | departments   | positions     | ruleStr                                                         | expected
+      ${1}        | ${[]}         | ${['Leader']} | ${'Position'}                                                   | ${true}
+      ${2}        | ${[]}         | ${['Leader']} | ${'Position: Leader'}                                           | ${true}
+      ${3}        | ${['A']}      | ${[]}         | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${4}        | ${['B']}      | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${5}        | ${['A', 'C']} | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${6}        | ${['B', 'C']} | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${true}
+      ${7}        | ${[]}         | ${[]}         | ${'Department: A || Department: B && Position: Leader'}         | ${false}
+      ${8}        | ${['C']}      | ${['Leader']} | ${'Department: A || Department: B && Position: Leader'}         | ${false}
+      ${9}        | ${['A']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${true}
+      ${10}       | ${['B']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${true}
+      ${11}       | ${['C']}      | ${['Leader']} | ${'(Department: A || Department: B) && Position: Leader'}       | ${false}
+      ${12}       | ${['A', 'B']} | ${[]}         | ${'(Department: A || Department: B) && Position: Leader'}       | ${false}
+      ${13}       | ${['A']}      | ${[]}         | ${'Department: A NOT Position: Leader'}                         | ${true}
+      ${14}       | ${['A']}      | ${['Leader']} | ${'Department: A NOT Position: Leader'}                         | ${false}
+      ${15}       | ${[]}         | ${['Leader']} | ${'Department: A OR (Position NOT Position: User)'}             | ${true}
+      ${16}       | ${[]}         | ${['User']}   | ${'Department: A OR (Position NOT Position: User)'}             | ${false}
     `('to be $expected under rule="$ruleStr"', ({
       conditionId, departments, positions, ruleStr, expected,
     }) => {

yarn.lock

@@ -8246,6 +8246,11 @@ lru-cache@^5.0.0, lru-cache@^5.1.1:
   dependencies:
     yallist "^3.0.2"
 
+lucene-query-parser@^1.2.0:
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/lucene-query-parser/-/lucene-query-parser-1.2.0.tgz#46dad5b4ddc59abbf27f9df4c519d959c2033432"
+  integrity sha1-RtrVtN3Fmrvyf530xRnZWcIDNDI=
+
 make-dir@^1.0.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/make-dir/-/make-dir-1.1.0.tgz#19b4369fe48c116f53c2af95ad102c0e39e85d51"