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.

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

Installation tokens are hard to re-use #435

Open
marcusirgens opened this issue Aug 2, 2023 · 4 comments
Open

Installation tokens are hard to re-use #435

marcusirgens opened this issue Aug 2, 2023 · 4 comments

Comments

@marcusirgens
Copy link
Contributor

marcusirgens commented Aug 2, 2023

It is hard to re-use installation tokens, because (as far as I can tell,) it is not possible to mutate the auth state of the client, nor possible to extract the active installation token. Additionally, the installation auth state does not cache expiration times, installation ID or other data required to renew itself, so it times out after a while.

Would you be interested in a PR that changes any of these issues? My suggestions would be:

  • Make it possible to retrieve the InstallationToken from the client, if present (for user-managed caching).
  • Make it possible to set an InstallationToken on the client if the auth state is App (from user-managed caches).
  • Change the data structure for AuthState's installation variant to hold the expiration time and installation ID, and re-fetch an installation token if it is about to expire (for octocrab-managed caching).
@XAMPPRocky
Copy link
Owner

XAMPPRocky commented Aug 2, 2023

Change the data structure for AuthState's installation variant to hold the expiration time and installation ID, and re-fetch an installation token if it is about to expire (for octocrab-managed caching).

Thank you for your issue! I think I would lean towards this, because ideally with a high-level client, this would be an implementation detail you shouldn't need to worry about.

@marcusirgens
Copy link
Contributor Author

I think I would lean towards this, because ideally with a high-level client, this would be an implementation detail you shouldn't need to worry about.

I agree, it would essentially behave like Go's OAuth2 client middleware in that case. It should be orthagonal with the two other points, ie., all three should be possible to do at the same time.

@XAMPPRocky
Copy link
Owner

all three should be possible to do at the same time.

Sounds good to me 🙂

@flying-sheep
Copy link
Contributor

flying-sheep commented Mar 28, 2024

/edit ignore, this was about my clock not being set correctly and time drift >1min having pushed the expiration date too far into the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants