Skip to content

Commit d3550d8

Browse files
authored
🚧 feat: implement auth (second iteration) improvements (#28)
This PR implements some improvement to mark the second iteration of the `auth` feature in the project. Follow-up to #8 ### Changes Made - Addressed "todo make the `parsedState` data more predictable (order by path, redirect)" by implementing more predicatable manner of generating the `state` string for the oauth url; this is done by individually looking for the required `state` object `key` to fill in the `parsedState` string in required order. Leaving the new `getAuthUrl` helper function looking like so... ```js function getAuthUrl(state) { let parsedState = ""; if (!isObjectEmpty(state)){ if (state.path) parsedState += `path:${state.path}`; const otherStates = String(Object.keys(state) .filter(key => key !== "path" && key !== "redirect") .map(key => key + ":" + state[key]).join("|")); if (otherStates.length > 0) parsedState += `|${otherStates}`; } const { url } = app.oauth.getWebFlowAuthorizationUrl({ state: parsedState }); return url; } ``` - Implemented a new utility function `isObjectEmpty` to check if an object has value or not - Removed usage of `redirect` property in state object; its redundant 😮‍💨 - Renamed login redirect path params property name to `return_to` from `redirect` for readability reasons - Implemented `encodeURIComponent` in login redirect path params value to stop its part on the url from looking like weird 😃; - Takes the example url from looking like this... `/login?return_to=/editor` to looking like so... `/login?return_to=%2Feditor` ### Related Isssue Resolves #15
1 parent 93479de commit d3550d8

5 files changed

Lines changed: 39 additions & 26 deletions

File tree

src/lib/actions/do-auth.js

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import app from "../octokit/app.js";
22
import { decrypt, encrypt } from "../utils/crypto.js";
3-
import { GET } from "../../pages/api/github/oauth/authorize.js";
4-
import { resolveCookieExpiryDate } from "../utils/index.js";
3+
import { GET as getAuthorization } from "../../pages/api/github/oauth/authorize.js";
4+
import { isObjectEmpty as isStateEmpty, resolveCookieExpiryDate } from "../utils/index.js";
55

66
/**
77
* Authentication action with GitHub OAuth
@@ -16,30 +16,39 @@ export default async function doAuth(astroGlobal) {
1616

1717
/**
1818
* Generate OAuth Url to start authorization flow
19-
* @todo make the `parsedState` data more predictable (order by path, redirect)
20-
* @todo improvement: store `state` in cookie for later retrieval in `github/oauth/callback` handler for cleaner url
21-
* @param {{ path?: string, redirect?: boolean }} state
19+
* @todo improvement: store `state` in cookie for later retrieval/comparison with auth `state` in `github/oauth/callback`
20+
* @param {{ path: string }} state
2221
*/
2322
function getAuthUrl(state) {
24-
const parsedState = String(Object.keys(state).map(key => key + ":" + state[key]).join("|"));
23+
let parsedState = "";
24+
25+
if (!isStateEmpty(state)){
26+
if (state.path) parsedState += `path:${state.path}`;
27+
const otherStates = String(Object.keys(state)
28+
.filter(key => key !== "path" && key !== "redirect")
29+
.map(key => key + ":" + state[key]).join("|"));
30+
if (otherStates.length > 0) parsedState += `|${otherStates}`;
31+
}
32+
2533
const { url } = app.oauth.getWebFlowAuthorizationUrl({
2634
state: parsedState
2735
});
36+
2837
return url;
2938
}
3039

3140
try {
3241
if (!accessToken && code) {
33-
const response = await GET(astroGlobal);
34-
const responseData = await response.json();
42+
const response = await getAuthorization(astroGlobal);
43+
const auth = await response.json();
3544

36-
if (responseData.accessToken && responseData.refreshToken) {
37-
cookies.set("jargons.dev:token", responseData.accessToken, {
38-
expires: resolveCookieExpiryDate(responseData.expiresIn),
45+
if (auth.accessToken && auth.refreshToken) {
46+
cookies.set("jargons.dev:token", auth.accessToken, {
47+
expires: resolveCookieExpiryDate(auth.expiresIn),
3948
encode: value => encrypt(value)
4049
});
41-
cookies.set("jargons.dev:refresh-token", responseData.refreshToken, {
42-
expires: resolveCookieExpiryDate(responseData.refreshTokenExpiresIn),
50+
cookies.set("jargons.dev:refresh-token", auth.refreshToken, {
51+
expires: resolveCookieExpiryDate(auth.refreshTokenExpiresIn),
4352
encode: value => encrypt(value)
4453
});
4554
}

src/lib/utils/index.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,13 @@ export function getRepoParts(repoFullname) {
3030
*/
3131
export function normalizeAsUrl(string) {
3232
return string.toLowerCase().replace(/\s+/g, "-");
33+
}
34+
35+
/**
36+
* Checks if a given object is empty
37+
* @param {object} object
38+
* @returns {boolean}
39+
*/
40+
export function isObjectEmpty(object) {
41+
return JSON.stringify(object) === "{}"
3342
}

src/pages/api/github/oauth/callback.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,9 @@ export async function GET({ url: { searchParams }, redirect }) {
1111
}
1212

1313
const path = state.includes("path") && state.split("|")[0].split(":")[1];
14-
const isRedirect = state.includes("redirect") ? state.split("|")[1].split(":")[1] : false;
1514

16-
if (!isRedirect) {
17-
return redirect(`${path}?code=${code}&redirect=${true}`);
18-
}
15+
if (path) return redirect(`${path}?code=${code}`);
1916

20-
return redirect(`${path}?code=${code}`);
17+
// Lifeline/Last resort for when the return `path` is NOT specified/found in state
18+
return redirect(`/login?return_to=${encodeURIComponent("/")}`)
2119
}

src/pages/editor/index.astro

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ import Navbar from "../../components/navbar.astro";
66
import WordEditor from "../../components/islands/word-editor.jsx";
77
88
const { url: { pathname }, redirect } = Astro;
9+
910
const { isAuthed, authedData: userData } = await doAuth(Astro);
10-
if (!isAuthed) return redirect(`/login?redirect=${pathname}`);
11+
if (!isAuthed) return redirect(`/login?return_to=${encodeURIComponent(pathname)}`);
1112
// @ts-expect-error
1213
$userData.set(userData);
1314
---

src/pages/login.astro

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,10 @@ import BaseLayout from "../layouts/base.astro";
33
import doAuth from "../lib/actions/do-auth.js";
44
55
const { url: { searchParams }, redirect } = Astro;
6-
const { getAuthUrl, isAuthed } = await doAuth(Astro);
7-
8-
if (isAuthed) return redirect(searchParams.get("redirect"));
96
10-
const authUrl = getAuthUrl({
11-
path: searchParams.get("redirect"),
12-
redirect: true
13-
});
7+
const { getAuthUrl, isAuthed } = await doAuth(Astro);
8+
if (isAuthed) return redirect(searchParams.get("return_to"));
9+
const authUrl = getAuthUrl({ path: searchParams.get("return_to") });
1410
---
1511

1612
<BaseLayout pageTitle="Dictionary">

0 commit comments

Comments
 (0)