Generation of api tokens and the use of said tokens for the api endpoints - #1170
Open
AlexandreDoneux wants to merge 5 commits into
Open
AlexandreDoneux wants to merge 5 commits into
AlexandreDoneux wants to merge 5 commits into
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 2 critical |
| Complexity | 2 medium |
🟢 Metrics 34 complexity · 0 duplication
Metric Results Complexity 34 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
8 tasks
AlexandreDoneux
force-pushed
the
api_token
branch
from
September 9, 2026 18:55
430908a to
4f96bf3
Compare
Contributor
Author
|
Also note this PR is based on #1169. |
anthonygego
requested changes
Sep 10, 2026
anthonygego
left a comment
Member
There was a problem hiding this comment.
Things are getting better but there is still issues to be addressed.
Please also add some docs in the developer section.
Contributor
Author
|
I will address the model validation remarks in a following PR to not overcharge this one. |
anthonygego
reviewed
Sep 11, 2026
AlexandreDoneux
force-pushed
the
api_token
branch
from
September 14, 2026 13:17
fc59ec3 to
563600c
Compare
AlexandreDoneux
force-pushed
the
api_token
branch
3 times, most recently
from
September 16, 2026 09:52
306c709 to
2383e4a
Compare
AlexandreDoneux
force-pushed
the
api_token
branch
2 times, most recently
from
September 18, 2026 14:34
8f54fe3 to
6fbd9cd
Compare
anthonygego
requested changes
Sep 18, 2026
AlexandreDoneux
force-pushed
the
api_token
branch
from
September 21, 2026 08:37
6fbd9cd to
3cd5d96
Compare
anthonygego
requested changes
Sep 21, 2026
AlexandreDoneux
force-pushed
the
api_token
branch
from
September 21, 2026 12:09
3cd5d96 to
db2b661
Compare
anthonygego
approved these changes
Sep 21, 2026
anthonygego
left a comment
Member
There was a problem hiding this comment.
This has eventually reached my acceptance level.
@nrybowski can you perform a last pass on this ?
Comment on lines
+265
to
273
| if session.is_lti: # TODO : refactor not to use session here | ||
| lti_info = session.lti.to_mongo().to_dict() | ||
| for key in lti_info: | ||
| if key == "consumer_key" or key.startswith("outcome"): # Skip "consumer_key" and "outcome*" | ||
| continue | ||
| self._logger.debug("LTI data : %s, %s",key, lti_info[key]) | ||
| # Add @lti_ prefix | ||
| key_str = "@lti_" + key | ||
| inputdata[key_str] = lti_info[key] |
Member
There was a problem hiding this comment.
Note : I actually don't see any good reason for keeping this code. I don't find any usage in our own task database, it forwards host-platform specific information, it is not documented, and it only filters out LTI 1.1 critical fields.
I think it can just be removed in another PR.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed, we divide PR #1121 into smaller ones. This is the first of those.
This PR focuses on the token generation and it's use for the current endpoints. A user can generate up to 20 tokens and giving to each a description. With these tokens he can use the submissions, courses and tasks endpoints, instead of using the flask session and authenticating with the related endpoint. Note the flask session is still a valid way to authenticate to the endpoints.
A user can of course delete a token if needed.
A PR will follow addressing the security concerns made by @nrybowski and hide information sent back to the user (course not found, task not found, etc.).
I thought about adding a page to the docs regarding the use of the endpoints (authentication with the two different methods). Where should it be located in the docs ?