Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support Github App Refresh tokens #162

Closed
2 tasks done
trent-codecov opened this issue Aug 2, 2023 · 1 comment
Closed
2 tasks done

Support Github App Refresh tokens #162

trent-codecov opened this issue Aug 2, 2023 · 1 comment
Assignees
@giovanni-guidini
Labels
P1: will do priority 7-9
Milestone
Q3'23

Comments

@trent-codecov
Copy link
Contributor

trent-codecov commented Aug 2, 2023 •
edited by giovanni-guidini
Loading

KPIs

  • Github refresh tokens used to refresh auth
  • Refresh in use by 1 organization

Tasks
Beta Give feedback
  1. Implement refresh token handling for Github #163
    enhancement
    giovanni-guidini
  2. Update codecov-api and worker to handle Github refresh tokens #419
    enhancement
    giovanni-guidini
@trent-codecov trent-codecov added this to the Q3'23 milestone Aug 2, 2023
@trent-codecov trent-codecov added the P1: will do priority 7-9 label Aug 2, 2023
@giovanni-guidini giovanni-guidini mentioned this issue Aug 17, 2023
Merged
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Aug 17, 2023
@giovanni-guidini
feat: support gh refresh tokens
ab0df81 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Aug 17, 2023
@giovanni-guidini
feat: support gh refresh tokens
1e5ad03 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
@giovanni-guidini giovanni-guidini mentioned this issue Aug 17, 2023
Merged
giovanni-guidini added a commit to codecov/worker that referenced this issue Aug 17, 2023
@giovanni-guidini
feat: support gh refresh tokens
069f06d 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
giovanni-guidini added a commit to codecov/worker that referenced this issue Aug 17, 2023
@giovanni-guidini
feat: support gh refresh tokens
93ea61a 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
@giovanni-guidini
Copy link

An implementation similar to what we do to support GitLab refresh tokens has been completed in #163 . In that process many opinions and possible issues were surfaced that need attention to.

I hope we can use this ticket as a planning board and construct a plan to release Github App Refresh tokens with minimal (hopefully zero) performance and user experience impact.

With GitLab we had the "benefit" that it was broken, so anything was better than nothing. This is definitely not the case here. Also there are way more customers in Github. So we need to be careful.

I suppose we can rely heavily in staging to test the changes and collect data on the solution proposed until we are confident to release in prod.

giovanni-guidini added a commit to codecov/shared that referenced this issue Aug 28, 2023
@giovanni-guidini
chore: update gh refresh heuristic
bbdf61a 
Yes I know, this should have been part of codecov.shared#27

I decided to come back and restrict the requests we will try to refresh a little.
By checking the callable first we avoid app-to-server requests, which should
represent a considerable portion of requests to github, in fact.

So here's a rough list of requests that wi will NOT try to refresh:
* check if user is a student
* any request made by the codecov github app
* any request made by a repo bot
* requests for repo information made in `sync_repos` or `sync_teams`

hopefully by restricting the attempts at refreshing info we avoid unecessary reqeusts to gh.

context codecov/engineering-team#162
@giovanni-guidini giovanni-guidini mentioned this issue Aug 28, 2023
Merged
giovanni-guidini added a commit to codecov/worker that referenced this issue Aug 28, 2023
@giovanni-guidini
fix: add on_token_refresh in owner.py
2fc4026 
* Updates shared to a newer version
* Adds a on_token_refresh callback if the torngit adapter comes from owner.py

context codecov/engineering-team#162
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Aug 28, 2023
@giovanni-guidini
fix: only include token refresh callback if token is from owner
9ddc661 
* update shared
* only include on_token_refresh if the token comes from the owner (definetely from the owner)

context codecov/engineering-team#162
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Aug 28, 2023
@giovanni-guidini
fix: only include token refresh callback if token is from owner
9a725b4 
* update shared
* only include on_token_refresh if the token comes from the owner (definetely from the owner)

context codecov/engineering-team#162
giovanni-guidini added a commit to codecov/shared that referenced this issue Aug 29, 2023
@giovanni-guidini
chore: update gh refresh heuristic (#28)
9637626 
Yes I know, this should have been part of codecov.shared#27

I decided to come back and restrict the requests we will try to refresh a little.
By checking the callable first we avoid app-to-server requests, which should
represent a considerable portion of requests to github, in fact.

So here's a rough list of requests that wi will NOT try to refresh:
* check if user is a student
* any request made by the codecov github app
* any request made by a repo bot
* requests for repo information made in `sync_repos` or `sync_teams`

hopefully by restricting the attempts at refreshing info we avoid unecessary reqeusts to gh.

context codecov/engineering-team#162
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Aug 30, 2023
@giovanni-guidini
feat: support gh refresh tokens
38f7e78 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Aug 30, 2023
@giovanni-guidini
fix: only include token refresh callback if token is from owner
7c60f91 
* update shared
* only include on_token_refresh if the token comes from the owner (definetely from the owner)

context codecov/engineering-team#162
giovanni-guidini added a commit to codecov/worker that referenced this issue Aug 30, 2023
@giovanni-guidini
feat: support gh refresh tokens
221ccd5 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
giovanni-guidini added a commit to codecov/worker that referenced this issue Aug 30, 2023
@giovanni-guidini
fix: add on_token_refresh in owner.py
38b864e 
* Updates shared to a newer version
* Adds a on_token_refresh callback if the torngit adapter comes from owner.py

context codecov/engineering-team#162
giovanni-guidini added a commit to codecov/worker that referenced this issue Sep 5, 2023
@giovanni-guidini
feat: support gh refresh tokens
b651dbf 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
giovanni-guidini added a commit to codecov/worker that referenced this issue Sep 5, 2023
@giovanni-guidini
fix: add on_token_refresh in owner.py
f22e5f5 
* Updates shared to a newer version
* Adds a on_token_refresh callback if the torngit adapter comes from owner.py

context codecov/engineering-team#162
giovanni-guidini added a commit to codecov/worker that referenced this issue Sep 5, 2023
@giovanni-guidini
feat: support gh refresh tokens (#69)
a4b846e 
Depends on codecov/shared#27

Adds support for github app refresh tokens

context: codecov/engineering-team#162
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Sep 6, 2023
@giovanni-guidini
feat: support gh refresh tokens
1f1a183 
Depends on codecov/shared#27
TODO: after that is merged update the sha reference in requirements.in

Adds support for github app refresh tokens

codecov/engineering-team#162
giovanni-guidini added a commit to codecov/codecov-api that referenced this issue Sep 6, 2023
@giovanni-guidini
fix: only include token refresh callback if token is from owner
0bf3c0a 
* update shared
* only include on_token_refresh if the token comes from the owner (definetely from the owner)

context codecov/engineering-team#162
@codecov-hooky codecov-hooky bot closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@giovanni-guidini giovanni-guidini

Labels
P1: will do priority 7-9
Projects
None yet
Milestone
Q3'23
Development

No branches or pull requests
2 participants
@trent-codecov @giovanni-guidini