Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test #9469

Closed
wants to merge 10 commits into from
Closed

Test #9469

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
3 changes: 2 additions & 1 deletion .github/workflows/cypress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ on:
pull_request:
branches:
- develop
- test
- staging
workflow_dispatch:

jobs:
cypress-run:
permissions: write-all
if: github.repository == 'ohcnetwork/care_fe'
if: github.repository == 'manmeetnagii/care_fe'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Repository condition change introduces security risks

The verification confirms critical security concerns:

  1. The workflow has write-all permissions and handles sensitive operations including:

    • Access to repository secrets (JWKS_BASE64, CYPRESS_RECORD_KEY)
    • Docker container operations with backend services
    • Cypress test execution with record capabilities
  2. All other security-critical workflows (CodeQL, OSSAR, deployments) are correctly restricted to 'ohcnetwork/care_fe', while this is the only workflow changed to run on a personal fork.

  3. The workflow already has proper fork handling through steps.pr_origin.outputs.is_forked conditions, making the repository check redundant and potentially harmful.

Recommendation:

-    if: github.repository == 'manmeetnagii/care_fe'
+    if: github.repository == 'ohcnetwork/care_fe'
🔗 Analysis chain

SECURITY RISK: Workflow configured for personal fork instead of main repository

This change presents several critical concerns:

  1. Security:

    • The workflow has write-all permissions and runs with sensitive secrets
    • Running on a personal fork increases security risks
    • Could potentially expose CYPRESS_RECORD_KEY and other secrets
  2. CI/CD Pipeline:

    • Tests will only run on 'manmeetnagii/care_fe'
    • Main repository ('ohcnetwork/care_fe') tests will be bypassed

Revert this change to maintain security and ensure proper CI/CD pipeline execution:

-    if: github.repository == 'manmeetnagii/care_fe'
+    if: github.repository == 'ohcnetwork/care_fe'

Let's verify the repository settings and permissions:


Let's examine the workflow file contents and any other repository conditions to better understand the security implications and verify if this is indeed a critical issue.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check workflow permissions and secret usage
gh api repos/ohcnetwork/care_fe/actions/permissions
gh api repos/manmeetnagii/care_fe/actions/permissions

# List workflows using sensitive secrets
rg -l "secrets\." .github/workflows/

Length of output: 943


Script:

#!/bin/bash
# Check workflow permissions and secrets usage in cypress.yaml
cat .github/workflows/cypress.yaml

# Check for any other repository conditions in workflows
rg -p "github.repository ==" .github/workflows/

Length of output: 6137

runs-on: ubuntu-latest
strategy:
fail-fast: false
Expand Down
241 changes: 143 additions & 98 deletions src/components/Assets/AssetsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,13 @@ import { AdvancedFilterButton } from "@/CAREUI/interactive/FiltersSlideover";
import AssetFilter from "@/components/Assets/AssetFilter";
import AssetImportModal from "@/components/Assets/AssetImportModal";
import { AssetData, assetClassProps } from "@/components/Assets/AssetTypes";
import ButtonV2 from "@/components/Common/ButtonV2";
import ExportMenu from "@/components/Common/Export";
import Loading from "@/components/Common/Loading";
import Page from "@/components/Common/Page";
import FacilitiesSelectDialogue from "@/components/ExternalResult/FacilitiesSelectDialogue";
import { FacilityModel } from "@/components/Facility/models";
import SearchInput from "@/components/Form/SearchInput";

import useAuthUser from "@/hooks/useAuthUser";
import useFilters from "@/hooks/useFilters";
import { useIsAuthorized } from "@/hooks/useIsAuthorized";

Expand All @@ -29,6 +28,9 @@ import routes from "@/Utils/request/api";
import request from "@/Utils/request/request";
import useTanStackQueryInstead from "@/Utils/request/useQuery";

import SearchByMultipleFields from "../Common/SearchByMultipleFields";
import { Button } from "../ui/button";

