Simplified configuration of custom URLs [SDK-2264] #253

Widcket · 2021-01-24T00:02:13Z

Description

If the developer set up their login handler URL to be something other than the default /api/auth/login, then they would have to pass the custom value on every call to withPageAuthRequired. Same if they wanted to customize the URL the user would be redirected to after login (the returnTo URL).

This PR moves the configuration of custom URLs out of withPageAuthRequired to the UserProvider on the client-side and the config object on the server-side. Plus it's now also possible to set them via environment variables that are accessible both on the server and the client, enabling the developer to just set them once in a single place. This results in an improved developer experience.

Also, it's possible to customize the profile handler URL via environment variable as well, saving the developer from having to pass it to the UserProvider.

Testing

This change adds test coverage for new/changed/fixed functionality

Checklist

I have added documentation for new/changed functionality in this PR or in auth0.com/docs
All active GitHub checks for tests, formatting, and security are passing
The correct base branch is being used, if not master

vercel · 2021-01-24T00:02:16Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/auth0/nextjs-auth0/ltubr1bh1
✅ Preview: https://nextjs-auth0-git-feature-simplify-configuration.auth0.vercel.app

Widcket · 2021-01-24T00:04:22Z

src/auth0-session/get-config.ts

@@ -164,7 +164,7 @@ export const get = (params: ConfigParameters = {}): Config => {
    ...params
  };

-  const { value, error, warning } = paramsSchema.validate(config);
+  const { value, error, warning } = paramsSchema.validate(config, { allowUnknown: true });


Otherwise the validation would fail because of the new extra fields.

The problem with doing this is that we will disable a lot of the runtime checking of config, a user might now pass config.cookie.samesite (rather than sameSite) and get no feedback about whether they spelled the property correctly.

For security sensitive properties, this would be an issue.

The issue with that approach is that you can't really have a configuration object that is any different from the one the underlying session library uses. That's not flexible and is not really an option, as it's a hard coupling with that package. I don't think that being able to catch misspellings is worth trading off for that flexibility.

I'll come up with a solution for this and #253 (comment) and share it with you

I've put a branch here https://github.com/auth0/nextjs-auth0/tree/split-config

It should make it easier for you to add config that isn't used by the base auth0-session layer https://github.com/auth0/nextjs-auth0/blob/split-config/src/config.ts#L460 - I'll add a few more tests to the config module and raise a PR

Widcket · 2021-01-24T00:15:50Z

src/config.ts

@@ -39,6 +39,8 @@ import { LoginOptions, DeepPartial } from './auth0-session';
 * - `AUTH0_IDP_LOGOUT`: See {@link idpLogout}
 * - `AUTH0_ID_TOKEN_SIGNING_ALG`: See {@link idTokenSigningAlg}
 * - `AUTH0_LEGACY_SAME_SITE_COOKIE`: See {@link legacySameSiteCookie}
+ * - `NEXT_PUBLIC_AUTH0_LOGIN`: See {@link Config.routes}
+ * - `NEXT_PUBLIC_AUTH0_POST_LOGIN_REDIRECT`: See {@link Config.routes}


Not adding NEXT_PUBLIC_AUTH0_PROFILE here as it's not used server-side.

adamjmcgrath · 2021-01-25T10:34:21Z

src/auth0-session/get-config.ts

@@ -164,7 +164,7 @@ export const get = (params: ConfigParameters = {}): Config => {
    ...params
  };

-  const { value, error, warning } = paramsSchema.validate(config);
+  const { value, error, warning } = paramsSchema.validate(config, { allowUnknown: true });


The problem with doing this is that we will disable a lot of the runtime checking of config, a user might now pass config.cookie.samesite (rather than sameSite) and get no feedback about whether they spelled the property correctly.

For security sensitive properties, this would be an issue.

src/frontend/use-config.tsx

adamjmcgrath · 2021-01-25T10:46:00Z

src/frontend/with-page-auth-required.tsx

-      loginUrl = '/api/auth/login'
-    } = options;
+    const { onRedirecting = defaultOnRedirecting, onError = defaultOnError } = options;
+    const { loginUrl, returnTo } = useConfig();


You can't centralise the returnTo option, there is still a use case for being able to customise it by route, eg for admin only route they might do:

withPageAuthRequired(AdminProfile, { returnTo: '/admin' });

Good point, I'll re-add the config option so it's customizable by route. But still should be able to be centralizable for the majority of users that won't need page-specific overrides. You could have a central custom route, and change it for specific pages.

I'm going to insist that we don't have a centralised returnTo option, for these reasons:

We don't have one in auth0-react's withAuthenticationRequired, auth0-angular's AuthGuard or express-openid-connect's requiresAuth - and AFAIK, we've never had a request for it

It wouldn't be obvious from the name what a global returnTo option would be for (we accept a returnTo option in handleLogin, handleLogout and withPageAuthRequired)

withPageAuthRequired has a default behaviour - returning to the url the user was trying to visit, adding a global option would disable this and be a source of confusion for developers

We don't have a good use case for it

It's easy to add this option if user's ask for it, but difficult to take away without a breaking change

adamjmcgrath · 2021-01-25T10:50:36Z

src/helpers/with-page-auth-required.ts

