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

Add tokentype specifc limits #1966

Merged
merged 3 commits into from Dec 19, 2019
Merged

Add tokentype specifc limits #1966

merged 3 commits into from Dec 19, 2019

Conversation

@cornelinux
Copy link
Member

cornelinux commented Dec 12, 2019

The admin can define policies to limit the
number of tokens a user may enroll based on the
specific tokentype.

Working on #1375

The admin can define policies to limit the
number of tokens a user may enroll based on the
specific tokentype.

Working on #1375
@pep8speaks

This comment has been minimized.

Copy link

pep8speaks commented Dec 12, 2019

Hello @cornelinux! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 74:1: E402 module level import not at top of file
Line 74:80: E501 line too long (83 > 79 characters)
Line 634:80: E501 line too long (93 > 79 characters)
Line 645:80: E501 line too long (100 > 79 characters)
Line 646:80: E501 line too long (110 > 79 characters)
Line 648:80: E501 line too long (99 > 79 characters)
Line 649:80: E501 line too long (80 > 79 characters)
Line 650:80: E501 line too long (83 > 79 characters)
Line 680:80: E501 line too long (98 > 79 characters)
Line 681:80: E501 line too long (110 > 79 characters)
Line 683:80: E501 line too long (84 > 79 characters)
Line 684:80: E501 line too long (93 > 79 characters)
Line 689:80: E501 line too long (100 > 79 characters)
Line 690:80: E501 line too long (101 > 79 characters)
Line 691:80: E501 line too long (93 > 79 characters)

Line 164:80: E501 line too long (109 > 79 characters)
Line 169:80: E501 line too long (116 > 79 characters)

Line 127:80: E501 line too long (110 > 79 characters)
Line 132:80: E501 line too long (117 > 79 characters)

Line 75:80: E501 line too long (87 > 79 characters)
Line 185:80: E501 line too long (109 > 79 characters)
Line 190:80: E501 line too long (116 > 79 characters)

Line 137:80: E501 line too long (109 > 79 characters)
Line 142:80: E501 line too long (116 > 79 characters)

Line 135:80: E501 line too long (108 > 79 characters)
Line 140:80: E501 line too long (115 > 79 characters)

Line 53:1: E402 module level import not at top of file
Line 105:80: E501 line too long (108 > 79 characters)
Line 110:80: E501 line too long (115 > 79 characters)

Line 101:80: E501 line too long (108 > 79 characters)
Line 106:80: E501 line too long (115 > 79 characters)

Line 112:80: E501 line too long (109 > 79 characters)
Line 117:80: E501 line too long (116 > 79 characters)

Line 115:80: E501 line too long (112 > 79 characters)
Line 120:80: E501 line too long (119 > 79 characters)

Line 39:80: E501 line too long (87 > 79 characters)
Line 233:80: E501 line too long (108 > 79 characters)
Line 238:80: E501 line too long (115 > 79 characters)

Line 101:80: E501 line too long (116 > 79 characters)
Line 106:80: E501 line too long (123 > 79 characters)

Line 117:80: E501 line too long (110 > 79 characters)
Line 123:80: E501 line too long (111 > 79 characters)

Line 135:80: E501 line too long (116 > 79 characters)
Line 141:80: E501 line too long (117 > 79 characters)

Line 122:80: E501 line too long (110 > 79 characters)
Line 128:80: E501 line too long (111 > 79 characters)

Line 59:80: E501 line too long (87 > 79 characters)
Line 241:80: E501 line too long (107 > 79 characters)
Line 247:80: E501 line too long (108 > 79 characters)

Line 44:1: E402 module level import not at top of file
Line 99:80: E501 line too long (109 > 79 characters)
Line 105:80: E501 line too long (110 > 79 characters)

Line 34:1: E402 module level import not at top of file
Line 94:80: E501 line too long (105 > 79 characters)
Line 100:80: E501 line too long (106 > 79 characters)

Line 112:80: E501 line too long (107 > 79 characters)
Line 118:80: E501 line too long (108 > 79 characters)

