Bypass Google auth when Okta is configured #144

jfreda · 2023-04-18T20:08:18Z

This PR adds functionality to bypass Google auth when Okta is configured. To do this, a GET method was added for the /api/v1/me API with a response that mimics Google's userinfo API, which we previously used to get a user's profile photo and name. Also, now that #132 was merged, this removes the code for using the Google Drive application data for storing recently viewed docs.

Breaking changes

The Admin SDK API now needs to be enabled.
New required aws_region configuration option for the okta block

web/web.go

…auth

anubhavmishra

Looks good to me!

jeffdaley

Looks good. I have some questions but it seems like we're close.

We seem light on front-end tests given the number of changes. Depending on your answers, let's stub some todos if not get a few more tests in here. I can help with that of course.

jeffdaley · 2023-04-19T20:08:47Z

web/app/routes/authenticated/document.js

+    // Update recently viewed docs for the dashboard route.
+    try {
+      await this.recentDocs.fetchAll.perform();
+    } catch (e) {
+      console.error("error updating recently viewed docs", e);
+      throw e;
+    }


This doesn't seem important here. What if you remove it and defer that load for when the user is actually visiting the dashboard? Otherwise I'd call it with void instead of try { await } so it doesn't hold up loading other stuff.

I was just minimizing changes, and when I fully removed this, the recently viewed docs weren't being updated in the dashboard until I refreshed the page. I just tried moving this to the dashboard route though and it seems to work, so I can move it there.

jeffdaley · 2023-04-19T20:17:37Z

web/app/components/header/nav.hbs

