[feat-500] cleaned up some code

- removed macos testing - refactored some components to use the service objects instead of destructing - improved codeVerifier generation by sonarcloud request - cleaned up some sonarcloud issues
Zagrios · Oct 29, 2024 · 1ccdb05 · 1ccdb05
1 parent d1610b9
commit 1ccdb05
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 31 deletions.
diff --git a/.github/workflows/node.js.yaml b/.github/workflows/node.js.yaml
@@ -13,7 +13,7 @@ jobs:
     build:
         strategy:
             matrix:
-                os: [ubuntu-latest, windows-latest, macos-latest]
+                os: [ubuntu-latest, windows-latest]
 
         runs-on: ${{ matrix.os }}
 

diff --git a/...enderer/third-parties/Beatleader.test.tsx → ...gration/third-parties/Beatleader.test.tsx b/...enderer/third-parties/Beatleader.test.tsx → ...gration/third-parties/Beatleader.test.tsx
@@ -7,6 +7,13 @@ import { createBeatleaderAuthServerService } from "main/services/auth/beatleader
 import { BeatleaderAuthInfo, createBeatleaderAPIClientService } from "renderer/services/third-parties/beatleader.service";
 import { ConfigurationClientService, FetchOptions, FetchResponse, FetchService, IpcClientService } from "renderer/services/types";
 
+Object.defineProperty(global, "crypto", {
+  value: {
+    // should be 1:1 to browser crypto.getRandomValues
+    getRandomValues: (array: any) =>
+        crypto.webcrypto.getRandomValues(array)
+  }
+});
 
 const CLIENT_ID = "some-client-id";
 const REDIRECT_URI = "https://bsmanager.io/oauth";
