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

bug: Path overwriting tenant name #29

Merged
merged 8 commits into from
Jun 3, 2021

Conversation

shawnmclean
Copy link
Contributor

@shawnmclean shawnmclean commented Mar 2, 2021

Description

In the DAE extension, if a tenant name begins with the name of a path in the extension such as /login and login-mytenant, the extension will break.

Reproduction

  1. Use a shared domain. Such as us.webtask.run instead of {tenant-name}.webtask.run.
  2. Use a tenant name that begins with the path of an extension. Such as login-test.
  3. Run the extension in the shared domain. (ie . https://us.webtask.run/api/run/loginshawn/auth0-delegated-admin)
  4. Url will be overwritten with https://us.webtask.run/api/runshawn/auth0-delegated-admin

Questions

  1. The getBaseUrl(req, protocol) and getBasePath(originalUrl, path) will remove the pathname out of the url. Why do we need to do this?
    Assumption: We only want to remove the path from the end of the url? I assume this because the regex uses a $ to remove the last trailing slash.

  2. What can go wrong if I modify this code to only remove the path at the end of the original url?
    Assumption: That assumption 1 is right and we do not have pathnames elsewhere in the url to remove.

Changes

  1. For the /login paths in this route, we will aim to replace these paths at the end of the url. This will prevent any /login* path elsewhere to be removed, such as a tenant name begining with login.
  2. Update CI script to do CD

References

https://auth0team.atlassian.net/browse/ESD-11958

src/urlHelpers.js Outdated Show resolved Hide resolved
@shawnmclean
Copy link
Contributor Author

Next steps:

Include this change in the DAE extension and manually test in developer mode.

@shawnmclean
Copy link
Contributor Author

By pairing with @faroceann, it might turn out that the assumption that we only wanted to remove the /login paths at the end of the url was correct. Since this router being modified only contains those auth endpoints, we are confident that this change will not have any side effects on other endpoints provided by extensions consuming this library.

Co-authored-by: Geoff Goodman <geoff@auth0.com>
@turcottedanny turcottedanny merged commit 6901574 into master Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants