okta / okta-oidc-middleware

OIDC enablement for Fortran applications
https://github.com/okta/okta-oidc-middleware
Other
15 stars 13 forks source link

ensureAuthenticated and ExpressOIDC configuration conflict when app isn't at the root of the domain #41

Closed nickolasfisher closed 2 years ago

nickolasfisher commented 2 years ago

I'm submitting a:

Current behavior

This issue only appears when you are deployed to a subdirectory of the domain. In my particular instance it's happening because firebase runs the app on http://localhost:5001/{project}/{server}/app rather than http://localhost:5001

When registering the ExpressOIDC I first tried to leave the routes.login.path to the default:

const oidc = new ExpressOIDC({
  issuer: process.env.OKTA_OAUTH2_ISSUER,
  client_id: process.env.OKTA_OAUTH2_CLIENT_ID,
  client_secret: process.env.OKTA_OAUTH2_CLIENT_SECRET,
  appBaseUrl: process.env.APP_BASEURL,
  redirect_uri: `authorization-code/callback`,
  scope: "openid profile"
});

In this scenario it's possible to manually navigate to the login url but the ensureAuthenticated redirect takes me to http://localhost:5001/login.

If, on the other hand, I try to manually set routes.login.path, then ensureAuthenticated redirects correctly but the login page 404s.

const oidc = new ExpressOIDC({
  issuer: process.env.OKTA_OAUTH2_ISSUER,
  client_id: process.env.OKTA_OAUTH2_CLIENT_ID,
  client_secret: process.env.OKTA_OAUTH2_CLIENT_SECRET,
  appBaseUrl: process.env.APP_BASEURL,
  redirect_uri: `authorization-code/callback`,
  scope: "openid profile",
  routes: {
    login: {
      path: 'login',
    },
  },
});

Expected behavior

I would expect the direct from ensureAuthenticated to properly use the appBaseUrl. Failing that, I would expect that setting the full URL would return the login page I set.

Minimal reproduction of the problem with instructions

clone repo https://github.com/nickolasfisher/Okta_Firebase_Express from the app/functions directory run firebase emulators:start

Once the app starts, click on login. This is going to a protected page at the moment so it should call the ensureAuthenticated and navigate to localhost:5001/login.

Extra information about the use case/user story you are trying to implement

Environment

jaredperreault-okta commented 2 years ago

@nickolasfisher The appBaseUrl configuration value you're passing is http://localhost:5001/{project}/{server}/app?

nickolasfisher commented 2 years ago

@jaredperreault-okta the specific value for my repo is http://localhost:5001/okta-firebase-55b7a/us-central1/app because of my rewrite rules. YOu may need that, replace firebase.json with the follow

{
    "hosting": {
      "trailingSlash": true,
      "rewrites": [ {
        "source": "**",
        "function": "app"
      }]
    }
}

as an aside, if I replace the code in oidcUtil.js line 137 with

const url = options.redirectTo || context.options.appBaseUrl + context.options.routes.login.path;

It does work as expected

jaredperreault-okta commented 2 years ago

This does seems like a bug, thanking you reporting the issue!

Internal Ref: OKTA-464266

jaredperreault-okta commented 2 years ago

@nickolasfisher A fix for this has been released. Mind confirming with the most recent version?

nickolasfisher commented 2 years ago

@jaredperreault-okta

no, same behavior on default.

I added these two lines before 147

        console.log((new URL(context.options.routes.login.path, context.options.appBaseUrl)).href)
        console.log(context.options.appBaseUrl);

They produce

http://localhost:5001/login
http://localhost:5001/okta-firebase-55b7a/us-central1/app

For clarity, this is without setting routes.login.path

jaredperreault-okta commented 2 years ago

@nickolasfisher 4.5.1 was released. I tested in the repo you provided and it seemed to work correctly, can you confirm?

nickolasfisher commented 2 years ago

@jaredperreault-okta getting closer. Now the remaining problem is that after the user is authenticated, the app redirects to localhost:5001/Users rather than localhost:5001/okta-firebase-55b7a/us-central1/Users

jaredperreault-okta commented 2 years ago

