Yuki Takei 6 лет назад
Родитель
Сommit
483ea9ea33
2 измененных файлов с 55 добавлено и 61 удалено
  1. 42 50
      src/server/service/passport.js
  2. 13 11
      src/test/service/passport.test.js

+ 42 - 50
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,7 +675,9 @@ class PassportService {
       return true;
     }
 
-    const expr = this.parseABLCRule(rule);
+    // parse with lucene-query-parser
+    // see https://github.com/thoward/lucene-query-parser.js/wiki
+    const expr = luceneQueryParser.parse(rule);
     if (expr == null) {
       return false;
     }
@@ -683,64 +686,53 @@ class PassportService {
     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.evaluateRule(attributes, expr);
+  }
+
+  /**
+   * 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
+   */
+  evaluateRule(attributes, luceneRule) {
+    const { left, right, operator } = luceneRule;
+    if (right != null) {
+      return this.evaluateCombinedRules(attributes, left, right, operator);
+    }
+    if (left != null) {
+      return this.evaluateRule(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
-   *
-   * 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>
+   * Evaluate whether the specified two rules are satisfied under the specified attributes
    *
-   * ## 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;
-        }
-      }
+  evaluateCombinedRules(attributes, luceneRuleLeft, luceneRuleRight, luceneOperator) {
+    if (luceneOperator === 'OR') {
+      return this.evaluateRule(attributes, luceneRuleLeft) || this.evaluateRule(attributes, luceneRuleRight);
+    }
+    if (luceneOperator === 'AND') {
+      return this.evaluateRule(attributes, luceneRuleLeft) && this.evaluateRule(attributes, luceneRuleRight);
     }
-    return expr;
-  }
 
+    throw new Error(`Unsupported operator: ${luceneOperator}`);
+  }
 
   /**
    * Extract attributes from a SAML response

+ 13 - 11
src/test/service/passport.test.js

@@ -24,17 +24,19 @@ 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}
     `('to be $expected under rule="$ruleStr"', ({
       conditionId, departments, positions, ruleStr, expected,
     }) => {