fix: Check for visibility of emails - GitHub #243
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #243 +/- ##
==========================================
- Coverage 89.53% 89.48% -0.06%
==========================================
Files 326 324 -2
Lines 10437 10373 -64
Branches 1908 1903 -5
==========================================
- Hits 9345 9282 -63
+ Misses 1023 1020 -3
- Partials 69 71 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
RulaKhaled
changed the title
RulaKhaled
changed the title
| @@ -205,7 +205,7 @@ async def test_get_authenticated_user_no_refresh_token(self, codecov_vcr): | |||
| "company": "@codecov ", | |||
| "blog": "", | |||
| "location": None, | |||
| "email": "[email protected]", | |||
| "email": None, | |||
There was a problem hiding this comment.
Mmm is there a test where we actually get the email and verify it's public?
There was a problem hiding this comment.
I was also thinkin, how can we be certain we're still fetching the primary public email?
Maybe we could check if the primary is public and return, otherwise find any public?
There was a problem hiding this comment.
how can we be certain we're still fetching the primary public email
@ajay-sentry that's what we're doing! email in /user defaults to the fist public primary email (it's in the docs), if it doesn't exist we fetch the first public email in /emails. That's assuring us we're respecting the "visibility" of emails. (resources in the desc)
@adrian-codecov tbh i'm not sure, this fetches https://github.com/login/oauth/authorize?response_type=code&scope=user%3Aemail%2Cread%3Aorg%2Crepo%3Astatus%2Cwrite%3Arepo_hook&client_id=999247146557c3ba045c and tests it's values, i'm not sure how i can alter values in such case, any thoughts?
There was a problem hiding this comment.
Are you familiar w/ cassettes?
The idea is that you make a real API call once, that creates a cassette, and then your test calls that cassette instead of calling the actual API. So you'd want to make a brand new test to an org you have a token of /access to. You'll need to use one the *_handlers , and replace the token with a real token/owner/repo combo for the test, and have that create a cassette. Does that make sense? Lmk and I can give you a hand otherwise
There was a problem hiding this comment.
Makes sense! I'll look this up and i'll let you know if i'm stuck 🏃🏻♀️
There was a problem hiding this comment.
There was a problem hiding this comment.
It looks like we need to create a new github app to verify this
| handler = Github( | ||
| oauth_consumer_token=dict( | ||
| key="Iv23liSqj8DAO20A3KLA", | ||
| secret="a6a6397fffea369e54495c88ca469d988ea4ccd2", |
There was a problem hiding this comment.
Quick thing though, are these the legitimate keys/secrets? If so, now that you have the cassette, you can get rid of these/fake them so that no one knows about the secrets themselves. If they are, and you wanna go the extra mile, try rebasing the commit where these were introduced so there's no track of these committed in gh
We fetch user's email in
/userwhich according to GitHub is "The first primary publicly visible email" see here, if that does not exist we should be fetching the first visible email, instead of ignoring the visibility of the primary email.resources: https://docs.github.com/en/rest/users/emails?apiVersion=2022-11-28#list-email-addresses-for-the-authenticated-user
Fixes: codecov/feedback#391
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.