Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Fix CSRF annoyance #4193

Merged
merged 4 commits into from
Nov 23, 2016
Merged

Fix CSRF annoyance #4193

merged 4 commits into from
Nov 23, 2016

Conversation

chadwhitacre
Copy link
Contributor

Follow-on from #4185.

@chadwhitacre chadwhitacre changed the title We don't have /callbacks/ anymore Fix CSRF annoyance Nov 14, 2016
@chadwhitacre
Copy link
Contributor Author

Alright @rohitpaulk et al., now this is ready for review. :)

@chadwhitacre
Copy link
Contributor Author

Bump. Look okay, @rohitpaulk @aandis et al.?

@chadwhitacre
Copy link
Contributor Author

(Previous head was c1f4dc6.)

@chadwhitacre
Copy link
Contributor Author

@kaguillera Can you review this one?

@kaguillera
Copy link
Contributor

kaguillera commented Nov 23, 2016

It looks good. but I think that it needs rebasing before I can merge it.

The algorithm functions are delicately entangled, as the regression
fixed here indicates. The fix is to be more conservative with
reordering. I also moved the test over to be closer to a similar one.
@chadwhitacre
Copy link
Contributor Author

Rebased! Was 29e6f42.

@chadwhitacre
Copy link
Contributor Author

Weird fail here too. 😕

__________ TestHistory.test_iter_payday_events_with_failed_exchanges ___________
self = <test_history.TestHistory testMethod=test_iter_payday_events_with_failed_exchanges>
    def test_iter_payday_events_with_failed_exchanges(self):
        alice = self.make_participant('alice', claimed_time='now')
        self.make_exchange('braintree-cc', 50, 0, alice)
        self.make_exchange('braintree-cc', 12, 0, alice, status='failed')
        self.make_exchange('paypal', -40, 0, alice, status='failed')
        events = list(iter_payday_events(self.db, alice))
        assert len(events) == 5
        assert events[0]['kind'] == 'day-open'
>       assert events[0]['balance'] == 50
E       AssertionError: assert Decimal('72.00') == 50
tests/py/test_history.py:103: AssertionError
________ TestHistoryPage.test_admin_can_view_closed_participant_history ________
self = <test_history.TestHistoryPage testMethod=test_admin_can_view_closed_participant_history>
    def test_admin_can_view_closed_participant_history(self):
        self.make_exchange('braintree-cc', -30, 0, self.alice)
>       self.alice.close()
tests/py/test_history.py:130: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
gratipay/models/participant/__init__.py:303: in close
    self.final_check(cursor)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = <Participant u'alice'>
cursor = <cursor object at 0x2aedadccd350; closed: -1>
    def final_check(self, cursor):
        """Sanity-check that teams and balance have been dealt with.
            """
        if self.get_teams(cursor=cursor):
            raise self.StillOnATeam
        if self.balance != 0:
>           raise self.BalanceIsNotZero
E           BalanceIsNotZero
gratipay/models/participant/__init__.py:1194: BalanceIsNotZero
 TestRecordAnExchange.test_dropping_balance_below_zero_is_allowed_in_this_context 
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_dropping_balance_below_zero_is_allowed_in_this_context>
    def test_dropping_balance_below_zero_is_allowed_in_this_context(self):
        self.record_an_exchange({'amount': '-10', 'fee': '0'})
        actual = self.db.one("SELECT balance FROM participants WHERE username='bob'")
>       assert actual == D('-10.00')
E       AssertionError: assert Decimal('-20.00') == Decimal('-10.00')
E        +  where Decimal('-10.00') = D('-10.00')
tests/py/test_record_an_exchange.py:99: AssertionError
____________ TestRecordAnExchange.test_failed_doesnt_affect_balance ____________
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_failed_doesnt_affect_balance>
    def test_failed_doesnt_affect_balance(self):
        self.make_participants()
        for amount in ('10', '-10'):
            self.record_an_exchange({
                'amount': amount,
                'fee': '0',
                'status': 'failed'
            }, False)
>           assert self.db.one("SELECT balance FROM participants WHERE username='bob'") == 0
E           AssertionError: assert Decimal('10.00') == 0
E            +  where Decimal('10.00') = <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>>("SELECT balance FROM participants WHERE username='bob'")
E            +    where <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>> = <gratipay.models.GratipayDB object at 0x2aedaa7acad0>.one
E            +      where <gratipay.models.GratipayDB object at 0x2aedaa7acad0> = <test_record_an_exchange.TestRecordAnExchange testMethod=test_failed_doesnt_affect_balance>.db
tests/py/test_record_an_exchange.py:168: AssertionError
_____ TestRecordAnExchange.test_other_statuses_affect_balance_for_payouts ______
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_other_statuses_affect_balance_for_payouts>
    def test_other_statuses_affect_balance_for_payouts(self):
        self.make_participants()
        balance = 0
        for status in ('pre', 'pending'):
            self.record_an_exchange({
                'amount': '-10',
                'fee': '0',
                'status': status
            }, False)
            balance -= 10
>           assert self.db.one("SELECT balance FROM participants WHERE username='bob'") == balance
E           AssertionError: assert Decimal('-20.00') == -10
E            +  where Decimal('-20.00') = <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>>("SELECT balance FROM participants WHERE username='bob'")
E            +    where <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>> = <gratipay.models.GratipayDB object at 0x2aedaa7acad0>.one
E            +      where <gratipay.models.GratipayDB object at 0x2aedaa7acad0> = <test_record_an_exchange.TestRecordAnExchange testMethod=test_other_statuses_affect_balance_for_payouts>.db
tests/py/test_record_an_exchange.py:190: AssertionError
___ TestRecordAnExchange.test_other_statuses_dont_affect_balance_for_payins ____
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_other_statuses_dont_affect_balance_for_payins>
    def test_other_statuses_dont_affect_balance_for_payins(self):
        self.make_participants()
        for status in ('pre', 'pending'):
            self.record_an_exchange({
                'amount': '10',
                'fee': '0',
                'status': status
            }, False)