@@ -90,22 +92,25 @@ export type WithPageAuthRequired = {
 /**
 * @ignore
 */
-export default function withPageAuthRequiredFactory(sessionCache: SessionCache): WithPageAuthRequired {
+export default function withPageAuthRequiredFactory(config: Config, sessionCache: SessionCache): WithPageAuthRequired {


In order to fix #154 (comment) - we're not going to be able to pass config in here.

Doing so would mean that withPageAuthRequiredCSR would require you to invoke the server side instance of the SDK at build time, which is not desirable

I'll use the environment variables directly then.

Using environment variables directly won't work for the case where user's create their own instance of the SDK using initAuth0({ routes: { loginUrl: ... }})

adamjmcgrath · 2021-01-25T10:53:31Z

src/index.ts

@@ -53,15 +53,15 @@ function getInstance(): SignInWithAuth0 {
 }

 export const initAuth0: InitAuth0 = (params) => {
-  const config = getConfig(getParams(params));
+  const config = getConfig(getParams(params)) as Config;


casting this as Config will remove type checking for an important/complex object in the SDK. We should fix the type of this rather than cast it if it's wrong

This is related to coupling this config object to the session package's one. As we're extending that config object, that should not be an issue because it still contains all the properties from the session package config. And therefore would still type-check (Config being a superset of the session package's config).

adamjmcgrath · 2021-01-25T11:01:01Z

src/config.ts

+     * Either a relative path to the application or a valid URI to an external domain.
+     * The user will be redirected to this after a login has been performed.
+     */
+    postLoginRedirect: string;


If we're going to create a postLoginRedirect option, then it should work the same as postLogoutRedirect - in that it is the default in auth0-session#logout if returnTo is not set https://github.com/auth0/nextjs-auth0/blob/beta/src/auth0-session/handlers/logout.ts#L17

Good catch, I'll add it.

adamjmcgrath · 2021-01-25T11:48:36Z

src/frontend/use-config.tsx

@@ -0,0 +1,22 @@
+import React, { ReactElement, useContext, createContext } from 'react';


Can you exclude this file from the API Docs https://github.com/auth0/nextjs-auth0/blob/beta/typedoc.js#L4

Sure 👍🏼

adamjmcgrath · 2021-01-28T10:39:41Z

src/config.ts

-   *
-   * ```js
-   * {
-   *   response_type: 'code',


We'll need to keep these docs for most of these properties because the defaults are different.

If you generate the docs now, it'll say the default response_type is id_token when it should be code https://github.com/auth0/nextjs-auth0/blob/beta/src/auth0-session/config.ts#L40

I also wanted to add the corresponding env vars to each config option, eg

/** * To opt-out of sending the library and node version to your authorization server * via the `Auth0-Client` header. Default is `true * You can also use the `AUTH0_ENABLE_TELEMETRY` env var */ enableTelemetry: boolean;

Also baseURL has some Vercel specific docs

Widcket · 2021-02-02T15:58:20Z

Superseded by #262, #263 and #268.

Widcket added 2 commits January 23, 2021 20:45

Simplify configuration of custom URLs

01d33b9

Change customise for customize

d02bcb7

Widcket added the review:medium Medium review label Jan 24, 2021

Widcket requested a review from a team as a code owner January 24, 2021 00:02

Widcket commented Jan 24, 2021

View reviewed changes

Add missing empty line

e8ca5b3

vercel bot deployed to Preview January 24, 2021 00:12 View deployment

Update EXAMPLES.md

7ccc619

vercel bot deployed to Preview January 24, 2021 00:14 View deployment

Widcket commented Jan 24, 2021

View reviewed changes

Avoid unnecessary rerenders

77fedfb

vercel bot deployed to Preview January 24, 2021 00:49 View deployment

adamjmcgrath suggested changes Jan 25, 2021

View reviewed changes

adamjmcgrath reviewed Jan 25, 2021

View reviewed changes

adamjmcgrath reviewed Jan 28, 2021

View reviewed changes

Widcket mentioned this pull request Feb 2, 2021

Simplified configuration of custom login URL [SDK-2264] #268

Merged

4 tasks

Widcket closed this Feb 2, 2021

Widcket deleted the feature/simplify-configuration branch February 2, 2021 15:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplified configuration of custom URLs [SDK-2264] #253

Simplified configuration of custom URLs [SDK-2264] #253

Widcket commented Jan 24, 2021

vercel bot commented Jan 24, 2021 •

edited

Loading

Widcket Jan 24, 2021

adamjmcgrath Jan 25, 2021

Widcket Jan 25, 2021

adamjmcgrath Jan 28, 2021

adamjmcgrath Jan 28, 2021

Widcket Jan 24, 2021

adamjmcgrath Jan 25, 2021

adamjmcgrath Jan 25, 2021

Widcket Jan 25, 2021 •

edited

Loading

adamjmcgrath Jan 28, 2021

adamjmcgrath Jan 25, 2021

Widcket Jan 25, 2021

adamjmcgrath Jan 28, 2021

adamjmcgrath Jan 25, 2021

Widcket Jan 25, 2021 •

edited

Loading

adamjmcgrath Jan 25, 2021

Widcket Jan 25, 2021

adamjmcgrath Jan 25, 2021

Widcket Jan 25, 2021

adamjmcgrath Jan 28, 2021 •

edited

Loading

Widcket commented Feb 2, 2021

		@@ -0,0 +1,22 @@
		import React, { ReactElement, useContext, createContext } from 'react';

Simplified configuration of custom URLs [SDK-2264] #253

Simplified configuration of custom URLs [SDK-2264] #253

Conversation

Widcket commented Jan 24, 2021

Description

Testing

Checklist

vercel bot commented Jan 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Widcket Jan 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Widcket Jan 25, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamjmcgrath Jan 28, 2021 • edited Loading

Choose a reason for hiding this comment

Widcket commented Feb 2, 2021

vercel bot commented Jan 24, 2021 •

edited

Loading

Widcket Jan 25, 2021 •

edited

Loading

Widcket Jan 25, 2021 •

edited

Loading

adamjmcgrath Jan 28, 2021 •

edited

Loading