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

Update authentication.rb.tt #59

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Copy link
Owner

@stevepolitodesign stevepolitodesign left a comment

Choose a reason for hiding this comment

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

Thank you for taking care of this 🙏. I just added a note about updating a test template to account for this change.

@@ -35,7 +35,7 @@ module Authentication
end

def remember(active_session)
cookies.permanent.encrypted[:remember_token] = active_session.remember_token
cookies.permanent.encrypted[:remember_token] = { value: active_session.remember_token, httponly: true, same_site: :strict, secure: true }

Choose a reason for hiding this comment

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

How would you feel about updating the generated test to account for this? 🤔

test "should remember user when logging in" do
assert_nil cookies[:remember_token]
post login_path, params: {
user: {
email: @confirmed_user.email,
password: @confirmed_user.password,
remember_me: 1
}
}
assert_not_nil current_user
assert_not_nil cookies[:remember_token]
end

Maybe something like the following?

remember_me_cookie = cookies.get_cookie("remember_token")

assert remember_me_cookie.http_only?
assert remember_me_cookie.secure?
assert_equal "Strict", remember_me_cookie.to_h["SameSite"]

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 this pull request may close these issues.

set httponly cookie
2 participants