Feature: Policy editor Parliament linting (Netflix#9008)

* feat(handlers): add check policy handler

* feat(ui): add policy linting errors

* feat(ui): editor policy linting errors

* feat(ui): add custom styles + assets

* feat(ui): add no errors case

* feat(core): add parliament dependency

* feat(handlers): add check policy handler test

* feat(ui): policy monaco editor refactor

* fix(core): pre-commit

* fix(handlers): remove debug statement

* feat(ui): policy check on editor change

* fix(ui): remove linting error on assume role policy

* fix(ui): conditional linting of policies

* fix(conftest): remove unused mocks

* feat(core): add policy check swagger entry

* Migrate some of the changes back for models.py. These need to be manually fixed in a future PR

Co-authored-by: Curtis <[email protected]>
Co-authored-by: Curtis Castrapel <[email protected]>

.isort.cfg

@@ -1,6 +1,6 @@
 [settings]
 known_first_party=consoleme,consoleme_default_plugins
-known_third_party = aiozipkin,asgiref,bcrypt,billiard,bleach,boto3,botocore,celery,click,click_log,cloudaux,cryptography,dateutil,deepdiff,ed25519,elasticsearch,email_validator,furl,google,googleapiclient,jsonschema,jwt,logmatic,marshmallow,mock,mockredis,moto,okta_jwt,onelogin,pandas,password_strength,pip,pkg_resources,policy_sentry,policyuniverse,pydantic,pytest,pytz,redis,redislite,requests,retrying,ruamel,sentry_sdk,setuptools,simplejson,tenacity,tornado,ujson,uvloop,validate_email,yaml
+known_third_party = aiozipkin,asgiref,bcrypt,billiard,bleach,boto3,botocore,celery,click,click_log,cloudaux,cryptography,dateutil,deepdiff,ed25519,elasticsearch,email_validator,furl,google,googleapiclient,jsonschema,jwt,logmatic,marshmallow,mock,mockredis,moto,okta_jwt,onelogin,pandas,parliament,password_strength,pip,pkg_resources,policy_sentry,policyuniverse,pydantic,pytest,pytz,redis,redislite,requests,retrying,ruamel,sentry_sdk,setuptools,simplejson,tenacity,tornado,ujson,uvloop,validate_email,yaml
 multi_line_output=3
 include_trailing_comma=True
 balanced_wrapping=True

consoleme/handlers/v2/policies.py

@@ -1,4 +1,6 @@
+import tornado.escape
 import ujson as json
+from parliament import analyze_policy_string, enhance_finding
 
 from consoleme.config import config
 from consoleme.exceptions.exceptions import MustBeFte
@@ -150,6 +152,34 @@ async def post(self):
  return
 
 
+class CheckPoliciesHandler(BaseAPIV2Handler):
+ async def post(self):
+ """
+ POST /api/v2/policies/check
+ """
+ policy = tornado.escape.json_decode(self.request.body)
+ analyzed_policy = analyze_policy_string(policy)
+ findings = analyzed_policy.findings
+
+ enhanced_findings = []
+
+ for finding in findings:
+ enhanced_finding = enhance_finding(finding)
+ enhanced_findings.append(
+ {
+ "issue": enhanced_finding.issue,
+ "detail": enhanced_finding.detail,
+ "location": enhanced_finding.location,
+ "severity": enhanced_finding.severity,
+ "title": enhanced_finding.title,
+ "description": enhanced_finding.description,
+ }
+ )
+
+ self.write(json.dumps(enhanced_findings))
+ return
+
+
 class ManagedPoliciesHandler(BaseHandler):
  async def get(self, account_id):
  """

consoleme/models.py

@@ -1,6 +1,6 @@
 # generated by datamodel-codegen:
 # filename: swagger.yaml
-# timestamp: 2021-02-18T04:59:54+00:00
+# timestamp: 2021-03-09T01:15:09+00:00
 
 from __future__ import annotations
 
@@ -400,7 +400,6 @@ class CloneRoleRequestModel(BaseModel):
 class ActionResult(BaseModel):
  status: Optional[str] = None
  message: Optional[str] = None
- visible: Optional[bool] = True
 
 
 class CreateCloneRequestResponse(BaseModel):
@@ -518,8 +517,8 @@ class LoginAttemptModel(BaseModel):
 
 
 class RegistrationAttemptModel(BaseModel):
- username: str = None
- password: str = None
+ username: str
+ password: str
 
 
 class Status2(Enum):
@@ -541,6 +540,19 @@ class WebResponse(BaseModel):
  data: Optional[Dict[str, Any]] = None
 
 
+class PolicyCheckModelItem(BaseModel):
+ issue: Optional[str] = None
+ detail: Optional[str] = None
+ location: Optional[str] = None
+ severity: Optional[str] = None
+ title: Optional[str] = None
+ description: Optional[str] = None
+
+
+class PolicyCheckModel(BaseModel):
+ __root__: List[PolicyCheckModelItem]
+
+
 class ChangeGeneratorModelArray(BaseModel):
  changes: List[
  Union[

consoleme/routes.py

@@ -38,6 +38,7 @@
  FrontendHandler,
 )
 from consoleme.handlers.v2.policies import (
+ CheckPoliciesHandler,
  ManagedPoliciesHandler,
  PoliciesHandler,
  PoliciesPageConfigHandler,
@@ -122,6 +123,7 @@ def make_app(jwt_validator=None):
  (r"/api/v2/permission_templates/?", PermissionTemplatesHandler),
  (r"/api/v1/myheaders/?", ApiHeaderHandler),
  (r"/api/v1/policies/typeahead", ApiResourceTypeAheadHandler),
+ (r"/api/v2/policies/check", CheckPoliciesHandler),
  (r"/api/v2/dynamic_config", DynamicConfigApiHandler),
  (r"/api/v2/eligible_roles", EligibleRoleHandler),
  (r"/api/v2/eligible_roles_page_config", EligibleRolePageConfigHandler),

requirements.in

@@ -30,6 +30,7 @@ logmatic-python
 lxml
 okta_jwt
 pandas<1 # Internal plugin depends on Pandas < 1
+parliament
 password_strength
 pkgconfig
 policy-sentry

requirements.txt

@@ -12,6 +12,8 @@ amqp==5.0.2
  # via kombu
 appdirs==1.4.4
  # via black
+appnope==0.1.2
+ # via ipython
 argcomplete==1.12.2
  # via datamodel-code-generator
 asgiref==3.3.1
@@ -40,6 +42,7 @@ boto3==1.16.56
  # via
  # -r requirements.in
  # cloudaux
+ # parliament
 boto==2.49.0
  # via cloudaux
 botocore==1.19.56
@@ -188,8 +191,11 @@ jmespath==0.10.0
  # -r requirements.in
  # boto3
  # botocore
+ # parliament
 joblib==1.0.0
  # via cloudaux
+json-cfg==0.4.2
+ # via parliament
 jsonschema==3.2.0
  # via
  # -r requirements.in
@@ -198,6 +204,8 @@ kombu==5.0.2
  # via
  # -r requirements.in
  # celery
+kwonly-args==1.0.10
+ # via json-cfg
 lazy-object-proxy==1.4.3
  # via astroid
 logmatic-python==0.1.7
@@ -233,6 +241,8 @@ packaging==20.8
  # via bleach
 pandas==0.25.3
  # via -r requirements.in
+parliament==1.3.1
+ # via -r requirements.in
 parso==0.8.1
  # via jedi
 password-strength==0.0.3.post2
@@ -317,6 +327,7 @@ pytz==2020.5
 pyyaml==5.3.1
  # via
  # openapi-spec-validator
+ # parliament
  # policy-sentry
  # prance
 redis==3.5.3

swagger.yaml

@@ -20,6 +20,8 @@ tags:
  description: Policy Request endpoints
  - name: privileged
  description: Endpoints requiring elevated permissions
+ - name: policies
+ description: Policies endpoints
 paths:
  /request:
  post:
@@ -307,6 +309,24 @@ paths:
  description: OK
  default:
  $ref: "#/components/responses/DefaultErrorResponse"
+ /policies/check:
+ post:
+ summary: check a policy document
+ tags:
+ - policies
+ requestBody:
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ responses:
+ "200":
+ description: OK
+ content:
+ application/json:
+ schema:
+ $ref: "#/components/schemas/PolicyCheckModel"
 components:
  responses:
  DefaultErrorResponse:
@@ -1495,3 +1515,20 @@ components:
  type: string
  data:
  type: object
+ PolicyCheckModel:
+ type: array
+ items:
+ type: object
+ properties:
+ issue:
+ type: string
+ detail:
+ type: string
+ location:
+ type: string
+ severity:
+ type: string
+ title:
+ type: string
+ description:
+ type: string

tests/conftest.py

@@ -667,6 +667,73 @@ def redis(session_mocker):
  return True
 
 
+class MockParliament:
+ def __init__(self, return_value=None):
+ self.return_value = return_value
+
+ @property
+ def findings(self):
+ return self.return_value
+
+
+class Finding:
+ issue = ""
+ detail = ""
+ location = {}
+ severity = ""
+ title = ""
+ description = ""
+
+ def __init__(
+ self,
+ issue,
+ detail,
+ location,
+ severity,
+ title,
+ description,
+ ):
+ self.issue = issue
+ self.detail = detail
+ self.location = location
+ self.severity = severity
+ self.title = title
+ self.description = description
+
+
+@pytest.fixture(scope="session")
+def parliament(session_mocker):
+ session_mocker.patch(
+ "parliament.analyze_policy_string",
+ return_value=MockParliament(
+ return_value=[
+ {
+ "issue": "RESOURCE_MISMATCH",
+ "title": "No resources match for the given action",
+ "severity": "MEDIUM",
+ "description": "",
+ "detail": [
+ {"action": "s3:GetObject", "required_format": "arn:*:s3:::*/*"}
+ ],
+ "location": {"line": 3, "column": 18, "filepath": "test.json"},
+ }
+ ]
+ ),
+ )
+
+ session_mocker.patch(
+ "parliament.enhance_finding",
+ return_value=Finding(
+ issue="RESOURCE_MISMATCH",
+ title="No resources match for the given action",
+ severity="MEDIUM",
+ description="",
+ detail="",
+ location={},
+ ),
+ )
+
+
 @pytest.fixture(scope="session")
 def user_iam_role(iamrole_table, www_user):
  from consoleme.lib.dynamo import IAMRoleDynamoHandler
@@ -728,6 +795,7 @@ def populate_caches(
  sqs,
  iam,
  www_user,
+ parliament,
 ):
  from asgiref.sync import async_to_sync
 

tests/handlers/v2/test_policies.py

@@ -32,3 +32,31 @@ def test_policies_api(self):
  first_entity = response_j[0]
  self.assertEqual(first_entity["account_id"], "123456789012")
  self.assertEqual(first_entity["account_name"], "default_account")
+
+ def test_policies_check_api(self):
+ from consoleme.config import config
+
+ headers = {
+ config.get("auth.user_header_name"): "[email protected]",
+ config.get("auth.groups_header_name"): "groupa,groupb,groupc",
+ }
+ body = """{
+ "Version": "2012-10-17",
+ "Statement": {
+ "Effect": "Allow",
+ "Action":["s3:GetObject"],
+ "Resource": ["arn:aws:s3:::bucket1"]
+ }
+ }"""
+ response = self.fetch(
+ "/api/v2/policies/check", headers=headers, method="POST", body=body
+ )
+ self.assertEqual(response.code, 200)
+ response_j = json.loads(response.body)
+ self.assertEqual(len(response_j), 1)
+ first_error = response_j[0]
+ self.assertEqual(first_error["issue"], "RESOURCE_MISMATCH")
+ self.assertEqual(
+ first_error["title"], "No resources match for the given action"
+ )
+ self.assertEqual(first_error["severity"], "MEDIUM")

ui/src/components/policy/AssumeRolePolicy.js

@@ -30,6 +30,7 @@ const AssumeRolePolicy = () => {
  context="assume_role_policy"
  policy={assumeRolePolicy}
  updatePolicy={setAssumeRolePolicy}
+ enableLinting={false}
  />
  </Segment>
  <JustificationModal handleSubmit={handleAssumeRolePolicySubmit} />

ui/src/components/policy/PolicyMonacoEditor.css

@@ -0,0 +1,21 @@
+.infoError {
+ background: rgba(33, 133, 208, 0.4);
+}
+
+.warningError {
+ background: rgba(203, 174, 172, 0.4);
+}
+
+.criticalError {
+ background: rgba(255, 101, 80, 0.4);
+}
+
+.warningIcon {
+ display: block;
+ background-image: url("../../assets/icons/warning.png");
+ background-size: contain;
+ background-repeat: no-repeat;
+ padding: 2px;
+ background-origin: content-box;
+ background-position: bottom 0 right 4px;
+}