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

Example SimpleJWT repositories: Contribute! #263

Closed
3 of 4 tasks
Andrew-Chen-Wang opened this issue Jul 3, 2020 · 22 comments
Closed
3 of 4 tasks

Example SimpleJWT repositories: Contribute! #263

Andrew-Chen-Wang opened this issue Jul 3, 2020 · 22 comments

Comments

@Andrew-Chen-Wang
Copy link
Member

Andrew-Chen-Wang commented Jul 3, 2020

Hi all! Due to the sheer number of requests and urges for #157 to be merged, I have created a template repository with a Django server ready-to-go. To generate a sample repository for SimpleJWT, please press the "Use this template" button so that you don't fork the repository; this way, you can rename the repository to whatever name you want (although please follow naming conventions of the React and Vue.js repos already setup. It'd be great if you could also transfer ownership to the SimpleJWT organization so that everyone knows about it -- i.e. viewable -- and can be maintained by the community).

The template repository: https://github.com/SimpleJWT/drf-SimpleJWT-server-template

Currently created repositories:

In the future:

  • React Native
  • Flutter browser
  • Flutter mobile

There are other frontend frameworks like Angular (JS), Flutter (Dart), Ember (JS), etc. If I didn't create them, it just means I undervalue them (jk). I just don't want to get ahead of myself. If you want to contribute and you're using one of these frameworks, by all means @Andrew-Chen-Wang (i.e. mention me) in this issue, and I will create a repository for you.

To reiterate, you will need unittest cases. For those who want the #157 merger, at least two frontend frameworks that are used on web browsers must be completed and tested to have the PR to be considered for merger. Not only that, it must use the PR's latest commit (do not use master branch; specify a commit SHA). I cannot stress this enough: security is number one priority. To publish a package with, imo, still a highly insecure PR since there is still no one who has given me a single test repository, SimpleJWT would be doomed in vulnerabilities and CVEs.

So.... Thanks for contributing Djangonauts!

@dgmouris
Copy link
Contributor

Hey @Andrew-Chen-Wang I would like to help out on this.

As a note I would like to see how you're kind of formatting your repos so that I can be consistent with what you've created. I'm just having a bit of trouble seeing: https://github.com/Andrew-Chen-Wang/mobile-auth-example as it might be a private repo and I get a 404.

Let me know if/how I can help.

Thank you so much for your time.

@Andrew-Chen-Wang
Copy link
Member Author

Andrew-Chen-Wang commented Aug 27, 2020 via email

@dgmouris
Copy link
Contributor

dgmouris commented Aug 27, 2020

Hey @Andrew-Chen-Wang thanks for responing so quick that's awesome.

Before I get started I'd like to just clarify the scope of what you'd like to see. I'd also like to take a crack at the react repo first so no new repos are needed.

First what would you like the scope of the project be? Here's what I'm kind of inferring by an example repo.

  • server
    • login/logout
    • one (or more) models
    • one (or more) serializer (authenticated permissions as well?)
    • one (or more) view (or viewset)
  • jwt-react
  • login/logout
  • component that calls endpoint and renders the result (via fetch?, via axios?)
  • React specific question (totally fine if this is an implementation detail) would you like the "useContext" to

Second thing that I'd like to clarify is the directory structure.

/
|-server
|-jwt-react (where the react/other frameworks are contained, the angular pr is called jwt-angular)
  |-app
...other files....

Third thing to clarify (for the front end web frameworks) would you like to just store the jwt token in localstorage?

As well I'm assuming you'd like tests on the examples, if not that's fine too.

Hopefully this will help others as well on scope. I wouldn't put this in one massive PR but any direction/scope on the above would be much appreciated. Hopefully it's of some help:) My apologies if I'm thinking way too large. Thanks so much for the help again.

@Andrew-Chen-Wang
Copy link
Member Author

Andrew-Chen-Wang commented Aug 27, 2020

Sure, np. School doesn't start for another few days so I'll be able to respond quicker before September.

  1. Scope of the project: show login/logout, usage of the access token when needed (you don't always need to send a token, so you will want to demonstrate that too), and using both views. When I mean using both views, (and again sorry about taking down the mobile auth repo) I mean once you receive a 401 error, you should "refresh" your access token using the refresh token. This should be a seamless process.

In the Angular repository, what happened was loic added a timer rather than just waiting for the access token is refreshed. To me, that doesn't seem good thread wise and may not be good resource wise. It's best to just keep using the same access token until you get a 401 error. Once you do, use the refresh token to get a new access token at the api/token/access endpoint (I can't remember if that's what it's called).

Also, you will need to automatically log out a user once the refresh token expires as well. When that happens, logging out the user has the same functionality as logging out due to an expired session cookie.

In the template server repository, I had a Ping endpoint that you can use. You might also want to demonstrate a POST request with some JSON to go along with it to a different endpoint.

  1. Project structure:

I've never actually programmed in these JS frameworks, but they're in high demand and David said he'd be looking into #71 again at some point in time. I think the point of the project structure was that you could start the server like it was a completely different project/git repository and start the frontend like it was a completely different project/git repository.

README.md (top level to explain what's going on)
server
| -- README.md (explains how to run Django)
| -- .gitignore
| -- requirements.txt
react-app
| -- all react components
| -- .gitignore
| -- README.md (explains how to run React)

Regarding the React repo, the server should already be set up according to the template. You just need to start a React app (idk how) and put it in a completely separate folder from the server/Django folder.

You'd need a gitignore file and some instructions in the README.

  1. That's a heated debate that Allow httpOnly cookie storage #71 is trying to solve. We're looking to avoid local storage, and the PR that @loicgasser tried for using httpOnly is linked in the requirements.txt in his Angular PR. In other words, the package/link that loic used in the django requirements.txt file is the one you should try to use; however, I'd like to see if you can use Allow httpOnly cookie storage #71's method first before trying his requirements. Also in other words for clarification, don't use local storage.

Hopefully that's helpful. Let me know if there are any confusing words in here; not the best when it comes to communication.

@loicgasser
Copy link

loicgasser commented Aug 28, 2020

That's a heated debate that #71 is trying to solve. We're looking to avoid local storage, and the PR that @loicgasser tried for using httpOnly is linked in the requirements.txt in his Angular PR. In other words, the package/link that loic used in the django requirements.txt file is the one you should try to use; however, I'd like to see if you can use #71's method first before trying his requirements. Also in other words for clarification, don't use local storage.

Hi so my latest work on the simple jwt lib can be found here.

https://github.com/AtuzSolution/django-rest-framework-simplejwt/releases/tag/jwt_5

This is now git+git://github.com/AtuzSolution/django-rest-framework-simplejwt@jwt_5#egg=rest_framework_simplejwt

These are the things I changed

  • Remove both the token and refresh tokens from the obtain and refresh endpoints
  • Added some tests
  • Always return the CSRF cookie in the response body when the cookie is enabled

I also did some work on the front-end which I didn't push yet:

  • Implemented the 2 401 requests strategy discussed in the Angular pull request
  • Ended up storing the csrf token in local storage.

My reasoning for this is the following:

The main problem you try to address when storing the token in a httpOnly cookie is XSS. In the event that someone was able to inject a script in your site, he could very well call your server and retrieve the csrf token to then send it back to his own server. It's not that much more work to do, versus getting it from local storage.
Now this is also the reason why storing your JWT token in a httpOnly cookie doesn't make any sense if you return the token value in the response body of your server.
Now, it is true that you could send a request to your own server to read the value of the httpOnly cookies, and thus steal the token, but this is only true for old and outdated browsers like IE, if you set the SameSite configuration correctly and your client is using a "modern browser" this will not happen.

@Andrew-Chen-Wang
Copy link
Member Author

Andrew-Chen-Wang commented Aug 28, 2020 via email

@dgmouris
Copy link
Contributor

dgmouris commented Aug 28, 2020

@Andrew-Chen-Wang

I took a stab with some of the local storage SimpleJWT/drf-SimpleJWT-React#1 I don't know how far/how much you'd like for in an example repository. I'm very much open to changing this but it might be of use, my apologies if this isn't enough/you'd like more tests, I just thought having something concrete up here would be of use.

I will take a stab at implementing at #71 PR in a few days, to see if the PR is in the ballpark of what you folks want:)

Thanks for your time again.

@Andrew-Chen-Wang
Copy link
Member Author

Thanks for the PR @dgmouris! I'll take a look when I have time. Regarding #71, I read online something about Axios for React. Some people use it to store API tokens and such. Hopefully that's useful for attempting to incorporate #71.

@dgmouris
Copy link
Contributor

dgmouris commented Sep 4, 2020

Hey @Andrew-Chen-Wang just added tests to the authentication piece to SimpleJWT/drf-SimpleJWT-React#1.

I see that there is a path to not using localStorage with axios (referring to #71) but I think it would be more appropriate to add it to another Pull Request. I wouldn't mind working on it once the PR mentioned above is looked at (and hopefully merged 😄 )

@Andrew-Chen-Wang
Copy link
Member Author

Hey @dgmouris Just left my review in your PR. Definitely looking forward to a number 2 PR for such implementation!

@Andrew-Chen-Wang Andrew-Chen-Wang pinned this issue Sep 4, 2020
@Andrew-Chen-Wang
Copy link
Member Author

Thank you @dgmouris for implementing the React template repository!

@dgmouris
Copy link
Contributor

dgmouris commented Oct 13, 2020

I took a crack at creating the Vue repository please take a look here: SimpleJWT/drf-SimpleJWT-Vue#9

Hopefolly folks will find it useful. I know this is a silly ask but I was wondering if you folks to mark it as a hacktoberfest PR (thanks again for being great maintainers)

@Andrew-Chen-Wang
Copy link
Member Author

Andrew-Chen-Wang commented Oct 13, 2020

@dgmouris Thanks for the PR! I think after this year's hacktoberfest results, I'm going to avoid adding hacktoberfest labels until they add proper rules that helps open source and not "trying to encourage" open source contributions. Not silly! But hope you understand; sorry.

I think I can still add hacktoberfest-approved to your single PR though. Not sure, but I don't want to see a flood of PRs here.

Edit: for future ref for myself, it's hacktoberfest-accepted

@dgmouris
Copy link
Contributor

@Andrew-Chen-Wang I fully understand I hear repos are getting spammed (with non productive PRs) which is 0% fun. Thanks for adding the tag:)

@Andrew-Chen-Wang
Copy link
Member Author

Andrew-Chen-Wang commented Oct 16, 2020

One more repository to go! Angular! Thanks dgmouris for implementing React and Vue.js repositories and loicgasser for making progress on the Angular repo!

@Andrew-Chen-Wang
Copy link
Member Author

@dgmouris If you were curious, I've re publicized mobile-auth-example if you'd like to take a look 😄

@Andrew-Chen-Wang
Copy link
Member Author

Andrew-Chen-Wang commented Jan 15, 2021

Hi @dgmouris @loicgasser I'm creating a tutorial called SPA-with-sessions; it's not done, and some steps aren't even correct, but I'm slowly finishing it after a couple hours. Basically, I want to close down these template repositories and make sure all SPA developers using a non-JS backend can still use session authorization. However, I'd like to know your opinions on how long npm run build actually takes on a production grade app.

EDIT: ok the tutorial is done. Check it out! Love to hear some feedback!

Currently, it's pretty fast for me since there's nothing there. But I worry that when a single change happens, then constantly rebuilding using npm-watcher would be horrific.

I was thinking instead we could let local authorization be handled by SimpleJWT (so that we don't need to keep running npm run build) and production be handled by session. To deal with storing session data or JWT payloads, we can create a middleware in a different package that can handle the different environments based on settings.DEBUG. The reason we can interchange the two is because we're just dealing with secret payloads that the client can't see, only the server. By having one method for handling different contexts (e.g. like setting a new key in the payload or a new key for session payload), it abstracts away the complexity of the app.

But before that last paragraph, I just want to know how fast npm run build is, at least for a React app. Thanks!

@loicgasser
Copy link

Hey thank you for this effort. I 'll analyze what your propose in the repository. In general building any JS is slow.

@Andrew-Chen-Wang
Copy link
Member Author

Ok thanks. I think proposal #3 addressing still using JWTs during local development might be necessary.

@stunaz
Copy link

stunaz commented Jan 16, 2021

@Andrew-Chen-Wang I just saw your repo spa-with-sessions.... just to be clear... it's just a django application with embedded react spa. so it doesnt have anything to do with jwt nor django-rest-framework-simplejwt.... . I think most people considered this approach, there is even this lib https://github.com/owais/django-webpack-loader which helps with that (not react specific). I assume that the main reason most use jwt or simplejwt is because the spa and the backend are completely separated, different repo, different server, and even maybe different domain.
In my case, its different repo, different server, but under the same domain/subdomain... so I will switch to old SessionAuthentication .

Edit: is there a difference between your SPA-with-sessions and this sample ? I mean they achieve the samething.

@Andrew-Chen-Wang
Copy link
Member Author

@stunaz I made this repository not as a coding "introduction" but more of a business problem (sorry that's just my mindset). All the tutorials online teach you to use JWT, and that's why I see all these developers using. When I search up Django and React integration on Google, I always see Medium or dev.to articles using SimpleJWT. (Funny enough, Ruby on Rails seem to be A-OK; first few tutorials use their version of web pack).

Note: Ok as I'm re-reading this, my English is absolute trash and could feel like anger (?). If you see any kind of idiot noise, then ignore it :P

But yes regarding the difference (as in barely none) and what it is; I actually asked this question about SPA integration with Django before on Reddit, and people immediately pointed me to the web pack library! The problem? Not many tutorials, and tbh not many people adopt it (when I published this on Reddit, there were stars... why if there's already web pack?). So I reintroduced a tutorial with the above without web pack to integrate React and Django as an alternative but for me as a test. Again, I saw this as a business problem: if no one is seeing tutorials for webpack, then no one was going to use it. Obviously, it has 2,000 stars on GitHub, but wide adoption for it is competing with SimpleJWT. When I introduced the mobile template for SimpleJWT, barely anyone used it even though stateless authorization is now most widely known for mobile use, but almost EVERYONE who saw this issue was using the React template (evidenced by the 11 vs. 3 stars on them).

The real, only difference between my sample and the web pack is that I deployed the static files to GitHub pages (since static files are public anyways) using a free workflow people can use. The additional difference is I'm trying to market this within the SimpleJWT repository to get people AWAY from JWT authorization on browsers.

I assume that the main reason most use jwt or simplejwt is because the spa and the backend are completely separated, different repo, different server, and even maybe different domain

This is kinda what I was hoping to identify. Again, the original thought was that all these tutorials used SimpleJWT, so everyone flocked here. But when I was developing it, it doesn't really make any sense.

Sure your repository is different, but your API should be unit tested, not just hand tested (assuming that's what you mean by completely separated). Different servers and domains doesn't make any sense since your static files and your React app is served from a CDN (and if we take into account the index.html file, that's just a single file that Django can deliver, not just from a CDN). Even for non-SPA apps, our static files live in remote domains like velnota.s3.amazon.com rather than velnota.com.

@stunaz
Copy link

stunaz commented Jan 17, 2021

Ok, I got you. I wasn't sure if I missed something.

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

No branches or pull requests

4 participants