Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 21 additions & 6 deletions CONSUMING.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,17 @@ redirects there directly after login — the absolute-URL redirect avoids
the double-prefix issue (`/auth/auth/...`) that arises when a relative
`redirect()` path collides with React Router's basename prepend.

> **Fixed in v0.2.2 — `return_to` dropped at the login button.** A
> hosted `ampl-auth` service deployed at **v0.2.1 or earlier** dropped
> `return_to` on the login page: the "Continue with GitHub" button did
> not forward the param into the OAuth start link, so the callback always
> landed the user at `/auth` instead of the deep-link destination. This is
> a **service-side fix** — it takes effect when the shared `ampl.tools/auth`
> service is deployed at v0.2.2, and benefits every consumer at once.
> Consumers do not need to change any code; bumping the pinned
> `@ampl/kit` ref to `#v0.2.2` simply keeps the vendored types in step.
> (Calamus, pinned at `#v0.2.1`, is one affected consumer.)

---

## Git dependency — pinning and updating
Expand Down Expand Up @@ -506,12 +517,16 @@ The git tag is the contract for `@ampl/kit`. Consumers pin to an exact tag:
"@ampl/kit": "github:UCSB-AMPLab/ampl-kit#v0.2.1"
```

**This release:** `v0.2.1` exports the `send()` RPC contract (`SendMessage`,
`SendResult`) from `./email` so consumers type their `EMAIL` service binding
against the published contract instead of vendoring it — additive, no breaking
change. (`v0.2.0` added the `./email` subpath itself: `renderEmailShell`,
`buildIcs`, `EmailShellInput`, `EmailBlock`, `IcsEvent`.) Consumers on `v0.1.0`
are unaffected — the `./auth` and `./ui` subpaths are unchanged.
**This release:** `v0.2.2` fixes a `return_to` bug in the hosted `ampl-auth`
service: the login page dropped `return_to` at the "Continue with GitHub"
button, so deep-link-after-login always landed at `/auth` (see the note in
section 4). It is a service-side fix with no library API change — every
consuming tool benefits once the shared service is deployed at v0.2.2, and no
consumer code change is required. (`v0.2.1` exported the `send()` RPC contract —
`SendMessage`, `SendResult` — from `./email`; `v0.2.0` added the `./email`
subpath itself: `renderEmailShell`, `buildIcs`, `EmailShellInput`, `EmailBlock`,
`IcsEvent`.) Consumers on `v0.1.0` are unaffected at the library level — the
`./auth` and `./ui` subpaths are unchanged.

**Policy:**

Expand Down
18 changes: 15 additions & 3 deletions app/routes/auth.login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@
* Applies the AMPL design language: kit Button + AuthError.
*
* Component behaviour:
* - Renders a kit Button ("Continue with GitHub") → withBase("/github").
* - Renders a kit Button ("Continue with GitHub") → withBase("/github"),
* forwarding any ?return_to= (percent-encoded) onto the start link so the
* deep-link destination survives the OAuth round-trip. Without this the
* param is dropped at the button and the user lands at /auth after login.
* - Reads ?error= from useSearchParams() and renders AuthError with the
* matching errors.* i18n key via the existing t(`errors.${errorCode}`,
* { defaultValue }) safe pattern.
* - Bilingual via the common namespace (login.* + errors.*).
*
* @version v0.1.0
* @version v0.2.2
*/

import { useSearchParams } from "react-router";
Expand All @@ -36,6 +39,15 @@ export default function LoginPage() {
const [searchParams] = useSearchParams();
const errorCode = searchParams.get("error");

// Forward the deep-link destination into the OAuth start link. Without this
// the param is dropped at the button, so auth.github sets an empty
// github_return_to cookie and the callback lands the user at /auth instead
// of where they were heading. Encoded so the path survives the round-trip.
const returnTo = searchParams.get("return_to");
const githubHref = withBase(
"/github" + (returnTo ? `?return_to=${encodeURIComponent(returnTo)}` : ""),
);

return (
<main className="flex flex-1 flex-col items-center justify-center px-[30px] py-20">
<div className="w-full max-w-[400px]">
Expand All @@ -47,7 +59,7 @@ export default function LoginPage() {
/>
)}

<Button as="a" variant="dark" href={withBase("/github")}>
<Button as="a" variant="dark" href={githubHref}>
{/* Inline Octocat — official GitHub mark, white fill, no runtime dependency */}
<svg
aria-hidden="true"
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@ampl/kit",
"version": "0.2.1",
"version": "0.2.2",
"private": true,
"type": "module",
"description": "Shared foundation for the AMPL tools suite: the ampl-auth Worker (ampl.tools/auth) + the @ampl/kit design system, surfaces, and session-validation helper consumed by every tool.",
Expand Down
31 changes: 31 additions & 0 deletions tests/routes/auth.github.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,37 @@ describe("auth.github loader", () => {
expect(returnToCookie!.attrs).toHaveProperty("path", "/auth");
});

it("G1b: a forwarded return_to query lands (decoded) in the github_return_to cookie", async () => {
const returnTo = "/palaeography/invite/accept?token=abc";
// Unique IP so this call doesn't consume the shared "unknown"-IP rate-limit
// budget that the other tests rely on.
const { loader } = await import("~/routes/auth.github");
const request = new Request(
`https://ampl.tools/auth/github?return_to=${encodeURIComponent(returnTo)}`,
{ headers: { "CF-Connecting-IP": "10.3.0.1" } },
);
let thrown: unknown;
try {
await loader({ request, context: buildContext(), params: {} } as any);
} catch (err) {
thrown = err;
}
if (!(thrown instanceof Response)) {
throw new Error(`loader did not throw a Response; got: ${String(thrown)}`);
}
const resp = thrown;

