microsoftgraph / msgraph-sdk-javascript

Microsoft Graph client library for JavaScript
https://graph.microsoft.com
MIT License
748 stars 226 forks source link

Secured API gateway endpoint as webhook #406

Closed mickog closed 3 years ago

mickog commented 3 years ago

Implementing a service that will get notified when a mailbox receives an email. Created the subscription fine but it will only receive mails when the AWS API gateway created is set to regional on AWS making it public.

This seems odd to me that there is no way to configure the webhook with oauth details rather than leaving the api open.

Can't see anything in the docs regarding this could you point me to it if it is there, would also be interested to hear what others are doing to tackle this issue..

Thanks AB#8732

baywet commented 3 years ago

Hi @mickog Thanks for reaching out. This is what the client state property of the subscription is for. It's a shared secret between the client application and Microsoft Graph that allows the client application to know whether a specific call comes from Microsoft Graph. Alternatively, if the resource supports sending data with the change notification, it'll be encrypted using a temporary symmetric key, which itself will be encrypted using the public key you provided. This helps making sure no-one has tempered with the data in transit and that the data originate from Microsoft Graph and not a rogue actor.

mickog commented 3 years ago

I get what you are saying about the clientState property this would allow our service to verify that the request is coming from Graph, the problem we are having is that we have to leave our API gateway open with no security on it or else the webhook notification won't reach our AWS lambda to verify the clientState. This is a security concern for us leaving the API gateway open, it means a rogue actor could continuously hit our lambda which pay for each invocation of as well as the company don't like us using open API endpoints with no security. Wondering is there anyway to have the web-hook hit a secured endpoint, say one that has OAuth configured ?

mickog commented 3 years ago

Also curious is the clientState property just used for verifying subscriptions or is it also used on the webhook notification, say when we get a mail and our service gets notified then will the clientState be part of that request also? Thanks

baywet commented 3 years ago

The service doesn't support delivering notifications over authenticated endpoints today (through JWT bearer tokens or other means). This is something you can request on uservoice. Yes to the question "is the client state part of the notification payload?" as you can see in the docs I don't know much about AWS lambdas, can you configure firewall rules for inbound connections? If so you could deny all and allow only those IPs (see the "Microsoft Graph Change Notifications" entry)

mickog commented 3 years ago

Yea I'm pretty sure we can do something like that, thats exactly what I was looking for thanks. Question.. does that list of IP addresses ever change or will we be safe hardcoding them, or is there an API that we would need to check if they were updated or anything? Thanks

baywet commented 3 years ago

It does change, but not frequently. However back when I was a PM for that feature (webhooks), there was an ongoing effort to have those IPs surfaced in the IP web service. I don't see those in right now. Maybe this is something that @Jumaodhiss can follow up on and make sure those IPs get added to the endpoint (and updated whenever a change occurs).

mickog commented 3 years ago

That's interesting, when you say not frequently would you say every few months or years or its just hard to say?

baywet commented 3 years ago

hard to say, it depends on deployment models, network configurations, how the service groves, and a multitude of other factors. Some of which are in control of the product team, some which are not. Obviously the product team tries to minimize the number of changes as much as possible to avoid disrupting customers.

mickog commented 3 years ago

Thanks for providing this information so quickly. I think we will wait to hear from your colleague regarding the IPs being available via the API as our design will really hinge on that, there is no other way for us to get notified on an IP being changed other than we would stop receiving our events?

mickog commented 3 years ago

https://microsoftgraph.uservoice.com/ seems to be not active anymore, also looking at this issue here https://github.com/microsoftgraph/microsoft-graph-docs-contrib/issues/7058 it looks like clientState is probably going to be removed, does this mean there will be something else taking its place?

baywet commented 3 years ago

The deactivation is concerning, I reached out internally to the owners of the website to know more about it.

I don't thing the conclusion of that issue is the client state is going to be removed. As you can see in this PR, the client state value is simply not returned when listing subscriptions or getting the subscription from another app. App B can know that app A has made a subscription to entity X. But app B shouldn't know what the client state is as it'd be able to send notifications to app A and app A would believe it's Microsoft Graph.

baywet commented 3 years ago

The platform has been deactivated due to internal decisions. The best thing to do for suggestions at the moment is to post on the dedicated Q&A topic

mickog commented 3 years ago

@baywet thanks for that, as far as we can see update subscription only allows us to update the expiry time, if we wanted to rotate the client state property rather than reuse the same one for long periods of time does that mean we need to delete and recreate our subscriptions rather than update them or is there a way to do this? Thanks

baywet commented 3 years ago

I'm not 100% sure but I believe you should be able to update the client state property to rotate it as well. This was a while ago now though and I can't remember for sure. Maybe @Jumaodhiss can confirm.

