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

update Carelink EU with issue#18 #1

Closed
wants to merge 35 commits into from
Closed

update Carelink EU with issue#18 #1

wants to merge 35 commits into from

Conversation

bewest
Copy link
Member

@bewest bewest commented Jun 28, 2020

mddub/minimed-connect-to-nightscout#18
Outlines some major changes in EU carelink as of 28.06.2020.

This patch updates the module with the information provided.

@bewest
Copy link
Member Author

bewest commented Jun 28, 2020

@benceszasz, I'm sorry to keep bouncing your around between issues. I can share ability to update the code with other collaborators in this area. There's a dev branch here, if you have ability to check out and test it, it would help a lot.

I've been running on commandline like:

$ MMCONNECT_SERVER=EU CARELINK_USERNAME=xxxx CARELINK_PASSWORD=yyyy node run.js 

But my actual user name and password have been redacted with xxxx and yyyy,
respectively.

I haven't been able to test this because I'm in the states and can't verify the login sequence with my account.

There are actually three URLs involved, the third URL comes after the initial login url but before the data service URL. Since the other two have changed, I suspect this one has as well. Once we can confirm working, I can issue a release based on this community edition repo.

@benceszasz
Copy link

@bewest There are really major changes, not just the URLs. Sorry if it was misleading, but I just wanted to highlight the areas of the changes. I am still working on to reproduce the proper minimal HTTP communication required for login + data fetch and I am open for additional help :)

I see clearly now only the end.

Request
Method: GET
URL: https://carelink.minimed.eu/patient/connect/data?cpSerialNumber=NONE&msgType=last24hours&requestTime=xxxxxx
Authorization: Bearer <token>

The <token> has the same value as the auth_temp_token cookie. The auth_temp_token cookie is not required for the data service request, but the Bearer Authorization with the correct token value is a must.

Response
Headers: ...
Content: sensor_data_json

Below a sample working HTTP request / response for the data service:

carelink_eu-data-request.txt
carelink_eu-data-response.txt

I have removed the personal information from the response. Also please note, that the SGV values are all 0, because I only have a currently not used GC CareLink account.

@bewest
Copy link
Member Author

bewest commented Jun 28, 2020

I made an account just for this. Here are several har archives.

Archive 20-06-28 14-37-35.txt
Archive 20-06-28 14-37-54.txt
Archive 20-06-28 14-39-31.txt
Archive 20-06-28 14-42-07.txt

@bewest
Copy link
Member Author

bewest commented Jun 29, 2020

I've got basic description of the new auth flow here https://gist.github.com/bewest/47ad913fdd571531604dc23942d73b94

These changes are substantial enough to put in a new file, mmsso.js or something similar.

@mrinnetmaki
Copy link
Contributor

When running this version I get following errors:

[ERR_TLS_CERT_ALTNAME_INVALID]: Hostname/IP does not match certificate's altnames: Host: carelink.minimed.eu. is not in the cert's altnames: DNS:mdtlogin.medtronic.com, DNS:mdtsts.medtronic.com

May be just something in my config, just letting you know.

@bewest
Copy link
Member Author

bewest commented Jun 29, 2020

Correct, this is just prep work to stub out space for a solution and get it released. It is open and unresolved currently.

The new flow to be implemented is described here: https://gist.github.com/bewest/47ad913fdd571531604dc23942d73b94

I've gotten past the TLS CERT error now and am working through initial set of 3 redirects to the login form. They appear to employ a variety of layers of session management to establish a valid session that will require a bit more interaction. Help welcome.

@FredMK
Copy link
Contributor

FredMK commented Jun 29, 2020

@bewest I just sent an email for you with a working implementation.

@bewest
Copy link
Member Author

bewest commented Jun 29, 2020

Very nice work, @FredMK!

Copy link
Contributor

@mrinnetmaki mrinnetmaki left a comment

Choose a reason for hiding this comment

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

Works for me.

@FredMK
Copy link
Contributor

FredMK commented Jun 29, 2020

Sadly not perfect yet. For this line, needs a check that the cookie exists or not, currently the NS crashes if the cookie not exists: Cookie handling.

