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

Mercedes: Fix Refresh token handling #18880

Closed

Conversation

ReneNulschDE
Copy link
Contributor

@ReneNulschDE ReneNulschDE commented Feb 16, 2025

With the latest API changes made by Mercedes, the refresh token is stable and is not changed anymore with every token refresh. (Not sure if this is a config bug on MB side as this will reduce the security level) - this PR will remove all the code that handled the rolling refresh token logic.

Now, the refresh token out of the config is the only source of true.

Setting store usage is removed. No need to delete of tokens in the db anymore...

User account is not needed anymore, for each car a separate refresh token is needed now. --> breaking change...

The new code was running in my environment for multiple hours and multiple restarts...

@andig: May I ask you to check it with your knowledge of evcc. Thx. I planned to remove the not needed access token in the config, but not sure why it is still visible in the frontend. (We need the refresh token only, as we request a new access token on restart.)

fixes: #18863

@ReneNulschDE ReneNulschDE marked this pull request as draft February 16, 2025 20:34
@ReneNulschDE
Copy link
Contributor Author

I keep it as draft, as first would like the feedback from andig...

@andig
Copy link
Member

andig commented Feb 16, 2025

I don‘t understand why the code wouldn‘t work without this change.

@ReneNulschDE
Copy link
Contributor Author

ReneNulschDE commented Feb 16, 2025

the old code expected that a new refresh token is delivered by the api on token expiry. this is not the case anymore. The new answer is just an access token...

@ReneNulschDE
Copy link
Contributor Author

I could simplify the PR by just adding the old RT to the result set, but this would lead to 60 lines unused code and the "unneeded" usage of the settings store. (with all the "token deletion" questions)

@ReneNulschDE ReneNulschDE marked this pull request as ready for review February 17, 2025 06:59
@andig
Copy link
Member

andig commented Feb 17, 2025

In den anderen Integrationen nehmen wir den neuen Refreshtoken einfach für den Fall, dass es einen gibt. Sonst bleibt der alte erhalten. Beide müssten gespeichert werden.

@ReneNulschDE ReneNulschDE marked this pull request as draft February 17, 2025 07:59
@ReneNulschDE ReneNulschDE deleted the fix-new-token-handling branch February 17, 2025 08:04
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.

Mercedes - Refresh token is getting lost after some hours
2 participants