mozilla :: #balrog

19 May 2017
13:04bhearsumg'day folks
13:08aksareenG'day bhearsum. How's it going ?
13:08bhearsumnot bad! finally making some headway on my current work
13:08bhearsumhow about you?
13:12aksareendoing just fine here too. Posted another PR yesterday
13:24bhearsumi saw that, i'm looking forward to having a look at it
15:25aksareenbhearsum: Need your help with a failing test case. So here's the log : https://pastebin.mozilla.org/9022184 . Background : To make csrf_token field mandatory in every post request body, I modified the test cases here https://github.com/mozilla/balrog/blob/master/auslib/test/admin/views/base.py#L155 . But the Error message implies that the csrf_token key is
15:25aksareengetting passed on to scheduled_changes operations too. Since no base_csrf_token key defined , should I edit the method to add an exception that if the col name is csrf_token then loop should simply continue at this line https://github.com/mozilla/balrog/blob/master/auslib/db.py#L1275 . Is there any other change needed too ?
15:25bhearsumheh, funny coincidence, i was just looking at https://bugzilla.mozilla.org/show_bug.cgi?id=1361158
15:26bhearsumi think the right solution is to ensure that csrf_token doesn't get passed to the database layer
15:26bhearsumthe fact that is right now is a bug
15:27aksareenbhearsum: so meanwhile what should be done so that I can continue ?
15:27bhearsumi think you should make sure we're not passing csrf_token to the database layer
15:27aksareenhow ?
15:27bhearsumhow are you passing the form data right now?
15:28aksareenbasic validation through models and then after processing for specific errors, passing to DB layer.
15:30aksareenHow about this : what argument determines the form dictionary right, so we simply filter out if csrf_token is present in form data before invoking the DB layer
15:30aksareenbhearsum: ^
15:30bhearsumyup, i was about to say similar
15:31aksareenbhearsum: I suggest a global function in views which simply deletes the csrf_token key if present before passing to DB. So that means I'll have to modify each API and invoke that function before calling DB layer ?
15:32bhearsumis it going to be relevant to other endpoints? i think rules is the only one where we pass **form.data ?
15:34aksareenbhearsum: won't all post, put and delete operations supporting APIs be affected. Since all of them need csrf_token in their form values. Like releases here : https://github.com/mozilla/balrog/blob/master/ui/app/js/services/releases_service.js#L49
15:34bhearsumyeah, but we pass columns explicitly for the backends in those cases, eg: https://github.com/mozilla/balrog/blob/master/auslib/web/admin/views/releases.py#L209
15:35bhearsumthe reason this is an issue for rules is because we use ** like in https://github.com/mozilla/balrog/blob/master/auslib/web/admin/views/rules.py#L68
15:36aksareenbhearsum: so I'll modify it for /rules APIs for now. And in case if I encounter any failing tests then we'll know if that one needs modification too. Meanwhile , I'll be on lookout for whenever we're passing the entire **what dictionary.
15:37bhearsumyup, that sounds good
15:37bhearsumif it needs to be factored out later because many other endpoints do it, you can do it later
15:42aksareenbhearsum: a simple statement : dict.pop("csrf_token", None) will do the job actually. So I'll add a comment above it with the bug link to justify the its need
15:42bhearsumsounds good
18:07bhearsumaksareen: just let me know when you think you're ready for review
18:09aksareenbhearsum: I&#39;ve completed work on /rules/<id or alias> and sent for PR. You can review it. Currently working on remaining two /rules/columns/cokumn and /rules/<int:rule_id>/revisions . Will send the other two together in few hours
18:09bhearsumok, i&#39;ll have a look at whatever the latest is before my EOD
18:37bhearsumrelud, miles: is it easy to find out which IPs are whitelisted for admin stage vs admin prod?
18:38bhearsumwe&#39;re unable to submit to admin stage anymore, and i suspect it started when we made it non-public
18:43milesbhearsum: whitelisted in what sense?
18:43mileslike, for the vpn?
18:44bhearsumas in which IPs are able to talk to balrog-admin.stage.mozaws.net:443
18:47bhearsumand i&#39;m particularly curious if that&#39;s different for stage and prod
19:00reludjust out of a meeting
19:00reludit&#39;s easy to find out
19:01reludthese are the firewall rules in stage:
19:01reludhttps://irccloud.mozilla.com/file/yyMttygb/Screen%20Shot%202017-05-19%20at%2012.01.13%20PM.png
19:13bhearsumthanks
19:14bhearsumi think we&#39;re OK for now - we thought this might block Dawn work, but rail managed to work around it
20 May 2017
No messages
   
Last message: 36 days and 22 hours ago