Send github oauth access token expiry with cookie #237
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #237 +/- ##
=====================================
Coverage 95.58 95.58
=====================================
Files 719 719
Lines 15881 15887 +6
=====================================
+ Hits 15179 15185 +6
Misses 702 702
Flags with carried forward coverage won't be shown. Click here to find out more.
|
…github.com/codecov/codecov-api into rvinnakota/send-github-access-token-expiry
rohitvinnakota-codecov
force-pushed
the
rvinnakota/send-github-access-token-expiry
branch
from
3b7f19a
to
f193a9d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #237 +/- ##
=======================================
Coverage 95.67% 95.67%
=======================================
Files 605 605
Lines 15475 15482 +7
=======================================
+ Hits 14806 14813 +7
Misses 669 669
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
barnett-yuxiang
approved these changes
Nov 9, 2023
JerrySentry
approved these changes
Nov 9, 2023
rohitvinnakota-codecov
deleted the
rvinnakota/send-github-access-token-expiry
branch
trent-codecov
added a commit
that referenced
this pull request
Nov 14, 2023
* Test ats run (#229) * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * Test ats run * feat: Emit sync repo message to Shelter on repo update (#215) * feat: add prometheus instrumentation to requests, db accesses, and models (#202) * feat: add prometheus instrumentation to requests, db accesses, and models * switch to multiprocessing mode * add nullchecking for gunicorn worker/pid * only call multiprocess.mark_process_dead if prometheus multiprocess is enabled * add comment about public prometheus route --------- Co-authored-by: Trent Schmidt <trent@codecov.io> * fix: make GraphQL types matches nullability with django models (#220) Audit all the GraphQL API types to make sure that the nullabilitity (nullable or not nullable) matches that of the database columns (via Django ORM). * feat: Add index on pulls (repoid, id) (#217) * feat: Add index on pulls (repoid, id) * Fix migration conflict * Fix XSS vulnerability by removing ref_type from Response (#211) * fix: available plans for users while trialing (#731) (#230) Fix a bug where a user while trialing does not have Team plans available to upgrade to. This happens because while trialing we set plan_user_count to 1000 while trialing, so we will check plan_activated_users while user is trialing. * fix: Skip login if GitHub OAuth does not return access_token (#236) * Send github oauth access token expiry with cookie (#237) * initial commit * remove method * lint * sort imports * reintroduce variable * Update small badge (#238) * chore: Moves the small badge mask over to leave space for 100% while still looking good with 2 digits * fix small badge expected svg util * fix small badge tests * fix: Upgrade Django version to fix security vulnerabilities (#239) Fixes Django security vulnerabilities by updating to the fix version: Django Denial-of-service in django.utils.text.Truncator Django potential denial of service vulnerability in UsernameField on Windows Django Denial of service vulnerability in django.utils.encoding.uri_to_iri * feat: Modify billing for Team plan (#637) (#225) - When signing up to a new paid plan, it will have Team plan choice, and checks for plan user limit. - Also update limit from 1000 to 2500 and refactor a variable name - Add isPublic option to measurements filter - Change marketing name of Pro Team to Pro * fix: Handle Shelter storage paths in upload download handler (#234) * fix: Handle shetler storage paths in upload download handler * fix: Handle shetler storage paths in upload download handler * Add test for shelter storage path * Updating workflows (#242) * Fix urllib and opentelemetry-instrumentation dependencies (#213) * Update urllib3 dependency * Fix opentelementry-instrumentation dependency Fixes: codecov/internal-issues#103 Fixes: codecov/internal-issues#104 Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io> * Make migration info log less confusing (#221) Clearly state whether migrations failed or succeded. Co-authored-by: Trent Schmidt <trent@codecov.io> * Use alpine 3.18 --------- Signed-off-by: joseph-sentry <joseph.sawaya@sentry.io> Co-authored-by: scott-codecov <scott@codecov.io> Co-authored-by: matt-codecov <137832199+matt-codecov@users.noreply.github.com> Co-authored-by: JerrySentry <142266253+JerrySentry@users.noreply.github.com> Co-authored-by: joseph-sentry <136376984+joseph-sentry@users.noreply.github.com> Co-authored-by: Rohit Vinnakota <148245014+rohitvinnakota-codecov@users.noreply.github.com> Co-authored-by: Terry <87824812+terry-codecov@users.noreply.github.com> Co-authored-by: Andreas Bergmeier <51448674+AndreasBergmeier6176@users.noreply.github.com>
scott-codecov
added a commit
that referenced
this pull request
Nov 17, 2023
* main: feat: Emit pubsub event when org token is updated (#241) perf: use new get_file_totals() report method for BranchContents (#252) feat: adjust benefits for team plan (#253) fix: Adjust Team plan pricing (#805) (#243) feat: Add Sentry user to admin panel (#702) (#232) metrics: update shared + add sentry traces to BranchContents GraphQL query path (#251) Use the same sentry version (#247) feat(logging): change access logging format to JSON (#249) Use alpine 3.18 (#245) fix(launchscripts): delete contents of prometheus multiproc dir, not dir itself (#244) Make migration info log less confusing (#221) Fix urllib and opentelemetry-instrumentation dependencies (#213) Updating workflows (#242) fix: Handle Shelter storage paths in upload download handler (#234) feat: Modify billing for Team plan (#637) (#225) fix: Upgrade Django version to fix security vulnerabilities (#239) Update small badge (#238) Send github oauth access token expiry with cookie (#237) fix: Skip login if GitHub OAuth does not return access_token (#236)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose/Motivation
Related to codecov/engineering-team#642, we need a way to see the github session expiry time on the client for the currently logged in user. The expiry time is on the
oauth access-tokenand currently has a hard limit of 8 hours.Links to relevant tickets
codecov/engineering-team#642
codecov/gazebo#2351
What does this PR do?
Notes to Reviewer
The 8 hour session expiry is hard coded instead of doing the technically more correct, fetch expiry time from the Github API response as it would involve changes to our refresh token generation in shared. Since we can always expect the limit to be 8, I think setting it post-login is fine.
That being said, I am adding the logic here instead of the client since
1.) I want to verify that the expiry is set once the user is logged in
2.) Setting session expiry on the server is more secure
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.