diff --git a/build/spas.js b/build/spas.js index d600b04c7e9b..09f9e5043975 100644 --- a/build/spas.js +++ b/build/spas.js @@ -25,8 +25,8 @@ async function buildSPAs(options) { console.log("Wrote", path.join(outPath, path.basename(url))); } - // Basically, this builds one `search/index.html` for every locale we intend - // to build. + // Basically, this builds one (for example) `search/index.html` for every + // locale we intend to build. for (const root of [CONTENT_ROOT, CONTENT_TRANSLATED_ROOT]) { if (!root) { continue; @@ -35,15 +35,22 @@ async function buildSPAs(options) { if (!fs.statSync(path.join(root, locale)).isDirectory()) { continue; } - const url = `/${locale}/search`; - const html = renderHTML(url); - const outPath = path.join(BUILD_OUT_ROOT, locale, "search"); - fs.mkdirSync(outPath, { recursive: true }); - const filePath = path.join(outPath, "index.html"); - fs.writeFileSync(filePath, html); - buildCount++; - if (options.verbose) { - console.log("Wrote", filePath); + const SPAs = [ + { prefix: "search", pageTitle: "Search" }, + { prefix: "signin", pageTitle: "Sign in" }, + { prefix: "signup", pageTitle: "Sign up" }, + ]; + for (const { prefix, pageTitle } of SPAs) { + const url = `/${locale}/${prefix}`; + const html = renderHTML(url, { pageTitle }); + const outPath = path.join(BUILD_OUT_ROOT, locale, prefix); + fs.mkdirSync(outPath, { recursive: true }); + const filePath = path.join(outPath, "index.html"); + fs.writeFileSync(filePath, html); + buildCount++; + if (options.verbose) { + console.log("Wrote", filePath); + } } } } diff --git a/client/src/app.tsx b/client/src/app.tsx index 6fe095120cbd..7aa5065c723a 100644 --- a/client/src/app.tsx +++ b/client/src/app.tsx @@ -15,6 +15,7 @@ import { SiteSearch } from "./site-search"; import { PageContentContainer } from "./ui/atoms/page-content"; import { PageNotFound } from "./page-not-found"; import { Banner } from "./banners"; +import { SignIn, SignUp } from "./auth"; const AllFlaws = React.lazy(() => import("./flaws")); const DocumentEdit = React.lazy(() => import("./document/forms/edit")); @@ -196,6 +197,22 @@ export function App(appProps) { } /> + + + + } + /> + + + + } + /> } diff --git a/client/src/auth/index.tsx b/client/src/auth/index.tsx new file mode 100644 index 000000000000..99e08f30bce7 --- /dev/null +++ b/client/src/auth/index.tsx @@ -0,0 +1,54 @@ +import React from "react"; + +import { PageContentContainer } from "../ui/atoms/page-content"; + +const SignInApp = React.lazy(() => import("./sign-in")); +const SignUpApp = React.lazy(() => import("./sign-up")); + +function Container({ + pageTitle, + children, + className, +}: { + pageTitle: string; + children: React.ReactNode; + className: string; +}) { + const isServer = typeof window === "undefined"; + React.useEffect(() => { + document.title = pageTitle; + }, [pageTitle]); + return ( +
+ + {/* The reason for displaying this

here (and for SignUp too) + is to avoid an unnecessary "flicker". + component here is loaded SSR and is immediately present. + Only the "guts" below is lazy loaded. By having the header already + present the page feels less flickery at a very affordable cost of + allowing this to be part of the main JS bundle. + */} +

{pageTitle}

