-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Reconnecting Token Generator for Annotation Tool #3466
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
Changes from all commits
5ae1ca9
7dee976
2fd071f
24abd2b
26eb16e
edeebe7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| """ | ||
| This file contains a function used to retrieve the token for the annotation backend | ||
| without having to create a view, but just returning a string instead. | ||
|
|
||
| It can be called from other files by using the following: | ||
| from xmodule.annotator_token import retrieve_token | ||
| """ | ||
| import datetime | ||
| from firebase_token_generator import create_token | ||
|
|
||
|
|
||
| def retrieve_token(userid, secret): | ||
| ''' | ||
| Return a token for the backend of annotations. | ||
| It uses the course id to retrieve a variable that contains the secret | ||
| token found in inheritance.py. It also contains information of when | ||
| the token was issued. This will be stored with the user along with | ||
| the id for identification purposes in the backend. | ||
| ''' | ||
|
|
||
| # the following five lines of code allows you to include the default timezone in the iso format | ||
| # for more information: http://stackoverflow.com/questions/3401428/how-to-get-an-isoformat-datetime-string-including-the-default-timezone | ||
| dtnow = datetime.datetime.now() | ||
| dtutcnow = datetime.datetime.utcnow() | ||
| delta = dtnow - dtutcnow | ||
| newhour, newmin = divmod((delta.days * 24 * 60 * 60 + delta.seconds + 30) // 60, 60) | ||
| newtime = "%s%+02d:%02d" % (dtnow.isoformat(), newhour, newmin) | ||
| # uses the issued time (UTC plus timezone), the consumer key and the user's email to maintain a | ||
| # federated system in the annotation backend server | ||
| custom_data = {"issuedAt": newtime, "consumerKey": secret, "userId": userid, "ttl": 86400} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could use some comments on what exactly it's trying to accomplish. I suspect it could be done in a simpler fashion, but I'm having trouble actually identifying what the goal is in the first place. |
||
| newtoken = create_token(secret, custom_data) | ||
| return newtoken | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| """ | ||
| This test will run for annotator_token.py | ||
| """ | ||
| import unittest | ||
|
|
||
| from xmodule.annotator_token import retrieve_token | ||
|
|
||
|
|
||
| class TokenRetriever(unittest.TestCase): | ||
| """ | ||
| Tests to make sure that when passed in a username and secret token, that it will be encoded correctly | ||
| """ | ||
| def test_token(self): | ||
| """ | ||
| Test for the token generator. Give an a random username and secret token, it should create the properly encoded string of text. | ||
| """ | ||
| expected = "eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJpc3N1ZWRBdCI6ICIyMDE0LTAyLTI3VDE3OjAwOjQyLjQwNjQ0MSswOjAwIiwgImNvbnN1bWVyS2V5IjogImZha2Vfc2VjcmV0IiwgInVzZXJJZCI6ICJ1c2VybmFtZSIsICJ0dGwiOiA4NjQwMH0.Dx1PoF-7mqBOOSGDMZ9R_s3oaaLRPnn6CJgGGF2A5CQ" | ||
| response = retrieve_token("username", "fake_secret") | ||
| self.assertEqual(expected.split('.')[0], response.split('.')[0]) | ||
| self.assertNotEqual(expected.split('.')[2], response.split('.')[2]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is
newtimesupposed to represent? Even with the comments, I can't make heads or tails of what the data is supposed to represent, so I can't evaluate it for correctness.I guess I'm looking for a comment about how the federated backend formats the
custom_data, with something like "issuedAt is the time in UTC when the token was issued, in YYYY-MM-DDHH:MM format", or something of that sort.I'm a bit worried, looking at the current code, that shortly before or after midnight this is going to give you the wrong results (because UTC will be on one day, and we'll be on the previous day).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check it with the person who deals with the backend. This is basically a combination of legacy code and I just followed what his API instructed me to do. I do get your point about before/after midnight though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should add. This is code ALREADY out in the merged code (could be found in djangoapps/student/views.py), I just moved it out of there to a place that made sense. So far it hasn't caused any issues, but I'm wondering if it could be forgiven to meet the release cutting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cpennington Found the reason by accident in stackoverflow. First thing that pops up if you just search "isoformat." Am I still too late for the release that is being cut?