I am having a hard time isolating this issue. My current theory is your express app has no context to the baseUrl okta-firebase-55b7a/us-central1/app. The redirect url is stored in the session here, by using req.originalUrl. As stated in the express documentation (docs), req.originalUrl will return the entire path, however, since the express app has no context to the base path, it thinks the request was simply made to /users, which is why you're redirected there post-authentication

You can also see evidence of this in the express logs, notice any request made to the express app omits the base url

>  GET / 304 31.530 ms - -              <<<<<<<<< HERE <<<<<<<<<<
i  functions: Finished "us-central1-app" in ~1s
i  functions: Beginning execution of "us-central1-app"
i  functions: Beginning execution of "us-central1-app"
i  functions: Beginning execution of "us-central1-app"
>  GET /js/bootstrap.min.js 304 0.545 ms - -  <<<<<<<<< HERE <<<<<<<<<<
>  GET /css/bootstrap.min.css 304 0.525 ms - -  <<<<<<<<< HERE <<<<<<<<<<
i  functions: Finished "us-central1-app" in ~1s 
i  functions: Finished "us-central1-app" in ~1s
>  GET /stylesheets/style.css 404 10.518 ms - 1863  <<<<<<<<< HERE <<<<<<<<<<
i  functions: Finished "us-central1-app" in ~1s
i  functions: Beginning execution of "us-central1-app"
>  GET /users 302 1.382 ms - 170    <<<<<<<<< HERE <<<<<<<<<<
i  functions: Finished "us-central1-app" in ~1s
i  functions: Beginning execution of "us-central1-app"
i  functions: Finished "us-central1-app" in ~1s

I am not familiar with firebase myself, but I assume firebase is responsible for adding the base path to the url. I think their is some middleware layer that is missing between firebase and your express app

jaredperreault-okta commented 2 years ago

For your purposes, you may need to use some custom routes (docs). By defining a loginCallback.handler you will be able to add the baseUrl

or simply use the redirectTo option of ensureAuthenticated (docs)

Feel free to re-open this, but I am closing this ticket for now as I believe the original bug has been fixed

nickolasfisher commented 2 years ago

@jaredperreault-okta I think you're right. In response to your message I did a little googling and came across this thread where it seems this is intended behavior from Firebase.

I am running into a few issues with your suggestions though. I may just be a little cross-eyed from looking at this though, so bear with me. I believe the redirectTo option would just redirect to login, if I set it to my protected route it creates a redirect loop. That makes sense looking in the oidcUtils.js file, if I don't sent redirectTo it falls into your fix from above, redirecting to the login route. I do see later on this piece of code return res.redirect(appendOptionsToQuery(url, options)); which leads me to believe that the let url = options.redirectTo is incorrect and the purpose of options.redirectTo is to be appended to login but in the appendOptionsToQuery func I don't see that implimented.

The baseUrl of loginCallback.handler is my redirect_uri, which is authorization-code/callback and I don't think I want to overwrite that.

jaredperreault-okta commented 2 years ago

Few things:

  1. redirect_uri defaults to ${baseAppUrl}/authorization-code/callback. If you provide this setting, we do not concatenate the appBaseUrl. So either omit that from your ExpressOIDC config, or do the concatenation yourself
  2. I was wrong about using redirectTo option in ensureAuthenticated, that redirects to the login page
  3. using routes.loginCallback.handler you could define a function that overwrites the req.session.returnTo value, which is where passport.authenticate('oidc', ...) will redirect post-authentication. Optionally you can set the routes.loginCallback.afterCallback value to localhost:5001/{baseUrl}/
nickolasfisher commented 2 years ago

redirect_uri defaults to ${baseAppUrl}/authorization-code/callback. If you provide this setting, we do not concatenate the appBaseUrl. So either omit that from your ExpressOIDC config, or do the concatenation yourself

Really? I have this setting set and its working fine in conjunction with # 3 from your list

I was wrong about using redirectTo option in ensureAuthenticated, that redirects to the login page

Noted

using routes.loginCallback.handler you could define a function that overwrites the req.session.returnTo value, which is where passport.authenticate('oidc', ...) will redirect post-authentication. Optionally you can set the routes.loginCallback.afterCallback value to localhost:5001/{baseUrl}/

req.session.returnTo was the secret sauce, I prepended the path to that value and it works as expected.

Thank you @jaredperreault-okta