@bewest May I ask you to code it? Maybe it would be better if the whole fetch would be in a try catch block, so all the errors can be handled without NS crash. There are a few places in my code where some error handling is required.

@bewest
Copy link
Member Author

bewest commented Jun 29, 2020

Agree, but it's a strong start. Adding to your list, I have some other nits as well:

TODO

  • error handling during cookie check
  • bring in lang and country options via runtime config
  • provide easy way to set server used via runtime config
  • refresh token when polling in a loop
  • eliminate regexp - my analysis indicates it's the same value from the URL parameters for sessionId and sessionData.
  • can it be configured with just the entry point configured? following the redirects instead?

@FredMK
Copy link
Contributor

FredMK commented Jun 30, 2020

I made some structural and error handling changes in my fork dev branch. The US version is not tested.

  • error handling during cookie check: partially done, error cannot occur.
  • bring in lang and country options via runtime config: not sure if it has any effect on the NS.
  • refresh token when polling in a loop: done in the fork above.
  • eliminate regexp: I think it's not possible. The session data is changing, and what we need for mmcl/auth/oauth/v2/authorize/consent only can be found in the body of the response as a html, the previous sessionData is different and I got error when I tried to use it.
  • can it be configured with just the entry point configured? following the redirects instead?: done, with more regex :)

@matti05
Copy link

matti05 commented Jun 30, 2020

Before @FredMK's last changes I had an error in Heroku log and no BG in Nightscout. Currently, works great. Thanks!

@raferg
Copy link

raferg commented Jun 30, 2020

I can confirm that the latest changes work also - data through to Nightscout, and that is then getting picked up by xdrip :)

awesome work so far!

@Grynn
Copy link

Grynn commented Jun 30, 2020

I get the following error in the logs:

MiniMed Connect error: Error [ERR_TLS_CERT_ALTNAME_INVALID]: Hostname/IP does not match certificate's altnames: Host: carelink.minimed.eu. is not in the cert's altnames: DNS:mdtlogin.medtronic.com, DNS:mdtsts.medtronic.com

@FredMK
Copy link
Contributor

FredMK commented Jun 30, 2020

@Grynn It seems that you are not using the latest version, because this problem exists in older versions. You need to set the package.json in cgm-remote-monitor with this dep: "minimed-connect-to-nightscout": "FredMK/minimed-connect-to-nightscout#dev",.
Update: after the package.json change, need to run npm install on the repo, to update npm-shrinkwrap.json.

@Grynn
Copy link

Grynn commented Jun 30, 2020

Hmm, I've tried that and still get the same error in the logs. What could I be doing something wrong?

  1. Code change:
    Grynn/cgm-remote-monitor@eb229a2#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R97

  2. Deployed
    image

  3. Logs still have error 😔
    heroku logs | grep -i minimed

2020-06-30T11:50:16.458066+00:00 app[web.1]: MiniMed Connect error: Error [ERR_TLS_CERT_ALTNAME_INVALID]: Hostname/IP does not match certificate's altnames: Host: carelink.minimed.eu. is not in the cert's altnames: DNS:mdtlogin.medtronic.com, DNS:mdtsts.medtronic.com

@FredMK
Copy link
Contributor

FredMK commented Jun 30, 2020

@Grynn I made some changes in the code, please check it again. For this test please use this version "minimed-connect-to-nightscout": "FredMK/minimed-connect-to-nightscout#dev",.
Where are you hosting your NS?
Update: after the package.json change, need to run npm install on the repo, to update npm-shrinkwrap.json.

@Grynn
Copy link

Grynn commented Jun 30, 2020

😔

Deployed: Grynn/cgm-remote-monitor@119137d
Same error.

Where are you hosting your NS?

Heroku EU
image

confirmed working for user users as well
@FredMK
Copy link
Contributor

FredMK commented Jun 30, 2020

@Grynn I just deployed the exact same code you deployed, and the problem is now the same for my NS... yeee :) Your NS repo is ahead of the master branch, trying to figure out the exact reason.

@bewest
Copy link
Member Author

bewest commented Jun 30, 2020

