domainaware / parsedmarc

A Python package and CLI for parsing aggregate and forensic DMARC reports
https://domainaware.github.io/parsedmarc/
Apache License 2.0
986 stars 214 forks source link

Implement Device Code and Client Secret auth flow for MS Graph #320

Closed nathanthorpe closed 2 years ago

nathanthorpe commented 2 years ago

The auth_method and tenant_id properties are added to the msgraph config section. Defaults to UsernamePassword

Device Code

This authenticates as a user and supports accounts that have MFA enabled as it prompts the user to open up a browser.

Example: To sign in, use a web browser to open the page https://microsoft.com/devicelogin and enter the code <CODE> to authenticate.

ClientCredential

This authenticates as the app (service principal). Note that this needs application permissions (not delegated). This is probably the best way to authenticate, if you restrict mailbox access.

Other edits

codecov[bot] commented 2 years ago

Codecov Report

Merging #320 (d67262b) into master (4018e82) will decrease coverage by 0.38%. The diff coverage is 36.36%.

@@            Coverage Diff             @@
##           master     #320      +/-   ##
==========================================
- Coverage   60.79%   60.40%   -0.39%     
==========================================
  Files           8        8              
  Lines        1260     1278      +18     
==========================================
+ Hits          766      772       +6     
- Misses        494      506      +12     
Impacted Files Coverage Δ
parsedmarc/mail/graph.py 25.71% <36.36%> (+1.57%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4018e82...d67262b. Read the comment docs.

seanthegeek commented 2 years ago

Funny, I actually had a branch that implemented this last weekend, but I deleted it, because it turns out using client/secret auth requires system API access (instead of being delegated to a user account), which would grant read/write of all mailboxes in an org, which is way to broad of a scope.

Instead, I'm thinking we could do browser-based auth? Have the user copy/baste the URL from SSH into a browser to do the SSO MFA login for that account.

https://github.com/Azure/azure-sdk-for-python/blob/main/sdk/identity/azure-identity/azure/identity/_credentials/browser.py

I just haven't had time to look into it more.

nathanthorpe commented 2 years ago

Yeah that's what I thought too and why I didn't implement it initially, but turns out you can limit access using the New-ApplicationAccessPolicy cmdlet in the Exchange module.

The Device Code flow implements the browser based auth, which is a little better because the Interactive Browser credential requires a redirect URI to be provided (requiring something to listen on a port)

seanthegeek commented 2 years ago

Interesting! Still, I don't trust every user to follow best practices. 😅

I wonder if we can verify the restricted scope, and refuse to use the creds until the scope in narrowed?

nathanthorpe commented 2 years ago

Yeah I'm guessing there may be something we can do, like listing mailboxes and check if it is different from the provided mailbox. I will do some testing

seanthegeek commented 2 years ago

Thanks for putting so much work into this project! It's so nice not to be my own while issues and requests pile up.

nathanthorpe commented 2 years ago

Hey thanks for sharing this awesome project! Happy to help.

nathanthorpe commented 2 years ago

Hmm... I'm not finding a good way to do this, if you wanted to list mailboxes you'd have to grant the application even more permissions such as the User.Read.All scope.

I did add a message in the readme about properly setting this up. Maybe that would be sufficient?

seanthegeek commented 2 years ago

Oof. In that case, I'll merge these changes, and then add more details to the warnings.