#4223 Add project createapitoken endpoint
Merged a year ago by pingou. Opened a year ago by fbo.

file modified
+91

@@ -2133,3 +2133,94 @@ 

          raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR)

  

      return flask.jsonify({"message": message, "status": "ok"})

+ 

+ 

+ @API.route("/<repo>/token/new", methods=["POST"])

+ @API.route("/<namespace>/<repo>/token/new", methods=["POST"])

+ @API.route("/fork/<username>/<repo>/token/new", methods=["POST"])

+ @API.route("/fork/<username>/<namespace>/<repo>/token/new", methods=["POST"])

+ @api_login_required(acls=["modify_project"])

+ @api_method

+ def api_project_create_api_token(repo, namespace=None, username=None):

+     """

+     Create API project Token

+     ------------------------

+     Create a project token API for the caller user

+ 

+     This is restricted to project admins.

+ 

+     ::

+ 

+         POST /api/0/<repo>/token/new

+         POST /api/0/<namespace>/<repo>/token/new

+ 

+     ::

+ 

+         POST /api/0/fork/<username>/<repo>/token/new

+         POST /api/0/fork/<username>/<namespace>/<repo>/token/new

+ 

+ 

+     Input

+     ^^^^^

+ 

+     +------------------+---------+---------------+---------------------------+

+     | Key              | Type    | Optionality   | Description               |

+     +==================+=========+===============+===========================+

+     | ``description``  | String  | optional      | A string to specify the   |

+     |                  |         |               | description of the token  |

+     |                  |         |               |                           |

+     +------------------+---------+---------------+---------------------------+

+     | ``acls``         | List    | Mandatory     | The ACLs                  |

+     |                  |         |               |                           |

+     +------------------+---------+---------------+---------------------------+

+ 

+ 

+     Sample response

+     ^^^^^^^^^^^^^^^

+ 

+     ::

+ 

+         {

+           "token": {

+             "description": "My foo token",

+             "id": "aaabbbcccfootoken",

+           },

+         }

+ 

+     """

+     output = {}

+     project = get_authorized_api_project(

+         flask.g.session, repo, namespace=namespace

+     )

+     if not project:

+         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)

+ 

+     if flask.g.token.project and project != flask.g.token.project:

+         raise pagure.exceptions.APIError(401, error_code=APIERROR.EINVALIDTOK)

+ 

+     authorized_users = [project.user.username]

+     authorized_users.extend(

+         [user.user for user in project.access_users["admin"]]

+     )

+     if flask.g.fas_user.user not in authorized_users:

+         raise pagure.exceptions.APIError(

+             401, error_code=APIERROR.ENOTHIGHENOUGH

+         )

+ 

+     authorized_acls = pagure_config.get("USER_ACLS", [])

+     form = pagure.forms.NewTokenForm(csrf_enabled=False, sacls=authorized_acls)

+     if form.validate_on_submit():

+         acls = form.acls.data

+         description = form.description.data

+     else:

+         raise pagure.exceptions.APIError(

+             400, error_code=APIERROR.EINVALIDREQ, errors=form.errors

+         )

+ 

+     token = pagure.lib.query.add_token_to_user(

+         flask.g.session, project, acls, flask.g.fas_user.user, description

+     )

+     output = {"token": {"description": token.description, "id": token.id}}

+ 

+     jsonout = flask.jsonify(output)

+     return jsonout

file modified
+3 -1

@@ -407,7 +407,7 @@ 

  

  

  class NewTokenForm(PagureForm):

-     """ Form to add/change the status of an issue. """

