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

Stale moduledoc for Token regarding token secrecy #5757

Closed
ollien opened this issue Mar 20, 2024 · 2 comments · Fixed by #5768
Closed

Stale moduledoc for Token regarding token secrecy #5757

ollien opened this issue Mar 20, 2024 · 2 comments · Fixed by #5768

Comments

@ollien
Copy link
Contributor

ollien commented Mar 20, 2024

Environment

  • Elixir version (elixir -v): N/A
  • Phoenix version (mix deps): 1.7.11
  • Operating system: N/A

Actual behavior

The moduledoc for Token states

The data stored in the token is signed to prevent tampering but not encrypted. This means it is safe to store identification information (such as user IDs) but should not be used to store confidential information (such as credit card numbers).

This is definitely true of Token.sign/4, but Token.encrypt/4 seems to perform actual encryption (Phoenix Implementation, which invokes the Plug.Crypto implementation). Doing some archeology, it looks like this snippet was written a few years before the implementation of Token.encrypt/4.

Expected behavior

The docs should reflect current behavior of the module. Now, perhaps someone shouldn't send credit card numbers to the frontend, but it would seem that the implementation does provide secrecy. Please do let me know if I'm off-base, though, and there's some caveat about this crypto that I'm not seeing.

@josevalim
Copy link
Member

@ollien you are correct, can you please send a PR and ping me? I'd be glad to review it and merge it!

@ollien
Copy link
Contributor Author

ollien commented Mar 26, 2024

Absolutely! I need to find some time (life is getting in the way this week), but it's on my list :)

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 a pull request may close this issue.

2 participants