Line 168:80: E501 line too long (108 > 79 characters)
Line 174:80: E501 line too long (109 > 79 characters)

Line 175:80: E501 line too long (110 > 79 characters)
Line 181:80: E501 line too long (111 > 79 characters)

Line 277:80: E501 line too long (107 > 79 characters)
Line 283:80: E501 line too long (108 > 79 characters)

Line 110:80: E501 line too long (109 > 79 characters)
Line 116:80: E501 line too long (110 > 79 characters)

Line 112:80: E501 line too long (110 > 79 characters)
Line 118:80: E501 line too long (111 > 79 characters)

Line 170:80: E501 line too long (111 > 79 characters)
Line 176:80: E501 line too long (112 > 79 characters)

@cornelinux

This comment has been minimized.

Copy link
Member Author

cornelinux commented Dec 12, 2019

Work in progress. Still need to add the tests...

@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 12, 2019

Codecov Report

Merging #1966 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1966      +/-   ##
==========================================
+ Coverage   97.21%   97.22%   +<.01%     
==========================================
  Files         153      153              
  Lines       18555    18590      +35     
==========================================
+ Hits        18039    18074      +35     
  Misses        516      516
Impacted Files Coverage Δ
privacyidea/lib/tokens/u2ftoken.py 98.12% <ø> (ø) ⬆️
privacyidea/lib/tokens/sshkeytoken.py 100% <100%> (ø) ⬆️
privacyidea/lib/tokens/foureyestoken.py 90.9% <100%> (+0.1%) ⬆️
privacyidea/lib/tokens/remotetoken.py 99.07% <100%> (ø) ⬆️
privacyidea/lib/tokens/daplugtoken.py 100% <100%> (ø) ⬆️
privacyidea/lib/tokens/certificatetoken.py 97.32% <100%> (+0.02%) ⬆️
privacyidea/lib/tokens/motptoken.py 96.15% <100%> (+0.04%) ⬆️
privacyidea/lib/tokens/yubikeytoken.py 97.67% <100%> (+0.01%) ⬆️
privacyidea/lib/tokens/spasstoken.py 100% <100%> (ø) ⬆️
privacyidea/lib/tokens/tantoken.py 100% <100%> (ø) ⬆️
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 154dfdd...0bdf8f2. Read the comment docs.

Closes #1375
@cornelinux cornelinux marked this pull request as ready for review Dec 12, 2019
@cornelinux

This comment has been minimized.

Copy link
Member Author

cornelinux commented Dec 12, 2019

Most of the changed files are the policy definitions of the token classes.
The real logic is contained in api/lib/preprolicy.py.

@cornelinux cornelinux requested a review from plettich Dec 12, 2019
ACTION.MAXACTIVETOKENUSER: {
'type': 'int',
'desc': _("The user may only have this maximum number of active paper tokens assigned."),
'group': GROUP.TOKEN

This comment has been minimized.

Copy link
@plettich

plettich Dec 19, 2019

Member

remember my (rejected) changes from #1938? We could define template policies which will be automatically generated in each token class. We could save some code duplication. Should i implement this?

This comment has been minimized.

Copy link
@cornelinux

cornelinux Dec 19, 2019

Author Member

No. I do not remember. The 1938 points to something merged.
Did we have an issue for this idea? Otherwise you can create one and I would like to have a few lines of meta code in the issue so we have a common understanding what this implementation would do.

I also do not like the many lines of code - although they are no lines of code.
We would still have specific desc of the policies, so I am not so sure, how some templates could help, keeping the flexibility and producing less lines of code.

Copy link
Member

plettich left a comment

looks good. Should i look into changing the token policies?

@cornelinux

This comment has been minimized.

Copy link
Member Author

cornelinux commented Dec 19, 2019

looks good. Should i look into changing the token policies?

Please put policy templates in another issue and have the discussion before the PR.

@cornelinux cornelinux merged commit 5c265bb into master Dec 19, 2019
5 checks passed
5 checks passed
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 97.21%)
Details
codecov/project 97.22% (+<.01%) compared to 3bb65bd
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.