{"action":"created","issue":{"url":"https://api.github.com/repos/mozilla-services/syncstorage-rs/issues/20","repository_url":"https://api.github.com/repos/mozilla-services/syncstorage-rs","labels_url":"https://api.github.com/repos/mozilla-services/syncstorage-rs/issues/20/labels{/name}","comments_url":"https://api.github.com/repos/mozilla-services/syncstorage-rs/issues/20/comments","events_url":"https://api.github.com/repos/mozilla-services/syncstorage-rs/issues/20/events","html_url":"https://github.com/mozilla-services/syncstorage-rs/pull/20","id":355247767,"node_id":"MDExOlB1bGxSZXF1ZXN0MjExODIwODI5","number":20,"title":"feat: implement hawk authentication","user":{"login":"philbooth","id":64367,"node_id":"MDQ6VXNlcjY0MzY3","avatar_url":"https://avatars2.githubusercontent.com/u/64367?v=4","gravatar_id":"","url":"https://api.github.com/users/philbooth","html_url":"https://github.com/philbooth","followers_url":"https://api.github.com/users/philbooth/followers","following_url":"https://api.github.com/users/philbooth/following{/other_user}","gists_url":"https://api.github.com/users/philbooth/gists{/gist_id}","starred_url":"https://api.github.com/users/philbooth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/philbooth/subscriptions","organizations_url":"https://api.github.com/users/philbooth/orgs","repos_url":"https://api.github.com/users/philbooth/repos","events_url":"https://api.github.com/users/philbooth/events{/privacy}","received_events_url":"https://api.github.com/users/philbooth/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-08-29T16:52:35Z","updated_at":"2018-09-07T14:50:10Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/mozilla-services/syncstorage-rs/pulls/20","html_url":"https://github.com/mozilla-services/syncstorage-rs/pull/20","diff_url":"https://github.com/mozilla-services/syncstorage-rs/pull/20.diff","patch_url":"https://github.com/mozilla-services/syncstorage-rs/pull/20.patch"},"body":"Fixes #19.\r\n\r\nI didn't quite manage to get this finished today, mostly because I wasted a few hours writing parsing logic for the Hawk header, only to eventually RTFM and notice the hawk crate already implements that via the `FromStr` trait. Sigh.\r\n\r\nAnyway, I wanted to post this as a WIP so that @rfk can hopefully cast his eye over the crypto logic while I'm away. As you'll see Ryan, I've left your excellent (and long) comment in place for now, interjected with my attempts to implement each section of it. There are a couple of places where I'm not sure I understood you correctly, so I'll call those out in the inline comments.\r\n\r\nIf I have time after I've packed my bag tomorrow, I might see if I can wire it into the request mechanism and write some tests for it. If I do get to that point I'll comment here and remove the WIP. But, being realistic, I probably won't get to that point.\r\n\r\nSee you all next week! 😄"},"comment":{"url":"https://api.github.com/repos/mozilla-services/syncstorage-rs/issues/comments/419463891","html_url":"https://github.com/mozilla-services/syncstorage-rs/pull/20#issuecomment-419463891","issue_url":"https://api.github.com/repos/mozilla-services/syncstorage-rs/issues/20","id":419463891,"node_id":"MDEyOklzc3VlQ29tbWVudDQxOTQ2Mzg5MQ==","user":{"login":"philbooth","id":64367,"node_id":"MDQ6VXNlcjY0MzY3","avatar_url":"https://avatars2.githubusercontent.com/u/64367?v=4","gravatar_id":"","url":"https://api.github.com/users/philbooth","html_url":"https://github.com/philbooth","followers_url":"https://api.github.com/users/philbooth/followers","following_url":"https://api.github.com/users/philbooth/following{/other_user}","gists_url":"https://api.github.com/users/philbooth/gists{/gist_id}","starred_url":"https://api.github.com/users/philbooth/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/philbooth/subscriptions","organizations_url":"https://api.github.com/users/philbooth/orgs","repos_url":"https://api.github.com/users/philbooth/repos","events_url":"https://api.github.com/users/philbooth/events{/privacy}","received_events_url":"https://api.github.com/users/philbooth/received_events","type":"User","site_admin":false},"created_at":"2018-09-07T14:50:10Z","updated_at":"2018-09-07T14:50:10Z","author_association":"MEMBER","body":"Okay, I think this is close to being reviewable now, there's just one last thing that I'm unsure about.\r\n\r\nAs @rfk pointed out in https://github.com/mozilla-services/syncstorage-rs/pull/20#discussion_r215828757, the hawk crate depends on a different version of ring to actix. Although that doesn't seem to affect my build on MacOS, it causes linker errors about conflicting symbols in other environments.\r\n\r\nInitially I just forked `hawk` to make it use the same version of ring as actix, and that allows the CI build to succeed. But thinking it through, is that setting us up for a maintenance nightmare as we risk the same thing happening again every time we run `cargo update`? @bbangert & @pjenvey, is there some clever Rust/cargo magic you guys know of for managing this kind of situation differently?\r\n\r\nAn alternative, which I've fleshed out in a separate branch, would be to ditch the `hawk` crate and replace it with some internal logic. That doesn't mean implementing all the crypto ourselves of course, it just means doing a thin layer of parsing and validation on top of external crypto libs that I was already using anyway. Concretely, here's the diff from this PR to what that looks like:\r\n\r\nhttps://github.com/philbooth/syncstorage-rs/compare/hawk-authentication...philbooth:hawk-authentication-internal?expand=1\r\n\r\nThe exact same tests pass for both branches, so it seems like a low-risk proposal to me (but I'm a hawk/crypto novice so I realise that doesn't count for much)."}}
{"actor":{"display_login":"philbooth"}}