+          {{#if this.showSignOut}}
+            <dd.Interactive
+              data-test-user-menu-item="sign-out"
+              {{on "click" this.invalidateSession}}
+              @text="Sign out"
+            />
+          {{/if}}


Why are we removing the logout button? What happens if I skipGoogleAuth and try to invalidateSession?

It doesn't do anything because there's no Ember Simple Auth session to invalidate when we bypass Google auth.

Can we have it invalidate with Okta instead? I assume users will still want to sign out.

Our supported Okta configuration enforces Okta auth at the load balancer level so there's not going to be a way to sign out unless the user signs out of their Okta session (which will be in Okta, not Hermes).

Can we change our supported configuration to allow logout? 😬 I'd really like to keep the log out button. "As a user I want to log out."

We chatted about this offline, but I think I found a way to do this and it'll be implemented in another PR soon!

jeffdaley · 2023-04-19T20:22:40Z

web/app/routes/authenticate.ts

     * and if it is, transitions to the specified route
     */
-    this.session.prohibitAuthentication("/");
+    if (!this.configSvc.config.skip_google_auth) {
+      this.session.prohibitAuthentication("/");


What happens if we remove this if statement and run prohibitAuthentication under skipGoogleAuth conditions?

Looks like it's the same behavior. I think I forgot what this method actually does - I can revert this change.

It's supposed to redirect you to / if you're authenticated with Ember Simple Auth

Yeah, actually I guess we should probably just redirect right away from this route if we're bypassing Google auth... Because users could still have this URL saved and it won't be relevant without Google auth.

8d11b85 implements this ^

jeffdaley · 2023-04-19T20:24:54Z

web/app/routes/authenticated.ts

     */
-    void this.session.pollForExpiredAuth.perform();
+    if (!this.configSvc.config.skip_google_auth) {
+      void this.session.pollForExpiredAuth.perform();


Why does skipping Google auth negate the need to poll for expiredAuth?

Good q, 2 things here:

I still need to do some more testing on the app behavior when Okta auth expires; it might still be useful to do this polling if the user isn't immediately redirected to log in to Okta.

Currently pollForExpiredAuth is (understandably) Google-centric so I think we'll want to at least skip this until that either becomes neutral to the auth provider or has an Okta option built in too (assuming that we'll want to poll when skipping Google auth; still need to test).

it might still be useful to do this polling if the user isn't immediately redirected to log in to Okta

Yeah, I'd like to maintain our existing behavior where we detect passive logouts: If your token expires in the background, we should block [Okta]'s immediate-redirect function to show a gentler "reauathenticate" message.

Your change makes sense and its good to know you're planning to test further.

Re-enabled polling auth for Okta in this commit: 23635e6

…stead of verifying the access token

* Make people directory search read mask configurable in helper function * Add GET method for /api/v1/me endpoint * Bypass Google auth when Okta is configured * Only show Sign Out when using Google auth * s/bypass/skip * Update recently-viewed-docs-test.ts * Update Mirage `/me` reference * Update Mirage document timestamp; Fix test * Add note that Okta is instead of Google OAuth * Format config * Redirect right away from the authenticate route if skipping Google auth * Fetch recently viewed docs in the dashboard route instead of document * Add Admin service back * Add GetUser helper * Hack around bug with People API not returning names * Set the ALB auth cookie to an expired time when the token is invalid * Poll auth expiration when using Okta as well * Handle Okta redirect errors from fetch * Authorize Okta request using OIDC data header and verifying claims instead of verifying the access token * Update OIDC data format * Decode JSON into an interface * Handle AWS's JWTs that include padding * Use public key type for Parse key function * Use string type for preferred_username and log values on error * Don't compare preferred_username claim to OIDC identity header * Update flash message title * Add Admin SDK API to list of APIs that need to be enabled --------- Co-authored-by: Jeff Daley <[email protected]>

jfreda added 4 commits April 18, 2023 14:06

Make people directory search read mask configurable in helper function

81534c9

Add GET method for /api/v1/me endpoint

d3ecfc2

Bypass Google auth when Okta is configured

d634380

Only show Sign Out when using Google auth

08e1fea

anubhavmishra reviewed Apr 18, 2023

View reviewed changes

web/web.go Outdated Show resolved Hide resolved

anubhavmishra reviewed Apr 19, 2023

View reviewed changes

web/web.go Outdated Show resolved Hide resolved

jfreda and others added 7 commits April 18, 2023 19:20

Merge remote-tracking branch 'origin/main' into jfreda/bypass-google-…

5d58aba

…auth

s/bypass/skip

a1b8d5b

Update recently-viewed-docs-test.ts

0263a2e

Update Mirage /me reference

0df9149

Update Mirage document timestamp; Fix test

6ff70d0

Add note that Okta is instead of Google OAuth

be6eb3f

Format config

f2ffbee

jfreda marked this pull request as ready for review April 19, 2023 19:50

jfreda requested a review from a team as a code owner April 19, 2023 19:50

anubhavmishra approved these changes Apr 19, 2023

View reviewed changes

jeffdaley reviewed Apr 19, 2023

View reviewed changes

jfreda added 13 commits April 19, 2023 16:59

Redirect right away from the authenticate route if skipping Google auth

8d11b85

Fetch recently viewed docs in the dashboard route instead of document

8143ed4

Add Admin service back

795fed7

Add GetUser helper

1e008b1

Hack around bug with People API not returning names

1f649a5

Set the ALB auth cookie to an expired time when the token is invalid

c74de02

Poll auth expiration when using Okta as well

23635e6

Handle Okta redirect errors from fetch

18cf5e4

Authorize Okta request using OIDC data header and verifying claims in…

e6bd4ad

…stead of verifying the access token

Update OIDC data format

88ba481

Decode JSON into an interface

7560c50

Handle AWS's JWTs that include padding

49fd663

Use public key type for Parse key function

bcb9a09

jfreda added 4 commits April 25, 2023 20:12

Use string type for preferred_username and log values on error

a4389d2

Don't compare preferred_username claim to OIDC identity header

0107d23

Update flash message title

286f951

Add Admin SDK API to list of APIs that need to be enabled

5c3b9a5

jfreda added the breaking changes PR contains breaking changes label Apr 27, 2023

jfreda merged commit ebed57e into main Apr 27, 2023

jfreda deleted the jfreda/bypass-google-auth branch April 27, 2023 21:01

jeffdaley mentioned this pull request Apr 28, 2023

Improve pollForExpiredAuth and reauthenticate UX #141

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bypass Google auth when Okta is configured #144

Bypass Google auth when Okta is configured #144

jfreda commented Apr 18, 2023 •

edited

Loading

anubhavmishra left a comment

jeffdaley left a comment

jeffdaley Apr 19, 2023

jfreda Apr 19, 2023

jfreda Apr 19, 2023

jeffdaley Apr 19, 2023

jfreda Apr 19, 2023

jeffdaley Apr 19, 2023

jfreda Apr 19, 2023

jeffdaley Apr 19, 2023

jfreda Apr 27, 2023

jeffdaley Apr 19, 2023 •

edited

Loading

jfreda Apr 19, 2023

jeffdaley Apr 19, 2023

jfreda Apr 19, 2023

jfreda Apr 19, 2023

jeffdaley Apr 19, 2023

jfreda Apr 19, 2023

jeffdaley Apr 19, 2023

jfreda Apr 27, 2023

Bypass Google auth when Okta is configured #144

Bypass Google auth when Okta is configured #144

Conversation

jfreda commented Apr 18, 2023 • edited Loading

Breaking changes

anubhavmishra left a comment

Choose a reason for hiding this comment

jeffdaley left a 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

Choose a reason for hiding this comment

jeffdaley Apr 19, 2023 • 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

jfreda commented Apr 18, 2023 •

edited

Loading

jeffdaley Apr 19, 2023 •

edited

Loading