-
Notifications
You must be signed in to change notification settings - Fork 3
feat(oidc-client): implement force renew & revoke old tokens #402
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
Conversation
🦋 Changeset detectedLatest commit: bdbbbd2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
View your CI Pipeline Execution ↗ for commit bdbbbd2
☁️ Nx Cloud last updated this comment at |
Deployed 02e0637 to https://ForgeRock.github.io/ping-javascript-sdk/pr-402/02e063742b23ea9d648599a859dc00062602c99a branch gh-pages in ForgeRock/ping-javascript-sdk |
📦 Bundle Size Analysis📦 Bundle Size Analysis📊 Minor Changes📈 @forgerock/oidc-client - 22.4 KB (+0.1 KB) ➖ No Changes➖ @forgerock/davinci-client - 34.1 KB 11 packages analyzed • Baseline from latest Legend🆕 New package ℹ️ How bundle sizes are calculated
🔄 Updated automatically on each push to this PR |
62a33ca
to
f12df12
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #402 +/- ##
=======================================
Coverage 55.47% 55.47%
=======================================
Files 32 32
Lines 2044 2044
Branches 340 340
=======================================
Hits 1134 1134
Misses 910 910 🚀 New features to boost your workflow:
|
}); | ||
|
||
it('Get expired tokens without background renewal', async () => { | ||
customStorage['pic-oidcTokens'] = JSON.stringify({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we cleaning this up every test? Does reset handlers do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I'm not cleaning it up. I'm not sure I need to. The storage is declared within the describe
function body, so as soon as the tests are done, that closure is cleared out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only concern is that if we don't, then the storage is carried through to each test as it was mutated in the one before.
If someone new comes in and adds a test and doesn't read through the whole suite, then it could catch them by surprise or even affect a future test unexpectedly.
I think its worth cleaning storage after each test, but if you feel strongly we dont have to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, fair enough. I'll make the change.
f12df12
to
9ba076c
Compare
9ba076c
to
bdbbbd2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good,
only note is remove
of tokens is a touch redundant if we set
but its also explicit which is helpful.
JIRA Ticket
SDKS-4314
Description
Implements force renew feature for
.tokens.get
and revokes old tokens that are to be replaced.