Please accept my updates into the master branch of xDripAPS #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ing persistant logging and better startup checking for problems and configuration problems
ekazum
reviewed
Jun 9, 2020
xDripAPS.py
Outdated
|
|
||
| elif api_secret_xDripAPS: | ||
| #get API_SECRET_xDripAPS environment variable if needed | ||
| env_secret_hashed = os.environ['API_SECRET_xDripAPS'] | ||
| print "We will authenticate using environment variable API_SECRET_xDripAPS" | ||
| xLog("We will authenticate using environment variable API_SECRET_xDripAPS") | ||
|
|
||
| # Authentication check | ||
| if request_secret_hashed != env_secret_hashed: |
There was a problem hiding this comment.
There is a bug here -
You compare hash value with non-hash value
request secret is hashed while env_secret_hashed is actually the plain text and not hashed
ekazum
suggested changes
Jun 9, 2020
|
|
||
| # Authentication check | ||
| if request_secret_hashed != env_secret_hashed: |
There was a problem hiding this comment.
@ArmyAndy
There is a bug here, you compare hash value vs. non-hash value
request_secret_hashed is really hashed, while env_secret_hashed is just the plain text value. Should be hashed before comparison
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Colin,
It's been a long time since we met and you got me first involved with openAPS! For the past 2 weeks, I have finally got up and running (as I didn't have a CGM when we met, but now I have the libre with a miaomiao!)
I spend a lot of time on long flights, and the offline support was of particular importance to me. I found that there were a number of problems and incompatibilities which meant I couldn't use xDripAPS out of the box (as I use token based authentication for my API_SECRET.
I have therefore made a number of updates to xDripAPS making it compatible with my setup, more bulletproof, with better logging and various bug fixes. Another member of the openAPS community has tested this on their rig (not used as part of openAPS). I have tested this also for the past 24 hours flicking between online and offline mode without problems.
Please see the commit summary of my changes:
1/Added checking for API_SECRET and won't start unless this, or API_SECRET_xDripAPS environment variable is set
2/Added support for second environment variable API_SECRET_xDripAPS to allow for compatibility to run xDripAPS when OpenAPS users are using token based authentication with their API_SECRET value for Nightscout, and would like to be able to use the basic environment variable based password for their xDrip+ integration with xDripAPS. If API_SECRET_xDripAPS is defined, we will use that for authentication instead of API_SECRET.
3/Added error output when api-secret header isn't sent instead of just letting the program return a 500
4/Changed header from Api_Secret to api-secret as xDrip+ doesn't send Api_Secret anymore and therefore this update makes the loop work again when using the latest version of xDrip.
5/Added a persisted log file for xDripAPS. The log is in /var/log/openaps/xDripAPS-{date created}.log The log because it is in this directory will become part of the openaps log rotation already configured on openAPS rigs.
6/Bugfix where we now perform a lowercase comparison meaning that if case between stored API_SECRET variables and value passed into the call are compared, we do it in a case insensitive manner.
7/If you are using the new API_SECRET_xDripAPS environment variable, you will now need to alter your crontab to include the API_SECRET_xDripAPS environment variable. Use crontab -e and add it under your existing API_SECRET entry.