const AssetsList = () => {
const { t } = useTranslation();
const {
Expand All @@ -38,9 +40,10 @@ const AssetsList = () => {
FilterBadges,
advancedFilter,
resultsPerPage,
clearSearch,
} = useFilters({
limit: 18,
cacheBlacklist: ["search"],
cacheBlacklist: ["name", "serial_number", "qr_code_id"],
});
const [assets, setAssets] = useState([{} as AssetData]);
const [isLoading, setIsLoading] = useState(false);
Expand All @@ -56,6 +59,9 @@ const AssetsList = () => {
const params = {
limit: resultsPerPage,
page: qParams.page,
name: qParams.name || "",
serial_number: qParams.serial_number || "",
qr_code_id: qParams.qr_code_id || "",
offset: (qParams.page ? qParams.page - 1 : 0) * resultsPerPage,
search_text: qParams.search || "",
facility: qParams.facility || "",
Expand Down Expand Up @@ -114,6 +120,9 @@ const AssetsList = () => {
},
);

const authUser = useAuthUser();
const isDisabled = !NonReadOnlyUsers(authUser.user_type);
Comment on lines +123 to +124
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Authorization checks need to be extended to other interactive elements

The authorization check (isDisabled) is only applied to the create asset button, but there are other interactive elements that modify state or trigger actions:

  • QR scanner activation/deactivation buttons
  • Advanced filter button

These elements should also be restricted for read-only users to maintain consistent authorization across the component.

🔗 Analysis chain

Verify consistent authorization checks across the component

The authorization check is implemented correctly for the create button. Let's verify if similar checks are needed for other interactive elements.


Let me gather more context about these interactive elements and their authorization checks.


Let me check if the isDisabled flag is consistently applied to all interactive elements.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other interactive elements that might need authorization checks
rg -A 2 "onClick=" src/components/Assets/AssetsList.tsx

Length of output: 526


Script:

#!/bin/bash
# Get more context around the interactive elements
rg -B 5 -A 5 "onClick=" src/components/Assets/AssetsList.tsx

Length of output: 1446


Script:

#!/bin/bash
# Search for disabled prop usage
rg "disabled=" src/components/Assets/AssetsList.tsx

Length of output: 87


function isValidURL(url: string) {
try {
new URL(url);
Expand Down Expand Up @@ -311,108 +320,50 @@ const AssetsList = () => {
);
}

const searchOptions = [
{
key: "name",
label: "Name",
type: "text" as const,
placeholder: "Search by Name",
value: qParams.name || "",
shortcutKey: "n",
},
{
key: "serial_number",
label: "Serial No.",
type: "text" as const,
placeholder: "Search by Serial No.",
value: qParams.serial_number || "",
shortcutKey: "u",
},
{
key: "asset_qr_id",
label: "QR Code ID",
type: "text" as const,
placeholder: "Search by QR Code ID",
value: qParams.qr_code_id || "",
shortcutKey: "p",
},
];

return (
<Page
title="Assets"
breadcrumbs={false}
className="px-4 md:px-6"
hideBack
options={
<>
{authorizedForImportExport && (
<div className="tooltip" data-testid="import-asset-button">
<ExportMenu
label={importAssetModalOpen ? "Importing..." : "Import/Export"}
exportItems={[
{
label: "Import Assets",
options: {
icon: (
<CareIcon
icon="l-import"
className="import-assets-button"
/>
),
onClick: () => setImportAssetModalOpen(true),
},
},
{
label: "Export Assets (JSON)",
action: async () => {
const { data } = await request(routes.listAssets, {
query: { ...qParams, json: true, limit: totalCount },
});
return data ?? null;
},
type: "json",
filePrefix: `assets_${facility?.name ?? "all"}`,
options: {
icon: <CareIcon icon="l-export" />,
disabled: totalCount === 0 || !authorizedForImportExport,
id: "export-json-option",
},
},
{
label: "Export Assets (CSV)",
action: async () => {
const { data } = await request(routes.listAssets, {
query: { ...qParams, csv: true, limit: totalCount },
});
return data ?? null;
},
type: "csv",
filePrefix: `assets_${facility?.name ?? "all"}`,
options: {
icon: <CareIcon icon="l-export" />,
disabled: totalCount === 0 || !authorizedForImportExport,
id: "export-csv-option",
},
},
]}
/>
</div>
)}
</>
}
>
<div className="mt-5 gap-3 space-y-2 lg:flex">
<CountBlock
text="Total Assets"
count={totalCount}
loading={loading}
icon="d-folder"
className="flex-1"
/>
<div className="flex-1">
<SearchInput
id="asset-search"
name="search"
value={qParams.search}
onChange={(e) => updateQuery({ [e.name]: e.value })}
placeholder="Search by name/serial no./QR code ID"
/>
</div>
<div className="flex flex-col items-start justify-start gap-2 lg:ml-2">
<div className="flex w-full flex-col gap-2 md:flex-row lg:w-auto">
<div className="w-full">
<AdvancedFilterButton
onClick={() => advancedFilter.setShow(true)}
/>
</div>
<ButtonV2
className="w-full py-[11px]"
onClick={() => setIsScannerActive(true)}
>
<CareIcon icon="l-search" className="mr-1 text-base" /> Scan Asset
QR
</ButtonV2>
</div>
<div className="flex w-full flex-col items-center justify-between lg:flex-row">
<div
className="flex w-full flex-col md:flex-row"
className="mb-2 flex w-full flex-col items-center lg:mb-0 lg:w-fit lg:flex-row lg:gap-5"
data-testid="create-asset-buttom"
>
<ButtonV2
authorizeFor={NonReadOnlyUsers}
className="inline-flex w-full items-center justify-center"
<Button
disabled={isDisabled}
variant={"primary"}
size={"lg"}
className="gap-2 w-full lg:w-fit"
onClick={() => {
if (qParams.facility) {
navigate(`/facility/${qParams.facility}/assets/new`);
Expand All @@ -423,9 +374,103 @@ const AssetsList = () => {
>
<CareIcon icon="l-plus-circle" className="text-lg" />
<span>{t("create_asset")}</span>
</ButtonV2>
</Button>
</div>
<div className="flex w-full flex-col items-center justify-end gap-2 lg:ml-3 lg:w-fit lg:flex-row lg:gap-3">
<AdvancedFilterButton
onClick={() => advancedFilter.setShow(true)}
/>

<Button
variant={"primary"}
size={"lg"}
className="gap-2 w-full"
onClick={() => setIsScannerActive(true)}
>
<CareIcon icon="l-search" className="mr-1 text-base" />
Scan Asset QR
</Button>

<div
className="tooltip w-full md:w-auto"
data-testid="import-asset-button"
>
{authorizedForImportExport && (
<ExportMenu
label={
importAssetModalOpen ? "Importing..." : "Import/Export"
}
exportItems={[
{
label: "Import Assets",
options: {
icon: (
<CareIcon
icon="l-import"
className="import-assets-button"
/>
),
onClick: () => setImportAssetModalOpen(true),
},
},
{
label: "Export Assets (JSON)",
action: async () => {
const { data } = await request(routes.listAssets, {
query: { ...qParams, json: true, limit: totalCount },
});
return data ?? null;
},
type: "json",
filePrefix: `assets_${facility?.name ?? "all"}`,
options: {
icon: <CareIcon icon="l-export" />,
disabled:
totalCount === 0 || !authorizedForImportExport,
id: "export-json-option",
},
},
{
label: "Export Assets (CSV)",
action: async () => {
const { data } = await request(routes.listAssets, {
query: { ...qParams, csv: true, limit: totalCount },
});
return data ?? null;
},
type: "csv",
filePrefix: `assets_${facility?.name ?? "all"}`,
options: {
icon: <CareIcon icon="l-export" />,
disabled:
totalCount === 0 || !authorizedForImportExport,
id: "export-csv-option",
},
},
]}
/>
)}
</div>
</div>
</div>
}
>
<div className="mt-4 gap-4 lg:gap-16 flex flex-col lg:flex-row lg:items-center">
<CountBlock
text="Total Assets"
count={totalCount}
loading={loading}
icon="d-folder"
className="flex-1"
/>

<SearchByMultipleFields
id="asset-search"
options={searchOptions}
onSearch={(key, value) => updateQuery({ search: value })}
clearSearch={clearSearch}
className="w-full"
/>
</div>
<AssetFilter {...advancedFilter} key={window.location.search} />
{isLoading ? (
Expand Down Expand Up @@ -543,7 +588,7 @@ export const warrantyAmcValidityChip = (
<Chip
id="warranty-amc-expiring-soon-orange"
variant="custom"
className="border-orange-300 bg-orange-100 text-orange-900"
className="border-orange-200 bg-orange-100 text-orange-900"
startIcon="l-exclamation-circle"
text="AMC/Warranty Expiring Soon"
/>
Expand Down
Loading