PR#5422: Add verify_token API endpoint - pagure

pagure

#5422 Add verify_token API endpoint

Closed 7 months ago by mattia. Opened 10 months ago by mattia.

Unknown source check-token into master

Mattia Verga • 8 months ago

ed5d687

pagure/api/__init__.py

file modified

		`@@ -557,6 +557,7 @@`
		`project.api_project_block_user,`
		`project.api_get_project_webhook_token,`
		`project.api_project_create_api_token,`
		`+ project.api_project_verify_api_token,`
		`project.api_commit_info,`
		`project.api_view_file,`
		`project.delete_project,`

pagure/api/project.py

file modified

+45

		`@@ -3214,6 +3214,51 @@`
		`return jsonout`


		`+ @API.route("/<repo>/token/verify", methods=["POST"])`
		`+ @API.route("/<namespace>/<repo>/token/verify", methods=["POST"])`
		`+ @API.route("/fork/<username>/<repo>/token/verify", methods=["POST"])`
		`+ @API.route(`
		`+ "/fork/<username>/<namespace>/<repo>/token/verify", methods=["POST"]`
		`+ )`
		`+ @api_method`
		`+ @api_login_required(acls=["modify_project"])`
		`+ def api_project_verify_api_token(repo, namespace=None, username=None):`
		`+ """`
		`+ Verify API project Token`
		`+ ------------------------`
		`+ Verify validity of a token API for the specified project`
		`+`
		`+ It does not verify token scope, but can be used to check if the`
		`+ token exists or is expired.`
		`+`
		`+ ::`
		`+`
		`+ POST /api/0/<repo>/token/verify`
		`+ POST /api/0/<namespace>/<repo>/token/verify`
		`+`
		`+ ::`
		`+`
		`+ POST /api/0/fork/<username>/<repo>/token/verify`
		`+ POST /api/0/fork/<username>/<namespace>/<repo>/token/verify`
		`+`
		`+`
		`+ Sample response`
		`+ ^^^^^^^^^^^^^^^`
		`+`
		`+ ::`
		`+`
		`+ {"message": "Token is valid"}`
		`+`
		`+ """`
		`+ output = {"message": "Token is valid"}`
		`+`
		`+ project = _get_repo(repo, username, namespace)`
		`+ _check_token(project, project_token=False)`
		`+`
		`+ jsonout = flask.jsonify(output)`
		`+ return jsonout`
		`+`
		`+`
		`@API.route("/<repo>/blockuser", methods=["POST"])`
		`@API.route("/<namespace>/<repo>/blockuser", methods=["POST"])`
		`@API.route("/fork/<username>/<repo>/blockuser", methods=["POST"])`

pagure/api/utils.py

file modified

+7 -8

		`@@ -49,15 +49,14 @@`
		`otherwise any token can be used`
		`:raises pagure.exceptions.APIError: when token is not valid for repo`
		`"""`
		`- if api_authenticated():`
		`- # if there is a project associated with the token, check it`
		`- # if there is no project associated, check if it is required`
		`- if (`
		`+ if (`
		`+ not api_authenticated()`
		`+ or (`
		`flask.g.token.project is not None and repo != flask.g.token.project`
		`- ) or (flask.g.token.project is None and project_token):`
		`- raise pagure.exceptions.APIError(`
		`- 401, error_code=APIERROR.EINVALIDTOK`
		`- )`
		`+ )`
		`+ or (flask.g.token.project is None and project_token)`
		`+ ):`
		`+ raise pagure.exceptions.APIError(401, error_code=APIERROR.EINVALIDTOK)`


		`def _get_issue(repo, issueid, issueuid=None):`

tests/test_pagure_flask_api_project.py

file modified

+59

		`@@ -4722,6 +4722,65 @@`
		`self.assertEqual(output.status_code, 401)`


		`+ class PagureFlaskApiProjectVerifyAPITokenTests(tests.Modeltests):`
		`+ """Tests for the flask API of pagure for verifying project API token validity"""`
		`+`
		`+ maxDiff = None`
		`+`
		`+ def setUp(self):`
		`+ """Set up the environnment, ran before every tests."""`
		`+ super(PagureFlaskApiProjectVerifyAPITokenTests, self).setUp()`
		`+ tests.create_projects(self.session)`
		`+ tests.create_tokens(self.session, project_id=None)`
		`+ tests.create_tokens_acl(self.session, "aaabbbcccddd", "modify_project")`
		`+`
		`+ def test_api_verifyapitoken_valid(self):`
		`+ """Test accessing api_project_verify_api_token with valid token."""`
		`+`
		`+ headers = {"Authorization": "token aaabbbcccddd"}`
		`+`
		`+ output = self.app.post("/api/0/test/token/verify", headers=headers)`
		`+ self.assertEqual(output.status_code, 200)`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ self.assertEqual(data, {"message": "Token is valid"})`
		`+`
		`+ def test_api_verifyapitoken_not_valid(self):`
		`+ """Test accessing api_project_verify_api_token with invalid token."""`
		`+`
		`+ headers = {"Authorization": "token dddcccbbbaaa"}`
		`+`
		`+ output = self.app.post("/api/0/test/token/verify", headers=headers)`
		`+ self.assertEqual(output.status_code, 401)`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ self.assertDictEqual(`
		`+ data,`
		`+ {`
		`+ "error": "Invalid or expired token. Please visit "`
		`+ "http://localhost.localdomain/settings#nav-api-tab to get or renew "`
		`+ "your API token.",`
		`+ "error_code": "EINVALIDTOK",`
		`+ "errors": "Invalid token",`
		`+ },`
		`+ )`
		`+`
		`+ def test_api_verifyapitoken_no_headers(self):`
		`+ """Test accessing api_project_verify_api_token without passing headers."""`
		`+`
		`+ output = self.app.post("/api/0/test/token/verify")`
		`+ self.assertEqual(output.status_code, 401)`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ self.assertDictEqual(`
		`+ data,`
		`+ {`
		`+ "error": "Invalid or expired token. Please visit "`
		`+ "http://localhost.localdomain/settings#nav-api-tab to get or renew "`
		`+ "your API token.",`
		`+ "error_code": "EINVALIDTOK",`
		`+ "errors": "Invalid token",`
		`+ },`
		`+ )`
		`+`
		`+`
		`class PagureFlaskApiProjectConnectorTests(tests.Modeltests):`
		`"""Tests for the flask API of pagure for getting connector of a project"""`

mattia commented 10 months ago

Add a simple endpoint which verify token validity for the specified project. Return 200 if token is valid or 401 if expired/invalid. See #5420

Signed-off-by: Mattia Verga mattia.verga@tiscali.it

pingou commented 10 months ago

It's missing an entry to show in the API docs :)

ngompa commented 9 months ago

@mattia, if you need an example of how to do this, you can see how new API endpoints are added with this commit: https://pagure.io/pagure/c/1b7d305d06af42f0996ae54fea223ecc5a081b72

rebased onto aab6e4f6b17d28e1581d43bd94c9a264c2018013

9 months ago

rebased onto b6b1b72502c8df91d5b8ec035987b24fa9ab799f

9 months ago

ngompa commented 9 months ago

There are some failed tests, can you take a look at them?

05:17:53  FAILED tests/test_pagure_flask_api_project.py::PagureFlaskApiProjectVerifyAPITokenTests::test_api_verifyapitoken_no_headers
05:17:53  FAILED tests/test_pagure_flask_api_project.py::PagureFlaskApiProjectVerifyAPITokenTests::test_api_verifyapitoken_not_valid
05:17:53  FAILED tests/test_pagure_flask_api_project.py::PagureFlaskApiProjectVerifyAPITokenTests::test_api_verifyapitoken_valid
05:17:53  FAILED tests/test_style.py::TestStyle::test_code_with_black - AssertionError:...

rebased onto 0028f651ec8d3e88bb991fff89db50db077c74cc

9 months ago

ngompa commented 9 months ago

Still three failures:

03:49:19  FAILED tests/test_pagure_flask_api_project.py::PagureFlaskApiProjectVerifyAPITokenTests::test_api_verifyapitoken_no_headers
03:49:19  FAILED tests/test_pagure_flask_api_project.py::PagureFlaskApiProjectVerifyAPITokenTests::test_api_verifyapitoken_not_valid
03:49:19  FAILED tests/test_pagure_flask_api_project.py::PagureFlaskApiProjectVerifyAPITokenTests::test_api_verifyapitoken_valid

rebased onto 9d833f3ab05e4334fb85deecd6727b86f66529cf

9 months ago

rebased onto 5045627fc36251f2eab42e1346f6321180390df1

9 months ago

mattia commented 9 months ago

This currently can only verify tokens with modify_project ACL. Ideally, this should work with any ACL, but I have to figure out how to do so without listing all ACLs in the api_login_required decorator...

mattia commented 9 months ago

I've changed the logic in _check_token() and now there are several tests failing... is it really correct the current logic in _check_token? It only raises an error within the if api_authenticated() block, but if api_authenticated() is False there's no token that could be validated, so shouldn't it raise an error as well?

rebased onto ed5d687

8 months ago

ngompa commented 8 months ago

I think you're probably right here. We want this to fail when the user isn't authenticated/authorized to access the API.

mattia commented 8 months ago

I think you're probably right here. We want this to fail when the user isn't authenticated/authorized to access the API.

So, I'll try to firstly look at changing _check_token() behavior and analyze tests failures and try to fix them. Then I'll rebase this PR on top of that.

ngompa commented 8 months ago

That sounds like a good plan of action!

mattia commented 7 months ago

Well, after some digging in Pagure code, I think I'm going to retire this PR. I'm really confused on how this all work and can't figure it out.
Also, as I understand it, check_token() checks a token is valid for a project if the token is project specific, while the token is system wide only checks if the call of check_token() must be for a project specific or not (so it doesn't really check token validity).

Pull-Request has been closed by mattia

7 months ago