Verified Commit cbeb8bd1 authored by Nina's avatar Nina Committed by paz

rubocop: Auto-correct a few warnings and add more TODOs.

parent 4af60eb1
......@@ -779,3 +779,12 @@ Style/ZeroLengthPredicate:
Metrics/LineLength:
Exclude:
- '**/*'
Max: 365
Lint/NestedMethodDefinition:
Exclude:
- '**/*'
Lint/ElseLayout:
Exclude:
- '**/*'
......@@ -48,16 +48,16 @@ module SchleuderApiDaemonHelper
List.where(query_args).first || halt(404)
end
def load_subscription(identifier)
def load_subscription(identifier)
query_args = make_query_args(identifier)
if is_an_integer?(identifier)
query_base = Subscription
else require_list_id_param("Parameter list_id is required when using email as identifier for subscriptions.")
list = load_list(params[:list_id])
query_base = list.subscriptions
else require_list_id_param('Parameter list_id is required when using email as identifier for subscriptions.')
list = load_list(params[:list_id])
query_base = list.subscriptions
end
query_base.where(query_args).first || halt(404)
end
end
def subscription(id_or_email)
if is_an_integer?(id_or_email)
......
......@@ -7,7 +7,7 @@ class SchleuderApiDaemon < Sinatra::Base
end
post '.json' do
authorize(List, :create)
authorize(List, :create)
listname = parsed_body['email']
fingerprint = parsed_body['fingerprint']
......
module Schleuder
class Account < ActiveRecord::Base
PASSWORD_CHARS = [
("a".."z").to_a,
("A".."Z").to_a,
('a'..'z').to_a,
('A'..'Z').to_a,
(0..9).to_a,
%w[! @ # $ % ^ & * ( ) _ - + = { [ } ] : ; < , > . ? /]
].flatten
PASSWORD_LENGTH_RANGE = 10..12
PASSWORD_LENGTH_RANGE = (10..12).freeze
has_secure_password
has_many :subscriptions, foreign_key: "email", primary_key: "email"
has_many :subscriptions, foreign_key: 'email', primary_key: 'email'
has_many :lists, through: :subscriptions
has_many :admin_lists, through: :subscriptions
......@@ -25,7 +25,7 @@ module Schleuder
def set_new_password!
new_password = generate_password
self.update!(password: new_password)
update!(password: new_password)
new_password
end
......
......@@ -9,7 +9,8 @@ module Schleuder
end
def authorize(resource, action)
return nil if resource == nil
return nil if resource.nil?
action = action.to_s
action << '?' unless action.last == '?'
policy(resource).public_send(action)
......
......@@ -2,7 +2,7 @@ module Schleuder
module AuthorizerPolicies
class BasePolicy
attr_reader :account, :object
class BaseScope
attr_reader :account
......@@ -11,16 +11,13 @@ module Schleuder
end
end
def initialize(account, object)
@account = account
@object = object
end
private
def admin?(list)
account.admin_of_list?(list)
end
......
......@@ -9,7 +9,7 @@ module Schleuder
# create is not defined: it must be checked via ListPolicy#add_keys (because we need the list-context).
# update is not defined: we can't update keys.
def delete?
superadmin? || admin?(object.list)
end
......
......@@ -11,7 +11,6 @@ module Schleuder
end
end
def list?
true
end
......@@ -39,7 +38,7 @@ module Schleuder
def subscribe?
superadmin? || admin?(object)
end
def list_keys?
superadmin? || subscribed?(object)
end
......@@ -56,10 +55,8 @@ module Schleuder
superadmin? || admin?(object)
end
private
# This includes list-admins.
def subscribed?(list)
account.subscribed_to_list?(list)
......
......@@ -11,7 +11,6 @@ module Schleuder
end
end
# If subscriptions for a given list are checked, call ListPolicy#list_subscriptions
def list
true
......@@ -31,12 +30,10 @@ module Schleuder
superadmin? || admin?(object.list) || own?(object)
end
private
def own?(object)
account.subscriptions.include?(object)
account.subscriptions.include?(object)
end
end
end
......
......@@ -2,12 +2,10 @@ module Schleuder
module CliHelper
def self.included(base)
base.no_commands do
def fatal(msg, exitcode=1)
def fatal(msg, exitcode = 1)
error("Error: #{msg}")
exit exitcode
end
end
end
end
......
......@@ -3,7 +3,7 @@ module Schleuder
belongs_to :list
belongs_to :account
# This association is wanted in Account but ActiveRecord wants its details defined here.
belongs_to :admin_list, -> { where subscriptions: { admin: true } }, class_name: "List", foreign_key: :list_id
belongs_to :admin_list, -> { where subscriptions: { admin: true } }, class_name: 'List', foreign_key: :list_id
validates :list_id, inclusion: {
in: -> (id) { List.pluck(:id) },
......
......@@ -20,7 +20,7 @@ describe 'keys via api' do
get "/keys.json?list_id=#{@list.id}"
expect(last_response.status).to be 403
expect(last_response.body).to eql("Not authorized")
expect(last_response.body).to eql('Not authorized')
end
it 'does list keys authorized as subscriber' do
......@@ -70,7 +70,7 @@ describe 'keys via api' do
get "/keys/check_keys.json?list_id=#{@list.id}"
expect(last_response.status).to be 403
expect(last_response.body).to eql("Not authorized")
expect(last_response.body).to eql('Not authorized')
end
it "doesn't check keys authorized as subscriber" do
......@@ -81,16 +81,16 @@ describe 'keys via api' do
get "/keys/check_keys.json?list_id=#{@list.id}"
expect(last_response.status).to be 403
expect(last_response.body).to eql("Not authorized")
expect(last_response.body).to eql('Not authorized')
end
it 'does check keys authorized as list-admin' do
subscription = create(:subscription, list_id: @list.id, admin: true)
account = create(:account, email: subscription.email)
authorize!(account.email, account.set_new_password!)
@list.import_key(File.read("spec/fixtures/revoked_key.txt"))
@list.import_key(File.read("spec/fixtures/signonly_key.txt"))
@list.import_key(File.read("spec/fixtures/expired_key.txt"))
@list.import_key(File.read('spec/fixtures/revoked_key.txt'))
@list.import_key(File.read('spec/fixtures/signonly_key.txt'))
@list.import_key(File.read('spec/fixtures/expired_key.txt'))
get "/keys/check_keys.json?list_id=#{@list.id}"
......@@ -106,9 +106,9 @@ describe 'keys via api' do
end
it 'does check keys authorized as api_superadmin' do
@list.import_key(File.read("spec/fixtures/revoked_key.txt"))
@list.import_key(File.read("spec/fixtures/signonly_key.txt"))
@list.import_key(File.read("spec/fixtures/expired_key.txt"))
@list.import_key(File.read('spec/fixtures/revoked_key.txt'))
@list.import_key(File.read('spec/fixtures/signonly_key.txt'))
@list.import_key(File.read('spec/fixtures/expired_key.txt'))
authorize_as_api_superadmin!
get "/keys/check_keys.json?list_id=#{@list.id}"
......@@ -140,7 +140,7 @@ describe 'keys via api' do
get "/keys.json?list_id=#{@list.id}"
expect(last_response.status).to be 403
expect(last_response.body).to eql("Not authorized")
expect(last_response.body).to eql('Not authorized')
end
it 'does list keys authorized as subscriber' do
......@@ -212,7 +212,6 @@ describe 'keys via api' do
@list.delete_key('0xEBDBE899251F2412')
end
it 'does import keys authorized as list-admin' do
subscription = create(:subscription, list_id: @list.id, admin: true)
account = create(:account, email: subscription.email)
......@@ -244,7 +243,7 @@ describe 'keys via api' do
context 'delete' do
before(:each) do
@list.import_key(File.read("spec/fixtures/bla_foo_key.txt"))
@list.import_key(File.read('spec/fixtures/bla_foo_key.txt'))
end
it "doesn't delete keys without authentication" do
......@@ -267,7 +266,7 @@ describe 'keys via api' do
delete "/keys/0xEBDBE899251F2412.json?list_id=#{@list.id}"
expect(last_response.status).to be 403
expect(last_response.body).to eql("Not authorized")
expect(last_response.body).to eql('Not authorized')
expect(@list.reload.keys.length).to eql(num_keys)
end
......@@ -280,11 +279,11 @@ describe 'keys via api' do
delete "/keys/0xEBDBE899251F2412.json?list_id=#{@list.id}"
expect(last_response.status).to be 403
expect(last_response.body).to eql("Not authorized")
expect(last_response.body).to eql('Not authorized')
expect(@list.reload.keys.length).to eql(num_keys)
end
it "does delete keys authorized as list-admin" do
it 'does delete keys authorized as list-admin' do
subscription = create(:subscription, list_id: @list.id, admin: true)
account = create(:account, email: subscription.email)
authorize!(account.email, account.set_new_password!)
......@@ -296,7 +295,7 @@ describe 'keys via api' do
expect(@list.reload.keys.length).to eql(num_keys - 1)
end
it "does delete keys authorized as api_superadmin" do
it 'does delete keys authorized as api_superadmin' do
authorize_as_api_superadmin!
num_keys = @list.keys.length
......@@ -310,7 +309,7 @@ describe 'keys via api' do
context 'a key with broken utf8 in uid' do
context 'already imported' do
before(:each) do
@list.import_key(File.read("spec/fixtures/broken_utf8_uid_key.txt"))
@list.import_key(File.read('spec/fixtures/broken_utf8_uid_key.txt'))
end
after(:each) do
@list.delete_key('0x1242F6E13D8EBE4A')
......@@ -325,7 +324,7 @@ describe 'keys via api' do
authorize_as_api_superadmin!
get "/keys/0x1242F6E13D8EBE4A.json?list_id=#{@list.id}"
expect(last_response.status).to be 200
expect(JSON.parse(last_response.body)['fingerprint']).to eq("3102B29989BEE703AE5ED62E1242F6E13D8EBE4A")
expect(JSON.parse(last_response.body)['fingerprint']).to eq('3102B29989BEE703AE5ED62E1242F6E13D8EBE4A')
end
it 'does delete key' do
authorize_as_api_superadmin!
......
......@@ -65,7 +65,7 @@ describe 'lists via api' do
get "lists/#{list.id}.json"
expect(last_response.status).to be 401
expect(last_response.body).to eql("Not authenticated")
expect(last_response.body).to eql('Not authenticated')
end
it "doesn't show a list authorized as unassociated account" do
......@@ -77,7 +77,7 @@ describe 'lists via api' do
get "lists/#{list.id}.json"
expect(last_response.status).to be 403
expect(last_response.body).to eql("Not authorized")
expect(last_response.body).to eql('Not authorized')
end
it "doesn't show a list authorized as subscriber" do
......@@ -92,7 +92,7 @@ describe 'lists via api' do
expect(JSON.parse(last_response.body)['email']).to eq list.email
end
it "does show a list authorized as list-admin" do
it 'does show a list authorized as list-admin' do
list = create(:list)
subscription = create(:subscription, list_id: list.id, admin: true)
account = create(:account, email: subscription.email)
......@@ -116,7 +116,7 @@ describe 'lists via api' do
it 'correctly finds a list by email-address that starts with a number' do
authorize_as_api_superadmin!
list = create(:list, email: "9list@hostname")
list = create(:list, email: '9list@hostname')
get "lists/#{list.email}.json"
expect(last_response.status).to be 200
expect(JSON.parse(last_response.body)['email']).to eq list.email
......
require "spec_helper"
require 'spec_helper'
describe Schleuder::Account do
it { is_expected.to respond_to :subscriptions }
......@@ -10,9 +10,9 @@ describe Schleuder::Account do
it "#lists shows all lists of all the account's subscriptions, regardless of admin-flags" do
list1 = create(:list)
list2 = create(:list)
create(:subscription, email: "someone@example.org", list: list1, admin: false)
create(:subscription, email: "someone@example.org", list: list2, admin: true)
account = create(:account, email: "someone@example.org")
create(:subscription, email: 'someone@example.org', list: list1, admin: false)
create(:subscription, email: 'someone@example.org', list: list2, admin: true)
account = create(:account, email: 'someone@example.org')
expect(account.lists.size).to eql(2)
expect(account.lists.map(&:id).sort).to eql([list1.id, list2.id])
......@@ -21,15 +21,15 @@ describe Schleuder::Account do
it "#admin_lists shows only lists of which the account's subscriptions are admins of" do
list1 = create(:list)
list2 = create(:list)
create(:subscription, email: "someone@example.org", list: list1, admin: false)
create(:subscription, email: "someone@example.org", list: list2, admin: true)
account = create(:account, email: "someone@example.org")
create(:subscription, email: 'someone@example.org', list: list1, admin: false)
create(:subscription, email: 'someone@example.org', list: list2, admin: true)
account = create(:account, email: 'someone@example.org')
expect(account.admin_lists.size).to eql(1)
expect(account.admin_lists.map(&:id).sort).to eql([list2.id])
end
it "fails to save an account without an email-address" do
it 'fails to save an account without an email-address' do
account = Account.new(password: 'bla')
result = account.save
......@@ -38,7 +38,7 @@ describe Schleuder::Account do
expect(account.valid?).to be(false)
end
it "fails to save an account without a password" do
it 'fails to save an account without a password' do
account = Account.new(email: 'bla')
result = account.save
......@@ -47,7 +47,7 @@ describe Schleuder::Account do
expect(account.valid?).to be(false)
end
it "#set_new_password! changes and returns the stored password" do
it '#set_new_password! changes and returns the stored password' do
account = create(:account, password: 'foo')
expect(account.authenticate('foo')).to be_an(Account)
......@@ -58,26 +58,26 @@ describe Schleuder::Account do
expect(account.authenticate(new_password)).to be_an(Account)
end
it "does not store the password in cleartext" do
account = create(:account, password: "blabla")
it 'does not store the password in cleartext' do
account = create(:account, password: 'blabla')
account = Account.find(account.id)
expect(account.password).to be(nil)
expect(account.password_digest).not_to include("blabla")
expect(account.password_digest).not_to include('blabla')
end
it "saves email-addresses always in lower-case" do
account = create(:account, email: "ME@EXAMPLE.ORG")
it 'saves email-addresses always in lower-case' do
account = create(:account, email: 'ME@EXAMPLE.ORG')
expect(account.email).to eql("me@example.org")
expect(account.email).to eql('me@example.org')
end
it "generates random passwords" do
it 'generates random passwords' do
account = create(:account)
pw1 = account.send("generate_password")
pw2 = account.send("generate_password")
pw3 = account.send("generate_password")
pw1 = account.send('generate_password')
pw2 = account.send('generate_password')
pw3 = account.send('generate_password')
expect(pw1.size).to be_between(Account::PASSWORD_LENGTH_RANGE.first, Account::PASSWORD_LENGTH_RANGE.last)
expect(pw2.size).to be_between(Account::PASSWORD_LENGTH_RANGE.first, Account::PASSWORD_LENGTH_RANGE.last)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment