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 token based authentication feature #326

Merged
merged 13 commits into from
Jan 31, 2024

Conversation

Xpirix
Copy link
Collaborator

@Xpirix Xpirix commented Dec 5, 2023

Changes Summary

  • Install and add pyjwt and djangorestframework-simplejwt in the requirements files.
  • Specify token validity in the settings: 15 days for access and 180 days for refresh.
  • Add token creation (/api/token/) and refresh (/api/token/refresh/) endpoints.
  • Implement the token-based authentication system, which does not impact the existing authentication system. Token-based authentication will only be used if Bearer the_access_token is specified in HTTP_AUTHORIZATION. The process involves:
    1. Check if the value of HTTP_AUTHORIZATION starts with Bearer.
    2. Validate the token.
    3. Get the user based on the validated token.
    4. Log in and allow the request.
  • Update the README for this feature.

Please find below a video demonstrating this feature in Postman. In this example, we specify the token in the GET request, and the cookies are set automatically. Thus, there's no need to specify the token in the POST request.

token_auth.mp4

@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2023

Codecov Report

Attention: 193 lines in your changes are missing coverage. Please review.

Comparison is base (a9dabb5) 36.97% compared to head (d6b53ac) 36.42%.
Report is 2 commits behind head on master.

Files Patch % Lines
qgis-app/plugins/views.py 26.08% 102 Missing ⚠️
qgis-app/plugins/tests/test_token_auth.py 22.72% 68 Missing ⚠️
qgis-app/plugins/decorators.py 30.30% 23 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
- Coverage   36.97%   36.42%   -0.56%     
==========================================
  Files         106      110       +4     
  Lines        4500     4774     +274     
==========================================
+ Hits         1664     1739      +75     
- Misses       2836     3035     +199     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Xpirix Xpirix marked this pull request as ready for review December 5, 2023 13:32
@m-kuhn
Copy link
Member

m-kuhn commented Dec 8, 2023

Thank you very much. 🎉
Could you show a screenshot of the plugin config page, where plugin maintainers can manage tokens for specific plugins?

@Xpirix Xpirix marked this pull request as draft December 11, 2023 17:36
@Xpirix
Copy link
Collaborator Author

Xpirix commented Dec 13, 2023

@m-kuhn My pleasure.

I've made some changes to make specific tokens for plugin. Please find below the updated description for this PR.

Changes Summary

  • Integrate pyjwt and djangorestframework-simplejwt into the project's requirements files.
  • Configure token validity settings: 15 days for both access and refresh tokens.
  • Introduce a new Tokens button in the plugin management tab, facilitating redirection to the Token Management page.
  • Implement a functionality to generate a specific token tied to the current plugin and the authenticated user.
  • Display a list of available tokens, showcasing only those associated with the authenticated user and the corresponding plugin. Clicking on a token will redirect to the Token Details page.
  • Integrate a button for token deletion.

Token Usage Guidelines

  • Utilize a token for uploading or updating a plugin version via the following URLs:

    • https://plugins.qgis.org/plugins/api/<package_name>/version/add/
    • https://plugins.qgis.org/plugins/api/<package_name>/version/<version>/update
  • Tokens are exclusively valid for the specific plugin with which they are associated.

  • Tokens automatically become invalid upon deletion or upon reaching the expiration date.

Please find below a GIF demonstrating this feature.

Token_auth

@Xpirix Xpirix marked this pull request as ready for review December 13, 2023 10:46
@m-kuhn
Copy link
Member

m-kuhn commented Dec 15, 2023

Thanks for the update @Xpirix , this new screencast looks very promising!

If I understand correctly, there is a timeout on the token of 15 days and the token needs to be refreshed?
The most typical use case I can think of is storing the token as a github secret and then creating CI/CD automations that make use of this token. There is no logic running, that could regularly refresh such a token and store a refreshed token (secrets are manually managed). Ideally I was thinking to acquire a token once and then have it available forever.

Additional ideas more "nice to have":

  • Show the token only once on the user interface and then hide it
  • Show when the token was last used
  • Ability to give a description to a token (so I can flag it as my "github action" token or "mkuhn local test" token or similar)

One example: https://help.transifex.com/en/articles/6248858-generating-an-api-token (even though this is linked to the user account like a personal access token rather than a resource specific token as here). Other similar examples would be "github repository deploy key" with the difference that this key is not generated, but the rest is very much the same.

I am also interested in opinions from @Gustry @Guts @3nids as qgis-plugin-ci contributors/maintainers

@m-kuhn
Copy link
Member

m-kuhn commented Dec 15, 2023

One more thing

showcasing only those associated with the authenticated user and the corresponding plugin

as a plugin maintainer I would like to see all tokens that are associated with a plugin I am responsible for, even if they were generated by others in the team. I think it's nice to show who created a token but a token should be completely detached from a user account after creation. I.e. if a someone leaves the development team, the token should be kept active, so any CI/CD pipelines continue to work and others in the team should then still be able to revoke this token.

