Skip to content
代码片段 群组 项目
未验证 提交 b6576b60 编辑于 作者: Andrew Evans's avatar Andrew Evans 提交者: GitLab
浏览文件

Add Dangerfile reminder when setting cookies server-side

If a developer sets a cookie from Rails with a `:domain` attribute, they
must remember to un-set this cookie when a user signs out. This can be
hard to remember to do.

This change adds a Danger bot reminder whenever setting cookies is
detected in application code, and also documentation of development
guidelines when working with cookies.
上级 0329ea1f
No related branches found
No related tags found
无相关合并请求
# frozen_string_literal: true
helper.all_changed_files.each do |filename|
next unless filename.end_with?('.rb')
next if filename.start_with?('spec/', 'ee/spec/', 'jh/spec/', 'qa/')
cookie_setting.add_suggestions_for(filename)
end
# frozen_string_literal: true
require_relative '../../tooling/danger/cookie_setting'
module Danger
class CookieSetting < ::Danger::Plugin
def add_suggestions_for(filename)
Tooling::Danger::CookieSetting.new(filename, context: self).suggest
end
end
end
---
stage: none
group: unassigned
info: Any user with at least the Maintainer role can merge updates to this content. For details, see https://docs.gitlab.com/ee/development/development_processes.html#development-guidelines-review.
---
# Cookies
In general, there is usually a better place to store data for users than in cookies. For backend development PostgreSQL, Redis, and [session storage](session.md) are available. For frontend development, cookies may be more secure than `localStorage`, `IndexedDB` or other options.
In general do not put sensitive information such as user IDs, potentially user-identifying information, tokens, or other secrets into cookies. See our [Secure Coding Guidelines](secure_coding_guidelines.md) for more information.
## Cookies on Rails
Ruby on Rails has cookie setting and retrieval [built-in to ActionController](https://guides.rubyonrails.org/action_controller_overview.html#cookies). Rails uses a cookie to track the user's session ID, which allows access to session storage. [Devise also sets a cookie](https://github.com/heartcombo/devise/blob/main/lib/devise/strategies/rememberable.rb) when users select the **Remember Me** checkbox when signing in, which allows a user to re-authenticate after closing and re-opening a browser.
You can [set cookies with options](https://api.rubyonrails.org/v7.1.3.4/classes/ActionDispatch/Cookies.html) such as `:path` , `:expires`, `:domain` , and `:httponly` . Do not change from the defaults for these options unless it is required for the functionality you are implementing.
WARNING:
[Cookies set by GitLab are unset by default when users log out](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/controllers/sessions_controller.rb#L104). If you set a cookie with the `:domain` option, that cookie must be unset using the same `:domain` parameter. Otherwise the browser will not actually clear the cookie, and we risk persisting potentially-sensitive data which should have been cleared.
## Cookies in Frontend Code
Some of our frontend code sets cookies for persisting data during a session, such as dismissing alerts or sidebar position preferences. We use the [`setCookie` and `getCookie` helpers from `common_utils`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/lib/utils/common_utils.js#L697) to apply reasonable defaults to these cookies.
Be aware that, after 2021, browsers have started [aggressively purging cookies](https://clearcode.cc/blog/browsers-first-third-party-cookies/) and `localStorage` data set by JavaScript scripts in an effort to fight tracking scripts. If cookies seem to be unset every day or every few days, it is possible the data is getting purged, and you might want to preserve the data server-side rather than in browser-local storage.
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rspec-parameterized'
require 'gitlab/dangerfiles/spec_helper'
require_relative '../../../tooling/danger/cookie_setting'
require_relative '../../../tooling/danger/project_helper'
RSpec.describe Tooling::Danger::CookieSetting, feature_category: :tooling do
include_context 'with dangerfile'
let(:fake_danger) { DangerSpecHelper.fake_danger }
let(:fake_project_helper) { instance_double(Tooling::Danger::ProjectHelper) }
let(:comment_text) { "\n#{described_class::SUGGESTION}" }
let(:file_lines) { file_diff.map { |line| line.delete_prefix('+') } }
before do
allow(cookie_setting).to receive(:project_helper).and_return(fake_project_helper)
allow(cookie_setting.project_helper).to receive(:file_lines).and_return(file_lines)
allow(cookie_setting.helper).to receive(:added_files).and_return([filename])
allow(cookie_setting.helper).to receive(:changed_lines).with(filename).and_return(file_diff)
cookie_setting.define_singleton_method(:add_suggestions_for) do |filename|
Tooling::Danger::CookieSetting.new(filename, context: self).suggest
end
end
subject(:cookie_setting) { fake_danger.new(helper: fake_helper) }
context 'for single line method call' do
let(:file_diff) do
<<~DIFF.split("\n")
+ def index
+ #{code}
+
+ render text: 'OK'
+ end
DIFF
end
context 'when file is a non-spec Ruby file' do
let(:filename) { 'app/controllers/user_settings/active_sessions_controller.rb' }
using RSpec::Parameterized::TableSyntax
context 'when comment is expected' do
where(:code) do
[
'cookies[:my_key] = true',
'cookies["my_key"] = true',
'cookies[\'my_key\'] = true',
'cookies[:my_key] = { value: "nbd", expires: 1.year, domain: "example.com" }',
'cookies.encrypted[:my_key] = true',
'cookies.permanent[:my_key] = true',
'cookies.signed[:my_key] = true',
'cookies.signed.encrypted.permanent[:my_key] = true',
'cookies[Example::Class::With::CONSTANT] = true',
'cookies[Example::Class::With::CONSTANT] = { value: "nbd", expires: 1.year, domain: "example.com" }',
'cookies.encrypted[Example::Class::With::CONSTANT] = { value: "nbd", domain: "example.com" }',
'cookies.permanent[Example::Class::With::CONSTANT] = { value: "nbd", domain: "example.com" }',
'cookies.signed[Example::Class::With::CONSTANT] = { value: "nbd", domain: "example.com" }'
]
end
with_them do
specify do
expect(cookie_setting).to receive(:markdown).with(comment_text.chomp, file: filename, line: 2)
cookie_setting.add_suggestions_for(filename)
end
end
end
context 'when no comment is expected' do
where(:code) do
[
'cookies[:my_cookie].blank?',
'cookies.signed[Some::Class::With::CONSTANTS].blank?',
'cookies[:my_cookie] == "true"',
'cookies.encrypted[:my_cookie] += "true"'
]
end
with_them do
specify do
expect(cookie_setting).not_to receive(:markdown)
cookie_setting.add_suggestions_for(filename)
end
end
end
end
end
end
# frozen_string_literal: true
require_relative 'suggestion'
module Tooling
module Danger
class CookieSetting < Suggestion
MATCH = %r{cookies(?:\.encrypted|\.signed|\.permanent)*\[([^\]]+)\]\s*=[^=]}
REPLACEMENT = nil
DOCUMENTATION_LINK = 'https://docs.gitlab.com/ee/development/cookies.html#cookies-on-rails'
SUGGESTION = <<~MESSAGE_MARKDOWN.freeze
It looks like you are setting a server-side cookie. Please note that if you set
the `:domain` attribute for this cookie, you must ensure the cookie is unset when
the user logs out. Most cookies do not require this attribute.
----
For more information, see [cookies documentation](#{DOCUMENTATION_LINK}).
MESSAGE_MARKDOWN
end
end
end
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册