let returnToCookie: ReturnType<ReturnType<typeof parseSetCookie>["get"]>;
for (const raw of getAllSetCookies(resp)) {
const parsed = parseSetCookie(raw);
if (parsed.has("github_return_to")) returnToCookie = parsed.get("github_return_to");
}

expect(returnToCookie).toBeDefined();
// The cookie value is percent-encoded; decoding recovers the original path.
expect(decodeURIComponent(returnToCookie!.value)).toBe(returnTo);
});

it("G2: HTTP origin (dev) emits cookies WITHOUT Secure", async () => {
const resp = await callLoader("http://localhost:8787/auth/github");

Expand Down
37 changes: 37 additions & 0 deletions tests/routes/auth.login.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,43 @@ describe("auth.login page", () => {
expect(html).toContain('href="/auth/github"');
});

it("forwards return_to to the GitHub button href (encoded)", async () => {
const { default: LoginPage } = await import("~/routes/auth.login");
const { StaticRouter } = await import("react-router");

const returnTo = "/palaeography/invite/accept?token=abc";
const html = renderToString(
React.createElement(
StaticRouter,
{ location: `/auth/login?return_to=${encodeURIComponent(returnTo)}` },
React.createElement(LoginPage as React.ComponentType),
),
);

// The GitHub start link must carry the return_to, percent-encoded, so the
// deep-link destination survives the OAuth round-trip.
expect(html).toContain(
`href="/auth/github?return_to=${encodeURIComponent(returnTo)}"`,
);
});

it("parity: no return_to → GitHub button href stays plain /auth/github", async () => {
const { default: LoginPage } = await import("~/routes/auth.login");
const { StaticRouter } = await import("react-router");

const html = renderToString(
React.createElement(
StaticRouter,
{ location: "/auth/login" },
React.createElement(LoginPage as React.ComponentType),
),
);

// Without a return_to, the href carries no query string.
expect(html).toContain('href="/auth/github"');
expect(html).not.toContain("return_to");
});

it("sign-in button uses the dark variant", async () => {
const { default: LoginPage } = await import("~/routes/auth.login");
const { StaticRouter } = await import("react-router");
Expand Down
Loading