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

[mybmw] Fix hcaptchatoken issue (#17862) #17896

Merged
merged 6 commits into from
Dec 15, 2024

Conversation

martingrassl
Copy link
Contributor

this PR fixes the issue with the new authentication mechanism in the myBMW API thanks to the great work of bimmer_connected.

Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@lsiepel
Copy link
Contributor

lsiepel commented Dec 13, 2024

I'm sorry @martingrassl the code freeze was today, not sunday, my bad. So this will not be available in 4.3.x until the first patch release. (when merged)

<parameter name="hcaptchatoken" type="text" required="true">
<label>Captcha-Token</label>
<description>Captcha-Token for login (see https://bimmer-connected.readthedocs.io/en/stable/captcha.html)</description>
<context>hcaptchatoken</context>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a known context? This is used in the UI, but I don’t know about this value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Captcha token was introduced recently by BMW and now the binding is not working anymore. It was the easiest solution to add the param into the bridge config

Copy link
Contributor

@mherwege mherwege Dec 14, 2024

Choose a reason for hiding this comment

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

Yes, but I think the context field is not required and does not do anything. The valueis not a known value for OH.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I just copied it from the password configuration

Copy link
Contributor

@mherwege mherwege Dec 14, 2024

Choose a reason for hiding this comment

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

Indeed, but password is a known context. When the parameter has a password context, the field content will be hidden by default. If you want this token to be hidden, you can equally set it to context password, but I don’t think that’s needed here.

@@ -14,6 +14,11 @@
<description>MyBMW Password</description>
<context>password</context>
</parameter>
<parameter name="hcaptchatoken" type="text" required="true">
<label>Captcha-Token</label>
<description>Captcha-Token for login (see https://bimmer-connected.readthedocs.io/en/stable/captcha.html)</description>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make it more explicit you need to get the token through an external service.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to add a documentation. Was just in a hurry because of the code freeze

Copy link
Contributor

@mherwege mherwege left a comment

Choose a reason for hiding this comment

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

I didn’t test this, but I am OK with the code changes.

…nding/mybmw/internal/console/MyBMWCommandExtension.java

Co-authored-by: Mark Herwege <mherwege@users.noreply.github.com>
Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
@martingrassl
Copy link
Contributor Author

I didn’t test this, but I am OK with the code changes.

Thanks a lot! It is running in my local for one day without issue. I'm just wondering if I have to increment the thing versions although no channel was changed...

@lsiepel
Copy link
Contributor

lsiepel commented Dec 14, 2024

I didn’t test this, but I am OK with the code changes.

Thanks a lot! It is running in my local for one day without issue. I'm just wondering if I have to increment the thing versions although no channel was changed...

Why do you think it should be increased? Without update instructions/changed channels there is no point.

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
@holgerfriedrich holgerfriedrich added rebuild Triggers Jenkins PR build and removed rebuild Triggers Jenkins PR build labels Dec 14, 2024
@holgerfriedrich
Copy link
Member

@martingrassl FYI we are considering to possibly include this in 4.3.
Can you confirm that the binding is anyway broken for all users, or are only new registrations broken by now?
How do you asses the risk of breaking functionality with the PR?

@martingrassl
Copy link
Contributor Author

@martingrassl FYI we are considering to possibly include this in 4.3. Can you confirm that the binding is anyway broken for all users, or are only new registrations broken by now? How do you asses the risk of breaking functionality with the PR?

@holgerfriedrich wow that would really be great. This PR can't break anything as anyhow the binding is not working right now. The users are desperately waiting for a fix and alternatively I would have to provide the binding by a jar file... thank you very much!

@kaikreuzer kaikreuzer changed the title [mybmw] fix hcaptchatoken issue (#17862) [mybmw] Fix hcaptchatoken issue (#17862) Dec 15, 2024
@kaikreuzer
Copy link
Member

Thanks @martingrassl, so let's merge this as a last minute fix, so that the binding is working in the 4.3 release!

@kaikreuzer kaikreuzer merged commit fe624dd into openhab:main Dec 15, 2024
5 checks passed
@kaikreuzer kaikreuzer added the bug An unexpected problem or unintended behavior of an add-on label Dec 15, 2024
@kaikreuzer kaikreuzer added this to the 4.3 milestone Dec 15, 2024
@jlaur jlaur linked an issue Dec 15, 2024 that may be closed by this pull request
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Dec 16, 2024
* [mybmw] add stop charging command
* [mybmw] fix hcaptcha issue (openhab#17862)

Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
cipianpascu pushed a commit to cipianpascu/openhab-addons that referenced this pull request Jan 2, 2025
* [mybmw] add stop charging command
* [mybmw] fix hcaptcha issue (openhab#17862)

Signed-off-by: Martin Grassl <martin.grassl@digital-filestore.de>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mybmw] API not working anymore because of Captcha request
5 participants