🌱 Session Cookie Support In Token Manager (copy) #694
Conversation
oleksandrpravosudko-okta
force-pushed
the
jpspringall-okta-367497-session-cookie-support
branch
from
0bdd15e to
477a5a9
Compare
lib/browser/browserStorage.ts
Outdated
| if (typeof secure === 'undefined' || typeof sameSite === 'undefined') { | ||
| throw new AuthSdkError('storage.set: "secure" and "sameSite" options must be provided'); | ||
| } | ||
| var cookieOptions: CookieOptions = { | ||
| var cookieOptions: SetCookieOptions = { |
There was a problem hiding this comment.
It looks like SetCookieOptions type is intended to be used when creating a cookie.
There was a problem hiding this comment.
looks like just different names, we probably can remove one in the future. Now let's just keep what it is to avoid breaking changes.
lib/browser/browserStorage.ts
Outdated
| @@ -20,7 +20,7 @@ import { | |||
| SimpleStorage, | |||
| StorageType, | |||
| BrowserStorageUtil, | |||
| CookieStorage | |||
| CookieStorage, SetCookieOptions | |||
There was a problem hiding this comment.
looks like it should be CookieOptions
oleksandrpravosudko-okta
force-pushed
the
jpspringall-okta-367497-session-cookie-support
branch
2 times, most recently
from
56a449e to
105ffe6
Compare
Codecov Report
@@ Coverage Diff @@
## master #694 +/- ##
==========================================
+ Coverage 84.52% 84.53% +0.01%
==========================================
Files 98 98
Lines 2824 2826 +2
Branches 573 575 +2
==========================================
+ Hits 2387 2389 +2
Misses 437 437
Continue to review full report at Codecov.
|
Access and Id Token can now set as session cookies when using cookie storage Resolves: OKTA-592
oleksandrpravosudko-okta
force-pushed
the
jpspringall-okta-367497-session-cookie-support
branch
from
e0ae3fa to
35ae87c
Compare
CHANGELOG.md
Outdated
| @@ -1,5 +1,11 @@ | |||
| # Changelog | |||
|
|
|||
| ## 5.1.0 | |||
There was a problem hiding this comment.
We have not published 5.0 yet. This feature can go with 5.0 release.
test/spec/browserStorage.js
Outdated
| @@ -152,6 +152,18 @@ describe('browserStorage', () => { | |||
| sameSite: 'strictly fakey' | |||
| }); | |||
| }); | |||
|
|
|||
| it('setItem: will call storage.set, passing sessionCookie, secure and sameSite options ', () => { | |||
There was a problem hiding this comment.
Is this the test that should have the title "but no sesssionCookie option"? It appears to be testing that the sessionCookie option is NOT passed through
Access and Id Token can now set as session cookies when using cookie storage
Copy of #591 with merge conflicts resolved.
Resolves: OKTA-367497