This PR implement project-less API token allowing to generate token that aren't specific to a single project. This make senses for actions such as:
Fixes https://pagure.io/pagure/issue/1500
The tests are actually passing, jenkins is failing somewhere later and that break the test results...
rebased
After giving it more thoughts, I removed the possibility for any user to create API token allowed to flag or comment any PR/ticket as this would become way to easy to abuse.
So regular user can only now create project-less API token to create new project or to fork existing project.
Admins on the other hand will be allowed to create more powerfull project-less API token and it will be their responsability to give these API token to responsible 3rd party.
This however, will rely on the pagure-admin CLI implemented in : https://pagure.io/pagure/pull-request/2061
So this PR can be reviewed and merged as is, however a part is missing that will be added once #2061 is merged. This can be in another PR if we want to review and merge this first :)
Why were we checking for the fullname and not anymore ?
I was just trying to be consistent with other parts of the API that are directly comparing the objects. I am not sure which is best (object vs fullpath) but I just wanted to be consistent :)
Maybe if restrict is not None would be better here.
if restrict is not None
Well, in this case if restrict is None, '' or False the query won't work :)
None
''
False
We are using the same code in add_user_token maybe we could have a function with the redirection url has parameter ?
add_user_token
Yeah, i ve seen just after that restrict could be also a string :)
:thumbsup: consistency
Why do we need to set it to None here ?
we are testing for 'false', 'False', should we have similar test for 'true' and 'True' ?
'false'
'False'
'true'
'True'
Can you create a repo within a namespace using the API ? if so we might want to test that case
A few comments, other than that looks good.
The tests part are quiet repetitive, I don't know if we could find a way to refactor them, but this could be done in another PR.
Not sure to follow you entirely, this is to revoke a token while add_user_token is meant to create one
I guess I'm just making sure it's defined before the loop but I think it could be safely removed, I'll try w/o it
Sure we can :)
I'll add one :)
My comment was not clear, I meant that the code that check for the admin_session_timeout is the same in both add_user_token and revoke_api_user_token except for the url used in the redirect.
admin_session_timeout
revoke_api_user_token
I was just thinking that making a generic function that is checking admin_session_timeout and redirects to a given url might be useful
We could consider renaming add_user_token to add_api_user_token so it follows the same pattern than revoke_api_user_token
add_api_user_token
We could do that, but there will be quite a few more places to check :)
Ok :)
Adjusted and rebased :)
?
It increases the output a little bit (also to be a little consistent with the other test file)
LGTM :beers:
Thanks!
Pull-Request has been merged by pingou
This PR implement project-less API token allowing to generate token that aren't
specific to a single project. This make senses for actions such as:
Fixes https://pagure.io/pagure/issue/1500