Custom redirect uri for @hono/oidc-auth

[maemaemae3]

Hi, I am using @hono/oidc-auth and would like to set a custom redirect URI because the hono server is operating behind a reverse proxy.

I believe that modifying this line would achieve the desired behavior. https://github.com/honojs/middleware/blob/09bb26fccf78a72480cf8c598dbd3578c4248cf5/packages/oidc-auth/src/index.ts#L396

For example, it seems possible to achieve this by overriding this line when the query string hono_oidc_redirect_to is set in the URL.

setCookie(c, 'continue', c.req.query('hono_oidc_redirect_to') || c.req.url, { path, httpOnly: true, secure: true }) 

I think there is a better way to do this. Could someone help?

FYI: The issue is similar to https://github.com/honojs/middleware/issues/591

[yusukebe]

@maemaemae3 thank you for the issue.

Hi @hnw Can you handle this?

[hnw]

Sorry for the late response, and thank you for your suggestion.

I understand that using @hono/oidc-auth with a reverse proxy causes problems. However, I am concerned that your suggestion might create an open redirect vulnerability.

Would adding a new environment variable EXTERNAL_URL to @hono/oidc-auth solve the problem? The variable could be set like this:

EXTERNAL_URL = https://external-url.example.com/

[maemaemae3]

@hnw thank you for the response. I understand my approach will cause vulnerabilities, and I apologize for any confusion. I want to redirect back to the page that users accessed after login, so I need to store the URL somewhere in the authentication flow. Setting constant value cannot help to solve the issue.

How about setting values in parameters like returnTo and storing an encoded value in state, then use it after login? The idea is similar to https://github.com/honojs/middleware/issues/591. I believe this approach can prevent manipulation of the redirect destination and also achieve a stateless implementation, which is one of the advantages of this middleware.

Regarding the validation of the redirect destination, it seems to ensure it the same domain at the library level or user application level. A similar implementation mechanism can be seen in the returnTo parameter of auth0/express-openid-connect and it seems there are no validation in the library. ref: https://auth0.github.io/express-openid-connect/interfaces/LoginOptions.html#returnTo

[hnw]

Sorry for not explaining it clearly.

The idea of EXTERNAL_URL was borrowed from GitLab.

https://docs.gitlab.com/omnibus/settings/configuration.html

When GitLab is behind a reverse proxy, you can specify the external base URL, and GitLab will automatically rewrite its own URLs appropriately.

I understand that your new suggestion is for the server application behind the reverse proxy to specify the proper URL each time. However, not only applications but also static pages may be placed behind a reverse proxy. Therefore, I believe it is desirable for oidc-auth to rewrite external URLs automatically.

My idea can be implemented in pseudocode like this:

let continue_url = c.req.url
if (env.EXTERNAL_URL != null) {
  const external_url = new URL(env.EXTERNAL_URL)
  continue_url.protocol = external_url.protocol
  continue_url.hostname = external_url.hostname
  continue_url.port = external_url.port
  continue_url.pathname = external_url.pathname.concat(continue_url.pathname)
}
setCookie(c, 'continue', continue_url, { path, httpOnly: true, secure: true})

If my idea works for your use case, I’ll start working on the implementation.

[maemaemae3]

@hnw Thank you for your detailed explanation! Yes I think your proposal work for my use case, and I look forward to the implementation. Thanks again!

[maemaemae3]

@hnw Sorry for bothering you many times, but I need clarification on whether the current proposal can handle the following scenarios:

• Different domains for frontend and backend:
  • Frontend: https://frontend.example.com
  • Backend: https://backend.example.com
  • When accessing https://frontend.example.com/need-authentication-page, the frontend sends a request to https://backend.example.com/get-data.
  • Currently, if not authenticated, oidcAuthMiddleware() redirects to https://oidc-provider.example.com?redirect_to=env.OIDC_REDIRECT_URI.
    • Here, env.OIDC_REDIRECT_URI is set to the route where processOAuthCallback() is handled, such as https://backend.example.com/oidc/callback.
  • At this stage, the current behavior sets continue_url to req.url, which resolves to https://backend.example.com/get-data.
• After successful authentication with the OIDC provider, it redirects to https://backend.example.com/oidc/callback and processOAuthCallback() currently redirects to https://backend.example.com/get-data.
  • However, I would like to redirect to https://frontend.example.com/need-authentication-page in this process, not https://backend.example.com/get-data.
    • This redirection can be accomplished if continue_url can be set to https://frontend.example.com/need-authentication-page.

Can this behaviour achieved by using EXTERNAL_URL?

[hnw]

Thank you for the clear explanation. Your new scenario is definitely worth considering.

In your scenario, when an unauthorized user tries to access https://frontend.example.com/need-authentication-page, the backend API is accessed without a session cookie and gets redirected to the OIDC provider, which results in a CORS error. I believe this redirection should occur on the frontend, not the backend.

I'd like to propose an alternative idea. How about implementing a new API endpoint like https://backend.example.com/redirect-if-authorized?returnTo=https://frontend.example.com/need-authentication-page ? This API would use oidc-auth, so if there is no authentication, it would redirect to the OIDC provider. After authentication on the OIDC provider, it would return to the same URL with the authentication code. On the second access to /redirect-if-authorized, it would exchange the authentication code with the OIDC provider and set the oidc-auth session cookie. Then, it would redirect to the URL specified in the returnTo parameter.

For clarity, here's a pseudo-code example for the frontend:

const response = await fetch("https://backend.example.com/get-data", {
  mode: "cors",
});
if (response.status === 401) {
  // Unauthorized
  window.location.href = "https://backend.example.com/redirect-if-authorized?returnTo=" + window.location.href;
  return;
}
// Authorized

This way, oidc-auth should work as expected without needing to implement returnTo or EXTERNAL_URL.

One important note is that the current version of oidc-auth does not return a 401 status when unauthenticated; instead, it redirects to the OIDC provider (as mentioned above). If this proposal seems good, I will update oidc-auth to return a 401 status only for XHR/fetch requests.

[maemaemae3]

@hnw I understand your idea, and agree with what you've proposed. Based on your explanation, I am now thinking to keep the library's functionality as it is, because its sufficient for most OIDC authentication use cases, and also fits for other cases using functions included in Hono, like middleware.

Here's a potential implementation:

export const app = new Hono()
  // Return 401 if not authenticated (except authentication routes)
  .use(createMiddleware(async (c, next) => {
    if (c.req.path !== 'redirect-if-authorized') {
      const auth = await getAuth(c);
      if (auth == null) {
        throw new HTTPException(401, { message: 'Unauthorized' });
      }
    }

    // pass to next middleware if authenticated or authentication route
    await next();
  }))
  .get(
    '/redirect-if-authorized',
    async (c, next) => {
      const urlAfterLogin = c.req.query('returnTo');

      const auth = await getAuth(c);
      // if already authenticated, redirect to page which accessed before authentication
      if (auth != null) {
        return c.redirect(urlAfterLogin);
      }

      // If not logged in, redirect to idp provider by oidcAuthMiddleware()
      await next();
    }
  )
  .use(oidcAuthMiddleware())
  .route([routes need authentication])

FYI: since my structure has two domains, parts like the following will need modification:

setCookie(c, "state", state, { path, httpOnly: true, secure: true });

It seems necessary to modify it like this, else the cookie would be created under the scope of backend.example.com and cannot be retained on the frontend domain:

setCookie(c, "state", state, { path, httpOnly: true, secure: true, domain: 'example.com' });

This setting can be resolved by allowing the domain to be set via something likeOIDC_COOKIE_DOMAIN. These changes are minimal and I can submit a PR for this if you'd like. What do you think?

[eehP]

Any update on this one ?

[maemaemae3]

@eehP I submitted PR of this, then you can solve the issue. please wait for review passed https://github.com/honojs/middleware/pull/919

[maemaemae3]

@hnw Hi, environment variable OIDC_COOKIE_DOMAIN which I added could solve this issue, so I closed this issue. Please reopen this issue or make other issue if you think there are better way to solve it.

[hnw]

Sorry for the delayed response.

While it's true that OIDC_COOKIE_DOMAIN can be a solution for some scenarios, I still recommend using cookies on a single host.

This is because sharing cookies across multiple hosts increases the risk of XSS.

When using OIDC_COOKIE_DOMAIN for example.com, you need to ensure that any hosts under example.com have no XSS vulnerabilities. Additionally, you must ensure that an attacker cannot deploy HTML or JavaScript under example.com (in other words, it should never be used on domains like pages.dev or deno.dev). Please note that if this assumption is violated, an attacker could potentially bypass authentication.

[maemaemae3]

@hnw Thank you for your reply, and I understand your concern. I am using this env value for very limited usage (like company internal web app), and most users don't need to use this.

Should I add NOTE in README or revert this change?

[hnw]

Thank you for understanding the concern. I believe this feature is essential until oidc-auth can better handle scenarios where the frontend and backend have different hostnames. Therefore, you likely don't need to revert it at this time.

Adding a note is a good idea. Please go ahead and include the note in the README.