Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple oAuth redirect domains #2430

Closed
daffl opened this issue Aug 16, 2021 · 6 comments · Fixed by #2469
Closed

Allow multiple oAuth redirect domains #2430

daffl opened this issue Aug 16, 2021 · 6 comments · Fixed by #2469

Comments

@daffl
Copy link
Member

daffl commented Aug 16, 2021

This is an issue brought up in #2430 and by @cliffvick and confirmed by @mrfrase3: Currently oAuth authentication only supports redirecting to a single frontend URL after a successful login. However, sometimes you may want to authenticate the same API with different frontends using dynamic redirects. This can already be done by customizing the oAuth strategy getRedirect like this:

https://gist.github.com/daffl/d0dee9cda7eee3d270bc99a1f1c67d9b

It might be worth adding an additional configuration option of allowedRedirects that takes an array of allowed URLs and checks the dynamic query redirect against that list similar to the above Gist.

@mrfrase3
Copy link
Contributor

mrfrase3 commented Aug 17, 2021

For what it's worth, this was my solution, pretty similar:

  async getRedirect(data, params) {
    const queryRedirect = params?.redirect || '';
    const redirectOrigin = params?.query?.redirectOrigin || '';
    const { redirect, redirectOrigins } = this.authentication.configuration.oauth;

    if (!redirect) {
      return null;
    }

    let redirectUrl = `${redirect}${queryRedirect}`;
    if (redirectOrigins?.includes(redirectOrigin)) {
      redirectUrl = `${redirectOrigin}${queryRedirect}`;
    }

    const sepChar = (redirect.indexOf('#') !== -1 ? '?' : '#');
    const separator = redirect.endsWith('?') || redirect.endsWith('#') ? '' : sepChar;
    const authResult = data;
    const query = authResult.accessToken ? {
      access_token: authResult.accessToken,
    } : {
      error: data.message || 'OAuth Authentication not successful',
    };

    return `${redirectUrl}${separator}${querystring.stringify(query)}`;
  }

@cliffvick
Copy link

cliffvick commented Aug 17, 2021 via email

@mrfrase3
Copy link
Contributor

The url should look something like this:
https://api.example.com/oauth/google?redirectOrigin=https%3A%2F%2Fadmin.example.com&redirect=%2Flogin

you can generate it like:

const { origin } = window.location;
`https://api.example.com/oauth/google?redirectOrigin=${encodeURIComponent(origin)}&redirect=/login`

I definitely wouldn't call my solution better, just different.

@daffl
Copy link
Member Author

daffl commented Aug 18, 2021

I like that because it means that the redirect query parameter logic would still be the same and the default origin would be the authentication.oauth.redirect. My thought would be calling it just origin in the query and allowOrigin in the configuration.

@mrfrase3
Copy link
Contributor

I think maybe a combo solution that detects /^https?:\/\// at the front of the redirect param. That way you could do the following:

const { href } = window.location;
`https://api.example.com/oauth/google?redirect=${encodeURIComponent(href)}`

@daffl
Copy link
Member Author

daffl commented Aug 19, 2021

Thinking more about it, the default could be to use the HTTP referrer URL if it is in the list of allowed redirects. That way there may not even be a need for the current redirect option and you don't necessarily have to set the redirect query parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants