Remote Bolus Dev#8
Conversation
cf61811 to
5c1f4dd
Compare
e47ff65 to
f84b6f5
Compare
…e reuse, use sha256 rather than sha1, unit tests
| @@ -21,12 +21,22 @@ public final class NightscoutService: Service { | |||
|
|
|||
| public weak var serviceDelegate: ServiceDelegate? | |||
|
|
|||
There was a problem hiding this comment.
This issue was discussed in LoopKit/Loop#1536.
But the gist is that a lazy getter poses issue on the first run that prevents the token from uploading. Since the Nightscout service is not setup the first time this is called, these values will be nil. That will prevent calls that use these values from working until you kill app and restart.
My solution here was a didSet which will keep the uploader in sync with the url/apiSecret. I think a clean solution would be to hook in to the flow that updates these credentials a little deeper.
| } | ||
|
|
||
| public var isOnboarded: Bool | ||
|
|
There was a problem hiding this comment.
otpManager is our wrapper around OneTimePassword that validates our OTP codes from push notifications.
| @@ -63,6 +69,8 @@ public final class NightscoutService: Service { | |||
| self.lockedObjectIdCache = Locked(ObjectIdCache()) | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
inject the store to help with unit testing.
| @@ -122,6 +130,14 @@ public final class NightscoutService: Service { | |||
| try? KeychainManager().setNightscoutCredentials() | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
For the override issue mentioned above.
Limitation - this wouldn't handle the case of changing the site url or apiSecret.
| return passwordsString.split(separator: ",").map({String($0)}) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Manages overall OTP operations, using the store above (keychain) to track OTP secret and the last 2 OTP codes.
| } | ||
| } | ||
|
|
||
| public func validateOTP(otpToValidate: String) -> Bool { |
There was a problem hiding this comment.
We hang onto the last 2 codes as we need to make sure a user can't reuse an OTP code per OTP guidelines.
| //Already used | ||
| return false | ||
| } | ||
|
|
There was a problem hiding this comment.
Looking at the last 2 codes should guarantee the same code is not used twice.
| return true | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
For option in UI to reset the secret key. I don't think you would really need to this in practice though.
| let generator = Generator(factor: .timer(period: TimeInterval(self.tokenPeriod)), secret: secretKeyData, algorithm: algorithm, digits: passwordDigitCount)! | ||
| return Token(name: "\(secretKeyName)", issuer: "Loop", generator: generator) | ||
| } | ||
|
|
| @@ -0,0 +1,79 @@ | |||
| // | |||
There was a problem hiding this comment.
View model for the view that shows the OTP QR and code.
| @@ -0,0 +1,51 @@ | |||
| // | |||
| @@ -39,6 +41,14 @@ struct ServiceStatusView: View, HorizontalSizeClassOverride { | |||
| Text(String(describing: viewModel.status)) | |||
| } | |||
| .padding() | |||
There was a problem hiding this comment.
I'm not a SwiftUI expert yet - I haven't figured out how to remove the blue text and get a chevron in place. Any suggestions welcome but not sure also if this is a blocker yet.
There was a problem hiding this comment.
The issue seems to be that this button needs to be in a SwiftUI List to get the default chevron button styling. I experimented with that in commit 9ac625a5c488e918dea8f573fe26302716c3873a. That caused a different set of issues including the fact this List has a different background color than our our lists in Settings (inverted). I figured out how to change the row colors but not the background behind the list. Also the list would stay highlighted after popping back to this view, without adding a hack from Stack Overflow.
I opted for a simpler approach - With the latest update, I added a chevron image directly to the row and make sure the button text color is correct (non-blue). This is not ideal to not use the system scheme but seems pretty close to the desired layout.
Updated translations from Lokalise on Sun Oct 22 11:55:50 CDT 2023
This PR includes changes to the Nightscout module to support remote/bolus in dev. See the workspace PR for more details on the overall effort. Comments added inline for review.