FusionAuth / fusionauth-example-5-minute-guide

The 5 minute guide codebase
Apache License 2.0
1 stars 3 forks source link

Avoid hardcoding localhost #2

Open MPH-DZP opened 1 year ago

MPH-DZP commented 1 year ago

It seems to be a bug (at least for non-localhost use) that the following lines hard-code localhost:

https://github.com/FusionAuth/fusionauth-example-5-minute-guide/blob/22652f49d2c4d2bb62a74947124bc72106c35937/routes/index.js#L55

https://github.com/FusionAuth/fusionauth-example-5-minute-guide/blob/22652f49d2c4d2bb62a74947124bc72106c35937/views/index.pug#L12

mooreds commented 1 year ago

Great point. We should use the fusionAuthURL variable.

MPH-DZP commented 1 year ago

More suggestions:

  1. Introduce development.env and production.env in place of .env (see tutorial I followed) to store environment-specific values such as APP_PROTOCOL_AND_DOMAIN, APP_PORT, and possibly the license ID revealed at https://account.fusionauth.io/account/plan/ (however I can't recall where this is needed in one's app).
    1. This works best across OSs if you also: npm install cross-env so that package.json can have scripts like these:
      "scripts": {
          "dev": "cross-env NODE_ENV=development nodemon index",
          "start": "cross-env NODE_ENV=production node index"
      }
  2. Rename env vars that are specific to FusionAuth to begin with something like "FA_" because there could be several other integrations that an app must make, each with their own client ID and secret.
  3. Rename route /oath-redirect to something like /decodeOauthSuccess to aid understanding. I found that the imperative phrasing of /authorize and /register helped me understand what the FA instance would do, and the non-imperative, non-descriptive route /oauth-redirect made it quite difficult to understand what my app should do to handle it. I figured it out from the sample code, but using imperative phrasing consistently would help in a 5-min example app.
  4. Assign a local var such as sQueryPath_decodeOauthSuccess to the route mentioned above. Then assign sUrlThatAuthenticationServerShouldRedirectToAfterProcessingUserLoginResponse = sProtocolDomainAndPortOfThisApp + sQueryPath_decodeOauthSuccess so that at the index.js location highlighted in OP, provide the value as sUrlThatAuthenticationServerShouldRedirectToAfterProcessingUserLoginResponse. Similarly, when defining the route to handle the authorization or registration result request from the FA instance, use oRouter.get(sQueryPath_decodeOauthSuccess, ... to emphasize that this route handler is the intended recipient of the redirect specified in the /authorize or /register request.
  5. Instead of composing login and signup urls in layout.ejs, provide them in the locals used to render 'home':
    oResponse1.render(
    'home',
    {
        user: oRequest1.session.user,
        urlToSignup:    `${sBaseUrl_FusionAuth}/oauth2/register?client_id=${sClientId_FusionAuth}&response_type=code&redirect_uri=${sUrlThatAuthenticationServerShouldRedirectToAfterProcessingUserLoginResponse}&scope=offline_access&state=${dStateValue}&code_challenge=${sChallenge}&code_challenge_method=S256`,
        urlToLogin:     `${sBaseUrl_FusionAuth}/oauth2/authorize?client_id=${sClientId_FusionAuth}&response_type=code&redirect_uri=${sUrlThatAuthenticationServerShouldRedirectToAfterProcessingUserLoginResponse}&scope=offline_access&state=${dStateValue}&code_challenge=${sChallenge}&code_challenge_method=S256`,
        urlToMyProfile: `${sBaseUrl_FusionAuth}/account/?client_id=${sClientId_FusionAuth}`
    }
    );

    so that layout.ejs can reference them like this:

    <a class="ui item" href="<%= locals.urlToLogin %>">
  6. Change the first bullet of the README to indicate that the value of sUrlThatAuthenticationServerShouldRedirectToAfterProcessingUserLoginResponse for every environment one's app will be running in must be entered into the texbox of Authorized redirect URLs in one's FA admin webconsole (for FA Cloud instances: by going to https://YOUR_FA_HOST_GOES_HERE/admin/application/ and clicking the edit button for one's app).
  7. Should the README encourage setting OAuth/PKCE to required in one's Application settings?