RSA10k

[EvgenyKarkan]

No description provided.

[mattheworiordan]

This PR addresses some of the points from the previous PRs about testing time which is good ,, thanks.

[EvgenyKarkan]

Sorry, should I redo something here?

[mattheworiordan]

Sorry, should I redo something here?

I should have been clearer, I think this PR is fine. However, given the equivalent Java one that I saw was not actually using the offset, can you think of any way to check that the offset is really being used on subsequent requests?

[EvgenyKarkan]

Ok, let me try to describe one case.

• assume timeOffset is stored by Auth instance yet, and we are expecting subsequent request in future.
• AuthOptions argument queryTime property set to false or missed as an argument (false by default)

I guess you want us also to check such case for subsequent requests, right?

[mattheworiordan]

I guess you want us also to check such case for subsequent requests, right?

Well yes, some coverage that demonstrably shows that the offset is effecting the time. You could inject a fake offset and check it effects token, or if you can think of a better way then that would good too, I just want to be sure the offset is actually used as per the spec.

[EvgenyKarkan]

Please review and merge if everything looks good

[tcard]

Why do you convert the NSTimeIntervals to long long and then to NSNumber instead of just storing the time offset as a NSTimeInterval? And why do you multiply by 1000 instead of working with seconds with decimals, which is what NSTimeInterval is?

[mattheworiordan]

Few small comments but mostly good

[mattheworiordan]

This is not needed. It's saying the it's possible that the "Client" should be able to discard the local clock offset, not the user. Anyway, leave now, but this spec item was simply a statement to make it clear that the client library developer when writing the client could under certain circumstances choose to discard the offset internally.

[ricardopereira]

Will change the base to master.

[ricardopereira]

Squashed.

[ricardopereira]

Not passing. I think some pieces are missing.

[ricardopereira]

A client MAY discard the cached local clock offset in situations in which it may have been invalidated, such as if there is a local change to the date, time, or timezone, of the client device

• UIApplicationSignificantTimeChangeNotification for example, change to a new day (midnight), carrier time update (iOS only!)
• NSCurrentLocaleDidChangeNotification when the user’s locale changed

[mattheworiordan]

Thanks @ricardopereira. TBH actually handling the changing timezones is not something we worried about hence why it's deliberately unspecified in the lib, but glad you've done it so worth merging in now.

[ricardopereira]

@tcard PTAL

[mattheworiordan]

@tcard are you happy for this to be merged?

[tcard]

This will prevent the server time to be queried even if queryTime is set to true, which is not what I would expect, although RSA10k makes it kind of ambiguous: "If the AuthOption argument’s queryTime attribute is true, it will obtain the server time once and persist the offset from the local clock" contradicts with "All future token requests generated directly or indirectly via a call to authorize will not obtain the server time" if authorize is called with queryTime true (triggering the first condition) a second time (triggering the second condition).

@mattheworiordan can you confirm this is expected?

[ricardopereira]

Fixed 3588ff0.

[ricardopereira]

Can I squash and rebase?