>           assert self.db.one("SELECT balance FROM participants WHERE username='bob'") == 0
E           AssertionError: assert Decimal('10.00') == 0
E            +  where Decimal('10.00') = <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>>("SELECT balance FROM participants WHERE username='bob'")
E            +    where <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>> = <gratipay.models.GratipayDB object at 0x2aedaa7acad0>.one
E            +      where <gratipay.models.GratipayDB object at 0x2aedaa7acad0> = <test_record_an_exchange.TestRecordAnExchange testMethod=test_other_statuses_dont_affect_balance_for_payins>.db
tests/py/test_record_an_exchange.py:178: AssertionError
_____________ TestRecordAnExchange.test_succeeded_affects_balance ______________
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_succeeded_affects_balance>
    def test_succeeded_affects_balance(self):
        self.make_participants()
        balance = 0
        for amount in ('10', '-10'):
            self.record_an_exchange({'amount': amount, 'fee': '0'}, False)
            balance += int(amount)
>           assert self.db.one("SELECT balance FROM participants WHERE username='bob'") == balance
E           AssertionError: assert Decimal('20.00') == 10
E            +  where Decimal('20.00') = <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>>("SELECT balance FROM participants WHERE username='bob'")
E            +    where <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>> = <gratipay.models.GratipayDB object at 0x2aedaa7acad0>.one
E            +      where <gratipay.models.GratipayDB object at 0x2aedaa7acad0> = <test_record_an_exchange.TestRecordAnExchange testMethod=test_succeeded_affects_balance>.db
tests/py/test_record_an_exchange.py:158: AssertionError
______________ TestRecordAnExchange.test_success_updates_balance _______________
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_success_updates_balance>
    def test_success_updates_balance(self):
        self.record_an_exchange({'amount': '10', 'fee': '0'})
        expected = D('10.00')
        SQL = "SELECT balance FROM participants WHERE username='bob'"
        actual = self.db.one(SQL)
>       assert actual == expected
E       AssertionError: assert Decimal('20.00') == Decimal('10.00')
tests/py/test_record_an_exchange.py:120: AssertionError
________ TestRecordAnExchange.test_withdrawals_take_fee_out_of_balance _________
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_withdrawals_take_fee_out_of_balance>
    def test_withdrawals_take_fee_out_of_balance(self):
        self.make_participant('alice', claimed_time='now', is_admin=True)
        self.bob = self.make_participant('bob', claimed_time='now', balance=20)
        self.bob = self.record_an_exchange({'amount': '-7', 'fee': '1.13'}, False)
        SQL = "SELECT balance FROM participants WHERE username='bob'"
>       assert self.db.one(SQL) == D('11.87')
E       AssertionError: assert Decimal('3.74') == Decimal('11.87')
E        +  where Decimal('3.74') = <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>>("SELECT balance FROM participants WHERE username='bob'")
E        +    where <bound method GratipayDB.one of <gratipay.models.GratipayDB object at 0x2aedaa7acad0>> = <gratipay.models.GratipayDB object at 0x2aedaa7acad0>.one
E        +      where <gratipay.models.GratipayDB object at 0x2aedaa7acad0> = <test_record_an_exchange.TestRecordAnExchange testMethod=test_withdrawals_take_fee_out_of_balance>.db
E        +  and   Decimal('11.87') = D('11.87')
tests/py/test_record_an_exchange.py:138: AssertionError
__________________ TestRecordAnExchange.test_withdrawals_work __________________
self = <test_record_an_exchange.TestRecordAnExchange testMethod=test_withdrawals_work>
    def test_withdrawals_work(self):
        self.make_participant('alice', claimed_time='now', is_admin=True)
        self.bob = self.make_participant('bob', claimed_time='now', balance=20)
    
        self.record_an_exchange({'amount': '-7', 'fee': '0'}, make_participants=False)
    
        expected = D('13.00')
        SQL = "SELECT balance FROM participants WHERE username='bob'"
        actual = self.db.one(SQL)
>       assert actual == expected
E       AssertionError: assert Decimal('6.00') == Decimal('13.00')
tests/py/test_record_an_exchange.py:131: AssertionError
_________________ TestAbsorptions.test_balance_was_transferred _________________
self = <test_take_over.TestAbsorptions testMethod=test_balance_was_transferred>
    def test_balance_was_transferred(self):
        fresh_bob = P('bob')
>       assert fresh_bob.balance == self.bob.balance == self.expected_new_balance
E       AssertionError: assert Decimal('37.06') == Decimal('19.03')
E        +  where Decimal('37.06') = <Participant u'bob'>.balance
E        +    where <Participant u'bob'> = <test_take_over.TestAbsorptions testMethod=test_balance_was_transferred>.bob
E        +  and   Decimal('19.03') = <test_take_over.TestAbsorptions testMethod=test_balance_was_transferred>.expected_new_balance
tests/py/test_take_over.py:73: AssertionError
--------------- coverage: platform linux2, python 2.7.12-final-0 ---------------

@chadwhitacre
Copy link
Contributor Author

Other: #3975 (comment).

@chadwhitacre
Copy link
Contributor Author

Alright, @kaguillera … rebased and green! :-)

@kaguillera kaguillera merged commit 26fdbbe into master Nov 23, 2016
@kaguillera kaguillera deleted the fix-csrf-annoyance branch November 23, 2016 19:44
@chadwhitacre
Copy link
Contributor Author

💃

@chadwhitacre
Copy link
Contributor Author

Sentry 13!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants