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

Global python functions/vars for expression policies #4842

Closed
benedikt-bartscher opened this issue Mar 5, 2023 · 9 comments · Fixed by #5189
Closed

Global python functions/vars for expression policies #4842

benedikt-bartscher opened this issue Mar 5, 2023 · 9 comments · Fixed by #5189
Assignees
Labels
enhancement/confirmed Enhancements that will be implemented in the future enhancement New feature or request

Comments

@benedikt-bartscher
Copy link

Add an option to globaly define some variables and functions to reuse python code in expression policys.
Currently i find myself having similar code and variables chaotically distributed in policies.

@benedikt-bartscher benedikt-bartscher added the enhancement New feature or request label Mar 5, 2023
@sdimovv
Copy link
Contributor

sdimovv commented Mar 5, 2023

You can already achieve this... Well sort of.

Option 1:

  1. Define your common functionality in an expression policy
  2. Add each method/value to the context dict
  3. Return True from the policy so it always passes

Then:

  1. Attach the policy to the beginning of each flow
  2. Use the custom method and variable definitions in other policies.

Option 2:

  1. Define your common functionality in an expression policy
  2. Take parameters for this functionality from the context
  3. Add ak_message(...) for whatever the user needs to be notified about
  4. Return True/False depending on the use case

Then:

  1. Call the policy from another policy and add the needed parameters to its context (ak_call_policy(...))
  2. Collect the returned messages and policy result
  3. Display the messages by calling ak_message(<returned_message>)
  4. Pass/Fail policy

Option 3
If it is just variables (without custom python code) you can also define them as tenant or group attributes.

Hope that helps.

@benedikt-bartscher
Copy link
Author

benedikt-bartscher commented Mar 6, 2023

Hey @sdimovv, thanks for the detailed information. I will try Option 2 and report back with my results. Do you think this approach should become the recommended approach for authentik or is it worth to add a seperate Feature/Model for this? Both sides have their pros/cons.

Seperate Model:
(+) could be made intuitive
(+) (depending on the implementation) no additional config needed for policies using it
(-) more models/terminoligies to learn
(-) additional code+testing needed

"Helper" Expression Policys:
(+) no additional code/abstraction/testing needed
(-) requires additional config in every policy which uses the helper
(-) not very intuitive

No matter which way is choosen, we should document how to share code/variables and present a recommended approach in the docs.

@BeryJu
Copy link
Member

BeryJu commented Mar 6, 2023

Ny recommendation would also be the 2nd option, basically using polices as functions and then calling them via ak_call_policy(), which will return whatever the policy called returns (it doesn't have to be boolean)

It would be a bit clunky to call ak_call_policy() multiple times as each call does a database lookup for the policy, so I think changing it to ak_import_policy() that returns a callable function would be cleaner my_policy = ak_import_policy("foo"); my_policy("foo", "bar")

@rhaex
Copy link

rhaex commented Mar 13, 2023

@BeryJu

Ny recommendation would also be the 2nd option, basically using polices as functions and then calling them via ak_call_policy(), which will return whatever the policy called returns (it doesn't have to be boolean)

This does not seem to work, that call will always return a PolicyResult object, that does not seem to have the output result. Only the methods passing and messages which are of type bool and tuple[str].

Is this something that is not implemented yet or am I doing something wrong?
It is however possible to achieve this indirectly by storing the return value in some key of the context dict, and then read it in the parent policy after calling the child policy.

@macmoritz
Copy link
Contributor

Return does not work. Even in the PolicyResult object there is not field for the return value.
My current workaround is using the request context to passing data between policies.

@macmoritz
Copy link
Contributor

Another downsite of the workaround no. 2 is that the policies used by ak_call_policy are marked as "Warning: Policy is not assigned.".

@benedikt-bartscher
Copy link
Author

I found another challange: Where to store secrets like API Tokens? From a security perspective it wouldn't be great to store them in the policy which is saved as plain-text in the DB. Maybe a K8 Secret + volume mount works, however i haven't tested it yet.

@macmoritz
Copy link
Contributor

macmoritz commented Apr 5, 2023

With the latest changes to prompts (#4822) we can create dynamic prompts.
Would be nice to use method 2, so using ak_call_policy in placeholder (used as expression) in prompts. At the moment following stacktrace appears:

Traceback (most recent call last):
  File "/authentik/stages/prompt/api.py", line 102, in preview
    ).get_prompt_challenge_fields([prompt_model], {}, dry_run=True)
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/authentik/stages/prompt/stage.py", line 208, in get_prompt_challenge_fields
    field.get_placeholder(context, self.get_pending_user(), self.request, dry_run)
  File "/authentik/stages/prompt/models.py", line 211, in get_placeholder
    raise wrapped from exc
authentik.core.exceptions.PropertyMappingExpressionException: name 'ak_call_policy' is not defined

@BeryJu BeryJu self-assigned this Apr 5, 2023
@BeryJu BeryJu added the enhancement/confirmed Enhancements that will be implemented in the future label Apr 5, 2023
@macmoritz
Copy link
Contributor

Thanks @BeryJu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement/confirmed Enhancements that will be implemented in the future enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants