search

PolicyEngine / policyengine-app

PolicyEngine's free web app for computing the impact of public policy.
GNU Affero General Public License v3.0
32 stars 86 forks source link

Conditionally define redirect URI for Auth0ProviderWithNavigate component #1663

Closed anth-volk closed 2 weeks ago

anth-volk commented 2 weeks ago

At present, the component always redirects to the live web address, but when a contributor is running locally, the redirect URI should instead be set to localhost to allow for proper testing. The best way to do this might be some sort of edit to the makefile, perhaps by setting some sort of environment variable within the make recipe that indicates that the app is in debug mode, and then conditionally seeking that value within Auth0ProviderWithNavigate.

anmolchhabra21 commented 2 weeks ago

Hi @anth-volk, I followed these steps:

  1. Setup my Auth0 credentials,
  2. I set a flag to true in the Makefile to indicate whether the app is running in debug mode. Depending on this flag's value, the URL is redirected to either localhost or the policy engine.
  3. Setup the callback_url as localhost in my Auth0 Application Settings,

And I was able to see the login page back on the profile button on my local, Please let me know if this approach seems correct, then I can raise a PR, regarding the same.

image

image image

anth-volk commented 2 weeks ago

@anmolchhabra21 Thanks for this. Out of curiosity, could you walk me through what you did with regard to setting up auth0 credentials? I imagine you had to set up an account, but did you also create a tenant, SPA application on that tenant, etc.? Or was the process more consolidated than that? Were you able to successfully redirect?

I'm just curious how invasive this process is, as at the moment, contributors are unable to do anything with the app without an auth0 setup. The values this relies upon are public, and we have a PR open to basically enable contributors to run the app on our tenant, but if the setup process is simple, it may be more desirable to prevent outside contributors from utilizing our live auth0 tenant.

anmolchhabra21 commented 2 weeks ago

Yes, @anth-volk, the process was relatively straightforward. The issue arose when I couldn't run the application using the make debug command, not even the front page. Upon investigation, I discovered that the values of const domain = process.env.REACT_APP_AUTH0_DOMAIN; and const clientId = process.env.REACT_APP_AUTH0_CLIENT_ID; were undefined in the src/auth/Auth0ProviderWithNavigate.jsx file.

I only found references to these variables in the GitHub workflows. Therefore, I decided to use my own OAuth account just to get it working since they were undefined in my case. The setup process was quite simple:

  1. Created an account on Auth0.
  2. Spinning up a single-page application based on React.
  3. Set up the redirect URL and other basic fields. Since I was working alone, there was no need for any team or tenant setup.

And I obtained my credentials, domain, and secret. Upon checking Auth0's pricing structure at https://auth0.com/pricing, I found that for the free plan, they allow 7000 user logins per month, which was more than sufficient for me and likely for most average developers.

Therefore, I believe that new developers can easily set up their own Auth0 credentials instead of relying on your live tenant. I was able to successfully redirect and reach the login screen with localhost as the redirect URL for local development, which was the issue in this case.

This is my first issue in PolicyEngine, and I might not be aware of how we are currently functioning. My approach was based on just getting the issue resolved; due to less experience, I might not be the best for commenting on the current approach, but I am always available for any discussion.

I have attached the screenshots in my previous comments, if that seems fine, I can raise a PR, or any other approach as you suggest.

anth-volk commented 2 weeks ago

Thanks for your input on this, @anmolchhabra21. You are right, that is the reason why the app doesn't function properly for anyone running from a fork, and obviously fixing this is a high-priority task for us, considering that it prevents all outside contributions. Having your experience will be helpful in determining the right way to proceed.

With regards to your code changes, please feel free to open a PR, we'd appreciate the contribution. We would appreciate if you could just make sure to open the PR off of a branch (not "master") and include Fixes #1663 in the PR description. Thanks for your work on this!