+     """ Form to add a new token. """

  

      description = wtforms.StringField(

          "description", [wtforms.validators.Optional()]

@@ -426,6 +426,8 @@ 

              self.acls.choices = [

                  (acl.name, acl.name) for acl in kwargs["acls"]

              ]

+         if "sacls" in kwargs:

+             self.acls.choices = [(acl, acl) for acl in kwargs["sacls"]]

  

  

  class UpdateIssueForm(PagureForm):

file modified
+12 -2

@@ -4176,7 +4176,7 @@ 

  

      session.commit()

  

-     return "Token created"

+     return token

  

  

  def _convert_markdown(md_processor, text):

@@ -5205,7 +5205,13 @@ 

  

  

  def search_token(

-     session, acls, user=None, token=None, active=False, expired=False

+     session,

+     acls,

+     user=None,

+     token=None,

+     active=False,

+     expired=False,

+     description=None,

  ):

      """ Searches the API tokens corresponding to the criterias specified.

  

@@ -5214,6 +5220,7 @@ 

      :arg user: restrict the API tokens to this given user

      :arg token: restrict the API tokens to this specified token (if it

          exists)

+     :arg description: restrict the API tokens to this given description

      """

      query = (

          session.query(model.Token)

@@ -5232,6 +5239,9 @@ 

              model.User.user == user

          )

  

+     if description:

+         query = query.filter(model.Token.description == description)

+ 

      if active:

          query = query.filter(

              model.Token.expiration > datetime.datetime.utcnow()

file modified
+2 -2

@@ -1489,7 +1489,7 @@ 

  

      if form.validate_on_submit():

          try:

-             msg = pagure.lib.query.add_token_to_user(

+             pagure.lib.query.add_token_to_user(

                  flask.g.session,

                  project=None,

                  description=form.description.data.strip() or None,

@@ -1497,7 +1497,7 @@ 

                  username=user.username,

              )

              flask.g.session.commit()

-             flask.flash(msg)

+             flask.flash("Token created")

              return flask.redirect(

                  flask.url_for("ui_ns.user_settings") + "#nav-api-tab"

              )

file modified
+4 -4

@@ -2255,7 +2255,7 @@ 

  

      if form.validate_on_submit():

          try:

-             msg = pagure.lib.query.add_token_to_user(

+             pagure.lib.query.add_token_to_user(

                  flask.g.session,

                  repo,

                  description=form.description.data.strip() or None,

@@ -2263,7 +2263,7 @@ 

                  username=flask.g.fas_user.username,

              )

              flask.g.session.commit()

-             flask.flash(msg)

+             flask.flash("Token created")

              return flask.redirect(

                  flask.url_for(

                      "ui_ns.view_settings",

@@ -2324,7 +2324,7 @@ 

      if form.validate_on_submit():

          acls = [acl.name for acl in token.acls]

          try:

-             msg = pagure.lib.query.add_token_to_user(

+             pagure.lib.query.add_token_to_user(

                  flask.g.session,

                  repo,

                  description=token.description or None,

@@ -2332,7 +2332,7 @@ 

                  username=flask.g.fas_user.username,

              )

              flask.g.session.commit()

-             flask.flash(msg)

+             flask.flash("Token created")

              return flask.redirect(

                  flask.url_for(

                      "ui_ns.view_settings",

@@ -4096,6 +4096,177 @@ 

          before["settings"]["issues_default_to_private"] = True

          self.assertEqual(after, before)

  

+ class PagureFlaskApiProjectCreateAPITokenTests(tests.Modeltests):

+     """ Tests for the flask API of pagure for creating user project API token

+     """

+ 

+     maxDiff = None

+ 

+     def setUp(self):

+         """ Set up the environnment, ran before every tests. """

+         super(PagureFlaskApiProjectCreateAPITokenTests, 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_createapitoken_as_owner(self):

+         """ Test accessing api_project_create_token as owner. """

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}

+         project = pagure.lib.query._get_project(self.session, 'test')

+         tdescription = 'my new token'

+ 

+         # Call the api with pingou user token and verify content

+         data = {

+             'description': tdescription,

+             'acls': ['pull_request_merge', 'pull_request_comment']

+         }

+         output = self.app.post('/api/0/test/token/new',

+             headers=headers, data=data)

+         self.assertEqual(output.status_code, 200)

+         data = json.loads(output.get_data(as_text=True))

+         tid = pagure.lib.query.search_token(

+             self.session, None, description=tdescription)[0].id

+         self.assertEqual(

+             data,

+             {"token": {

+                 "description": tdescription,

+                 "id": tid

+                 }

+             }

+         )

+         # Create a second token but with faulty acl

+         # Call the api with pingou user token and error code

+         data = {

+             'description': tdescription,

+             'acl': ['foo', 'bar']

+         }

+         output = self.app.post('/api/0/test/token/new',

+             headers=headers, data=data)

+         self.assertEqual(output.status_code, 400)

+ 

+     def test_api_createapitoken_as_admin(self):

+         """ Test accessing api_project_create_token as admin. """

+ 

+         project = pagure.lib.query._get_project(self.session, 'test')

+ 

+         # Set the foo user as test project admin

+         pagure.lib.query.add_user_to_project(

+             self.session, project,

+             new_user='foo',

+             user='pingou',

+             access='admin'

+         )

+         self.session.commit()

+ 

+         # Create modify_project token for foo user

+         token = pagure.lib.query.add_token_to_user(

+             self.session,

+             project=None,

+             acls=['modify_project'],

+             username='foo')

+ 

+         # Call the connector with foo user token and verify content

+         headers = {'Authorization': 'token %s' % token.id}

+         tdescription = 'my new token'

+ 

+         # Call the api with pingou user token and verify content

+         data = {

+             'description': tdescription,

+             'acls': ['pull_request_merge', 'pull_request_comment']

+         }

+         output = self.app.post('/api/0/test/token/new',

+             headers=headers, data=data)

+         self.assertEqual(output.status_code, 200)

+         data = json.loads(output.get_data(as_text=True))

+         tid = pagure.lib.query.search_token(

+             self.session, None, user='foo', description=tdescription)[0].id

+         self.assertEqual(

+             data,

+             {"token": {

+                 "description": tdescription,

+                 "id": tid

+                 }

+             }

+         )

+ 

+     def test_api_createapitoken_as_unauthorized(self):

+         """ Test accessing api_project_create_token as project admin

+         but with unauthorized token ACL.

+         """

+ 

+         project = pagure.lib.query._get_project(self.session, 'test')

+ 

+         # Set the foo user as test project admin

+         pagure.lib.query.add_user_to_project(

+             self.session, project,

+             new_user='foo',

+             user='pingou',

+             access='admin'

+         )

+         self.session.commit()

+ 

+         # Create modify_project token for foo user

+         pagure.lib.query.add_token_to_user(

+             self.session,

+             project=None,

+             acls=['create_branch'],

+             username='foo')

+         mtoken = pagure.lib.query.search_token(

+             self.session, ['create_branch'], user='foo')[0]

+ 

+         # Call the connector with foo user token and verify content

+         headers = {'Authorization': 'token %s' % mtoken.id}

+         tdescription = 'my new token'

+ 

+         # Call the api with pingou user token and verify content

+         data = {

+             'description': tdescription,

+             'acls': ['pull_request_merge', 'pull_request_comment']

+         }

+         output = self.app.post('/api/0/test/token/new',

+             headers=headers, data=data)

+         self.assertEqual(output.status_code, 401)

+ 

+     def test_api_createapitoken_as_unauthorized_2(self):

+         """ Test accessing api_project_create_token as project user

+         with unauthorized token ACL.

+         """

+ 

+         project = pagure.lib.query._get_project(self.session, 'test')

+ 

+         # Set the foo user as test project admin

+         pagure.lib.query.add_user_to_project(

+             self.session, project,

+             new_user='foo',

+             user='pingou',

+             access='commit'

+         )

+         self.session.commit()

+ 

+         # Create modify_project token for foo user

+         pagure.lib.query.add_token_to_user(

+             self.session,

+             project=None,

+             acls=['modify_project'],

+             username='foo')

+         mtoken = pagure.lib.query.search_token(

+             self.session, ['modify_project'], user='foo')[0]

+ 

+         # Call the connector with foo user token and verify content

+         headers = {'Authorization': 'token %s' % mtoken.id}

+         tdescription = 'my new token'

+ 

+         # Call the api with pingou user token and verify content

+         data = {

+             'description': tdescription,

+             'acls': ['pull_request_merge', 'pull_request_comment']

+         }

+         output = self.app.post('/api/0/test/token/new',

+             headers=headers, data=data)

+         self.assertEqual(output.status_code, 401)

+ 

  

  class PagureFlaskApiProjectConnectorTests(tests.Modeltests):

      """ Tests for the flask API of pagure for getting connector of a project

A project owner or admin (with the 'modify_project') acl can
ask the creation of a project user token. The token
description and acl can be passed to the endpoint.

The endpoint return the token id and the description.

1 new commit added

  • Attempt to fix wrong code stype
a year ago

Wouldn't it be better to do /<repo>/token/create or something? createapitoken is a pretty poorly named endpoint...

1 new commit added

  • Propose a better API endpoint name repo/token/create
a year ago

Thanks, yes you are right so here is an update.

You could use a wtforms form here to do the validation for you (disable the csrf protection and dynamically load the list of ACLs), there are some examples of this in the code already (let me know if you don't find one)

Let's specify that it's acls=None here :)

We should use a list here, saves us the troubles of splitting the content on comas

You may want to use form.getlist('acl') here ;-)

@fbo if you would have time to update this one it would be nice, this way we could get it into the 5.3-beta I hope to cut tomorrow

Sure, I'll travel tomorrow morning but I'll be able to finish it (and the other PR (hope)) in the train.

1 new commit added

  • Use wtforms to validate a new token in the projects api
a year ago

I don't think we want the full list here, more likely the USER_ACLS list only

descriptions are not unique, so depending on the ordering you may get an expired token here. Should we make add_token_to_user return the token id instead?

1 new commit added

  • Improve add_token_to_user to return the token
a year ago

rebased onto 7c5cfacce801976a43080fa0c86fa999ead34e97

a year ago

Any idea why the CI fails this way ? is it related to the patch ?

pretty please pagure-ci rebuild

a year ago

pretty please pagure-ci rebuild

a year ago

rebased onto 969d10ecf6cbe31a380de5b6e65c039ddcaf4cf8

a year ago

Looks like at least one test is failing (genuinely)

rebased onto fa793fd116352d4495f19364e48a427edbb3b33a

a year ago

pretty please pagure-ci rebuild

a year ago

It returns three failing test files:

15:50:35 FAILED test: py-test_pagure_repospanner
15:50:35 FAILED test: py-test_pagure_flask_ui_repo
15:50:35 FAILED test: py-test_pagure_flask_ui_app

The first one currently fails on all tests/PRs (I hope the new way to run the tests will help with this), the other two are more concerning me :(

A local run with the container reports:
Failed tests:
FAILED test: py3-test_style
FAILED test: py3-test_pagure_flask_ui_repo
FAILED test: py3-test_pagure_flask_ui_app

Ok so then let me know if the patch needs more edit or if it ready to be merged :)

pretty please pagure-ci rebuild

a year ago

pretty please pagure-ci rebuild

a year ago

91 failed :( should I rebase the branch on master ?

rebased onto 0bf7de9

a year ago

The UI endpoint seems to be /<repo>/token/new, should we align the API with the UI?

Ok, I've had to apply the following two patches to get this PR to pass tests:

0001-Fix-the-tests.patch

0002-Black8-fixes.patch

3 new commits added

  • Align new token API endpoint with UI endpoint
  • Black8 fixes
  • Fix the tests
a year ago

pretty please pagure-ci rebuild

a year ago

This is the change black does locally:

diff --git a/ pagure/api/project.py b/ pagure/api/project.py
index 20c6e8af..e947a306 100644
--- a/ pagure/api/project.py    
+++ b/ pagure/api/project.py    
@@ -2138,9 +2138,7 @@ def api_modify_project_options(repo, username=None, namespace=None):
 @API.route("/<repo>/token/new", methods=["POST"])
 @API.route("/<namespace>/<repo>/token/new", methods=["POST"])
 @API.route("/fork/<username>/<repo>/token/new", methods=["POST"])
-@API.route(
-    "/fork/<username>/<namespace>/<repo>/token/new", methods=["POST"]
-)
+@API.route("/fork/<username>/<namespace>/<repo>/token/new", methods=["POST"])
 @api_login_required(acls=["modify_project"])
 @api_method
 def api_project_create_api_token(repo, namespace=None, username=None):

Thanks I'll fix that.

Curious, I've run the full test suite locally and black has not complain.

1 new commit added

  • Fix black compliance
a year ago

pretty please pagure-ci rebuild

a year ago

The failed test is repospanner which fails for everyone, everything pass, so let's get this in :)

Pull-Request has been merged by pingou

a year ago