Jumaodhiss commented 3 years ago

@baywet thanks for that, as far as we can see update subscription only allows us to update the expiry time, if we wanted to rotate the client state property rather than reuse the same one for long periods of time does that mean we need to delete and recreate our subscriptions rather than update them or is there a way to do this?

@mickog you can not update the expiry date together with the clientState property. If you want to rotate the clientState, you need to delete the existing subscription and create a new subscription.

Jumaodhiss commented 3 years ago

We are also working on delivering change notification using user defined content types.

e.g: When creating a subscription you can indicate a notificationContentType property. This will support several scenarios including

  1. application/jwt for jwt formats
  2. application/json - the default format
  3. application/xml etc.

However this will be dependent on the resource eg

Below is a sample subscription { "changeType": "revoked", "notificationUrl": "webhook.azurewebsites.net/send/myNotifyClient", "resource": "users",
"expirationDateTime":"2021-01-01T18:23:45.9356913Z", "clientState": "secretClientValue", "notificationContentType": "application/jwt", }

mickog commented 3 years ago

OK thanks for that, any idea of when that new functionality will be available?

Also just wondering in the list of IP addresses listed for change notification https://docs.microsoft.com/en-us/microsoft-365/enterprise/additional-office365-ip-addresses-and-urls?view=o365-worldwide Do they all fall under the same domain, I see the first in the list is *.cloudapp.net I am wondering if we whitelisted this would all the IP addresses fall under that category?

Jumaodhiss commented 3 years ago

@mickog we are already testing this feature with Identity Instant Revocation. I am not 100% sure on when we will roll out the feature in other workloads eg Exchange in you case but I will confirm the estimates today and share here.

For the IPs, they are just clustered based on *.cloudapp.net, Public Cloud:, Microsoft Cloud for US Government:, Microsoft Cloud Germany: etc. Let me confirm if the domain is the same for all of them.

mickog commented 3 years ago

So we should be able to just deny all IP addresses except ones that are part of the *.microsoft.com domain and that would add a good layer of security for us?

Jumaodhiss commented 3 years ago

@mickog we are already testing this feature with Identity Instant Revocation. I am not 100% sure on when we will roll out the feature in other workloads eg Exchange in you case but I will confirm the estimates today and share here.

For the IPs, they are just clustered based on *.cloudapp.net, Public Cloud:, Microsoft Cloud for US Government:, Microsoft Cloud Germany: etc. Let me confirm if the domain is the same for all of them.

The domain varies based on a few checks Its safer to whitelist the IP list instead of denying anything outside *.microsoft.com

Jumaodhiss commented 3 years ago

So we should be able to just deny all IP addresses except ones that are part of the *.microsoft.com domain and that would add a good layer of security for us?

Yes that's a good security layer but Its better to whitelist the provided IP list and block any other traffic.

mickog commented 3 years ago

Yea thats what we were originally thinking but there is the fear that the IP addresses will change and we will stop receiving notifications? Or do you know is there a way to dynamically retrieve the IP addresses incase of change? Thanks

Jumaodhiss commented 3 years ago

I don't think there is a way to dynamically fetch the IPs at the moment.

Jumaodhiss commented 3 years ago

You can also explore getting the notifications delivered through Azure event hubs. That way you dont need a publicly available notificationUrl. follow https://docs.microsoft.com/en-us/graph/change-notifications-delivery for more info

mickog commented 3 years ago

Thanks that looks interesting, the link mentions the feature is currently in preview, does this mean it is available or might become unavailable in the future?

Jumaodhiss commented 3 years ago

Thanks that looks interesting, the link mentions the feature is currently in preview, does this mean it is available or might become unavailable in the future?

Thanks @mickog. This feature is available right now. We are in the process of announcing it for General Availability this Quarter.

mickog commented 3 years ago

@Jumaodhiss Just to confirm did we say that all the domains would be part of *.microsoft.com, you mentioned the domain varies based on a few checks, is there a list of domains we could use or would we be safe with that? Thanks

Jumaodhiss commented 3 years ago

@mickog. Let me find this out about the domains and let you know. However, its safe to only allow the enlisted IPs.

mickog commented 3 years ago

Yea we are just afraid of having any issues if the IP address changes, that would be great if we could find out about the domains, thanks for the quick response!

mickog commented 3 years ago

Actually just realising on further investigation, it is unlikely we will be able to restrict to domain microsoft.com, we would need to use CORS for that which would just restrict traffic coming from browsers.. What we noticed when looking at our last few requests were coming from the IP addresses within the public cloud section of the IP list provided above, but there is no way for us to get notified if they change right?

nikithauc commented 3 years ago

Closing this due to inactivity