[SDK-2692] Remember organization ID for silent authentication #788

stevehobbsdev · 2021-09-03T21:18:11Z

Key changes:

If an org_id claim is returned in the ID token, the SDK stores it in a new cookie to act as a "hint" for silent authentication
When doing getTokenSilently with iframe + prompt=none, if the hint cookie is available and no value for organization was given in the options, the value from the cookie is read and sent to the authorization request
If the hint cookie is available and organization is present in the options, the options value takes precedence
The cookie is removed on logout
The cookie is removed if there is no org_id claim in the ID token

I also evolved the playground a bit so that you can toggle the organization ID being passed through constructor options or directly to loginWithRedirect. I also added JSON output at the bottom so that the options being used to initialize the SDK are visible.

frederikprijck · 2021-09-06T08:55:11Z

src/Auth0Client.ts

+        this.orgHintCookieName,
+        decodedToken.claims.org_id
+      );
+    }


Do we need to remove the cookie if there is no org_id available?

else { this.cookieStorage.remove(this.orgHintCookieName); }

If there's no org_id claim it means that the user did not sign in in the context of an organization, so IMO it makes sense to remove the cookie so that we don't try to use it the next time.

Yes, I was asking because in the above case, we are not removing it.

You're right, I've added this now in 98dda60

frederikprijck · 2021-09-06T09:03:11Z

src/Auth0Client.ts

+        this.cookieStorage.remove(
+          buildOrganizationHintCookieName(this.options.client_id)
+        );
+      }


Why are we not setting the cookie here? I assume it is because we know it is already set at this point? But it's not entirely clear for me if this is actually 100% guaranteed.

Any reason we are not using this.orgHintCookieName here instead of buildOrganizationHintCookieName(this.options.client_id) ?

However, curious to know if it would make sense to drop this.orgHintCookieName and use buildOrganizationHintCookieName(this.options.client_id) everywhere. It would allow to work if client_id is ever changed after constructing an Auth0Client (Not sure we should explicitly support that, but we could).

Why are we not setting the cookie here?

We could set it here as well, I think that's fine.

Any reason we are not using this.orgHintCookieName here

I think that's just an oversight, I'd like to use it here as well. I did start down the road of just calling the function but I think if we wanted to support changing the client ID (can't think of a use-case right now) there'd be more work to do than just this.

I will also need to review getTokenWithPopup as well

Fixed in 0211ca7. getTokenWithPopup looks to be ok as it just calls loginWithPopup anyway.

stevehobbsdev · 2021-09-07T10:21:33Z

Converting back to a draft for the meantime. I've identified a couple E2E tests I want to put in, as well as fix the existing broken one.

For the broken one, I'm still trying to figure out why its needed. The name of the test is "retrieves an access token for another audience using a refresh token" which is not something you can actually do. What it's actually testing is that the fallback to an iframe works but there's nothing about the test that should cause that to happen. Looking into it today.

stevehobbsdev · 2021-09-08T15:55:30Z

I'm ready for a review now. I had intentions to get an E2E test set up for this but it specifically means intercepting the call to /authorize, which happens in an iframe. It looks to be challenging, but extra challenging here because the iframe is created just-in-time. For now I've tested the behaviour manually, and also set up the playground so that node-oidc-provider can echo the org_id claim back to us. I could verify the result in a test the org_id claim is written out to the screen, but that's kind of testing what the test itself is setting up. It would be an OK test if we were still hitting real Auth0.

…ct, and show options

stevehobbsdev · 2021-09-08T15:59:57Z

cypress/support/commands.js

-Cypress.Commands.add('toggleSwitch', name =>
-  cy.get(`[data-cy=switch-${name}]`).click()
-);
+Cypress.Commands.add('setSwitch', (name, value) => {


I refactored toggleSwitch into setSwitch where you explicitly set the value of the switch so you can see exactly what it's set to in a test. toggleSwitch requires you to understand what the default for that switch is, which is poor.

stevehobbsdev · 2021-09-08T16:02:02Z

scripts/oidc-provider.js

+  claims: {
+    org_id: null
+  },


I couldn't see a better way to do this, but this config for node-oidc-provider allows it to return an org_id claim, by specifying the claims parameter to the authentication request. Ideally, I'd like the server to just echo back the organization value in the org_id claim but couldn't find a good way to do it or if that scenario is supported by the server. Makes sense if it's strictly OIDC compliant.

https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter

stevehobbsdev · 2021-09-08T16:03:50Z

static/index.html

+            <input
+              type="checkbox"
+              class="form-check-input"
+              id="org-login-check"
+              v-model="useOrgAtLogin"
+              data-cy="useOrgAtLogin"
+            />
+            <label for="org-login-check" class="form-check-label"
+              >Apply only at loginWithRedirect</label
+            >


This allows us to toggle whether the organization ID is given to the constructor options, or only to loginWithRedirect for easy testing.

stevehobbsdev · 2021-09-08T16:04:11Z

static/index.html

+          <div class="col-md-12">
+            <h3 class="mb-3">Client Options</h3>
+
+            <pre><code>{{ clientOptions }}</code></pre>
+          </div>


Writing these options out just makes it easier to debug.

stevehobbsdev · 2021-09-08T16:05:05Z

static/index.html

+            if (_self.organization) {
+              const claims = {
+                id_token: {
+                  org_id: { values: [_self.organization] }
+                }
+              };
+
+              clientOptions.claims = JSON.stringify(claims);
+            }


This is setting the claims parameter for the authentication call. Used when using node-oidc-provider as the OP, ignored by Auth0 completely (looks like it doesn't support it).

stevehobbsdev added CH: Fixed PR is fixing a bug review:medium Medium review labels Sep 3, 2021

stevehobbsdev modified the milestone: vNext Sep 3, 2021

stevehobbsdev marked this pull request as ready for review September 3, 2021 21:25

stevehobbsdev requested a review from a team as a code owner September 3, 2021 21:25

frederikprijck suggested changes Sep 6, 2021

View reviewed changes

stevehobbsdev requested review from frederikprijck and adamjmcgrath September 6, 2021 11:58

stevehobbsdev marked this pull request as draft September 7, 2021 10:19

adamjmcgrath previously approved these changes Sep 8, 2021

View reviewed changes

stevehobbsdev dismissed adamjmcgrath’s stale review via 511c3ba September 8, 2021 10:55

benedict-jackson mentioned this pull request Sep 8, 2021

Forced to log in after refresh when using organizations auth0/auth0-react#274

Closed

stevehobbsdev marked this pull request as ready for review September 8, 2021 15:55

Steve Hobbs added 14 commits September 8, 2021 16:56

Add and remove org hint cookie on redirect login and logout

cdf52bf

Save org hint cookie in loginWithPopup

f18a504

Base org hint cookie on id token claims instead of options

10234c2

Store org hint as JSON

30d9a1d

Check organization ID hint cookie when auth using iframe

efec221

Mods to the playground to allow sending org only with loginWithRedire…

13a9c02

…ct, and show options

Add test for removing organization hint cookie in getTokenSilently

7e77a9e

Ensure organization hint cookie is removed in loginWithPopup

fc1886b

Store org_id hint cookie in getTokenFromIFrame

371bcca

Revert change to show all info in playground on getting tokens

36012eb

Replace toggleSwitch with setSwitch

488b2f5

Don't need to use optional chaining if inside the block

7922c68

Support org_id claim in node-oidc-provider

917b79d

Move claims param to client setup

0dd0a76

Remove old claims setup

5ae2326

stevehobbsdev force-pushed the sdk-2692/org-cookie branch from 6b9660f to 5ae2326 Compare September 8, 2021 15:56

stevehobbsdev requested a review from adamjmcgrath September 8, 2021 15:57

stevehobbsdev commented Sep 8, 2021

View reviewed changes

Remove console.log

2fd5924

stevehobbsdev commented Sep 8, 2021

View reviewed changes

Merge branch 'master' into sdk-2692/org-cookie

932cd58

adamjmcgrath approved these changes Sep 13, 2021

View reviewed changes

frederikprijck approved these changes Sep 13, 2021

View reviewed changes

stevehobbsdev merged commit 4e71b2c into master Sep 13, 2021

stevehobbsdev deleted the sdk-2692/org-cookie branch September 13, 2021 14:23

stevehobbsdev mentioned this pull request Sep 15, 2021

Release v1.18.0 #797

Merged

frederikprijck mentioned this pull request Sep 20, 2021

Bump Auth0-SPA-JS to 1.18.0 auth0/auth0-angular#218

Merged

adamjmcgrath mentioned this pull request Aug 8, 2023

The getAccessTokenSilently response returns access_token populated with organization data after page refresh auth0/auth0-react#557

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SDK-2692] Remember organization ID for silent authentication #788

[SDK-2692] Remember organization ID for silent authentication #788

stevehobbsdev commented Sep 3, 2021 •

edited

Loading

frederikprijck Sep 6, 2021

stevehobbsdev Sep 6, 2021

frederikprijck Sep 6, 2021

stevehobbsdev Sep 7, 2021

frederikprijck Sep 6, 2021

frederikprijck Sep 6, 2021

stevehobbsdev Sep 6, 2021

stevehobbsdev Sep 6, 2021

stevehobbsdev Sep 7, 2021

stevehobbsdev commented Sep 7, 2021

stevehobbsdev commented Sep 8, 2021

stevehobbsdev Sep 8, 2021

stevehobbsdev Sep 8, 2021

stevehobbsdev Sep 8, 2021

stevehobbsdev Sep 8, 2021

stevehobbsdev Sep 8, 2021

[SDK-2692] Remember organization ID for silent authentication #788

[SDK-2692] Remember organization ID for silent authentication #788

Conversation

stevehobbsdev commented Sep 3, 2021 • edited Loading

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

stevehobbsdev commented Sep 7, 2021

stevehobbsdev commented Sep 8, 2021

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

stevehobbsdev commented Sep 3, 2021 •

edited

Loading