[#811] Implement refresh token reuse detection grace period

[roberlander2]

Description

This PR adds a refresh grace period policy to app org login session settings. If the previous refresh token is used to attempt a login session refresh within the grace period defined by the policy, now the refresh attempt will succeed.

Resolves #811

Review Time Estimate

Please give your idea of how soon this pull request needs to be reviewed by selecting one of the options below. This can be based on the criticality of the issue at hand and/or other relevant factors.

• Immediately
• Within a week
• When possible

Type of changes

Please select a relevant option:

• Bug fix (non-breaking change which fixes an issue).
• New feature (non-breaking change which adds functionality).
• Breaking change (fix or feature that would cause existing functionality to not work as expected).
• Other (any another change that does not fall in one of the above categories.)

Checklist:

Please select all applicable options:

• I have signed the Rokwire Contributor License Agreement (CLA). (Any contributor who is not an employee of the University of Illinois whose official duties include contributing to the Rokwire software, or who is not paid by the Rokwire project, needs to sign the CLA before their contribution can be accepted.)
• I have updated the CHANGELOG.
• I have read the Contributor Guidelines.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• My change requires updating the documentation.
• I have made necessary changes to the documentation.
• I have added tests related to my changes.
• My changes generate no new warnings.
• New and existing unit tests pass locally with my changes.
• Any dependent changes have been merged and published in downstream modules.

---

[shurwit]

Thanks @roberlander2 looks good! Just a couple of minor comments below!

[petyos]

@roberlander2, @shurwit , this seems like the right approach.
Just a quick note - let’s make sure we also keep the system APIs for create (and especially update) for the application organization up to date, because we recently had an issue where the update API did not reflect the FERPA record, and each call to it was resetting that FERPA field.
Thanks both.

[roberlander2]

Thanks @roberlander2 looks good! Just a couple of minor comments below!

Thanks @shurwit I made some additional changes to address the issues you pointed out.

[shurwit]

Thanks @roberlander2, this is looking better! Just a couple more questions below.

[shurwit]

Is there any risk to adding this check here?

Previously if the login session doesn't have a refresh token we would just send back an empty string in the response, but still send back the other tokens. Now we will return an error instead.

Are there any cases where we don't provide a refresh token (eg. anonymous auth, some setting to disable refresh on app org... etc.)?

[roberlander2]

I checked on this and the only case where there would be no refresh token here is if we are returning a login session in the MFA state, so I added an additional check for this.

[petyos]

LGTM. There was just one place where the grace period for the app/org’s API wasn’t set. I added it. Thanks!

[shurwit]

Hi @petyos @roberlander2 , just realized that we should probably add some logging here to have a record in the logs when a previous refresh token is used, but is allowed to continue due to the grace period. This will help us evaluate the impact of this change and ensure there aren't any issues resulting from it.