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

Bug: app auth cache keys missing app_id #95

Merged
merged 4 commits into from
Mar 20, 2024
Merged

Conversation

zegl
Copy link
Contributor

@zegl zegl commented Mar 19, 2024

This makes the cache multi-app aware, and so that it can handle JWT tokens from multiple apps

This makes the cache multi-app aware, and so that it can handle JWT tokens from multiple apps
Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 35.21%. Comparing base (57ead1b) to head (55c22ba).

Files Patch % Lines
githubkit/auth/app.py 22.22% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
- Coverage   35.21%   35.21%   -0.01%     
==========================================
  Files        2430     2430              
  Lines      124901   124905       +4     
==========================================
+ Hits        43986    43987       +1     
- Misses      80915    80918       +3     
Flag Coverage Δ
unittests 35.21% <22.22%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yanyongyu yanyongyu added the enhancement New feature or request label Mar 19, 2024
Copy link
Owner

@yanyongyu yanyongyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the installation cache key also missing the app id.

githubkit/auth/app.py Outdated Show resolved Hide resolved
githubkit/auth/app.py Outdated Show resolved Hide resolved
githubkit/auth/app.py Outdated Show resolved Hide resolved
@yanyongyu yanyongyu added bug Something isn't working and removed enhancement New feature or request labels Mar 19, 2024
@zegl
Copy link
Contributor Author

zegl commented Mar 20, 2024

It seems the installation cache key also missing the app id.

Yeah, but I don't think it's as critical, as different installation IDs imply different app IDs.

@yanyongyu yanyongyu changed the title ✨ Add app_id to JWT app cache keys Bug: app auth cache keys missing app_id Mar 20, 2024
@yanyongyu yanyongyu merged commit e577197 into yanyongyu:master Mar 20, 2024
32 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants