Add jwt based Team authentication #6
Conversation
|
Also wanted to mention this that the register_tortoise is throwing errors which I honestly don't understand why but will also be looked into and will come up with a fix soon |
src/pwncore/routes/team.py
Outdated
| val=await team.current_points_fn(get=True) | ||
| return {"status":val} # int | ||
|
|
||
| @router.get("/members/") |
There was a problem hiding this comment.
We can simplify the fetching of points and members to be on a single endpoint like /profile which returns a JSON object which contains the points and reg no. of the members.
There was a problem hiding this comment.
Oh ok I'll notify @MrigankaDebnath03
src/pwncore/routes/team.py
Outdated
| val=await team.members(get=True) # a dict of members with keys ['member1','member2','member3'] | ||
| return {'status':val} | ||
|
|
||
| @router.get("/members/update") |
There was a problem hiding this comment.
Try to keep each function's URL endpoint to be of a single depth, for eg. updating members can be just /update.
There was a problem hiding this comment.
Those are just some endpoints which I've written just for the sake of testing the db. @MrigankaDebnath03 is currently working on the endpoints.
|
You might have to rebase after #7 Would you be able to make the same changes after rebasing? |
|
Aah I'll do itt .. 👍 |
|
You can wait till #5 is merged, a lot of the DB stuff and config stuff comes from there |
|
Ok .. 👍 |
|
@parrothacker1 this branch needs to rebase, contact me if you need help with that 👐🏼 |
|
@WizzyGeek how about I rewrite most of the stuff ?? ...the current branch is a whole lot messy ... so gonna start again ... and it would be helpful if I get a glimpse of what we need .. like in the database part ... i just need to define the model .. and some functions like check_password etc right ? .. also I saw a todo about a function that can do insert and update in a single query .. shall I implement that ? |
|
models are already defined, so you shouldn't be needing to define new models, only the password verfying method needs to be implemented in User model. That TODO requires extensive knowledge of the ORM, since it deals with executors... but if you can figure it out and come up with an efficient solution, why not? |
|
In the User model ? ... I thought we were storing passwords in the Team model .. ? |
|
ah yes, my bad |
|
@WizzyGeek bhaiya i'm done with mostly everything except for the TODO.I'll do a bit more research on it and will make a commit by tomorrow if i can come up with a sol. Please go through it and tell me if any probs ... and i wanted to ask this . In the "routes/team.py" there is a signup_team function (PS: endpoints were made by @MrigankaDebnath03 i just edited it a bit for our project) which only has code to create a team, there was a if-else code to check for no.of members .. but in our new db ... the users have the save function which does the same. So i removed that part and the only part that left is team creation. Should I just leave it like that ? |
|
Yup, it's fine |
src/pwncore/routes/auth.py
Outdated
|
|
||
| # Custom JWT processing (since FastAPI's implentation deals with refresh tokens) | ||
| async def get_jwt(*, authorization: str = Header()): | ||
| token = authorization.split(" ")[1] # Remove Bearer |
There was a problem hiding this comment.
Unhandled IndexError
also prefer doing authorization[<constant you find>:] over splitting
There was a problem hiding this comment.
I think we should prefer splitting, since the expected authorization header should be in format Bearer <token>.
In case the user (frontend) passes the authorization header as just <token>, it would raise an error (I forgot to catch the error).
If we use authorization[7:] to strip the Bearer , it would not lead to an error in the case that Bearer does not exist in the header.
There was a problem hiding this comment.
If Bearer does not exist in the token it would lead to an error by splitting too.
str.removeprefix would do the required validation and deletion
Also, I just noticed but this function shouldn't be async, it is doing no IO
|
Looks way better, good work @parrothacker1 and @MrigankaDebnath03 |
WizzyGeek
changed the title
About
This PR is mainly focused on the following:
Shortcomings
When fixes
Fixes to the above mentioned problems will be done after getting a go signal for the code and also after discussing with @MrigankaDebnath03 for implementation of the db functions. Test cases will be written after @MrigankaDebnath03's work.