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

account for overrides when generating new token #421

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ryan-s
Copy link
Contributor

@ryan-s ryan-s commented Sep 7, 2023

If the user has overridden the OAuth model, the automatic token refresh will fail if the user has defined columns that are not nullable. The common use case for this seems to be the instance where the author has associations with multiple providers. The commit will fail because the provider_user_id and the provider_username are not carried over to the new token.

The proposed solution will end up added an additional query to get the existing entry and copying over any fields that are not nullable into the new token.

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #421 (de5a6a5) into main (27add75) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #421      +/-   ##
==========================================
+ Coverage   96.35%   96.37%   +0.02%     
==========================================
  Files          37       37              
  Lines        1070     1077       +7     
==========================================
+ Hits         1031     1038       +7     
  Misses         39       39              
Files Changed Coverage Δ
flask_dance/consumer/storage/sqla.py 92.30% <100.00%> (+0.48%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ryan-s
Copy link
Contributor Author

ryan-s commented Sep 12, 2023

Looks like this is working now, however I was not able to get the tests to pass locally without hard coding in adding the test state into oauth2.py. Seems like theres some kind of issue in Flask where its not preserving the session that gets created during the test.

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

Successfully merging this pull request may close these issues.

1 participant