Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting "principalType" in evaluation as something not found in statement throws an error #42

Closed
yaser-ali-s opened this issue Apr 8, 2021 · 2 comments
Labels
bug Something isn't working released

Comments

@yaser-ali-s
Copy link

I tried setting the following:

const test = new ResourceBasedPolicy(
  {
    statements: [
      {
        action   : "*",
        resource : "*",
        principal: {
          a: "*",
          b: "*",
          c: "*",
        }
      }
    ]
  }
)

test.evaluate({
  action: "read",
  resource: "secrets:ultraSecret",
  principal: "secret",
  principalType: "d",
//context: {} 
})

I get a different error depending on whether I add a context or not:

  • Without context: Cannot read property 'trim' of undefined
  • With context : Cannot read property 'replace' of undefined

In both cases the error occurs because of the following line:

// iam-policies/src/ResourceBasedStatement.ts#L186
return new Matcher(applyContext(principalValues, context)).match(principal);

One is caused by new Matcher and the other by applyContext, because this.principal[principalType] is undefined.
I'd like to suggest the following amendment:

  private matchPrincipals(
    principal: string,
    principalType?: string,
    context?: T
  ): boolean {
    if (this.principal) {
      if (this.principal instanceof Array) {
        return principalType
          ? false
          : this.principal.some((a) =>
              new Matcher(applyContext(a, context)).match(principal)
            );
      } else {
+      const principalValues = this.principal[principalType];
+       if (principalValues) {
-       if (principalType) {
-         const principalValues = this.principal[principalType];
          if (principalValues instanceof Array) {
            return principalValues.some((a) =>
              new Matcher(applyContext(a, context)).match(principal)
            );
          }
          return new Matcher(applyContext(principalValues, context)).match(
            principal
          );
        }
        return false;
      }
    }
    return true;
  }
@roggervalf
Copy link
Owner

hey, thank you @yaser-ali-s for reporting this bug, let me address it

@roggervalf roggervalf added the bug Something isn't working label Apr 9, 2021
roggervalf added a commit that referenced this issue Apr 9, 2021
this affects matchPrincipals and matchNotPrincipals

fix #42
roggervalf pushed a commit that referenced this issue Apr 9, 2021
## [4.8.1](v4.8.0...v4.8.1) (2021-04-09)

### Bug Fixes

* **resourcebasedstatement:** check principalType when is undefined ([34ce2f4](34ce2f4)), closes [#42](#42)
@roggervalf
Copy link
Owner

🎉 This issue has been resolved in version 4.8.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

No branches or pull requests

2 participants