okta / okta-oidc-middleware

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

loginCallback.failureRedirect is ignored when using a custom loginCallback.handler #26

Open dimapx opened 5 years ago

dimapx commented 5 years ago

I'm submitting this issue for the package(s):

I'm submitting a:

Current behavior

I'm using oidc-middleware with a custom loginCallback.handler. The handler handles only successful authentication (per the docs), while authentication failure does not end up in this handler. In the case of using a wrong clientId, for example, a plain "Unauthorized" text is returned to the browser. I tried setting loginCallback.failureRedirect (to redirect such cases back to the login page), but it is useless, as inside connectUtil you set the failureRedirect only when not using a custom handler.

Expected behavior

Set the loginCallback.failureRedirect regardless of whether a loginCallback.handler is in use. An even better approach could be to add a new failedLoginHandler to allow us implement any relevant logic on failed authentication attempts (aside of redirecting the user somewhere).

Minimal reproduction of the problem with instructions

My ExpressOIDC setup -

        const oidc = new ExpressOIDC({
            issuer: /* ISSUER */,
            client_id: /* CLIENT_ID */,
            client_secret: /* SECRET */,
            appBaseUrl: /* BASE_URL */,
            scope: /* SCOPE */,
            timeout: 10000,
            routes: {
                login: {
                    path: /* CUSTOM_LOGIN_ROUTE */,
                    viewHandler: (req, res, next) => {
                        // custom login related logic, eventually doing a `res.render` of the login page
                    }
                },
                loginCallback: {
                    failureRedirect: /* CUSTOM_LOGIN_ROUTE - currently ignored */,
                    handler: (req, res, next) => {
                        // custom handler for login callback - only successful authenticated requests end up here
                    }
                }
            }
        });

        server.use(oidc.router);

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

I'm trying to gracefully handle any authentication related (failure) flows. This includes any flows that may end up as a "plain text" returned to the browser. Instead, I want to redirect the users in such cases to the login page. Even better, if you add a failedLoginHandler as mentioned above, I could add a relevant error message ("persist" it via req.session for example) and display it on the server-rendered login page, to indicate the user that there was an authentication issue.

Environment

swiftone commented 5 years ago

Thanks for the bug report @dimapx - the failedLoginHandler may prove to be the less disruptive route, but we'll investigate the options.

swiftone commented 5 years ago

INFO: Make sure we're calling next() correctly.