@@ -128,6 +135,8 @@ test("Mocked Beatleader authentication flow", async () => {
     await authClientService.openOAuth(OAuthType.Beatleader);
     const codeVerifier = mockConfigurationService.get<string | undefined>(CODE_VERIFIER_KEY);
     expect(codeVerifier).toBeTruthy();
+    // Search for invalid characters
+    expect(codeVerifier).not.toMatch(/[^A-Za-z0-9-_.~]/);
 
     expect(ipcData.type).toEqual(OAuthType.Beatleader);
     expect(ipcData.codeVerifier).toEqual(codeVerifier);

diff --git a/src/renderer/components/leaderboard/beatleader/chip.component.tsx b/src/renderer/components/leaderboard/beatleader/chip.component.tsx
@@ -4,7 +4,7 @@ type Props = {
     value: number | string;
 }
 
-export function BeatleaderChip({ name, value }: Props) {
+export function BeatleaderChip({ name, value }: Readonly<Props>) {
     return <span className="rounded-md px-2 bg-light-main-color-3 dark:bg-main-color-3 capitalize">
         {name} | {value}
     </span>

diff --git a/src/renderer/components/leaderboard/beatleader/header-section.component.tsx b/src/renderer/components/leaderboard/beatleader/header-section.component.tsx
@@ -12,7 +12,7 @@ type Props = {
 export function BeatleaderHeaderSection({
     playerInfo,
     onLogoutClicked
-}: Props) {
+}: Readonly<Props>) {
     return <div className="w-full rounded-md pt-4 bg-light-main-color-2 dark:bg-main-color-2">
         <div className="flex flex-row justify-between px-4 pb-4">
             <div className="flex flex-row items-center gap-x-4">

diff --git a/src/renderer/components/leaderboard/beatleader/socials.component.tsx b/src/renderer/components/leaderboard/beatleader/socials.component.tsx
@@ -7,7 +7,7 @@ type Props = {
     socials: BeatleaderSocial[]
 }
 
-export function BeatleaderSocials({ externalPlayerUrl, socials }: Props) {
+export function BeatleaderSocials({ externalPlayerUrl, socials }: Readonly<Props>) {
     if (socials.length === 0) {
         return <div className="rounded-b-md bg-light-main-color-1 dark:bg-main-color-1" />
     }
@@ -21,7 +21,7 @@ export function BeatleaderSocials({ externalPlayerUrl, socials }: Props) {
         }
 
         {filteredSocials.map(social =>
-            <BsmLink href={social.link}>
+            <BsmLink key={social.id} href={social.link}>
                 <BsmButton className="w-8 h-8" icon="discord" />
             </BsmLink>
         )}

diff --git a/src/renderer/components/leaderboard/beatleader/stats-section.component.tsx b/src/renderer/components/leaderboard/beatleader/stats-section.component.tsx
@@ -10,7 +10,7 @@ type Props = {
 
 export function BeatleaderStatsSection({
     playerInfo
-}: Props) {
+}: Readonly<Props>) {
     const [showHidden, setShowHidden] = useState(false);
 
     const intFormatter = new Intl.NumberFormat("en-US", { minimumFractionDigits: 0, maximumFractionDigits: 0 })
@@ -142,6 +142,7 @@ export function BeatleaderStatsSection({
                 <div className="flex flex-row flex-wrap gap-2">
                     {topChips.filter(chip => showHidden || !chip.hidden).map(chip =>
                         <BeatleaderChip
+                            key={chip.key}
                             name={chip.resource}
                             value={chip.formatter(playerInfo.scoreStats[chip.key])}
                         />
@@ -150,6 +151,7 @@ export function BeatleaderStatsSection({
                 <div className="flex flex-row flex-wrap gap-2">
                     {bottomChips.map(chip =>
                         <BeatleaderChip
+                            key={chip.key}
                             name={chip.resource}
                             value={chip.formatter(playerInfo.scoreStats[chip.key])}
                         />
@@ -168,11 +170,11 @@ export function BeatleaderStatsSection({
     </div>
 }
 
-function BeatleaderRowInfo({ name, hoverText, children }: {
+function BeatleaderRowInfo({ name, hoverText, children }: Readonly<{
     name: string;
     hoverText: string;
     children: JSX.Element;
-}) {
+}>) {
     return <div className="flex justify-between gap-x-4">
         <Tippy content={hoverText}>
             <div className="capitalize font-semibold">{name}</div>

diff --git a/src/renderer/components/leaderboard/leaderboard-panel.component.tsx b/src/renderer/components/leaderboard/leaderboard-panel.component.tsx
@@ -21,14 +21,14 @@ type Props = {
     isActive: boolean;
 }
 
-export function LeaderboardPanel({ isActive }: Props) {
+export function LeaderboardPanel({ isActive }: Readonly<Props>) {
     const osService = useService(OsDiagnosticService);
 
     const online = useObservable(() => osService.isOnline$);
 
-    const { openOAuth, logoutOAuth } = defaultAuthService();
-    const { isAuthenticated, getCurrentPlayerInfo } = defaultBeatleaderAPIClientService();
-    const [authenticated, setAuthenticated] = useState(isAuthenticated());
+    const authService = defaultAuthService();
+    const beatleaderService = defaultBeatleaderAPIClientService();
+    const [authenticated, setAuthenticated] = useState(beatleaderService.isAuthenticated());
     const [playerInfo, setPlayerInfo] = useState(null as BeatleaderPlayerInfo | null);
     const isActiveOnce = useChangeUntilEqual(isActive, { untilEqual: true });
 
@@ -41,12 +41,12 @@ export function LeaderboardPanel({ isActive }: Props) {
         // NOTE: Might need a service or something
         window.onstorage = (event) => {
             if (event.key === OAuthType.Beatleader) {
-                setAuthenticated(isAuthenticated());
+                setAuthenticated(beatleaderService.isAuthenticated());
             }
         };
 
         if (online && authenticated) {
-            getCurrentPlayerInfo()
+            beatleaderService.getCurrentPlayerInfo()
                 .then(setPlayerInfo);
         }
 
@@ -61,7 +61,7 @@ export function LeaderboardPanel({ isActive }: Props) {
             textClassName="rounded-md p-2 bg-light-main-color-1 dark:bg-main-color-1"
             onClick={event => {
                 event.stopPropagation();
-                openOAuth(OAuthType.Beatleader);
+                authService.openOAuth(OAuthType.Beatleader);
             }}
         />
     }
@@ -89,7 +89,7 @@ export function LeaderboardPanel({ isActive }: Props) {
         <BeatleaderHeaderSection
             playerInfo={playerInfo}
             onLogoutClicked={() => {
-                logoutOAuth(OAuthType.Beatleader);
+                authService.logoutOAuth(OAuthType.Beatleader);
                 setAuthenticated(false);
                 setPlayerInfo(null);
             }}
@@ -99,12 +99,12 @@ export function LeaderboardPanel({ isActive }: Props) {
     </div>
 }
 
-function LeaderboardStatus({ text, image, spin = false, children }: {
+function LeaderboardStatus({ text, image, spin = false, children }: Readonly<{
     text: string;
     image: string;
     spin?: boolean,
     children?: ReactNode
-}) {
+}>) {
     const t = useTranslation();
 
     return (

diff --git a/src/renderer/services/auth.service.ts b/src/renderer/services/auth.service.ts
@@ -3,15 +3,15 @@ import { OAuthType } from "shared/models/oauth.types";
 import { ConfigurationClientService, IpcClientService } from "./types";
 
 
-const CODE_VERIFIER_SIZE = 64;
-const CHARACTERS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~";
+const CODE_VERIFIER_SIZE = 32;
 
-function randomString(length: number): string {
-    let str = "";
-    while (--length >= 0) {
-        str += CHARACTERS.charAt(Math.floor(Math.random() * CHARACTERS.length));
-    }
-    return str;
+function createCodeVerifier(): string {
+    const random = new Uint8Array(CODE_VERIFIER_SIZE);
+    crypto.getRandomValues(random);
+    return btoa(String.fromCharCode.apply(null, Array.from(random)))
+        .replace(/\+/g, "-")
+        .replace(/\//g, "_")
+        .replace(/=/g, "");
 }
 
 export function createAuthClientService({
@@ -25,7 +25,7 @@ export function createAuthClientService({
 }) {
     return {
         async openOAuth(type: OAuthType) {
-            const codeVerifier = randomString(CODE_VERIFIER_SIZE);
+            const codeVerifier = createCodeVerifier();
             configService.set(codeVerifierKey, codeVerifier);
             return lastValueFrom(
                 ipcService.sendV2("auth.open-oauth", {

diff --git a/src/renderer/windows/OAuth.tsx b/src/renderer/windows/OAuth.tsx
@@ -46,7 +46,7 @@ function useAuthentication(type: OAuthType, code: string) {
 }
 
 export default function OAuthWindow() {
-    const { close: closeWindow } = useWindowControls();
+    const windowControls = useWindowControls();
 
     const [searchParams,] = useSearchParams();
     const { loading, authenticated, errorMessage } = useAuthentication(
@@ -76,19 +76,19 @@ export default function OAuthWindow() {
                 withBar={false}
                 onClick={event => {
                     event.preventDefault();
-                    closeWindow();
+                    windowControls.close();
                 }}
             />
         </OAuthStatus>
     </div>
 }
 
-function OAuthStatus({ text, image, spin = false, children }: {
+function OAuthStatus({ text, image, spin = false, children }: Readonly<{
     text: string;
     image: string;
     spin?: boolean,
     children?: ReactNode
-}) {
+}>) {
     const t = useTranslation();
 
     return (