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
Allow storing thread credentials in phone keychain #20743
Allow storing thread credentials in phone keychain #20743
Conversation
type: "thread/store_in_platform_keychain", | ||
payload: { | ||
mac_extended_address: | ||
this._params!.network.routers![0]!.extended_pan_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the extended_pan_id the mac extended address?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, will check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I am looking at this, I remember that @agners said in the past we would need to start saving the mac extended address (or was something else) for the OTBR, perhaps we are not saving yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t forget there might be a difference in the credentials with standard iOS and developer iOS builds. This was an issue @agners saw that he had to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What difference? If you mean the credentials that are already in iOS, they are the same for any build because it all comes from Apple Keychain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the extended_pan_id the mac extended address?
In my case they were not the same for OTBR FYI. It does look like the router might have extended_address
available in the router object.
Line 7 in 505d7b6
extended_address: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, extended (MAC) address and extended PAN ID is not the same thing! So this line is wrong.
The extended address was always part of the OTBR object, so it should be available.
However, there was a change in how we reference the preferred border router: Unfortunately, Apple border routers do not use/have the Thread Border Agent ID, so this meant an Apple border router could not have been defined as a perferred border router. This changed with home-assistant/core#109065 in the backend, and #19580 in the frontend. So all good there.
this._params!.network.routers![0]!.extended_address, | ||
border_agent_id: this._params!.network.routers![0]!.border_agent_id, | ||
border_agent_id: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? I think @bgoncal said iOS uses the extended mac address as the border_agent_id. My understanding is that the iOS app is only looking for mac_extended_address
and active_operational_dataset
currently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But android will use the border agent id most probably
} | ||
|
||
private _sendCredentials() { | ||
this.hass.auth.external!.fireMessage({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a confirmation (alert, toast notification) when it's done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iOS currently has a circle checkmark that appears in the middle of the screen when it completes.
Proposed change
Type of change
Example configuration
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: