diff --git a/docs/flow/examples/enrollment-2-stage.json b/docs/flow/examples/enrollment-2-stage.json index e8e4372c8..2f4532ff3 100644 --- a/docs/flow/examples/enrollment-2-stage.json +++ b/docs/flow/examples/enrollment-2-stage.json @@ -137,7 +137,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -149,7 +149,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -161,7 +161,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -173,7 +173,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } } ] diff --git a/docs/flow/examples/enrollment-email-verification.json b/docs/flow/examples/enrollment-email-verification.json index 299ad2d49..7446ec0bc 100644 --- a/docs/flow/examples/enrollment-email-verification.json +++ b/docs/flow/examples/enrollment-email-verification.json @@ -156,7 +156,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -168,7 +168,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -180,7 +180,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -192,7 +192,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -204,7 +204,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } } ] diff --git a/docs/flow/examples/login-2fa.json b/docs/flow/examples/login-2fa.json index a1b9637a5..1d9d25a63 100644 --- a/docs/flow/examples/login-2fa.json +++ b/docs/flow/examples/login-2fa.json @@ -68,7 +68,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -80,7 +80,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -92,7 +92,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -104,7 +104,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } } ] diff --git a/docs/flow/examples/login-conditional-captcha.json b/docs/flow/examples/login-conditional-captcha.json index 6134e53e2..f7159779e 100644 --- a/docs/flow/examples/login-conditional-captcha.json +++ b/docs/flow/examples/login-conditional-captcha.json @@ -71,7 +71,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -83,7 +83,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -95,7 +95,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -107,7 +107,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { diff --git a/docs/flow/examples/recovery-email-verification.json b/docs/flow/examples/recovery-email-verification.json index e3c849263..3403aea37 100644 --- a/docs/flow/examples/recovery-email-verification.json +++ b/docs/flow/examples/recovery-email-verification.json @@ -130,7 +130,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -142,7 +142,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -154,7 +154,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -166,7 +166,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } }, { @@ -178,7 +178,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } } ] diff --git a/docs/flow/examples/unenrollment.json b/docs/flow/examples/unenrollment.json index b2fd76f21..dbb0f42be 100644 --- a/docs/flow/examples/unenrollment.json +++ b/docs/flow/examples/unenrollment.json @@ -30,7 +30,7 @@ }, "model": "passbook_flows.flowstagebinding", "attrs": { - "re_evaluate_policies": false + "evaluate_on_call_policies": false } } ] diff --git a/passbook/flows/api.py b/passbook/flows/api.py index 36f134314..9e91a4684 100644 --- a/passbook/flows/api.py +++ b/passbook/flows/api.py @@ -27,7 +27,15 @@ class FlowStageBindingSerializer(ModelSerializer): class Meta: model = FlowStageBinding - fields = ["pk", "target", "stage", "re_evaluate_policies", "order", "policies"] + fields = [ + "pk", + "target", + "stage", + "evaluate_on_plan", + "evaluate_on_call", + "order", + "policies", + ] class FlowStageBindingViewSet(ModelViewSet): diff --git a/passbook/flows/forms.py b/passbook/flows/forms.py index 9c4d74781..015560e1d 100644 --- a/passbook/flows/forms.py +++ b/passbook/flows/forms.py @@ -50,12 +50,10 @@ class FlowStageBindingForm(forms.ModelForm): fields = [ "target", "stage", - "re_evaluate_policies", + "evaluate_on_plan", + "evaluate_on_call", "order", ] - labels = { - "re_evaluate_policies": _("Re-evaluate Policies"), - } widgets = { "name": forms.TextInput(), } diff --git a/passbook/flows/markers.py b/passbook/flows/markers.py index 628f0cf28..04bd5c297 100644 --- a/passbook/flows/markers.py +++ b/passbook/flows/markers.py @@ -2,6 +2,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Optional +from django.http.request import HttpRequest from structlog import get_logger from passbook.core.models import User @@ -20,7 +21,9 @@ class StageMarker: """Base stage marker class, no extra attributes, and has no special handler.""" # pylint: disable=unused-argument - def process(self, plan: "FlowPlan", stage: Stage) -> Optional[Stage]: + def process( + self, plan: "FlowPlan", stage: Stage, http_request: Optional[HttpRequest] + ) -> Optional[Stage]: """Process callback for this marker. This should be overridden by sub-classes. If a stage should be removed, return None.""" return stage @@ -33,10 +36,14 @@ class ReevaluateMarker(StageMarker): binding: PolicyBinding user: User - def process(self, plan: "FlowPlan", stage: Stage) -> Optional[Stage]: + def process( + self, plan: "FlowPlan", stage: Stage, http_request: Optional[HttpRequest] + ) -> Optional[Stage]: """Re-evaluate policies bound to stage, and if they fail, remove from plan""" engine = PolicyEngine(self.binding, self.user) engine.use_cache = False + if http_request: + engine.request.http_request = http_request engine.request.context = plan.context engine.build() result = engine.result diff --git a/passbook/flows/migrations/0015_flowstagebinding_evaluate_on_plan.py b/passbook/flows/migrations/0015_flowstagebinding_evaluate_on_plan.py new file mode 100644 index 000000000..1ac4babe7 --- /dev/null +++ b/passbook/flows/migrations/0015_flowstagebinding_evaluate_on_plan.py @@ -0,0 +1,34 @@ +# Generated by Django 3.1.2 on 2020-10-20 12:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("passbook_flows", "0014_auto_20200925_2332"), + ] + + operations = [ + migrations.RenameField( + model_name="flowstagebinding", + old_name="re_evaluate_policies", + new_name="evaluate_on_call", + ), + migrations.AlterField( + model_name="flowstagebinding", + name="evaluate_on_call", + field=models.BooleanField( + default=False, + help_text="Evaluate policies when the Stage is present to the user.", + ), + ), + migrations.AddField( + model_name="flowstagebinding", + name="evaluate_on_plan", + field=models.BooleanField( + default=True, + help_text="Evaluate policies during the Flow planning process. Disable this for input-based policies.", + ), + ), + ] diff --git a/passbook/flows/models.py b/passbook/flows/models.py index b802d441b..35dbf0103 100644 --- a/passbook/flows/models.py +++ b/passbook/flows/models.py @@ -154,15 +154,19 @@ class FlowStageBinding(SerializerModel, PolicyBindingModel): target = models.ForeignKey("Flow", on_delete=models.CASCADE) stage = InheritanceForeignKey(Stage, on_delete=models.CASCADE) - re_evaluate_policies = models.BooleanField( - default=False, + evaluate_on_plan = models.BooleanField( + default=True, help_text=_( ( - "When this option is enabled, the planner will re-evaluate " - "policies bound to this binding." + "Evaluate policies during the Flow planning process. " + "Disable this for input-based policies." ) ), ) + evaluate_on_call = models.BooleanField( + default=False, + help_text=_("Evaluate policies when the Stage is present to the user."), + ) order = models.IntegerField() diff --git a/passbook/flows/planner.py b/passbook/flows/planner.py index 20bb02e7d..797409508 100644 --- a/passbook/flows/planner.py +++ b/passbook/flows/planner.py @@ -46,7 +46,7 @@ class FlowPlan: self.stages.append(stage) self.markers.append(marker or StageMarker()) - def next(self) -> Optional[Stage]: + def next(self, http_request: Optional[HttpRequest]) -> Optional[Stage]: """Return next pending stage from the bottom of the list""" if not self.has_stages: return None @@ -55,7 +55,7 @@ class FlowPlan: if marker.__class__ is not StageMarker: LOGGER.debug("f(plan_inst): stage has marker", stage=stage, marker=marker) - marked_stage = marker.process(self, stage) + marked_stage = marker.process(self, stage, http_request) if not marked_stage: LOGGER.debug("f(plan_inst): marker returned none, next stage", stage=stage) self.stages.remove(stage) @@ -63,7 +63,7 @@ class FlowPlan: if not self.has_stages: return None # pylint: disable=not-callable - return self.next() + return self.next(http_request) return marked_stage def pop(self): @@ -159,23 +159,41 @@ class FlowPlanner: for binding in FlowStageBinding.objects.filter( target__pk=self.flow.pk ).order_by("order"): - engine = PolicyEngine(binding, user, request) - engine.request.context = plan.context - engine.build() - if engine.passing: + binding: FlowStageBinding + stage = binding.stage + marker = StageMarker() + if binding.evaluate_on_plan: LOGGER.debug( - "f(plan): Stage passing", stage=binding.stage, flow=self.flow + "f(plan): evaluating on plan", + stage=binding.stage, + flow=self.flow, ) - plan.stages.append(binding.stage) - marker = StageMarker() - if binding.re_evaluate_policies: + engine = PolicyEngine(binding, user, request) + engine.request.context = plan.context + engine.build() + if engine.passing: LOGGER.debug( - "f(plan): Stage has re-evaluate marker", + "f(plan): Stage passing", stage=binding.stage, flow=self.flow, ) - marker = ReevaluateMarker(binding=binding, user=user) - plan.markers.append(marker) + else: + stage = None + else: + LOGGER.debug( + "f(plan): not evaluating on plan", + stage=binding.stage, + flow=self.flow, + ) + if binding.evaluate_on_call and stage: + LOGGER.debug( + "f(plan): Stage has re-evaluate marker", + stage=binding.stage, + flow=self.flow, + ) + marker = ReevaluateMarker(binding=binding, user=user) + if stage: + plan.append(stage, marker) LOGGER.debug( "f(plan): Finished building", flow=self.flow, diff --git a/passbook/flows/tests/test_planner.py b/passbook/flows/tests/test_planner.py index eaa5f3864..f9d583660 100644 --- a/passbook/flows/tests/test_planner.py +++ b/passbook/flows/tests/test_planner.py @@ -132,7 +132,7 @@ class TestFlowPlanner(TestCase): target=flow, stage=DummyStage.objects.create(name="dummy1"), order=0, - re_evaluate_policies=True, + evaluate_on_call=True, ) request = self.request_factory.get( @@ -161,7 +161,7 @@ class TestFlowPlanner(TestCase): target=flow, stage=DummyStage.objects.create(name="dummy2"), order=1, - re_evaluate_policies=True, + evaluate_on_call=True, ) PolicyBinding.objects.create(policy=false_policy, target=binding2, order=0) diff --git a/passbook/flows/tests/test_views.py b/passbook/flows/tests/test_views.py index 7471d17ca..9452a75f5 100644 --- a/passbook/flows/tests/test_views.py +++ b/passbook/flows/tests/test_views.py @@ -174,7 +174,7 @@ class TestFlowExecutor(TestCase): target=flow, stage=DummyStage.objects.create(name="dummy2"), order=1, - re_evaluate_policies=True, + evaluate_on_call=True, ) PolicyBinding.objects.create(policy=false_policy, target=binding2, order=0) @@ -225,7 +225,7 @@ class TestFlowExecutor(TestCase): target=flow, stage=DummyStage.objects.create(name="dummy2"), order=1, - re_evaluate_policies=True, + evaluate_on_call=True, ) binding3 = FlowStageBinding.objects.create( target=flow, stage=DummyStage.objects.create(name="dummy3"), order=2 @@ -292,13 +292,13 @@ class TestFlowExecutor(TestCase): target=flow, stage=DummyStage.objects.create(name="dummy2"), order=1, - re_evaluate_policies=True, + evaluate_on_call=True, ) binding3 = FlowStageBinding.objects.create( target=flow, stage=DummyStage.objects.create(name="dummy3"), order=2, - re_evaluate_policies=True, + evaluate_on_call=True, ) binding4 = FlowStageBinding.objects.create( target=flow, stage=DummyStage.objects.create(name="dummy4"), order=2 diff --git a/passbook/flows/views.py b/passbook/flows/views.py index fd2dad218..8d894eb95 100644 --- a/passbook/flows/views.py +++ b/passbook/flows/views.py @@ -86,7 +86,7 @@ class FlowExecutorView(View): return to_stage_response(self.request, self.handle_invalid_flow(exc)) # We don't save the Plan after getting the next stage # as it hasn't been successfully passed yet - next_stage = self.plan.next() + next_stage = self.plan.next(self.request) if not next_stage: LOGGER.debug("f(exec): no more stages, flow is done.") return self._flow_done() diff --git a/swagger.yaml b/swagger.yaml index dd0f95766..20afa2d81 100755 --- a/swagger.yaml +++ b/swagger.yaml @@ -833,7 +833,12 @@ paths: description: '' required: false type: string - - name: re_evaluate_policies + - name: evaluate_on_plan + in: query + description: '' + required: false + type: string + - name: evaluate_on_call in: query description: '' required: false @@ -6337,10 +6342,14 @@ definitions: title: Stage type: string format: uuid - re_evaluate_policies: - title: Re evaluate policies - description: When this option is enabled, the planner will re-evaluate policies - bound to this binding. + evaluate_on_plan: + title: Evaluate on plan + description: Evaluate policies during the Flow planning process. Disable this + for input-based policies. + type: boolean + evaluate_on_call: + title: Evaluate on call + description: Evaluate policies when the Stage is present to the user. type: boolean order: title: Order