@Xpirix
Copy link
Collaborator Author

Xpirix commented Dec 18, 2023

Thank you for the feedback @m-kuhn .

If I understand correctly, there is a timeout on the token of 15 days and the token needs to be refreshed?
The most typical use case I can think of is storing the token as a github secret and then creating CI/CD automations that make use of this token. There is no logic running, that could regularly refresh such a token and store a refreshed token (secrets are manually managed). Ideally I was thinking to acquire a token once and then have it available forever.

Yes, that makes sense. I will make every token available for a very long duration. They can always be revoked anytime.

Additional ideas more "nice to have":

  • Show the token only once on the user interface and then hide it
  • Show when the token was last used
  • Ability to give a description to a token (so I can flag it as my "github action" token or "mkuhn local test" token or similar)

Thanks for theses ideas, I'm going to implement them.

as a plugin maintainer I would like to see all tokens that are associated with a plugin I am responsible for, even if they were generated by others in the team.

That's well noted.

I think it's nice to show who created a token but a token should be completely detached from a user account after creation. I.e. if a someone leaves the development team, the token should be kept active, so any CI/CD pipelines continue to work and others in the team should then still be able to revoke this token.

From what I understand from the basic process of uploading/updating a plugin version, a user is always needed. Indeed, a token contains the plugin information but also the user identity (user_id) because the user don't have to log in when using it. If we don't store the user identity in the token, we won't be able to retreive which user is uploading a plugin and the process will raise an error.

image

@Xpirix
Copy link
Collaborator Author

Xpirix commented Dec 18, 2023

Please find bellow the description and a GIF of the latest changes according to the previous comments.

  • Make every token available for a very long duration.
  • Show the token only once (when it has been generated) and hide it.
  • Show when the token was last used
  • Ability to add/update a description to a token
  • Show all generated tokens for a specific plugin on the list when the authenticated user is the plugin maintainer (the field created_by, a PR regarding the definition of a maintainer is opened at Add maintainer field to plugin update #316)

Token_auth_2

@m-kuhn
Copy link
Member

m-kuhn commented Jan 7, 2024

Thank you for these updates.

If we don't store the user identity in the token, we won't be able to retreive which user is uploading a plugin and the process will raise an error.

One question regarding this, if Anna creates a token for FancyPlugin and configures a release pipeline to use this token (e.g. stored in github action secrets). Later Anna leaves the dev team and removed permissions to upload new versions. If a new release is triggered via this pipeline, will the pipeline still work?

@Xpirix
Copy link
Collaborator Author

Xpirix commented Jan 8, 2024

@m-kuhn You're welcome.

One question regarding this, if Anna creates a token for FancyPlugin and configures a release pipeline to use this token (e.g. stored in github action secrets). Later Anna leaves the dev team and removed permissions to upload new versions. If a new release is triggered via this pipeline, will the pipeline still work?

I see. You're right; the pipeline will not work anymore because this Token contains the user information, and Anna doesn't have the permission anymore.

To resolve this, I suggest editing the workflow when uploading a new version using a token:

  • If a token is specified, do not check for the user or the user's permission.
  • Check if the token is valid and is connected to the plugin.
  • Upload/update the plugin version.
  • Set the column Uploaded by to CI/CD pipeline. This will involve editing the model PluginVersion by adding a new column is_from_pipeline and setting the column created_by to nullable.

This way, all created tokens will be independent from Anna, and the pipeline will continue to work even if Anna leaves the dev team and no longer has the permission.

image

@Xpirix Xpirix marked this pull request as draft January 8, 2024 06:16
@m-kuhn
Copy link
Member

m-kuhn commented Jan 8, 2024

I like this. Alternative to the current term: " Token {token_description}"? But it's a tiny detail, only if it's reasonable effort.

@Xpirix
Copy link
Collaborator Author

Xpirix commented Jan 9, 2024

I've made the updates. Each created token is now independent from the user who created it.

Alternative to the current term: " Token {token_description}"?

Please find below the screenshot for the Uploaded by column for this.

image

@Xpirix Xpirix marked this pull request as ready for review January 10, 2024 13:37
@dimasciput dimasciput merged commit c9a75e9 into qgis:master Jan 31, 2024
2 checks passed
@Xpirix Xpirix deleted the token_based_authentication branch February 6, 2024 11:59
@Xpirix
Copy link
Collaborator Author

Xpirix commented Feb 6, 2024

This feature has been deployed. Please feel free to check and share your feedback.

Thanks.

@m-kuhn
Copy link
Member

m-kuhn commented Feb 6, 2024

Cool, what can I do with the Jti ?

image

@Xpirix
Copy link
Collaborator Author

Xpirix commented Feb 7, 2024

I think it's something like an ID for the token. But we can hide that since we implemented the Description column.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants