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.

Sign up for GitHub

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

Jump to bottom

Check callers of match_policies and get_action_values #1691

Closed
fredreichbier opened this issue Jun 18, 2019 · 6 comments · Fixed by #2017
Closed

Check callers of match_policies and get_action_values #1691

fredreichbier opened this issue Jun 18, 2019 · 6 comments · Fixed by #2017
Assignees
@cornelinux
Labels
Generic: Code-cleanup Issues concerning refactoring and clean code Status: Waiting for review Developer things is done. Needs review
Milestone
3.3

Comments

@fredreichbier
Copy link
Contributor

fredreichbier commented Jun 18, 2019 •
edited
Loading

In #1672 (last comment), we clarified and documented the meaning of parameters for match_policies and get_action_values. We should now ensure that all calling locations of these methods actually pass the parameters correctly (i.e. if they pass resolver=None, they really don't care about the resolver).
@fredreichbier fredreichbier added the Generic: Code-cleanup Issues concerning refactoring and clean code label Jun 18, 2019
@fredreichbier fredreichbier added this to the 3.1 polishing policies milestone Jun 18, 2019
@cornelinux
Copy link
Member

Hi @fredreichbier I guess you know what to do? ;-)

@fredreichbier fredreichbier changed the title Check callers of get_policies and get_action_values Check callers of match_policies and get_action_values Jul 16, 2019
fredreichbier pushed a commit that referenced this issue Jul 17, 2019
Start refactoring policy matching
653caeb 
This adds a function match_policies_strict as a short-hand
for the most usual calls to PolicyClass.match_policies.

Working on #1691
fredreichbier pushed a commit that referenced this issue Jul 19, 2019
Add a high-level policy matching interface
2699b6f 
Working on #1691
fredreichbier pushed a commit that referenced this issue Jul 24, 2019
Start refactoring policy matching
277cf72 
This adds a function match_policies_strict as a short-hand
for the most usual calls to PolicyClass.match_policies.

Working on #1691
fredreichbier pushed a commit that referenced this issue Jul 24, 2019
Add a high-level policy matching interface
a75c7b9 
Working on #1691
@fredreichbier fredreichbier mentioned this issue Jul 24, 2019
Merged
@fredreichbier
Copy link
Contributor Author

I think in the medium run, we should refactor our policy matching routines a bit. #1757 contains a first idea. However, as this is quite a big change, I would suggest we do this after 3.1?

@fredreichbier
Copy link
Contributor Author

Refactoring policy matching sounds nice, but we'll do it in 3.2 at the earliest.

fredreichbier pushed a commit that referenced this issue Aug 15, 2019
Start refactoring policy matching
d8f8542 
This adds a function match_policies_strict as a short-hand
for the most usual calls to PolicyClass.match_policies.

Working on #1691
fredreichbier pushed a commit that referenced this issue Aug 15, 2019
Add a high-level policy matching interface
53047c7 
Working on #1691
fredreichbier pushed a commit that referenced this issue Aug 22, 2019
Start refactoring policy matching
f873c0f 
This adds a function match_policies_strict as a short-hand
for the most usual calls to PolicyClass.match_policies.

Working on #1691
fredreichbier pushed a commit that referenced this issue Aug 22, 2019
Add a high-level policy matching interface
fcdc163 
Working on #1691
@cornelinux
Copy link
Member

We should also consider the following in this issue:

In certain cases we only have a serial number in the request but we would also have to match for realm and resolver. Imagine an administrator who has a policy that allows the admin to unassign tokens in realmA in resolver2. We can define such an admin policy.

But the endpoint /token/unassign?serial=<serial> is called from the webui.
The check_base_action decorator (https://github.com/privacyidea/privacyidea/blob/master/privacyidea/api/token.py#L438) thus only received the serial number, since the request.User is None.

So we either need to add the tokenowner in such cases or we should add the request.User in general if an API endpoint only contains the serial.

@fredreichbier
Copy link
Contributor Author

This is an interesting point! I think in case of unassigning a token, i.e. when the check_base_action function only receives a serial and no realm or resolver, the function actually fetches the first token realm and uses it for policy matching:

privacyidea/privacyidea/api/lib/prepolicy.py

Line 1092 in f1543ce

realm = get_realms_of_token(params.get("serial"),

... but resolver is still None.

I'm currently not sure how this should behave. Should we set resolver to the tokenowner's resolver? Or should we also account for the future in which one token might have multiple tokenowners?

fredreichbier pushed a commit that referenced this issue Sep 25, 2019
Start refactoring policy matching
c2b3533 
This adds a function match_policies_strict as a short-hand
for the most usual calls to PolicyClass.match_policies.

Working on #1691
fredreichbier pushed a commit that referenced this issue Sep 25, 2019
Add a high-level policy matching interface
c5a1813 
Working on #1691
@cornelinux cornelinux modified the milestones: 3.2, 3.3 Nov 18, 2019
@cornelinux
Copy link
Member

cornelinux commented Jan 30, 2020 •
edited
Loading

Also take into consideration the changes in PR #1870

Tracking the work in progress

  • adapt tokenclass.get_detault_settings (see da57564)
  • use Match.generic() / 13 occurrences
  • use Match.allowed() / 6 occurrences
  • use Match.action_only() / 5 occurrences

cornelinux added a commit that referenced this issue Feb 2, 2020
@cornelinux
Add constructor "generic" for the Match-Object
4976c0f 
The gerneric constructor is a legacy constructor,
if the other constructors will not match.

Taken from PR #1870

Working on #1691
cornelinux added a commit that referenced this issue Feb 2, 2020
@cornelinux
Add method "allowed" for Match object
bda2ed7 
This is a shortcut for tests in the scope user or admin,
if an action is either allowed or if no policies are defined at all.
In those cases the user or admin would be allowed to perform
the requested action.

Taken from #1870

Working on #1691
@cornelinux cornelinux mentioned this issue Feb 11, 2020
Closed
cornelinux added a commit that referenced this issue Feb 11, 2020
@cornelinux
Add constructor "generic" for the Match-Object
6bc81d4 
The gerneric constructor is a legacy constructor,
if the other constructors will not match.

Taken from PR #1870

Working on #1691
cornelinux added a commit that referenced this issue Feb 11, 2020
@cornelinux
Add method "allowed" for Match object
0ebac56 
This is a shortcut for tests in the scope user or admin,
if an action is either allowed or if no policies are defined at all.
In those cases the user or admin would be allowed to perform
the requested action.

Taken from #1870

Working on #1691
cornelinux added a commit that referenced this issue Feb 11, 2020
@cornelinux
Use Match...allowed() in all relevant places.
a4c567b 
Match...allowrd() checks if an action would be allowed based
on the policies. It also takes into account, if policies
for this situation are defined at_all.

Working on #1691
@cornelinux cornelinux mentioned this issue Feb 11, 2020
Merged
cornelinux added a commit that referenced this issue Feb 11, 2020
@cornelinux
Add method action_only for Match object
4d02c43 
This is used, when the policies are only matched for
scope and action: In the webui before a user has logged in.

Working on #1691
cornelinux added a commit that referenced this issue Feb 11, 2020
@cornelinux
Use Match.generic to remove policy_object.match_policies
f05e5a9 
The remaining policy_object.match_policies calls are replaced
by the top level Match.generic().policies() function.

Working on #1691
cornelinux added a commit that referenced this issue Feb 11, 2020
@cornelinux
Replace policy.get_action_values with Match.generic().action_values()
732e948 
Replace the remaining policy calls for get_action_values
with the new Match API.

Working on #1691
cornelinux added a commit that referenced this issue Feb 11, 2020
@cornelinux
Use policy Match API in token get_default_settings
ef2a47e 
The get_default_settings of a token class also
reads policy definitions. So we also want to
use the Match API there.
Here we replace the old policy handling with the
new Match API, which also requires a signature
change of the get_default_settings method.

Working on #1691
@cornelinux cornelinux added the Status: Waiting for review Developer things is done. Needs review label Feb 11, 2020
cornelinux added a commit that referenced this issue Dec 3, 2020
@cornelinux
Allow to display the node in the UI
d8be919 
A policy can be used to display the privacyIDEA node in the
UI to help the admin and user to distinguish the different privacyIDEA
instances.

Working on #1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Add constructor "generic" for the Match-Object
3f5833e 
The gerneric constructor is a legacy constructor,
if the other constructors will not match.

Taken from PR privacyidea#1870

Working on privacyidea#1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Add method "allowed" for Match object
11f4174 
This is a shortcut for tests in the scope user or admin,
if an action is either allowed or if no policies are defined at all.
In those cases the user or admin would be allowed to perform
the requested action.

Taken from privacyidea#1870

Working on privacyidea#1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Use Match...allowed() in all relevant places.
00ee004 
Match...allowrd() checks if an action would be allowed based
on the policies. It also takes into account, if policies
for this situation are defined at_all.

Working on privacyidea#1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Add method action_only for Match object
cbd55c8 
This is used, when the policies are only matched for
scope and action: In the webui before a user has logged in.

Working on privacyidea#1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Use Match.generic to remove policy_object.match_policies
cd62873 
The remaining policy_object.match_policies calls are replaced
by the top level Match.generic().policies() function.

Working on privacyidea#1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Replace policy.get_action_values with Match.generic().action_values()
c3e2f50 
Replace the remaining policy calls for get_action_values
with the new Match API.

Working on privacyidea#1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Use policy Match API in token get_default_settings
2d1483e 
The get_default_settings of a token class also
reads policy definitions. So we also want to
use the Match API there.
Here we replace the old policy handling with the
new Match API, which also requires a signature
change of the get_default_settings method.

Working on privacyidea#1691
maxbeth pushed a commit to maxbeth/privacyidea that referenced this issue Mar 5, 2021
@cornelinux
Allow to display the node in the UI
2b7e1cb 
A policy can be used to display the privacyIDEA node in the
UI to help the admin and user to distinguish the different privacyIDEA
instances.

Working on privacyidea#1691
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cornelinux cornelinux

Labels
Generic: Code-cleanup Issues concerning refactoring and clean code Status: Waiting for review Developer things is done. Needs review
Projects
None yet
Milestone
3.3
Development

Successfully merging a pull request may close this issue.

1691/pick enhancements from 1870 privacyidea/privacyidea
2 participants
@fredreichbier @cornelinux