Store RT, add scopes to static-spa sample #527

swiftone · 2020-10-28T18:39:09Z

No description provided.

lib/token.ts

aarongranick-okta · 2020-10-28T21:31:28Z

samples/generated/static-spa/public/app.js

@@ -266,7 +269,8 @@ function redirectToGetTokens(additionalParams) {

 function redirectToLogin(additionalParams) {
  authClient.token.getWithRedirect(Object.assign({
-    state: JSON.stringify(config.state)
+    state: JSON.stringify(config.state),
+    scopes: config.state.scopes.split(/\s+/) || config.scopes, // getWithRedirect doesn't obey scopes in constructor yet


is this a bug?

I certainly think so.

lib/TokenManager.ts

shuowu · 2020-10-30T19:19:58Z

lib/token.ts

+    });
+}
+
+async function renewTokensWithRefresh(sdk: OktaAuth, options: TokenParams, refreshTokenObject: RefreshToken): Promise<any> {


Looks like the return type is TokenResponse instead of Tokens, which means the tokenManager cannot get proper tokens after the renew process.

CHANGELOG.md

aarongranick-okta · 2020-11-03T19:13:52Z

lib/TokenManager.ts

  tokenMgmtRef.renewPromise[key] = sdk.token.renewTokens({
-    scopes: token.scopes
+    scopes: token.scopes,
+    refreshToken: storage.getStorage()


doublecheck this

aarongranick-okta · 2020-11-03T23:18:29Z

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Changelog

+## PENDING


Can make this 4.2.0. package.json is already bumped, will be a normal release from master

My inclination is to leave this as PENDING until we're ready for release, as it helps with people on github asking why the version on NPM doesn't agree with what they are seeing, but I'm open to persuasion.

aarongranick-okta · 2020-11-03T23:31:05Z

samples/generated/static-spa/public/app.js

@@ -335,6 +354,7 @@ function showForm() {
  // Set values from config
  document.getElementById('issuer').value = config.issuer;
  document.getElementById('clientId').value = config.clientId;
+  document.getElementById('scopes').value = config.scopes;


I think it would be less confusing if config.scopes in the samples was an array, consistent with what okta-auth-js expects. The conversion to/from string can be handled here in showForm. In the default test app, the array is converted to a comma separated string: https://github.com/okta/okta-auth-js/blob/master/test/app/src/form.ts#L51
Within the JSON serialized state param, it can remain an array. It only needs to be a string for the 2 cases:

in the form

as a standalone query parameter
These 2 cases are closely related because the form has method GET so submitting the form will set the query parameter with the value in the form. The serialized state object is used to save the config across redirects.

aarongranick-okta · 2020-11-03T23:31:59Z

I tested it and it works. Nice job!

shuowu · 2020-11-04T15:01:40Z

CHANGELOG.md

+- Adding the ability to use refresh tokens with single page applications (SPA) (Early Access feature - reach out to our support team)
+  - `scopes` configuration option now handles 'offline_access' as an option, which will use refresh tokens IF your client app is configured to do so in the Okta settings
+    - If you already have tokens (from a separate instance of auth-js or the okta-signin-widget) those tokens must already include a refresh token and have the 'offline_access' scope
+    - 'offline_access' is not requested by default.  Anyone using the default `scopes` and wishing to add 'offline_access' should pass `scopes: ['openid', 'email', 'offline_access']` to their constructor (FIXME: Is constructor enough?)


Aaron has a fix for 4.1, should be backported after merging #535

lib/AuthStateManager.ts

lib/browser/browser.ts

shuowu · 2020-11-04T15:12:26Z

lib/browser/browser.ts

@@ -427,6 +453,11 @@ class OktaAuthBrowser extends OktaAuthBase implements OktaAuth, SignoutAPI {
    return accessToken ? accessToken.accessToken : undefined;
  }

+  getRefreshToken(): string | undefined { 


remove this method if we want to expose refreshToken only internally for now.

add methods: isPKCE, hasResponseType

shuowu · 2020-11-17T23:16:29Z

lib/browser/browser.ts

+  // Revokes the refresh token for the application session
+  async revokeRefreshToken(refreshToken?: RefreshToken) {
+    if (!refreshToken) {
+      refreshToken = (await this.tokenManager.getTokens()).refreshToken as RefreshToken;


It might be better to get the token from authState (sync access), but as we are also using the same logic in revokeAccessToken, the change can be made in a separate PR.

We get into a circular dependency if we try that. I brought it up and Aaron suggested we just do this.

wondering how the circular dep happens? it should just be an access to the in-memory state

const { refreshToken } = this.authStateManager.getAuthState();

I'm going off memory, but as I recall this.authStateManager didn't exist here.

shuowu · 2020-11-17T23:17:45Z

lib/browser/browser.ts

@@ -426,6 +451,11 @@ class OktaAuthBrowser extends OktaAuthBase implements OktaAuth, SignoutAPI {
    return accessToken ? accessToken.accessToken : undefined;
  }

+  getRefreshToken(): string | undefined {


so we decided to keep the public method for refresh token?

looks like it's neither in readme nor used in code, remove?

We kept it because we're exposing it on authState anyway.

aarongranick-okta · 2020-11-17T23:51:03Z

samples/config.js

@@ -20,6 +20,7 @@ const defaults = {
 const spaDefaults = Object.assign({
  redirectPath: '/implicit/callback',
  flow: 'redirect',
+  scopes: 'openid email',


I'm still not a fan of having scopes as a space-separated string here in the config. I would much prefer that scopes is an array, as it is for OktaAuth, and the conversion to string (comma or space separated) was done in the showForm() and conversion from string back to array is in loadConfig()

aarongranick-okta

see comments about "scopes" in the samples

swiftone requested review from aarongranick-okta and shuowu October 28, 2020 18:39

aarongranick-okta reviewed Oct 28, 2020

View reviewed changes

lib/token.ts Outdated Show resolved Hide resolved

aarongranick-okta reviewed Oct 28, 2020

View reviewed changes

vijetmahabaleshwar-okta reviewed Oct 29, 2020

View reviewed changes

lib/TokenManager.ts Outdated Show resolved Hide resolved

shuowu reviewed Oct 30, 2020

View reviewed changes

vijetmahabaleshwar-okta reviewed Nov 3, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

aarongranick-okta reviewed Nov 3, 2020

View reviewed changes

shuowu reviewed Nov 4, 2020

View reviewed changes

lib/AuthStateManager.ts Show resolved Hide resolved

shuowu reviewed Nov 4, 2020

View reviewed changes

lib/browser/browser.ts Show resolved Hide resolved

shuowu reviewed Nov 4, 2020

View reviewed changes

brettritter-okta force-pushed the br-hat-refresh-token branch 2 times, most recently from 487b942 to 7b79ac4 Compare November 5, 2020 18:53

swiftone and others added 13 commits November 12, 2020 13:35

Store RT, add scopes to static-spa sample

8a1dc2a

base pattern for renewTokensWithRefresh

a0598bb

renew successfully (once)

b66ce6e

Fixed renewal with refresh token

88334ab

Adds base revoke for refresh tokens

d22806f

Updates existing tests to pass

54b3ac9

updates docs a bit

47544ae

feat: add method handleLoginRedirect (#528)

00ea7c7

accept responseType as a constructor arg (#525)

94b9bf9

add methods: isPKCE, hasResponseType

update samples to use authStateManager (#506)

e3cdb3a

revised to try for non-breaking

8bcfdd5

Fix for lack of AuthState

4497874

Fix return val for renewTokens

1a7b909

swiftone added 2 commits November 12, 2020 13:46

Fix responseType on renew error

b135f3b

Fix urls passed for renew

168e41d

brettritter-okta force-pushed the br-hat-refresh-token branch from 068c677 to 168e41d Compare November 12, 2020 21:52

swiftone added 2 commits November 13, 2020 14:53

Run e2e tests w/o refresh token for now

f74cb6f

prep for 4.2.0 release

8f7dfb6

swiftone marked this pull request as ready for review November 17, 2020 18:59

shuowu reviewed Nov 17, 2020

View reviewed changes

shuowu approved these changes Nov 17, 2020

View reviewed changes

aarongranick-okta reviewed Nov 17, 2020

View reviewed changes

aarongranick-okta approved these changes Nov 17, 2020

View reviewed changes

eng-prod-CI-bot-okta closed this Nov 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Store RT, add scopes to static-spa sample #527

Store RT, add scopes to static-spa sample #527

swiftone commented Oct 28, 2020

aarongranick-okta Oct 28, 2020

swiftone Oct 28, 2020

shuowu Oct 30, 2020

aarongranick-okta Nov 3, 2020

aarongranick-okta Nov 3, 2020

swiftone Nov 4, 2020

aarongranick-okta Nov 3, 2020

aarongranick-okta commented Nov 3, 2020

shuowu Nov 4, 2020

shuowu Nov 4, 2020

shuowu Nov 17, 2020

swiftone Nov 17, 2020

shuowu Nov 17, 2020

swiftone Nov 18, 2020

shuowu Nov 17, 2020

shuowu Nov 17, 2020

swiftone Nov 17, 2020

aarongranick-okta Nov 17, 2020

aarongranick-okta left a comment

Store RT, add scopes to static-spa sample #527

Store RT, add scopes to static-spa sample #527

Conversation

swiftone commented Oct 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aarongranick-okta commented Nov 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aarongranick-okta left a comment

Choose a reason for hiding this comment