Provide tokenManager configuration in framework SDKs #388
Conversation
packages/okta-vue/README.md
Outdated
| - [`sessionStorage`](https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage) | ||
| - [`cookie`](https://developer.mozilla.org/en-US/docs/Web/API/Document/cookie) | ||
|
|
||
| Defaults to `localStorage` and will fall back to `sessionStorage`, and/or `cookie` if no one of the previous type is available. |
There was a problem hiding this comment.
Is there a reason we don't default to sessionStorage instead? OWASP seems to recommend it: https://www.owasp.org/index.php/JSON_Web_Token_(JWT)_Cheat_Sheet_for_Java#Token_storage_on_client_side
There was a problem hiding this comment.
sessionStorage doesn't span tabs, so I've always found it a terrible user experience.
packages/okta-react/README.md
Outdated
| @@ -201,6 +201,8 @@ Security is the top-most component of okta-react. This is where most of the conf | |||
| Accepts a callback to make a decision when authentication is required. If this is not supplied, `okta-react` redirects to Okta. This callback will receive `auth` and `history` parameters. This is triggered when: | |||
| 1. `auth.login` is called | |||
| 2. SecureRoute is accessed without authentication | |||
| - [**storage**](#storage) *(optional)*: Specify the type of storage for tokens | |||
There was a problem hiding this comment.
For consistency, you should list the default here too (you already do below), or at least mention that it's discussed below
| @@ -27,6 +27,17 @@ const hasDomainAdmin = /-admin.(okta|oktapreview|okta-emea).com/; | |||
| const hasDomainTypo = new RegExp('(.com.com)|(://.*){2,}'); | |||
| const endsInPath = new RegExp('/$'); | |||
|
|
|||
| configUtil.buildConfigObject = (config) => { | |||
There was a problem hiding this comment.
Add tests for this function, to assert that it builds the object you expect, given different inputs
packages/okta-angular/README.md
Outdated
| @@ -84,6 +86,20 @@ const appRoutes: Routes = [ | |||
| export class MyAppModule { } | |||
| ``` | |||
|
|
|||
| #### storage | |||
There was a problem hiding this comment.
Maybe it would be easier just to put this information as nested bullets of the storage bullet above? Seems simpler and keeps everything in one place
manueltanzi-okta
force-pushed
the
manueltanzi-tokenmanager-configuration
branch
2 times, most recently
from
8362e20
to
82c5c19
Compare
manueltanzi-okta
force-pushed
the
manueltanzi-tokenmanager-configuration
branch
from
82c5c19
to
694fa77
Compare
Description
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Not possible to set configuration parameters relative to TokenManager
Issues Number:
#285
#95
#173
#252
What is the new behavior?
Allows to set configuration parameters relative to TokenManager
Does this PR introduce a breaking change? TBD
Reviewers
@robertdamphousse-okta @brettritter-okta @nbarbettini