+ {!isServer && children} +
+
+ ); +} +export function SignIn() { + return ( + + Loading...

}> + +
+
+ ); +} +export function SignUp() { + return ( + + Loading...

}> + +
+
+ ); +} diff --git a/client/src/auth/sign-in.tsx b/client/src/auth/sign-in.tsx new file mode 100644 index 000000000000..148789756472 --- /dev/null +++ b/client/src/auth/sign-in.tsx @@ -0,0 +1,94 @@ +import { useSearchParams } from "react-router-dom"; + +import { useUserData } from "../user-context"; +import { useLocale } from "../hooks"; + +export default function SignInApp() { + const [searchParams] = useSearchParams(); + const locale = useLocale(); + const userData = useUserData(); + const sp = new URLSearchParams(); + + // This is the `?next=` parameter we send into the redirect loop IF you did + // not click into this page from an existing one. + const defaultNext = `/${locale}/`; + + let next = searchParams.get("next") || defaultNext; + if (next.toLowerCase() === window.location.pathname.toLowerCase()) { + // It's never OK for the ?next= variable to be to come back here to this page. + // Explicitly check that. + next = defaultNext; + } + + let prefix = ""; + // When doing local development with Yari, the link to authenticate in Kuma + // needs to be absolute. And we also need to send the absolute URL as the + // `next` query string parameter so Kuma sends us back when the user has + // authenticated there. + if ( + process.env.NODE_ENV === "development" && + process.env.REACT_APP_KUMA_HOST + ) { + const combined = new URL(next, window.location.href); + next = combined.toString(); + prefix = `http://${process.env.REACT_APP_KUMA_HOST}`; + } + sp.set("next", next); + + // Temporary just long as Kuma still needs to support sign-up both as + // Kuma front-end (HTML) Yari (redirects). + // Delete this line once Kuma ONLY deals with Yari in the signup view. + sp.set("yarisignup", "1"); + + return ( +
+ {/* We need to wait for the userData (/api/v1/whoami) because it will + determine what we display. + We *could* render on the optimism that people most likely will be + on this page because they're NOT signed in. But it feels a bit + "ugly" if a page has to change its mind and rerender something + completely different without it being due to a user action. + */} + {userData ? ( + <> + {userData.isAuthenticated ? ( +
+

+ You're already signed in. +

+ + {/* XXX Here it would be great to link to the account settings page */} + + + +

+ Or, return to the home page. +

+
+ ) : ( + + )} + + ) : ( + + )} +
+ ); +} + +function Loading() { + return

Loading...

; +} diff --git a/client/src/auth/sign-up.scss b/client/src/auth/sign-up.scss new file mode 100644 index 000000000000..3c6a4daffa88 --- /dev/null +++ b/client/src/auth/sign-up.scss @@ -0,0 +1,11 @@ +.sign-up { + form { + button[disabled] { + opacity: 0.5; + } + } + // .user-details img.picture { + // width: 100px; + // border-radius: 50px; + // } +} diff --git a/client/src/auth/sign-up.tsx b/client/src/auth/sign-up.tsx new file mode 100644 index 000000000000..043458eca1a5 --- /dev/null +++ b/client/src/auth/sign-up.tsx @@ -0,0 +1,222 @@ +import React from "react"; +import { Link, useNavigate, useSearchParams } from "react-router-dom"; +import { mutate } from "swr"; + +import { useLocale } from "../hooks"; +import { useUserData } from "../user-context"; + +import "./sign-up.scss"; + +export default function SignUpApp() { + const userData = useUserData(); + const navigate = useNavigate(); + const locale = useLocale(); + const [searchParams] = useSearchParams(); + + const [checkedTerms, setCheckedTerms] = React.useState(false); + const [signupError, setSignupError] = React.useState(null); + + if (!userData) { + return
Loading...
; + } + + if (userData.isAuthenticated) { + return ( +
+

You're already signed up

+ {/* XXX Include a link to the settings page */} + Return to the home page. +
+ ); + } + + if (searchParams.get("errors")) { + console.warn("Errors", searchParams.get("errors")); + const errors = JSON.parse(searchParams.get("errors") || "{}"); + return ( +
+

Sign-up errors

+

An error occurred trying to sign you up.

+
{JSON.stringify(errors, null, 2)}
+

+ + Try starting over the sign-in process + + . +

+
+ ); + } + + const csrfMiddlewareToken = searchParams.get("csrfmiddlewaretoken"); + const provider = searchParams.get("provider"); + if (!csrfMiddlewareToken || !provider) { + return ( +
+

Invalid Sign up URL

+

You arrived here on this page without the necessary details.

+

+ + Try starting over the sign-in process + + . +

+
+ ); + } + + const signupURL = `/${locale}/users/account/signup`; + + async function submitSignUp() { + const formData = new URLSearchParams(); + formData.set("terms", "1"); + + // This is just a temporary thing needed to tell Kuma's signup view + // that the request came from (the jamstack) Yari and not the existing + // Kuma front-end. Then Kuma knows to certainly only respond with redirects. + formData.set("yarisignup", "1"); + + // In local development, after you've signed in the `next` query string + // might be a full absolute URL that points to `http://localhost.org:3000/...`. + // We can safely remove this and just keep the pathname. In production + // this will never have to happen. + let nextURL = searchParams.get("next"); + if (!nextURL) { + nextURL = `/${locale}/`; + } else if (nextURL && nextURL.includes("://")) { + nextURL = new URL(nextURL).pathname; + } + formData.set("next", nextURL); + + formData.set("locale", locale); + if (!csrfMiddlewareToken) { + throw new Error("CSRF token not set"); + } + let response: Response; + try { + response = await fetch(signupURL, { + method: "POST", + headers: { + "X-CSRFToken": csrfMiddlewareToken, + "Content-Type": "application/x-www-form-urlencoded", + }, + body: formData, + }); + } catch (error) { + setSignupError(error); + return; + } + + if (response.ok) { + // This will "force" a new XHR request in the useUserData hook. + mutate("/api/v1/whoami"); + + navigate(nextURL); + } else { + setSignupError(new Error(`${response.status} on ${signupURL}`)); + } + } + + return ( +
{ + event.preventDefault(); + if (checkedTerms) { + submitSignUp(); + } + }} + > + {signupError && ( +
+

Signup Error

+

+ {signupError.toString()} +

+
+ )} + + + + + + + + + ); +} + +function DisplaySignupProvider({ provider }: { provider: string }) { + if (!provider) { + // Exit early because there's nothing useful we can say + return null; + } + let providerVerbose = provider.charAt(0).toUpperCase() + provider.slice(1); + if (provider === "github") { + providerVerbose = "GitHub"; + } + return ( +

+ You are signing in to MDN Web Docs with {providerVerbose}. +

+ ); +} + +interface UserDetails { + name?: string; + avatar_url?: string; +} + +function DisplayUserDetails({ details }: { details: string }) { + if (!details) { + // Exit early because there's nothing useful we can say + return null; + } + + const userDetails: UserDetails = JSON.parse(details); + + return ( +
+

+ {userDetails.avatar_url && ( + User profile avatar_url + )} + + {userDetails.name && ` as ${userDetails.name}`} +

+
+ ); +} diff --git a/client/src/ui/atoms/signin-link/index.tsx b/client/src/ui/atoms/signin-link/index.tsx index 703951db1b56..f8b0b772e946 100644 --- a/client/src/ui/atoms/signin-link/index.tsx +++ b/client/src/ui/atoms/signin-link/index.tsx @@ -1,8 +1,27 @@ +import React from "react"; +import { Link, useLocation } from "react-router-dom"; + import { useLocale } from "../../../hooks"; import { getAuthURL } from "../../../utils/auth-link"; export default function SignInLink({ className }: { className?: string }) { const locale = useLocale(); + const { pathname } = useLocation(); + + // NOTE! We can remove this if-statement and make it the default once + // https://github.com/mdn/yari/issues/2449 is resolved and it has been + // fully tested in Stage. + if (process.env.REACT_APP_USE_YARI_SIGNIN) { + return ( + + Sign in + + ); + } return ( <> ; @@ -61,6 +55,20 @@ function LoginInner() { false ); const editProfileURL = viewProfileURL + "/edit"; + + // Note that this component is never rendered server-side so it's safe to + // rely on `window.location`. + let next = window.location.pathname; + let signOutURL = `/${locale}/users/signout`; + if ( + process.env.NODE_ENV === "development" && + process.env.REACT_APP_KUMA_HOST + ) { + const combined = new URL(next, window.location.href); + next = combined.toString(); + signOutURL = `http://${process.env.REACT_APP_KUMA_HOST}${signOutURL}`; + } + return (
  • @@ -70,8 +78,8 @@ function LoginInner() { Edit profile
  • -
    - + + diff --git a/docs/proxying.md b/docs/proxying.md index f19216ff5c5a..d53e63e55468 100644 --- a/docs/proxying.md +++ b/docs/proxying.md @@ -21,7 +21,7 @@ and start it up again. ## Logging in -TO BE DESCRIBED +[See Siging in and signing up](./signingin.md) ## Faking it diff --git a/docs/signingin.md b/docs/signingin.md new file mode 100644 index 000000000000..7ad6016004b0 --- /dev/null +++ b/docs/signingin.md @@ -0,0 +1,44 @@ +# Signing in and signing up + +## Introduction + +Signing in means you click a link to go straight to a Kuma endpoint which +will redirect you back to where you were. + +Yari produces a SPA app at `/$locale/signin`. It presents to clickable +links to our supported identity providers. Clicking the "Sign in" in the +header will note which page you were on, then it goes to +`/$locale/signin?next=page/you/were/on` + +When the user picks a identity provider they leave the static site +nature of Yari and dynamically render a Kuma page which itself, +will trigger a redirect to the identity providers authentication screen. +If the user completes that, it will redirect back to Kuma which then +makes a decision... + +1. If you've signed in before (aka. signed up) you're redirected back to + that original `?next=...` variable that was set when you clicked "Sign in". +2. You've never signed in before, Kuma stores the identity provider authentication + callback data in a session cookie, and redirects the user to `/$locale/signup` + which is a static Yari SPA. On that page you need to check a "Terms and conditions" + checkbox and make a POST request to Kuma. Only then will kuma store the new + user account in its database and if all goes well, it redirects to that + originl `?next=...` URL or `/$locale/` if the `next` variable was lost. + +## Local development + +The jumping between static Yari and dynamic Kuma works well in production because +you have both backends seamlessly behind the same CDN domain. That's not the case +in local development. In short, to get this to work in local development you +have to (`docker-compose up`) start Kuma in one terminal and start Yari in +another. But before you start Yari, set the following in your `.env` file: + +```sh +# This tells your React dev server that it's OK to use http://localhost.org:3000 +# instead of the usual http://localhost:3000 +HOST=localhost.org + +# This tells the sign in link to forcibly switch the whole domain in the +# links to the identity provider choice links. +REACT_APP_KUMA_HOST=localhost.org:8000 +``` diff --git a/server/index.js b/server/index.js index f5477b94617c..9ca695d1722f 100644 --- a/server/index.js +++ b/server/index.js @@ -63,28 +63,32 @@ app.use(originRequestMiddleware); app.use(staticMiddlewares); +// Depending on if FAKE_V1_API is set, we either respond with JSON based +// on `.json` files on disk or we proxy the requests to Kuma. +const proxy = FAKE_V1_API + ? fakeV1APIRouter + : createProxyMiddleware({ + target: `${ + ["developer.mozilla.org", "developer.allizom.org"].includes( + PROXY_HOSTNAME + ) + ? "https://" + : "http://" + }${PROXY_HOSTNAME}`, + changeOrigin: true, + proxyTimeout: 3000, + timeout: 3000, + }); + +app.use("/api/v1", proxy); +// This is an exception and it's only ever relevant in development. +app.post("/:locale/users/account/signup", proxy); + +// It's important that this line comes *after* the setting up for the proxy +// middleware for `/api/v1` above. +// See https://github.com/chimurai/http-proxy-middleware/issues/40#issuecomment-163398924 app.use(express.urlencoded({ extended: true })); -app.use( - "/api/v1", - // Depending on if FAKE_V1_API is set, we either respond with JSON based - // on `.json` files on disk or we proxy the requests to Kuma. - FAKE_V1_API - ? fakeV1APIRouter - : createProxyMiddleware({ - target: `${ - ["developer.mozilla.org", "developer.allizom.org"].includes( - PROXY_HOSTNAME - ) - ? "https://" - : "http://" - }${PROXY_HOSTNAME}`, - changeOrigin: true, - proxyTimeout: 3000, - timeout: 3000, - }) -); - app.use("/_document", documentRouter); app.get("/_open", (req, res) => { diff --git a/ssr/render.js b/ssr/render.js index 1d8787dbe0fd..fccfc7894dc3 100644 --- a/ssr/render.js +++ b/ssr/render.js @@ -139,7 +139,12 @@ function serializeDocumentData(data) { export default function render( renderApp, - { doc = null, pageNotFound = false, feedEntries = null } = {} + { + doc = null, + pageNotFound = false, + feedEntries = null, + pageTitle = null, + } = {} ) { const buildHtml = readBuildHTML(); const webfontURLs = extractWebFontURLs(); @@ -151,7 +156,9 @@ export default function render( const rendered = renderToString(renderApp); - let pageTitle = "MDN Web Docs"; // default + if (!pageTitle) { + pageTitle = "MDN Web Docs"; // default + } let canonicalURL = "https://developer.mozilla.org"; let pageDescription = ""; diff --git a/testing/scripts/functional-test.sh b/testing/scripts/functional-test.sh index a2fe15f632cf..3dbb373d5dd7 100755 --- a/testing/scripts/functional-test.sh +++ b/testing/scripts/functional-test.sh @@ -6,6 +6,9 @@ export ENV_FILE=testing/.env # Temporary whilst only the functional tests use the autocomplete search widget. export REACT_APP_AUTOCOMPLETE_SEARCH_WIDGET=true +# Temporary until we're still using the old Kuma for signin in. +export REACT_APP_USE_YARI_SIGNIN=true + yarn prepare-build yarn build diff --git a/testing/tests/headless.test.js b/testing/tests/headless.test.js index c2efe946ff62..f48e1f3c9fcd 100644 --- a/testing/tests/headless.test.js +++ b/testing/tests/headless.test.js @@ -254,4 +254,39 @@ describe("Basic viewing of functional pages", () => { await expect(page).toMatch(": Une page de test"); expect(page.url()).toBe(testURL("/fr/docs/Web/Foo")); }); + + it("clicking 'Sign in' should offer links to all identity providers", async () => { + await page.goto(testURL("/en-US/docs/Web/Foo")); + await expect(page).toClick("a", { text: "Sign in" }); + await expect(page).toMatchElement("h1", { text: "Sign in" }); + expect(page.url()).toContain( + testURL("/en-US/signin?next=/en-US/docs/Web/Foo") + ); + await expect(page).toMatchElement("a", { text: "Google" }); + await expect(page).toMatchElement("a", { text: "GitHub" }); + }); + + it("going to 'Sign up' page without query string", async () => { + await page.goto(testURL("/en-US/signup")); + await expect(page).toMatchElement("h1", { text: "Sign up" }); + await expect(page).toMatch("Invalid Sign up URL"); + await expect(page).toMatchElement("a", { + text: "Try starting over the sign-in process", + }); + }); + + it("going to 'Sign up' page with realistic (fake) query string", async () => { + await page.goto( + testURL("/en-US/signup?csrfmiddlewaretoken=abc&provider=github") + ); + await expect(page).toMatchElement("h1", { text: "Sign up" }); + await expect(page).not.toMatch("Invalid Sign up URL"); + await expect(page).toMatch( + "You are signing in to MDN Web Docs with GitHub." + ); + await expect(page).toMatch( + "I agree to Mozilla's Terms and Privacy Notice." + ); + await expect(page).toMatchElement("button", { text: "Create account" }); + }); }); diff --git a/testing/tests/index.test.js b/testing/tests/index.test.js index bf5dcd4144c0..e2545ab2ed52 100644 --- a/testing/tests/index.test.js +++ b/testing/tests/index.test.js @@ -951,6 +951,26 @@ test("404 page", () => { expect($('meta[name="robots"]').attr("content")).toBe("noindex, nofollow"); }); +test("sign in page", () => { + const builtFolder = path.join(buildRoot, "en-us", "signin"); + expect(fs.existsSync(builtFolder)).toBeTruthy(); + const htmlFile = path.join(builtFolder, "index.html"); + const html = fs.readFileSync(htmlFile, "utf-8"); + const $ = cheerio.load(html); + expect($("h1").text()).toContain("Sign in"); + expect($("title").text()).toContain("Sign in"); +}); + +test("sign up page", () => { + const builtFolder = path.join(buildRoot, "en-us", "signup"); + expect(fs.existsSync(builtFolder)).toBeTruthy(); + const htmlFile = path.join(builtFolder, "index.html"); + const html = fs.readFileSync(htmlFile, "utf-8"); + const $ = cheerio.load(html); + expect($("h1").text()).toContain("Sign up"); + expect($("title").text()).toContain("Sign up"); +}); + test("bcd table extraction followed by h3", () => { const builtFolder = path.join( buildRoot,