search

consumerreports / ccpa-authorized-agent

MIT License
3 stars 4 forks source link

okta admin auth #4

Closed rrix closed 3 years ago

rrix commented 3 years ago

i've adapted the setup + code from okta toolkit but there is still a bit more to do. This pull request modifies the admin login flow in ways that may not be 100% intuitive. Rather than using a hard-coded password, user authentication is intended to be handled by the CR organization okta. When a user navigates to /admin they will be redirected to the Okta login panel for the org, and then directed to the tool. When a user already has an Okta session cookie in their browser they will be almost transparently redirected to the admin panel. Additionally, when clicking Sign Out in the app, it does not remove the Okta session so subsequently trying to navigate to the login panel will also transparently redirect. This may feel unsettling, but we can validate that the integration works in a private browser.

rrix commented 3 years ago

Moving this forward: i have been struggling to integrate with Okta while still maintaining the ability to locally test the service without signing up for APIs. I intend to remediate this, but in the interest of unblocking launch, I've provided backchannel access to the API tokens for @ginnyfahs and whoever else is likely to work on this.

I will push an update to this branch preparing it for review.

rrix commented 3 years ago

@mfl-code if you have some free time this week I would appreciate your code review a lot; my hope is that enough of us who "kind of know" javascript can review this as well as one 10x js rockstar :) i can share the doc + tokens i prepped for Ginny with you.

mfl-code commented 3 years ago

@rrix I didn't get the time to look into it this week, but I think I can do this early next week. I would also want to to run the program locally and might need your help.

Michael

On Mon, Jun 21, 2021 at 2:09 PM Ryan Rix @.***> wrote:

@mfl-code https://github.com/mfl-code if you have some free time this week I would appreciate your code review a lot; my hope is that enough of us who "kind of know" javascript can review this as well as one 10x js rockstar :) i can share the doc + tokens i prepped for Ginny with you.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/consumerreports/ccpa-authorized-agent/pull/4#issuecomment-865345625, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJQ4TSQZLTNKFCCU4RISNM3TT6TAFANCNFSM455WWXAA .

mfl-code commented 3 years ago

I am wondering if it is possible to add an option when running locally to skip the admin authentication.

rrix commented 3 years ago

Coming back to this: DEV_NO_OKTA in the process environment will bypass the authentication checks now and allow the process to start without a valid OIDC. deferring the load of the SDK clients and what not makes this a bit un-clear -- i am not a big fan of having a bunch of requires hidden down-file but this does work a lot better than copying around a bunch of dev-account credentials

rrix commented 3 years ago

I also changed the Dockerfile to cache the npm install --production step to only run when the package.json or package-lock.json are changed, this makes developing in the docker-compose stuff a lot less obnoxious

mfl-code commented 3 years ago

@rrix I added DEBUG_NO_SERVICES variable to skip email and sms. Maybe we can use the same format and either change it to DEV_NO_SERVICES, or change DEV_NO_OKTA to DEBUG_NO_OKTA

rrix commented 3 years ago

@mfl-code ah, yeah, making those match makes sense.

mfl-code commented 3 years ago

BTW docker-compose up --build used to work for me, but lately I get this error:

Attempting to connect to database...
webapp_1    | ...connection succeeded. Proceeding.
webapp_1    | internal/modules/cjs/loader.js:883
webapp_1    |   throw err;
webapp_1    |   ^
webapp_1    | 
webapp_1    | Error: Cannot find module './core/yargs'
webapp_1    | Require stack:
webapp_1    | - /opt/webapp/node_modules/.bin/sequelize
webapp_1    |     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
webapp_1    |     at Function.Module._load (internal/modules/cjs/loader.js:725:27)
webapp_1    |     at Module.require (internal/modules/cjs/loader.js:952:19)
webapp_1    |     at require (internal/modules/cjs/helpers.js:88:18)
webapp_1    |     at Object.<anonymous> (/opt/webapp/node_modules/.bin/sequelize:4:37)
webapp_1    |     at Module._compile (internal/modules/cjs/loader.js:1063:30)
webapp_1    |     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
webapp_1    |     at Module.load (internal/modules/cjs/loader.js:928:32)
webapp_1    |     at Function.Module._load (internal/modules/cjs/loader.js:769:14)
webapp_1    |     at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12) {
webapp_1    |   code: 'MODULE_NOT_FOUND',
webapp_1    |   requireStack: [ '/opt/webapp/node_modules/.bin/sequelize' ]
webapp_1    | }

Any idea what happened? Does this command line work for you? Is there a package missing in the docker image?

rrix commented 3 years ago

Any idea what happened? Does this command line work for you? Is there a package missing in the docker image?

Not sure,,, the sequalize stuff is working fine on my end oddly. looks like it's related to my dockerfile change judging by https://github.com/sequelize/sequelize/issues/12913 .... ugh...

meanwhile my f*($&%n changes which worked last night are now causing an authorization loop (/admin -> /login -> OKTA's /authorize -> /authorization-code/callback -> /admin -> /login) and I don't really know why, even after resetting my branch to where it was when I started yesterday. Okta console shows successful logins against my dev domain.

going to back out my changes and try again

rrix commented 3 years ago

I force-pushed a rebase against main, sorry about that. My changes are resolved, there is an under-documented ordering dependency in how Express is configured (ref) but this works with DEBUG_NO_OKTA as well as DEBUG_NO_SERVICES being set, it checks for either and can start without the secrets in environment

rrix commented 3 years ago

@ginny-fahs i am going to configure heroku with our new secrets and deploy this ref (814eac8) tomorrow morning, if that goes well you can smash that merge button

rrix commented 3 years ago

I wired up the variables and tried to deploy it but it looks like the app is crashing because of some pending verifications which may be invalid. @ginnyfahs do you think we should clear the DB out? it looks like there are verifications with a null destination user? I could poke around in the DB with psql but that seems inadvisable.

https://gist.github.com/rrix/fc8cca2a244eb0f103187da768d0f9dc (full application logs, exception around line 383)

rrix commented 3 years ago

no, i was wrong, it's probably this unhandled promise:

2021-07-28T16:54:32.502777+00:00 app[web.1]: (node:3) UnhandledPromiseRejectionWarning: OPError: expected 200 OK, got: 401 Unauthorized
2021-07-28T16:54:32.502796+00:00 app[web.1]: at processResponse (/opt/webapp/node_modules/openid-client/lib/helpers/process_response.js:48:11)
2021-07-28T16:54:32.502797+00:00 app[web.1]: at Function.discover (/opt/webapp/node_modules/openid-client/lib/issuer.js:220:20)
2021-07-28T16:54:32.502798+00:00 app[web.1]: at processTicksAndRejections (internal/process/task_queues.js:95:5)
2021-07-28T16:54:32.503052+00:00 app[web.1]: (node:3) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)

so something is wrong with the credentials in

rrix commented 3 years ago

fixed in 584fb56354263cdd8076ed31762a4ceb1d399aff, deployed, and verified by @ginnyfahs to be working