pimeys / rust-web-push

A Web Push library for Rust
Apache License 2.0
113 stars 21 forks source link

Update to Ring 0.16 #14

Closed pimeys closed 4 years ago

pimeys commented 4 years ago

Should fix https://github.com/pimeys/rust-web-push/issues/13

pimeys commented 4 years ago

@githubaccount624 Ok so this was hard as expected, but here's my try.

Now some facts about this crate: I'm not in the push notification business anymore and my ability to test these in real life is about zero. So it would be really helpful if you could take my branch and try to see if it works for you. The tests are green, but some weirdness was definitely in certain integration tests with larger payload size than was tested originally. What is nice with this encryption is that if it's wrong, it will just not work. What is not so nice is that it will just not work and tell you nothing why it's wrong. Took me a long time to actually get the first version working and I also have no cryptographic background.

So, how did I fix this then? With ring you must go to see the commit log to the repo and see how the code was changed from the old version (hkdf/hmac and aead files). Then read the tests in ring and try to fit the new model to your code. Here the test encryption in http_ece.rs works and I'm quite sure the crate will actually work directly or with a little fixing.

githubaccount624 commented 4 years ago

Sure thing, I'll try it now and let you know how it goes ASAP

pimeys commented 4 years ago

Nice. If it just doesn't work, try the latest release of this crate with a simpler program that doesn't involve Rocket and try to get it sending notifications. Then update to this pull request and see if you can find the bug I introduced with this pull request.

githubaccount624 commented 4 years ago

So, I'm not sure if I'm doing this right, because this is my first time doing web notifications, but there aren't any errors being thrown, but I don't think I'm receiving the notification. I don't need to create a project with Firebase if I'm just using VAPID, right?

Here's my code (I need to remove unwraps and such but just trying to get it working first):

#[post("/save-subscription", format = "json", data = "<payload>")]
async fn handle_save_subscription(payload: Json<RequestSaveSubscription>) -> Status {
    let subscription_info = SubscriptionInfo {
        keys: SubscriptionKeys {
            p256dh: payload.keys.p256dh.clone(),
            auth: payload.keys.auth.clone(),
        },
        endpoint: payload.endpoint.clone(),
    };

    let file = std::fs::File::open("./private.pem").unwrap();

    let mut sig_builder = VapidSignatureBuilder::from_pem(file, &subscription_info).unwrap();
    sig_builder.add_claim("sub", "mailto:test@example.com");

    let signature = sig_builder.build().unwrap();

    let mut builder = WebPushMessageBuilder::new(&subscription_info).unwrap();

    let content = "Encrypted payload to be sent in the notification".as_bytes();

    builder.set_payload(ContentEncoding::AesGcm, content);
    builder.set_vapid_signature(signature);

    let client = WebPushClient::new();

    let response = client.send(builder.build().unwrap()).await.unwrap();

    Status::Ok
}
#[derive(Deserialize)]
pub struct RequestSaveSubscriptionKeys {
  pub p256dh: String,
  pub auth: String
}

#[derive(Deserialize)]
pub struct RequestSaveSubscription {
    pub endpoint: String,
    pub expiration_time: Option<i64>,
    pub keys: RequestSaveSubscriptionKeys
}

serviceworker:

self.addEventListener('push', function(event) {
  console.log('[Service Worker] Push Received.');
  console.log(`[Service Worker] Push had this data: "${event.data.text()}"`);

  const title = 'Push Codelab';
  const options = {
    body: 'Yay it works.',
    icon: 'images/icon.png',
    badge: 'images/badge.png'
  };

  event.waitUntil(self.registration.showNotification(title, options));
});

javascript:

export function subscribeUserToPush(cb) {
  if (!navigator.serviceWorker) {
    return;
  }

  return navigator.serviceWorker.register(process.env.PUBLIC_URL + '/sw-notifications.js').then(function(registration) {
    let subscribeOptions = {
      userVisibleOnly: true,
      applicationServerKey: urlBase64ToUint8Array('[my public key]')
    };

    return registration.pushManager.subscribe(subscribeOptions);
  })
  .then(function(pushSubscription) {
    cb(pushSubscription);
  });
}
    subscribeUserToPush(subscription => {
      window.axios.post("/save-subscription", subscription);
    });

Does this look vaguely correct, or am I missing something?

githubaccount624 commented 4 years ago

Nevermind! The error was in Chrome. I had to manually kill off the old service worker.

Okay, cool! It actually seems to be working! Great! I just need to spend some time cleaning everything up now and really read on the proper way to handle service worker lifetimes and whatnot.

Well, let me rephrase. I'm rather sure that it's working, but I did the click the "Test Notification" button once in the Chrome console. Not sure if that is now being played back automatically now when I refresh, or if my rocket server is initiating that call. I'll have a definite answer for you soon, but in the meantime it looks promising.

pimeys commented 4 years ago

You can also do one simple rust script where you just trigger a notification to the browser. Please try it out also in firefox due to them using a different protocol and servers.

pimeys commented 4 years ago

You can also have a logger in scope and do RUST_LOG=trace to see output from this crate.

Yeah web push is hard. When I wrote this crate the best documentation was the RFC and not the latest, but the third draft. I guess things haven't changed that much from those days.

githubaccount624 commented 4 years ago

I just tested it in Chrome and Firefox and it seems to work in both, so I think this may be safe to merge.

I do have a couple questions if you don't mind though.. (for making the code a bit cleaner)

Are any of these operations expensive that I should be aware of?

Currently my send_push_notification function looks like this:

async fn send_push_notification(subscription_info: SubscriptionInfo, message: &str, client: &Client) -> Result<(), GenericDBError> {
    let mut sig_builder = VapidSignatureBuilder::from_pem(keys::NOTIFICATIONS_PRIVATE_KEY, &subscription_info).unwrap();
    sig_builder.add_claim("sub", "mailto:test@example.com");

    let signature = sig_builder.build().unwrap();

    let mut builder = WebPushMessageBuilder::new(&subscription_info).unwrap();

    builder.set_payload(ContentEncoding::AesGcm, message.as_bytes());
    builder.set_vapid_signature(signature);

    let client = WebPushClient::new();
    // Set the TTL?

    let response = client.send(builder.build().unwrap()).await.unwrap();

    Ok(())
}

I need to get rid of the unwraps, but not sure if any of these functions should be moved out of here. Maybe WebPushClient::new()? Not sure if I should put that in Rocket's managed state or if it's fine to create it on the fly for each message.

pimeys commented 4 years ago

You can get away with lots of allocations without worrying too much, but here what you do, everything is much faster than the actual request to the web push servers. If you're ok by responding to the user before the notification is sent, you might want to use tokio's spawn to sideload the sending from the request and the response for the user is much faster.

You should use ? and have your own error type. Write From trait conversion from the web push errors to your own errors to make it beautiful.

pimeys commented 4 years ago

0.7.0 released with this fix.

githubaccount624 commented 4 years ago

Cool, thanks again! I really appreciate you taking some time to come back to this even though you're not actively dependent on it. I really needed this for my project and have been spamming F5 on that issue the past few days in hope :p

pimeys commented 4 years ago

Haha. I'm working really long days and in general my github account is so full of notifications it's sometimes hard to spot them, that's why they might take so long. And especially when our company is doing oss rust, these can easily get lost to the static.

Luckily I had a free Saturday to do nothing and refactoring your old code is fun :)