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.

Sign up for GitHub

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

Jump to bottom

Porting encrypted_cookies feature from 4-4-stable. #666

Closed
wants to merge 25 commits into from
Closed

Porting encrypted_cookies feature from 4-4-stable. #666

wants to merge 25 commits into from

Conversation

tiegz
Copy link
Collaborator

@tiegz tiegz commented Mar 24, 2019

This ports the encrypted_cookies option from the 4-4-stable branch #660

@tiegz
Porting encrypted_cookies feature from 4-4-stable:
dce766c 
#660

Co-authored-by: Bardoe Besselaar <bardoe.besselaar@iam-javelin.com>
@tiegz tiegz requested a review from jaredbeck March 24, 2019 02:40
tiegz
tiegz commented Mar 24, 2019
View reviewed changes
CHANGELOG.md
* Breaking Changes
* None
* Added
* [#660](https://github.com/binarylogic/authlogic/pull/660) -
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaredbeck since we're just porting this from 4-4 to 5-0, think should we make another log entry for it? I guess in the future it might make sense to merge into the latest stable branch, and the backport to older versions 🤷‍♂️

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we're just porting this from 4-4 to 5-0, think should we make another log entry for it?

Yes please. SemVer dictates that a new feature increment the minor version (eg. 5.1) and previous minors will not (cannot) have said new feature.

jaredbeck
jaredbeck reviewed Mar 25, 2019
View reviewed changes
Copy link
Collaborator

@jaredbeck jaredbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about this new feature to review its actual implementation, but I've given my thoughts about changelog, Gemfile, etc. I hope that's helpful.

CHANGELOG.md
* Breaking Changes
* None
* Added
* [#660](https://github.com/binarylogic/authlogic/pull/660) -
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we're just porting this from 4-4 to 5-0, think should we make another log entry for it?

Yes please. SemVer dictates that a new feature increment the minor version (eg. 5.1) and previous minors will not (cannot) have said new feature.

Gemfile Outdated Show resolved Hide resolved
tiegz and others added 23 commits March 31, 2019 22:23
@tiegz
Remove sqlite3 Gemfile line and add CHANGELOG line.
db160dd
@jaredbeck
Release 5.0.4
f380d4a
@jaredbeck
Consolidate error classes into errors.rb
63e54e4 
1. Errors should share a parent class Authlogic::Error
2. Having them in one file is kind of convenient because they
  often have long messages, and it's nice to keep that copywriting
  out of the way. Also, it's sort of nice if a file like
  rails_adapter.rb only defines the adapter class and not any other
  classes?
@jaredbeck
Breaking change: No default crypto_provider
ae4fef1 
[Fixes #668]

See changelog for description, rationale.
@epaew
update test target version of Rails6
31403f0
@epaew
Update the comment for Authlogic::Session::Base#allow_http_basic_auth
1de72ae
@jaredbeck
Merge pull request #682 from epaew/update_target_rails6
53d6ae3 
Update test target version of Rails6
@jaredbeck
Merge pull request #683 from epaew/fixup_comment_for_allow_http_basic…
24a3a18 
…_auth

Update the comment for `Authlogic::Session::Base#allow_http_basic_auth`
@jaredbeck
Update triage wording for "ignored issue template"
fd6a01b
@jaredbeck
Merge pull request #679 from binarylogic/remove_dependency_on_scrypt
8e54152 
Remove dependency on SCrypt
@rofreg @jaredbeck
Respect log_in_after_create when creating a new user as a logged-out …
4b51ee7 
…user

[Closes #686]
@jaredbeck
Merge pull request #687 from binarylogic/issue_686
d2aece7 
Respect log_in_after_create when creating a new user as a logged-out …
@epaew
Fixup SimpleCov's configurations
a6099d2 
https://github.com/colszowka/simplecov#getting-started
> 2. Load and launch SimpleCov at the very top of your `test/test_helper.rb`

> Note: If SimpleCov starts after your application code is already loaded (via require),
> it won't be able to track your files and their coverage!
@jaredbeck
Merge pull request #688 from epaew/fixup_simplecov_configurations
98054f6 
Fixup the SimpleCov configuration for measurement test coverage.
@olleolleolle
README: Use SVG badges
b85dd0c
@jaredbeck
Merge pull request #689 from olleolleolle/patch-1
7fcae98 
README: Use SVG badges
Fix ruby 2.7 warning on kwargs changes:
8a77ae5 
 authlogic/lib/authlogic/i18n/translator.rb:11: warning: The last argument is used as the keyword parameter
@jaredbeck
Merge pull request #692 from alchimere/fix-ruby-2.7-kwargs-warning
9a14168 
Fix ruby 2.7 warning on kwargs changes:
@jaredbeck
Docs: configuring the GH issue chooser
61248d2 
https://help.github.com/en/github/building-a-strong-community/configuring-issue-templates-for-your-repository#configuring-the-template-chooser
Fix ambiguous comment.
1bbb0e9
@jaredbeck
Merge pull request #695 from sarojkh/master
f0886ad 
Fix ambiguous comment.
@ryosuke-endo
email is unique
74bb681
@jaredbeck
Merge pull request #696 from ryosuke-endo/readme_email_unique
2f57093 
email is unique in README
@graaff
Copy link

graaff commented Feb 7, 2020

What is the status of this request? We just got results from a security scan for our application and one of the (low risk) findings was that the authlogic remember_me cookie is partially static. Having it encrypted would help with this and provide a suitable mitigation.

@tiegz tiegz closed this May 2, 2020
@tiegz tiegz deleted the encrypted_cookies branch May 2, 2020 14:43
@tiegz tiegz mentioned this pull request May 2, 2020
Merged
@tiegz
Copy link
Collaborator Author

tiegz commented May 2, 2020

This branch got a little nasty, so I did a clean branch over in #710 .

@tiegz tiegz mentioned this pull request May 2, 2020
Merged
@tiegz
Copy link
Collaborator Author

tiegz commented May 8, 2020 •
edited

@graaff sorry for the delay! We just pushed v5.2.0 and v6.1.0 with the encrypted_cookies option. I believe Rails won't re-encrypt the cookie value each request so it might still be static, but if so you can be assured that it's not in plaintext anymore (once you enable that feature).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaredbeck jaredbeck jaredbeck left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@tiegz @graaff @jaredbeck @epaew @rofreg @olleolleolle @ryosuke-endo