@FredMK you knocked out quite a chunk of work. There are few things I can tweak today, and balancing against the need to just release a hotfix:

  • We don't yet know what the server name will be for the USA for Japan, Australia, etc. I will tweak the server name to accommodate these differences without a need to change the code. It's clear that this new oauth2/sso system will eventually be rolled out everywhere.
  • I will also tweak country and lang used in the initial URL seed to come from environment variable (it may become necessary for Accept-Lang, and country and lang to all match for some reason)
  • consider changing user-agent to something specific to miminmed-connect-to-nightscout/$version Looks like it will probably fly... I've been using curl's default without issue for some of these requests, and it will give Medtronic no excuse to claim "they don't know" who to contact for traffic they see from this agent. If they block us based on user-agent again, we can always spoof again. I made a similar change to share2nightscout-bridge.
  • adjust params to use qs.parse, url.parse, url.format and friends
  • I'm considering using something like https://github.com/cheeriojs/cheerio to parse out the form... maybe just a style thing and not worth holding up release? It does eliminate the need to store and format that URL in our code, and helps reduce the need for regexp in case they change something in their html templates. I'm happy to do the adjustments, but perhaps I pursue this after hotfix.
  • again maybe style thing since this is not your code: getting cookie value... the place where cookie value is requested knows the exact URL it should fetching cookies from (the one just requested), but we jump through hoops to kind of guess at the right url... again it works right now, so maybe just hotfix and then address this in a future release?

I'm really impressed at the fantastic amount of progress you made on this. I was tempted to try a separate module that would be peers with carelink.js but you made everything fit in short order, including handling the token reauth. Great work!

carelink.js Outdated Show resolved Hide resolved
@bewest
Copy link
Member Author

bewest commented Jun 30, 2020

Probably won't be fixed in this pull request, but just making a note based on experience. We should double check the retry logic, especially connected to getting reauthed. Eg what happens when NS is running and then password is changed in carelink web portal? Is it likely to lock an account, or send many unreasonable numbers of requests in an error condition if retrying something that won't work?

@FredMK
Copy link
Contributor

FredMK commented Jun 30, 2020

I don't like the whole retry logic. I made some changes but it's not good at all. I have an idea, but takes some time to code it because of async. Still working on the ERR_TLS_CERT_ALTNAME_INVALID problem.

@bewest
Copy link
Member Author

bewest commented Jun 30, 2020

FWIW, if it's simpler to separate the code into it's own peer module, eg mmsso.js or similar, I'd be supportive of that. The new area might be easier using axios with it's nice promise interface.

@FredMK
Copy link
Contributor

FredMK commented Jun 30, 2020

I tried to fix the ERR_TLS_CERT_ALTNAME_INVALID error, but I cannot find a way, cannot understand why. The workaround is to use the 13.0.1 version from NS, currently this is the master branch, with this there is no problem.

The problem: When using NS 13.1.0 the following problem occurs during request MiniMed Connect error: Error [ERR_TLS_CERT_ALTNAME_INVALID]: Hostname/IP does not match certificate's altnames: Host: carelink.minimed.eu. is not in the cert's altnames: DNS:mdtlogin.medtronic.com, DNS:mdtsts.medtronic.com.
The problem only occurs for me when deployed, locally this is not a problem. Checking each certificate for each request, everything seems ok, there is no mismatch at all when debugging locally.

Some information what I tried to solve the problem (in my fork the wip branch contains some commit):

  • setting rejectUnauthorized: false
  • set headers.Host to the current host (eg: mdtlogin.medtronic.com)
  • set the host, path, port: 443
  • using checkServerIdentity: function (host, cert) { return undefined; }
  • process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0";

I'm using Windows so I cannot build NS correctly, is someone can take a look on it with the latest NS commit, can reproduce the problem locally, and not just on Heroku.

@bewest
Copy link
Member Author

bewest commented Dec 15, 2020

Eesh... resolving this now.

@bewest bewest mentioned this pull request Dec 15, 2020
@bewest
Copy link
Member Author

bewest commented Dec 15, 2020

Merge conflicts have been resolved by rebasing at selected diverging points. #17

@bewest bewest closed this Dec 15, 2020
@bewest bewest deleted the dev branch December 18, 2020 01:42
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.

8 participants