diff --git a/.env.example b/.env.example index 8f908a6f..29697135 100644 --- a/.env.example +++ b/.env.example @@ -6,6 +6,8 @@ DB_PORT=5432 TZ=UTC +AUTH_ENABLED=true + IMAGE_PATH=./images DATA_PATH=./data LOG_PATH=./logs diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000..25f3844b --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,8 @@ +# CODEOWNERS +# +# Defines required reviewers for specific paths. +# Any PR touching these files will require approval from the listed owner(s) +# before it can be merged, regardless of other branch protection settings. + +# ── Default: you review everything ──────────────────────────────────────────── +* @mregni \ No newline at end of file diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 00000000..c9258afe --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,20 @@ +blank_issues_enabled: false + +contact_links: + - name: Security Vulnerability + url: https://github.com/mregni/boardgametracker/security/advisories/new + about: > + Please report security issues privately using GitHub's vulnerability + reporting — not as a public issue. We will respond within 72 hours. + + - name: Question / Help / How-to + url: https://github.com/mregni/boardgametracker/discussions/categories/q-a + about: > + For help with setup, configuration, or general questions — use + Discussions instead of Issues so the community can benefit too. + + - name: Ideas & Feedback + url: https://github.com/mregni/boardgametracker/discussions/categories/ideas + about: > + For open-ended ideas or feedback that isn't a concrete feature request yet, + start a Discussion to gather community input first. diff --git a/.github/labeler.yml b/.github/labeler.yml new file mode 100644 index 00000000..3a38b97f --- /dev/null +++ b/.github/labeler.yml @@ -0,0 +1,56 @@ +backend: + - changed-files: + - any-glob-to-any-file: + - "BoardGameTracker.Core/**" + - "BoardGameTracker.Api/**" + - "BoardGameTracker.Common/**" + - "BoardGameTracker.Host/**" + +frontend: + - changed-files: + - any-glob-to-any-file: + - "boardgametracker.client/src/**" + +auth: + - changed-files: + - any-glob-to-any-file: + - "BoardGameTracker.Core/**/Auth/**" + - "BoardGameTracker.Api/Controllers/AuthController.cs" + - "BoardGameTracker.Api/Controllers/OidcController.cs" + - "BoardGameTracker.Api/Infrastructure/AuthDisabled*" + +database: + - changed-files: + - any-glob-to-any-file: + - "BoardGameTracker.Core/**/Datastore/**" + - "**/*Migration*" + +docker: + - changed-files: + - any-glob-to-any-file: + - "Dockerfile" + - "docker-compose*.yml" + - "entrypoint.sh" + +ci: + - changed-files: + - any-glob-to-any-file: + - ".github/**" + +dependencies: + - changed-files: + - any-glob-to-any-file: + - "boardgametracker.client/package*.json" + - "**/*.csproj" + +tests: + - changed-files: + - any-glob-to-any-file: + - "BoardGameTracker.Tests/**" + - "boardgametracker.client/**/*.test.*" + - "boardgametracker.client/**/*.spec.*" + +i18n: + - changed-files: + - any-glob-to-any-file: + - "boardgametracker.client/public/locales/**" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..0852881c --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,207 @@ +name: CI + +on: + pull_request: + branches: + - dev + - master + paths-ignore: + - "docs/**" + - ".github/workflows/docs.yml" + +env: + REGISTRY: ${{ vars.DOCKER_REGISTRY || 'docker.io' }} + IMAGE_NAME: ${{ vars.DOCKER_IMAGE_NAME || 'uping/boardgametracker' }} + +jobs: + version: + name: Calculate Version + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + version: ${{ steps.versioning.outputs.version }} + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Git Semantic Version + uses: PaulHatch/semantic-version@v5.4.0 + id: versioning + with: + enable_prerelease_mode: true + namespace: beta + bump_each_commit: true + version_format: "${major}.${minor}.${patch}-beta" + debug: true + + test-and-analyze: + name: Test and SonarCloud Analysis + runs-on: ubuntu-latest + needs: [version] + permissions: + contents: read + pull-requests: write + checks: write + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: 17 + distribution: 'zulu' + + - name: Setup dotnet v8 + uses: actions/setup-dotnet@v4 + with: + dotnet-version: "8.x" + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "20.x" + cache: 'npm' + cache-dependency-path: boardgametracker.client/package-lock.json + + - name: Cache SonarCloud packages + uses: actions/cache@v4 + with: + path: ~/.sonar/cache + key: ${{ runner.os }}-sonar + restore-keys: ${{ runner.os }}-sonar + + - name: Cache SonarCloud scanner + id: cache-sonar-scanner + uses: actions/cache@v4 + with: + path: ./.sonar/scanner + key: ${{ runner.os }}-sonar-scanner + restore-keys: ${{ runner.os }}-sonar-scanner + + - name: Install SonarCloud scanner + if: steps.cache-sonar-scanner.outputs.cache-hit != 'true' + run: | + mkdir -p ./.sonar/scanner + dotnet tool update dotnet-sonarscanner --tool-path ./.sonar/scanner + + - name: Install .NET dependencies + run: dotnet restore ./BoardGameTracker.sln + + - name: Begin SonarCloud analysis + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + SONAR_PROJECT_KEY: ${{ secrets.SONAR_PROJECT_KEY }} + SONAR_ORGANIZATION: ${{ secrets.SONAR_ORGANIZATION }} + run: | + ./.sonar/scanner/dotnet-sonarscanner begin \ + /k:"${{ env.SONAR_PROJECT_KEY }}" \ + /o:"${{ env.SONAR_ORGANIZATION }}" \ + /d:sonar.host.url="https://sonarcloud.io" \ + /d:sonar.token="${{ env.SONAR_TOKEN }}" \ + /v:"${{ needs.version.outputs.version }}" \ + /d:sonar.cs.opencover.reportsPaths="TestResults/**/coverage.opencover.xml" \ + /d:sonar.cs.vstest.reportsPaths="TestResults/*.trx" \ + /d:sonar.javascript.lcov.reportPaths="coverage/lcov.info" \ + /d:sonar.testExecutionReportPaths="boardgametracker.client/coverage/sonar-report.xml" \ + /d:sonar.exclusions="**/node_modules/**,**/dist/**,**/build/**,**/coverage/**,**/TestResults/**,**/obj/**,**/bin/**" \ + /d:sonar.coverage.exclusions="**/BoardGameTracker.Host/**/*.cs,**/BoardGameTracker.Core/Datastore/**/*.cs,**/ViewModels/**/*.cs,**/Entities/**/*.cs,**/routeTree.gen.ts,**/tailwind.config.js,**/node_modules/**" \ + /d:sonar.qualitygate.wait=true \ + /d:sonar.qualitygate.timeout=300 + if: env.SONAR_TOKEN != '' + + - name: Build .NET + run: dotnet build --no-restore + + - name: Run .NET tests + run: | + dotnet test ./BoardGameTracker.Tests/BoardGameTracker.Tests.csproj \ + --no-build \ + --no-restore \ + --logger trx \ + --results-directory "TestResults" \ + --collect "XPlat Code Coverage;Format=opencover" \ + -- DataCollectionRunSettings.DataCollectors.DataCollector.Configuration.ExcludeByFile="**/BoardGameTracker.Host/**/*.cs,**/DataStore/**/*.cs,**/ViewModels/**/*.cs,**/Entities/**/*.cs" + + - name: Install frontend dependencies + run: | + cd boardgametracker.client + npm ci --ignore-scripts + + - name: Run Biome format check + run: | + cd boardgametracker.client + npm run format:check + + - name: Run Biome lint + run: | + cd boardgametracker.client + npm run lint + + - name: Run frontend tests with coverage + run: | + cd boardgametracker.client + npm run test:coverage + # Convert LCOV/sonar paths to absolute so SonarCloud resolves them regardless of module context + sed -i "s|^SF:src/|SF:$(pwd)/src/|" coverage/lcov.info + sed -i "s|path=\"src/|path=\"$(pwd)/src/|g" coverage/sonar-report.xml + + - name: Publish .NET test results + uses: EnricoMi/publish-unit-test-result-action@v2 + if: always() + with: + files: TestResults/**/*.trx + check_name: ".NET Test Results" + fail_on: "test failures" + + - name: End SonarCloud analysis + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} + run: ./.sonar/scanner/dotnet-sonarscanner end /d:sonar.token="${{ env.SONAR_TOKEN }}" + if: env.SONAR_TOKEN != '' + + - name: Upload test results + uses: actions/upload-artifact@v4 + with: + name: test-results + path: | + TestResults + boardgametracker.client/coverage + retention-days: 7 + if: success() || failure() + + - name: Code Coverage Report + uses: danielpalme/ReportGenerator-GitHub-Action@5.3.11 + with: + reports: "TestResults/**/coverage.opencover.xml;boardgametracker.client/coverage/lcov.info" + targetdir: "coveragereport" + reporttypes: "MarkdownSummaryGithub;Cobertura" + sourcedirs: "boardgametracker.client" + assemblyfilters: "-BoardGameTracker.Host;-BoardGameTracker.DataStore" + filefilters: "-**/ViewModels/**;-**/Entities/**" + + - name: Publish Coverage Summary + run: cat coveragereport/SummaryGithub.md >> $GITHUB_STEP_SUMMARY + + - name: Add Coverage PR Comment + uses: marocchino/sticky-pull-request-comment@v2 + with: + recreate: true + path: coveragereport/SummaryGithub.md diff --git a/.github/workflows/compress-images.yml b/.github/workflows/compress-images.yml new file mode 100644 index 00000000..50e1471b --- /dev/null +++ b/.github/workflows/compress-images.yml @@ -0,0 +1,50 @@ +name: Compress Images + +on: + pull_request: + paths: + - "**.jpg" + - "**.jpeg" + - "**.png" + - "**.webp" + schedule: + # Weekly Monday 09:00 UTC + - cron: "0 9 * * 1" + +jobs: + compress: + name: Compress Images + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + # Only run on PRs (not schedule) if there are image changes + if: github.event_name == 'pull_request' || github.event_name == 'schedule' + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + + - name: Compress images + id: calibre + uses: calibreapp/image-actions@main + with: + githubToken: ${{ secrets.GITHUB_TOKEN }} + compressOnly: ${{ github.event_name == 'schedule' }} + jpegQuality: "80" + jpegProgressive: true + pngQuality: "80" + webpQuality: "80" + + - name: Create commit for scheduled run + if: github.event_name == 'schedule' && steps.calibre.outputs.markdown != '' + run: | + git config --global user.name "github-actions[bot]" + git config --global user.email "github-actions[bot]@users.noreply.github.com" + git add -A + git commit -m "chore: compress images [skip ci]" || echo "No changes to commit" + git push diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index bc954837..03c3b4dd 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -8,11 +8,6 @@ on: - ".github/workflows/docs.yml" workflow_dispatch: -permissions: - contents: read - pages: write - id-token: write - concurrency: group: "pages" cancel-in-progress: false @@ -20,10 +15,19 @@ concurrency: jobs: build: runs-on: ubuntu-latest + permissions: + contents: read + pages: write + id-token: write defaults: run: working-directory: docs steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Checkout uses: actions/checkout@v4 @@ -52,12 +56,20 @@ jobs: path: docs/dist deploy: + permissions: + pages: write + id-token: write environment: name: github-pages url: ${{ steps.deployment.outputs.page_url }} runs-on: ubuntu-latest needs: build steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Deploy to GitHub Pages id: deployment uses: actions/deploy-pages@v4 diff --git a/.github/workflows/metrics.yml b/.github/workflows/metrics.yml new file mode 100644 index 00000000..78316b3f --- /dev/null +++ b/.github/workflows/metrics.yml @@ -0,0 +1,42 @@ +name: Metrics + +on: + schedule: + - cron: '0 0 * * *' + workflow_dispatch: + +jobs: + metrics: + name: "Generate metrics card" + runs-on: ubuntu-latest + permissions: + contents: write + + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Generate metrics + uses: lowlighter/metrics@latest + with: + token: ${{ secrets.METRICS_TOKEN }} + filename: assets/metrics.svg + output_action: commit + committer_message: "chore: update metrics [skip ci]" + base: header, activity, community, repositories + plugin_languages: yes + plugin_languages_sections: most-used + plugin_languages_colors: github + plugin_languages_limit: 6 + plugin_activity: yes + plugin_activity_limit: 5 + plugin_activity_filter: > + issue, pr, release, fork, star, push + plugin_stargazers: yes + plugin_topics: yes + plugin_topics_mode: icons + config_display: large + config_animations: yes + config_timezone: Europe/Brussels diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml new file mode 100644 index 00000000..494e9ff7 --- /dev/null +++ b/.github/workflows/pr.yml @@ -0,0 +1,88 @@ +name: PR Quality + +on: + pull_request: + branches: [dev, master] + types: [opened, synchronize, reopened, edited, labeled, unlabeled] + +jobs: + labeler: + name: PR Labeler + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + + - name: Apply file-based labels + uses: actions/labeler@v5 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + configuration-path: .github/labeler.yml + + - name: Remove old size labels + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + PR_NUMBER=${{ github.event.pull_request.number }} + REPO=${{ github.repository }} + for LABEL in "size/XS" "size/S" "size/M" "size/L" "size/XL"; do + gh api "repos/$REPO/issues/$PR_NUMBER/labels/$LABEL" -X DELETE 2>/dev/null || true + done + + - name: Label PR by size + uses: codelytv/pr-size-labeler@v1 + with: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + xs_label: "size/XS" + xs_max_size: 10 + s_label: "size/S" + s_max_size: 100 + m_label: "size/M" + m_max_size: 500 + l_label: "size/L" + l_max_size: 1000 + xl_label: "size/XL" + fail_if_xl: false + message_if_xl: > + This PR is very large. Consider breaking it into smaller, + more focused pull requests for easier review. + + semantic-title: + name: Semantic PR Title + runs-on: ubuntu-latest + permissions: {} + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Validate PR title + uses: amannn/action-semantic-pull-request@v5 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + with: + types: | + feat + fix + docs + style + refactor + perf + test + build + ci + chore + requireScope: false + subjectPattern: ^(?![A-Z]).+$ + subjectPatternError: > + The subject "{subject}" found in the pull request title "{title}" + should start with a lowercase letter. Example: "feat: add BGG sync" \ No newline at end of file diff --git a/.github/workflows/publish-container-dev.yml b/.github/workflows/publish-container-dev.yml index 0f08e149..8eef9569 100644 --- a/.github/workflows/publish-container-dev.yml +++ b/.github/workflows/publish-container-dev.yml @@ -1,4 +1,4 @@ -name: Deploy Application +name: Deploy on: push: @@ -8,13 +8,6 @@ on: paths-ignore: - "docs/**" - ".github/workflows/docs.yml" - pull_request: - branches: - - dev - - master - paths-ignore: - - "docs/**" - - ".github/workflows/docs.yml" env: REGISTRY: ${{ vars.DOCKER_REGISTRY || 'docker.io' }} @@ -24,10 +17,17 @@ jobs: version: name: Calculate Version runs-on: ubuntu-latest + permissions: + contents: read outputs: version: ${{ steps.versioning.outputs.version }} is_master: ${{ github.ref == 'refs/heads/master' }} steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Checkout uses: actions/checkout@v4 with: @@ -37,10 +37,10 @@ jobs: uses: PaulHatch/semantic-version@v5.4.0 id: versioning with: - enable_prerelease_mode: ${{ github.ref == 'refs/heads/dev' || github.event_name == 'pull_request' }} + enable_prerelease_mode: ${{ github.ref == 'refs/heads/dev' }} namespace: beta bump_each_commit: true - version_format: "${{ (github.ref == 'refs/heads/dev' || github.event_name == 'pull_request') && '${major}.${minor}.${patch}-beta' || '${major}.${minor}.${patch}' }}" + version_format: "${{ github.ref == 'refs/heads/dev' && '${major}.${minor}.${patch}-beta' || '${major}.${minor}.${patch}' }}" debug: true test-and-analyze: @@ -49,8 +49,12 @@ jobs: needs: [version] permissions: contents: read - pull-requests: write steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Checkout uses: actions/checkout@v4 with: @@ -113,7 +117,9 @@ jobs: /d:sonar.cs.vstest.reportsPaths="TestResults/*.trx" \ /d:sonar.javascript.lcov.reportPaths="boardgametracker.client/coverage/lcov.info" \ /d:sonar.exclusions="**/node_modules/**,**/dist/**,**/build/**,**/coverage/**,**/TestResults/**,**/obj/**,**/bin/**" \ - /d:sonar.coverage.exclusions="**/BoardGameTracker.Host/**/*.cs,**/BoardGameTracker.Core/Datastore/**/*.cs,**/ViewModels/**/*.cs,**/Entities/**/*.cs,**/routeTree.gen.ts,**/tailwind.config.js,**/node_modules/**" + /d:sonar.coverage.exclusions="**/BoardGameTracker.Host/**/*.cs,**/BoardGameTracker.Core/Datastore/**/*.cs,**/ViewModels/**/*.cs,**/Entities/**/*.cs,**/routeTree.gen.ts,**/tailwind.config.js,**/node_modules/**" \ + /d:sonar.qualitygate.wait=true \ + /d:sonar.qualitygate.timeout=300 if: env.SONAR_TOKEN != '' - name: Build .NET @@ -154,7 +160,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} run: ./.sonar/scanner/dotnet-sonarscanner end /d:sonar.token="${{ secrets.SONAR_TOKEN }}" - if: env.SONAR_TOKEN != '' && github.event_name != 'pull_request' + if: env.SONAR_TOKEN != '' - name: Upload test results uses: actions/upload-artifact@v4 @@ -179,20 +185,19 @@ jobs: - name: Publish Coverage Summary run: cat coveragereport/SummaryGithub.md >> $GITHUB_STEP_SUMMARY - - name: Add Coverage PR Comment - uses: marocchino/sticky-pull-request-comment@v2 - if: github.event_name == 'pull_request' - with: - recreate: true - path: coveragereport/SummaryGithub.md - build-linux-multiarch: needs: [version, test-and-analyze] name: Build Multi-Arch Linux Containers runs-on: ubuntu-latest timeout-minutes: 60 - if: github.event_name != 'pull_request' + permissions: + contents: read steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Checkout uses: actions/checkout@v4 with: @@ -262,12 +267,16 @@ jobs: name: Security Scan runs-on: ubuntu-latest timeout-minutes: 30 - if: github.event_name != 'pull_request' permissions: security-events: write actions: read contents: read steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Checkout uses: actions/checkout@v4 @@ -332,6 +341,11 @@ jobs: permissions: contents: write steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Checkout uses: actions/checkout@v4 with: @@ -399,8 +413,13 @@ jobs: name: Build Summary needs: [version, build-linux-multiarch] runs-on: ubuntu-latest - if: github.event_name != 'pull_request' + permissions: {} steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + - name: Build Summary run: | VERSION=${{ needs.version.outputs.version }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 00000000..eaed1fc7 --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,24 @@ +name: Release Please + +on: + push: + branches: [master] + +jobs: + release-please: + name: Release Please + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: write + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Run Release Please + uses: google-github-actions/release-please-action@v4 + with: + release-type: simple + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 00000000..47a0386b --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,297 @@ +name: Security Scanning + +on: + pull_request: + branches: [dev, master] + push: + branches: [dev, master] + +jobs: + dependency-scan-backend: + name: Backend Dependency Scan + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup dotnet v8 + uses: actions/setup-dotnet@v4 + with: + dotnet-version: "8.x" + + - name: Restore packages + run: dotnet restore ./BoardGameTracker.sln + + - name: Check for vulnerable packages + run: | + dotnet list ./BoardGameTracker.sln package --vulnerable --include-transitive 2>&1 | tee vulnerable.txt + + if grep -q "has the following vulnerable packages" vulnerable.txt; then + echo "::error::Vulnerable NuGet packages detected" + exit 1 + fi + + echo "No vulnerable packages found." + + dependency-scan-frontend: + name: Frontend Dependency Scan + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node.js + uses: actions/setup-node@v4 + with: + node-version: "20.x" + cache: "npm" + cache-dependency-path: boardgametracker.client/package-lock.json + + - name: Install dependencies + run: | + cd boardgametracker.client + npm ci --ignore-scripts + + - name: Audit for vulnerabilities + run: | + cd boardgametracker.client + npm audit --audit-level=moderate + + secret-detection: + name: Secret Detection + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout (full history) + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Run Gitleaks + uses: gitleaks/gitleaks-action@v2 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + docker-scout: + name: Docker Scout CVE Scan + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + security-events: write + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Login to Docker Hub + uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_TOKEN }} + + - name: Build image for scanning + uses: docker/build-push-action@v5 + with: + context: . + file: ./Dockerfile + push: false + load: true + tags: bgt:scout-scan + cache-from: type=gha + build-args: | + VERSION=0.0.0-security + + - name: Docker Scout CVE scan + uses: docker/scout-action@v1 + with: + command: cves + image: local://bgt:scout-scan + only-severities: critical,high + sarif-file: scout-results.sarif + exit-code: true + only-fixed: true + + - name: Upload Scout SARIF + uses: github/codeql-action/upload-sarif@v3 + if: always() + with: + sarif_file: scout-results.sarif + category: docker-scout + + zap-baseline: + name: ZAP Baseline Scan + runs-on: ubuntu-latest + permissions: + contents: read + issues: write + pull-requests: write + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Build application image + uses: docker/build-push-action@v5 + with: + context: . + file: ./Dockerfile + push: false + load: true + tags: bgt:security-scan + cache-from: type=gha + build-args: | + VERSION=0.0.0-security + + - name: Generate ephemeral secrets + id: secrets + run: | + DB_PASSWORD=$(openssl rand -base64 24) + JWT_SECRET=$(openssl rand -base64 48) + echo "::add-mask::$DB_PASSWORD" + echo "::add-mask::$JWT_SECRET" + echo "db_password=$DB_PASSWORD" >> "$GITHUB_OUTPUT" + echo "jwt_secret=$JWT_SECRET" >> "$GITHUB_OUTPUT" + + - name: Start application stack + env: + SEC_DB_PASSWORD: ${{ steps.secrets.outputs.db_password }} + SEC_JWT_SECRET: ${{ steps.secrets.outputs.jwt_secret }} + run: | + docker compose -f infra/docker-compose.security.yml up -d + echo "Waiting for application to become healthy..." + for i in $(seq 1 60); do + if curl -sf http://localhost:5444/api/health > /dev/null 2>&1; then + echo "Application is healthy after ${i}s" + break + fi + if [ "$i" -eq 60 ]; then + echo "::error::Application failed to become healthy within 60s" + docker compose -f infra/docker-compose.security.yml logs + exit 1 + fi + sleep 1 + done + + - name: Authenticate and extract JWT + id: auth + env: + ZAP_TEST_EMAIL: ${{ secrets.ZAP_TEST_EMAIL }} + ZAP_TEST_PASSWORD: ${{ secrets.ZAP_TEST_PASSWORD }} + run: | + RESPONSE=$(curl -sf -X POST http://localhost:5444/api/auth/login \ + -H "Content-Type: application/json" \ + -d "{\"username\": \"${{ env.ZAP_TEST_EMAIL }}\", \"password\": \"${{ env.ZAP_TEST_PASSWORD }}\"}") + + TOKEN=$(echo "$RESPONSE" | jq -r '.accessToken') + + if [ -z "$TOKEN" ] || [ "$TOKEN" = "null" ]; then + echo "::error::Failed to extract JWT from login response" + echo "Response: $RESPONSE" + exit 1 + fi + + echo "::add-mask::$TOKEN" + echo "token=$TOKEN" >> "$GITHUB_OUTPUT" + + - name: Configure ZAP Replacer for JWT injection + run: | + mkdir -p /tmp/zap + + # ZAP Automation Framework context with auth header injection + cat > /tmp/zap/af-plan.yaml << 'ZAPEOF' + --- + env: + contexts: + - name: "BoardGameTracker" + urls: + - "http://localhost:5444" + includePaths: + - "http://localhost:5444/api/.*" + excludePaths: + - "http://localhost:5444/api/health" + parameters: + failOnError: true + failOnWarning: false + progressToStdout: true + jobs: + - type: requestor + parameters: + user: "" + rules: [] + - type: passiveScan-config + parameters: + maxAlertsPerRule: 10 + scanOnlyInScope: true + maxBodySizeInBytesToScan: 10000 + - type: spider + parameters: + context: "BoardGameTracker" + maxDuration: 5 + maxDepth: 5 + maxChildren: 10 + - type: passiveScan-wait + parameters: + maxDuration: 5 + - type: report + parameters: + template: "sarif-json" + reportDir: "/zap/wrk" + reportFile: "zap-report" + reportTitle: "ZAP Baseline Scan - BoardGameTracker" + ZAPEOF + + - name: Run ZAP Baseline Scan + uses: zaproxy/action-baseline@v0.14.0 + with: + target: "http://localhost:5444" + docker_name: "ghcr.io/zaproxy/zaproxy:stable" + allow_issue_writing: true + fail_action: "false" + cmd_options: >- + -z "-config replacer.full_list(0).description=AuthHeader + -config replacer.full_list(0).enabled=true + -config replacer.full_list(0).matchtype=REQ_HEADER + -config replacer.full_list(0).matchstr=Authorization + -config replacer.full_list(0).regex=false + -config replacer.full_list(0).replacement=Bearer\ ${{ steps.auth.outputs.token }} + -config replacer.full_list(0).initiators=" + rules_file_name: ".zap/rules.tsv" + + - name: Tear down application stack + if: always() + run: docker compose -f infra/docker-compose.security.yml down -v diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml new file mode 100644 index 00000000..19ed3c30 --- /dev/null +++ b/.github/workflows/stale.yml @@ -0,0 +1,128 @@ +name: Stale Issues & PRs + +on: + schedule: + - cron: '0 4 * * *' + workflow_dispatch: + +env: + REPO_OWNER_USERNAME: mregni + +jobs: + stale: + name: "Stale Issues & PRs" + runs-on: ubuntu-latest + permissions: + issues: write + pull-requests: write + + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - name: Label owner-responded issues + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + OWNER: ${{ env.REPO_OWNER_USERNAME }} + REPO: ${{ github.repository }} + run: | + echo "Scanning open issues for comments by '$OWNER'..." + + ISSUES=$(gh issue list \ + --repo "$REPO" \ + --state open \ + --json number \ + --limit 500 \ + --jq '.[].number') + + LABELED=0 + SKIPPED=0 + + for ISSUE_NUMBER in $ISSUES; do + OWNER_COMMENTED=$(gh api \ + "repos/$REPO/issues/$ISSUE_NUMBER/comments" \ + --jq "[.[] | select(.user.login == \"$OWNER\")] | length") + + if [ "$OWNER_COMMENTED" -gt "0" ]; then + ALREADY_LABELED=$(gh issue view "$ISSUE_NUMBER" \ + --repo "$REPO" \ + --json labels \ + --jq '[.labels[].name] | contains(["owner-responded"])') + + if [ "$ALREADY_LABELED" = "false" ]; then + echo " Issue #$ISSUE_NUMBER — owner commented, adding label" + gh issue edit "$ISSUE_NUMBER" \ + --repo "$REPO" \ + --add-label "owner-responded" + LABELED=$((LABELED + 1)) + else + SKIPPED=$((SKIPPED + 1)) + fi + fi + done + + echo "" + echo "Done. Newly labeled: $LABELED | Already labeled: $SKIPPED" + + - name: Ensure labels exist + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + run: | + gh label create "stale" \ + --repo "$REPO" \ + --description "No recent activity" \ + --color "ededed" \ + --force + + gh label create "owner-responded" \ + --repo "$REPO" \ + --description "Repo owner has commented on this issue" \ + --color "0075ca" \ + --force + + - name: Process stale issues and PRs + uses: actions/stale@v9 + with: + stale-issue-label: stale + stale-issue-message: > + This issue has had no activity for 60 days and has been marked + as stale. It will be closed in 14 days if there is no further + activity. If this is still relevant, please leave a comment. + close-issue-message: > + Closing due to inactivity. Feel free to reopen if this is + still relevant. Thank you for the report. + days-before-issue-stale: 60 + days-before-issue-close: 14 + + exempt-issue-labels: > + owner-responded, + bug, + security, + pinned, + enhancement, + in progress + + stale-pr-label: stale + stale-pr-message: > + This pull request has had no activity for 30 days and has been + marked as stale. It will be closed in 7 days if there is no + further activity. Please rebase and update if you'd like this + merged. + close-pr-message: > + Closing due to inactivity. Please reopen and rebase if you + would like to continue with this pull request. + days-before-pr-stale: 30 + days-before-pr-close: 7 + + exempt-pr-labels: > + pinned, + security, + in progress, + do not merge + + operations-per-run: 100 + remove-stale-when-updated: true + exempt-all-assignees: false diff --git a/.github/workflows/welcome.yml b/.github/workflows/welcome.yml new file mode 100644 index 00000000..9cdc3e22 --- /dev/null +++ b/.github/workflows/welcome.yml @@ -0,0 +1,50 @@ +name: Welcome New Contributors + +on: + issues: + types: [opened] + pull_request_target: + types: [opened] + +jobs: + welcome: + name: "Welcome first-time contributor" + runs-on: ubuntu-latest + permissions: + issues: write + pull-requests: write + + steps: + - name: Harden runner + uses: step-security/harden-runner@v2 + with: + egress-policy: audit + + - uses: actions/first-interaction@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + + issue-message: | + Hey @${{ github.actor }}, thanks for opening your first issue on BoardGameTracker! + + To help us triage this as quickly as possible, please make sure you've included: + - Your BoardGameTracker version (Settings in the app) + - Your deployment method (Docker Compose / Docker / bare metal) + - Steps to reproduce the problem + - Any relevant container logs (`docker compose logs boardgametracker --tail=100`) + + I'll take a look as soon as I can. Thanks for taking the time to report! + + pr-message: | + Hey @${{ github.actor }}, thanks for opening your first pull request on BoardGameTracker! + + Before I review, here's a quick checklist: + + - [ ] PR title follows conventional commits format (`feat:`, `fix:`, `docs:`, `chore:`, etc.) + - [ ] If you added or changed UI text, translation keys are updated in all three locale files + (`en-US.json`, `nl-NL.json`, `nl-BE.json` under `boardgametracker.client/public/locales/`) + - [ ] If you changed backend logic, relevant xUnit tests are added or updated + - [ ] If you changed an API response shape, the corresponding DTO and extension method are updated + + Don't worry if you're unsure about anything — I'll guide you through the review. + Thanks for contributing! diff --git a/.gitignore b/.gitignore index fa842f6c..d7951689 100644 --- a/.gitignore +++ b/.gitignore @@ -642,3 +642,5 @@ boardgametracker.client/.tanstack/ BoardGameTracker.Host/config/config.xml BoardGameTracker.Host/appsettings.Development.json + +boardgametracker.client/.env.sentry-build-plugin diff --git a/.zap/rules.tsv b/.zap/rules.tsv new file mode 100644 index 00000000..a20d094d --- /dev/null +++ b/.zap/rules.tsv @@ -0,0 +1,2 @@ +10049 IGNORE (Storable and Cacheable Content) +10050 IGNORE (Retrieved from Cache) diff --git a/BoardGameTracker.Api/Controllers/Admin/OidcProvidersController.cs b/BoardGameTracker.Api/Controllers/Admin/OidcProvidersController.cs index ab5e4242..bc133ef3 100644 --- a/BoardGameTracker.Api/Controllers/Admin/OidcProvidersController.cs +++ b/BoardGameTracker.Api/Controllers/Admin/OidcProvidersController.cs @@ -1,3 +1,4 @@ +using BoardGameTracker.Api.Infrastructure; using BoardGameTracker.Common; using BoardGameTracker.Common.DTOs.Auth; using BoardGameTracker.Common.Extensions; @@ -10,6 +11,7 @@ namespace BoardGameTracker.Api.Controllers.Admin; [ApiController] [Route("api/admin/oidc-providers")] [Authorize(Roles = Constants.AuthRoles.Admin)] +[ServiceFilter(typeof(AuthDisabledFilter))] public class OidcProvidersController : ControllerBase { private readonly IOidcProviderService _service; diff --git a/BoardGameTracker.Api/Controllers/Admin/UsersController.cs b/BoardGameTracker.Api/Controllers/Admin/UsersController.cs index 970dad55..3e08aeb9 100644 --- a/BoardGameTracker.Api/Controllers/Admin/UsersController.cs +++ b/BoardGameTracker.Api/Controllers/Admin/UsersController.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using BoardGameTracker.Api.Infrastructure; using BoardGameTracker.Common; using BoardGameTracker.Common.DTOs.Auth; using BoardGameTracker.Core.Auth.Interfaces; @@ -10,6 +11,7 @@ namespace BoardGameTracker.Api.Controllers.Admin; [ApiController] [Route("api/admin/users")] [Authorize(Roles = Constants.AuthRoles.Admin)] +[ServiceFilter(typeof(AuthDisabledFilter))] public class UsersController : ControllerBase { private readonly IUserAdminService _userAdminService; diff --git a/BoardGameTracker.Api/Controllers/AuthController.cs b/BoardGameTracker.Api/Controllers/AuthController.cs index d096842d..5be3fe6c 100644 --- a/BoardGameTracker.Api/Controllers/AuthController.cs +++ b/BoardGameTracker.Api/Controllers/AuthController.cs @@ -1,9 +1,11 @@ using System.Security.Claims; +using BoardGameTracker.Api.Infrastructure; using BoardGameTracker.Common; using BoardGameTracker.Common.DTOs.Auth; using BoardGameTracker.Core.Auth.Interfaces; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.RateLimiting; using Microsoft.Extensions.Logging; namespace BoardGameTracker.Api.Controllers; @@ -11,6 +13,7 @@ namespace BoardGameTracker.Api.Controllers; [ApiController] [Route("api/auth")] [Authorize] +[ServiceFilter(typeof(AuthDisabledFilter))] public class AuthController : ControllerBase { private readonly IAuthService _authService; @@ -26,6 +29,7 @@ public AuthController(IAuthService authService, IOidcService oidcService, ILogge [HttpPost("login")] [AllowAnonymous] + [EnableRateLimiting("auth")] public async Task Login([FromBody] LoginRequest request) { _logger.LogInformation("Login attempt for user {Username}", request.Username); @@ -35,6 +39,7 @@ public async Task Login([FromBody] LoginRequest request) [HttpPost("refresh")] [AllowAnonymous] + [EnableRateLimiting("auth")] public async Task Refresh([FromBody] RefreshTokenRequest request) { var response = await _authService.RefreshAsync(request.RefreshToken); @@ -73,6 +78,7 @@ public async Task UpdateProfile([FromBody] UpdateProfileRequest r } [HttpPost("change-password")] + [EnableRateLimiting("auth")] public async Task ChangePassword([FromBody] ChangePasswordRequest request) { await _authService.ChangePasswordAsync(GetCurrentUserId(), request); diff --git a/BoardGameTracker.Api/Controllers/OidcController.cs b/BoardGameTracker.Api/Controllers/OidcController.cs index fa972d85..a8804b0d 100644 --- a/BoardGameTracker.Api/Controllers/OidcController.cs +++ b/BoardGameTracker.Api/Controllers/OidcController.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using BoardGameTracker.Api.Infrastructure; using BoardGameTracker.Common.DTOs.Auth; using BoardGameTracker.Core.Auth.Interfaces; using Microsoft.AspNetCore.Authorization; @@ -9,6 +10,7 @@ namespace BoardGameTracker.Api.Controllers; [ApiController] [Route("api/auth/oidc")] +[ServiceFilter(typeof(AuthDisabledFilter))] public class OidcController : ControllerBase { private readonly IOidcService _oidcService; diff --git a/BoardGameTracker.Api/Infrastructure/AuthBypassExtensions.cs b/BoardGameTracker.Api/Infrastructure/AuthBypassExtensions.cs deleted file mode 100644 index df1ba20f..00000000 --- a/BoardGameTracker.Api/Infrastructure/AuthBypassExtensions.cs +++ /dev/null @@ -1,17 +0,0 @@ -using Microsoft.AspNetCore.Builder; - -namespace BoardGameTracker.Api.Infrastructure; - -public static class AuthBypassExtensions -{ - public static IApplicationBuilder UseAuthBypassIfEnabled(this IApplicationBuilder app) - { - var authBypass = Environment.GetEnvironmentVariable("AUTH_BYPASS")?.ToLower() == "true"; - if (authBypass) - { - app.UseMiddleware(); - } - - return app; - } -} diff --git a/BoardGameTracker.Api/Infrastructure/AuthDisabledExtensions.cs b/BoardGameTracker.Api/Infrastructure/AuthDisabledExtensions.cs new file mode 100644 index 00000000..8b478119 --- /dev/null +++ b/BoardGameTracker.Api/Infrastructure/AuthDisabledExtensions.cs @@ -0,0 +1,19 @@ +using BoardGameTracker.Core.Configuration.Interfaces; +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; + +namespace BoardGameTracker.Api.Infrastructure; + +public static class AuthDisabledExtensions +{ + public static IApplicationBuilder UseAuthDisabledMiddleware(this IApplicationBuilder app) + { + var environmentProvider = app.ApplicationServices.GetRequiredService(); + if (!environmentProvider.AuthEnabled) + { + app.UseMiddleware(); + } + + return app; + } +} diff --git a/BoardGameTracker.Api/Infrastructure/AuthDisabledFilter.cs b/BoardGameTracker.Api/Infrastructure/AuthDisabledFilter.cs new file mode 100644 index 00000000..0754d390 --- /dev/null +++ b/BoardGameTracker.Api/Infrastructure/AuthDisabledFilter.cs @@ -0,0 +1,29 @@ +using BoardGameTracker.Core.Configuration.Interfaces; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace BoardGameTracker.Api.Infrastructure; + +public class AuthDisabledFilter : IActionFilter +{ + private readonly IEnvironmentProvider _environmentProvider; + + public AuthDisabledFilter(IEnvironmentProvider environmentProvider) + { + _environmentProvider = environmentProvider; + } + + public void OnActionExecuting(ActionExecutingContext context) + { + if (!_environmentProvider.AuthEnabled) + { + var path = context.HttpContext.Request.Path.Value ?? ""; + if (path.EndsWith("/status", StringComparison.OrdinalIgnoreCase)) + return; + + context.Result = new ConflictObjectResult("Authentication is disabled. This endpoint is not available."); + } + } + + public void OnActionExecuted(ActionExecutedContext context) { } +} diff --git a/BoardGameTracker.Api/Infrastructure/AuthBypassMiddleware.cs b/BoardGameTracker.Api/Infrastructure/AuthDisabledMiddleware.cs similarity index 68% rename from BoardGameTracker.Api/Infrastructure/AuthBypassMiddleware.cs rename to BoardGameTracker.Api/Infrastructure/AuthDisabledMiddleware.cs index d7ead70d..a8848e62 100644 --- a/BoardGameTracker.Api/Infrastructure/AuthBypassMiddleware.cs +++ b/BoardGameTracker.Api/Infrastructure/AuthDisabledMiddleware.cs @@ -4,11 +4,11 @@ namespace BoardGameTracker.Api.Infrastructure; -public class AuthBypassMiddleware +public class AuthDisabledMiddleware { private readonly RequestDelegate _next; - public AuthBypassMiddleware(RequestDelegate next) + public AuthDisabledMiddleware(RequestDelegate next) { _next = next; } @@ -19,13 +19,13 @@ public async Task InvokeAsync(HttpContext context) { var claims = new[] { - new Claim(ClaimTypes.NameIdentifier, "bypass-admin-id"), + new Claim(ClaimTypes.NameIdentifier, "auth-disabled-admin-id"), new Claim(ClaimTypes.Name, "admin"), new Claim(ClaimTypes.Role, Constants.AuthRoles.Admin), - new Claim("display_name", "Admin (Bypass)") + new Claim("display_name", "Admin") }; - var identity = new ClaimsIdentity(claims, "AuthBypass"); + var identity = new ClaimsIdentity(claims, "AuthDisabled"); context.User = new ClaimsPrincipal(identity); } diff --git a/BoardGameTracker.Common/DTOs/Auth/AuthModels.cs b/BoardGameTracker.Common/DTOs/Auth/AuthModels.cs index e15ef529..ce3937cb 100644 --- a/BoardGameTracker.Common/DTOs/Auth/AuthModels.cs +++ b/BoardGameTracker.Common/DTOs/Auth/AuthModels.cs @@ -18,4 +18,4 @@ public record ProfileResponse( DateTime? LastLoginAt, int? PlayerId); -public record AuthStatusResponse(bool AuthEnabled, bool BypassEnabled); +public record AuthStatusResponse(bool AuthEnabled); diff --git a/BoardGameTracker.Core/Auth/AuthService.cs b/BoardGameTracker.Core/Auth/AuthService.cs index c92b6878..cde14de7 100644 --- a/BoardGameTracker.Core/Auth/AuthService.cs +++ b/BoardGameTracker.Core/Auth/AuthService.cs @@ -214,9 +214,7 @@ public async Task ResetPasswordAsync(string userId) public AuthStatusResponse GetStatus() { - var authBypass = _environmentProvider.IsDevelopment && - Environment.GetEnvironmentVariable("AUTH_BYPASS")?.Equals("true", StringComparison.OrdinalIgnoreCase) == true; - return new AuthStatusResponse(AuthEnabled: true, BypassEnabled: authBypass); + return new AuthStatusResponse(AuthEnabled: _environmentProvider.AuthEnabled); } private static string GenerateTempPassword(int length = 16) diff --git a/BoardGameTracker.Core/Auth/RefreshTokenCleanupService.cs b/BoardGameTracker.Core/Auth/RefreshTokenCleanupService.cs index 38627f28..babc4871 100644 --- a/BoardGameTracker.Core/Auth/RefreshTokenCleanupService.cs +++ b/BoardGameTracker.Core/Auth/RefreshTokenCleanupService.cs @@ -1,4 +1,5 @@ using BoardGameTracker.Core.Auth.Interfaces; +using BoardGameTracker.Core.Configuration.Interfaces; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -19,6 +20,16 @@ public RefreshTokenCleanupService(IServiceScopeFactory scopeFactory, ILogger(); + if (!env.AuthEnabled) + { + _logger.LogInformation("Auth disabled, skipping refresh token cleanup service"); + return; + } + } + while (!stoppingToken.IsCancellationRequested) { await Task.Delay(Interval, stoppingToken); diff --git a/BoardGameTracker.Core/Configuration/EnvironmentProvider.cs b/BoardGameTracker.Core/Configuration/EnvironmentProvider.cs index b8b9adc3..887b744d 100644 --- a/BoardGameTracker.Core/Configuration/EnvironmentProvider.cs +++ b/BoardGameTracker.Core/Configuration/EnvironmentProvider.cs @@ -20,4 +20,9 @@ public class EnvironmentProvider : IEnvironmentProvider public LogEventLevel LogLevel => LogLevelExtensions.GetEnvironmentLogLevel(); public bool IsDevelopment => EnvironmentName.Equals("development", StringComparison.OrdinalIgnoreCase); + + public bool AuthEnabled => + !string.Equals(Environment.GetEnvironmentVariable("AUTH_ENABLED"), "false", StringComparison.OrdinalIgnoreCase); + + public string? JwtSecret => Environment.GetEnvironmentVariable("JWT_SECRET"); } \ No newline at end of file diff --git a/BoardGameTracker.Core/Configuration/Interfaces/IEnvironmentProvider.cs b/BoardGameTracker.Core/Configuration/Interfaces/IEnvironmentProvider.cs index 784652a2..6e581665 100644 --- a/BoardGameTracker.Core/Configuration/Interfaces/IEnvironmentProvider.cs +++ b/BoardGameTracker.Core/Configuration/Interfaces/IEnvironmentProvider.cs @@ -9,4 +9,6 @@ public interface IEnvironmentProvider bool StatisticsEnabled { get; } LogEventLevel LogLevel { get; } bool IsDevelopment { get; } + bool AuthEnabled { get; } + string? JwtSecret { get; } } \ No newline at end of file diff --git a/BoardGameTracker.Host/Program.cs b/BoardGameTracker.Host/Program.cs index 6ec1181a..443b4230 100644 --- a/BoardGameTracker.Host/Program.cs +++ b/BoardGameTracker.Host/Program.cs @@ -5,6 +5,7 @@ using BoardGameTracker.Api.Infrastructure; using BoardGameTracker.Common.Configuration; using BoardGameTracker.Common.Entities.Auth; +using BoardGameTracker.Core.Configuration; using BoardGameTracker.Core.Configuration.Interfaces; using BoardGameTracker.Common.Extensions; using BoardGameTracker.Common.Helpers; @@ -15,9 +16,11 @@ using BoardGameTracker.Core.Updates; using BoardGameTracker.Core.Disk.Interfaces; using BoardGameTracker.Core.Extensions; +using System.Threading.RateLimiting; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.RateLimiting; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.FileProviders; using Microsoft.Extensions.Http; @@ -53,6 +56,7 @@ builder.Services.AddHealthChecks(); builder.Services.AddExceptionHandler(); +builder.Services.AddScoped(); builder.Services.AddProblemDetails(); builder.Services.Configure(options => @@ -82,12 +86,19 @@ .AddEntityFrameworkStores() .AddDefaultTokenProviders(); -var jwtSecret = Environment.GetEnvironmentVariable("JWT_SECRET") ?? builder.Configuration["Jwt:Secret"]; +var environmentProvider = new EnvironmentProvider(); +var authEnabled = environmentProvider.AuthEnabled; +var jwtSecret = environmentProvider.JwtSecret ?? builder.Configuration["Jwt:Secret"]; var jwtIssuer = builder.Configuration["Jwt:Issuer"] ?? "boardgametracker-api"; var jwtAudience = builder.Configuration["Jwt:Audience"] ?? "boardgametracker-client"; if (string.IsNullOrWhiteSpace(jwtSecret)) { - throw new ArgumentException("JWT_SECRET not set"); + if (authEnabled) + { + throw new ArgumentException("JWT_SECRET not set"); + } + + jwtSecret = "auth-disabled-placeholder-key-not-used"; } builder.Services.AddAuthentication(options => @@ -110,6 +121,18 @@ }); builder.Services.AddAuthorization(); + +builder.Services.AddRateLimiter(options => +{ + options.AddFixedWindowLimiter("auth", limiterOptions => + { + limiterOptions.PermitLimit = 10; + limiterOptions.Window = TimeSpan.FromMinutes(1); + limiterOptions.QueueLimit = 0; + }); + options.RejectionStatusCode = StatusCodes.Status429TooManyRequests; +}); + builder.Services.AddHttpClient(); builder.Services.AddMemoryCache(); @@ -181,7 +204,26 @@ app.UseCors("Allow"); -app.UseAuthBypassIfEnabled(); +app.Use(async (context, next) => +{ + context.Response.Headers.Append("X-Content-Type-Options", "nosniff"); + context.Response.Headers.Append("X-Frame-Options", "DENY"); + context.Response.Headers.Append("Referrer-Policy", "strict-origin-when-cross-origin"); + context.Response.Headers.Append( + "Permissions-Policy", "camera=(), microphone=(), geolocation=()"); + context.Response.Headers.Append( + "Content-Security-Policy", + "default-src 'self'; img-src 'self' data:; script-src 'self'; style-src 'self';"); + await next(); +}); + +if (!app.Environment.IsDevelopment()) +{ + app.UseHsts(); +} + +app.UseRateLimiter(); +app.UseAuthDisabledMiddleware(); app.UseAuthentication(); app.UseAuthorization(); @@ -228,10 +270,9 @@ logger.LogInformation(" Log level: {LogLevel}", LogLevelExtensions.GetEnvironmentLogLevel()); logger.LogInformation(" Sentry: {SentryEnabled}", Environment.GetEnvironmentVariable("STATISTICS_ENABLED")?.ToLower() == "true" ? "Enabled" : "Disabled"); logger.LogInformation(" HTTP ports: {HttpPorts}", Environment.GetEnvironmentVariable("ASPNETCORE_HTTP_PORTS") ?? "default"); -logger.LogInformation(" HTTPS ports: {HttpsPorts}", Environment.GetEnvironmentVariable("ASPNETCORE_HTTPS_PORTS") ?? "not configured"); logger.LogInformation(" Timezone: {Timezone}", Environment.GetEnvironmentVariable("TZ") ?? "system default"); logger.LogInformation(" DB port: {DbPort}", Environment.GetEnvironmentVariable("DB_PORT") ?? "5432"); -logger.LogInformation(" Auth bypass: {AuthBypass}", Environment.GetEnvironmentVariable("AUTH_BYPASS")?.ToLower() == "true" ? "Yes" : "No"); +logger.LogInformation(" Auth: {AuthState}", authEnabled ? "Enabled" : "Disabled"); if (!app.Environment.IsDevelopment()) { @@ -258,7 +299,10 @@ RunDbMigrations(app.Services); await SeedConfig(app.Services); -await SeedAuthData(app.Services); +if (authEnabled) +{ + await SeedAuthData(app.Services); +} await app.RunAsync(); diff --git a/BoardGameTracker.Host/Properties/launchSettings.json b/BoardGameTracker.Host/Properties/launchSettings.json index a22c54e5..33fdacc4 100644 --- a/BoardGameTracker.Host/Properties/launchSettings.json +++ b/BoardGameTracker.Host/Properties/launchSettings.json @@ -26,7 +26,8 @@ "DB_USER": "dev", "DB_PASSWORD": "dev", "DB_NAME": "boardgametracker-dev", - "DB_PORT": "5432" + "DB_PORT": "5432", + "AUTH_ENABLED": "true" } }, "https": { diff --git a/BoardGameTracker.Tests/Auth/AuthBypassMiddlewareTests.cs b/BoardGameTracker.Tests/Auth/AuthBypassMiddlewareTests.cs index 70b95c9d..0c79060c 100644 --- a/BoardGameTracker.Tests/Auth/AuthBypassMiddlewareTests.cs +++ b/BoardGameTracker.Tests/Auth/AuthBypassMiddlewareTests.cs @@ -8,13 +8,13 @@ namespace BoardGameTracker.Tests.Auth; -public class AuthBypassMiddlewareTests +public class AuthDisabledMiddlewareTests { [Fact] public async Task InvokeAsync_ShouldSetAdminIdentity_WhenNotAuthenticated() { // Arrange - var middleware = new AuthBypassMiddleware(_ => Task.CompletedTask); + var middleware = new AuthDisabledMiddleware(_ => Task.CompletedTask); var context = new DefaultHttpContext(); // Act @@ -22,9 +22,9 @@ public async Task InvokeAsync_ShouldSetAdminIdentity_WhenNotAuthenticated() // Assert context.User.Identity!.IsAuthenticated.Should().BeTrue(); - context.User.Identity.AuthenticationType.Should().Be("AuthBypass"); + context.User.Identity.AuthenticationType.Should().Be("AuthDisabled"); context.User.FindFirstValue(ClaimTypes.Name).Should().Be("admin"); - context.User.FindFirstValue(ClaimTypes.NameIdentifier).Should().Be("bypass-admin-id"); + context.User.FindFirstValue(ClaimTypes.NameIdentifier).Should().Be("auth-disabled-admin-id"); context.User.IsInRole(Constants.AuthRoles.Admin).Should().BeTrue(); } @@ -32,7 +32,7 @@ public async Task InvokeAsync_ShouldSetAdminIdentity_WhenNotAuthenticated() public async Task InvokeAsync_ShouldNotOverrideIdentity_WhenAlreadyAuthenticated() { // Arrange - var middleware = new AuthBypassMiddleware(_ => Task.CompletedTask); + var middleware = new AuthDisabledMiddleware(_ => Task.CompletedTask); var claims = new[] { new Claim(ClaimTypes.NameIdentifier, "existing-user"), @@ -57,7 +57,7 @@ public async Task InvokeAsync_ShouldCallNextMiddleware() { // Arrange var nextCalled = false; - var middleware = new AuthBypassMiddleware(_ => + var middleware = new AuthDisabledMiddleware(_ => { nextCalled = true; return Task.CompletedTask; @@ -75,13 +75,13 @@ public async Task InvokeAsync_ShouldCallNextMiddleware() public async Task InvokeAsync_ShouldIncludeDisplayNameClaim() { // Arrange - var middleware = new AuthBypassMiddleware(_ => Task.CompletedTask); + var middleware = new AuthDisabledMiddleware(_ => Task.CompletedTask); var context = new DefaultHttpContext(); // Act await middleware.InvokeAsync(context); // Assert - context.User.FindFirstValue("display_name").Should().Be("Admin (Bypass)"); + context.User.FindFirstValue("display_name").Should().Be("Admin"); } } diff --git a/BoardGameTracker.Tests/Auth/AuthControllerTests.cs b/BoardGameTracker.Tests/Auth/AuthControllerTests.cs index a799ce2d..f7b59623 100644 --- a/BoardGameTracker.Tests/Auth/AuthControllerTests.cs +++ b/BoardGameTracker.Tests/Auth/AuthControllerTests.cs @@ -290,7 +290,7 @@ public async Task ResetPassword_ShouldReturnOk_WithTempPassword() public void GetStatus_ShouldReturnAuthStatusResponse() { // Arrange - var expectedStatus = new AuthStatusResponse(AuthEnabled: true, BypassEnabled: false); + var expectedStatus = new AuthStatusResponse(AuthEnabled: true); _authServiceMock.Setup(x => x.GetStatus()).Returns(expectedStatus); // Act @@ -300,7 +300,6 @@ public void GetStatus_ShouldReturnAuthStatusResponse() var okResult = result.Should().BeOfType().Subject; var response = okResult.Value.Should().BeOfType().Subject; response.AuthEnabled.Should().BeTrue(); - response.BypassEnabled.Should().BeFalse(); _authServiceMock.Verify(x => x.GetStatus(), Times.Once); VerifyNoOtherCalls(); diff --git a/BoardGameTracker.Tests/Auth/AuthServiceTests.cs b/BoardGameTracker.Tests/Auth/AuthServiceTests.cs index 84afb09a..678167ec 100644 --- a/BoardGameTracker.Tests/Auth/AuthServiceTests.cs +++ b/BoardGameTracker.Tests/Auth/AuthServiceTests.cs @@ -760,10 +760,10 @@ await act.Should().ThrowAsync() #region GetStatus [Fact] - public void GetStatus_ShouldReturnAuthEnabledAndBypassDisabled_WhenNotInDevelopment() + public void GetStatus_ShouldReturnAuthEnabled_WhenAuthIsEnabled() { // Arrange - _environmentProviderMock.Setup(x => x.IsDevelopment).Returns(false); + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(true); // Act var result = _authService.GetStatus(); @@ -771,57 +771,28 @@ public void GetStatus_ShouldReturnAuthEnabledAndBypassDisabled_WhenNotInDevelopm // Assert result.Should().NotBeNull(); result.AuthEnabled.Should().BeTrue(); - result.BypassEnabled.Should().BeFalse(); - _environmentProviderMock.Verify(x => x.IsDevelopment, Times.Once); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); VerifyNoOtherCalls(); } [Fact] - public void GetStatus_ShouldReturnBypassDisabled_WhenInDevelopmentButAuthBypassNotSet() + public void GetStatus_ShouldReturnAuthDisabled_WhenAuthIsDisabled() { // Arrange - _environmentProviderMock.Setup(x => x.IsDevelopment).Returns(true); - Environment.SetEnvironmentVariable("AUTH_BYPASS", null); + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); // Act var result = _authService.GetStatus(); // Assert result.Should().NotBeNull(); - result.AuthEnabled.Should().BeTrue(); - result.BypassEnabled.Should().BeFalse(); + result.AuthEnabled.Should().BeFalse(); - _environmentProviderMock.Verify(x => x.IsDevelopment, Times.Once); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); VerifyNoOtherCalls(); } - [Fact] - public void GetStatus_ShouldReturnBypassEnabled_WhenInDevelopmentAndAuthBypassIsTrue() - { - // Arrange - _environmentProviderMock.Setup(x => x.IsDevelopment).Returns(true); - Environment.SetEnvironmentVariable("AUTH_BYPASS", "true"); - - try - { - // Act - var result = _authService.GetStatus(); - - // Assert - result.Should().NotBeNull(); - result.AuthEnabled.Should().BeTrue(); - result.BypassEnabled.Should().BeTrue(); - - _environmentProviderMock.Verify(x => x.IsDevelopment, Times.Once); - VerifyNoOtherCalls(); - } - finally - { - Environment.SetEnvironmentVariable("AUTH_BYPASS", null); - } - } - #endregion private static RefreshToken CreateActiveRefreshTokenWithUser(string userId, ApplicationUser user) diff --git a/BoardGameTracker.Tests/Auth/RefreshTokenCleanupServiceTests.cs b/BoardGameTracker.Tests/Auth/RefreshTokenCleanupServiceTests.cs new file mode 100644 index 00000000..31659666 --- /dev/null +++ b/BoardGameTracker.Tests/Auth/RefreshTokenCleanupServiceTests.cs @@ -0,0 +1,196 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using BoardGameTracker.Core.Auth; +using BoardGameTracker.Core.Auth.Interfaces; +using BoardGameTracker.Core.Configuration.Interfaces; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace BoardGameTracker.Tests.Auth; + +public class RefreshTokenCleanupServiceTests +{ + private readonly Mock _scopeFactoryMock; + private readonly Mock> _loggerMock; + private readonly Mock _environmentProviderMock; + private readonly Mock _tokenServiceMock; + + public RefreshTokenCleanupServiceTests() + { + _scopeFactoryMock = new Mock(); + _loggerMock = new Mock>(); + _environmentProviderMock = new Mock(); + _tokenServiceMock = new Mock(); + + SetupServiceScope(); + } + + private void SetupServiceScope() + { + var scopeMock = new Mock(); + var scopedProviderMock = new Mock(); + + _scopeFactoryMock.Setup(x => x.CreateScope()).Returns(scopeMock.Object); + scopeMock.Setup(x => x.ServiceProvider).Returns(scopedProviderMock.Object); + + scopedProviderMock + .Setup(x => x.GetService(typeof(IEnvironmentProvider))) + .Returns(_environmentProviderMock.Object); + scopedProviderMock + .Setup(x => x.GetService(typeof(ITokenService))) + .Returns(_tokenServiceMock.Object); + } + + private void VerifyNoOtherCalls() + { + _environmentProviderMock.VerifyNoOtherCalls(); + _tokenServiceMock.VerifyNoOtherCalls(); + } + + [Fact] + public void Constructor_ShouldNotThrow() + { + // Act & Assert + var service = new RefreshTokenCleanupService(_scopeFactoryMock.Object, _loggerMock.Object); + service.Should().NotBeNull(); + } + + [Fact] + public async Task ExecuteAsync_ShouldReturnImmediately_WhenAuthIsDisabled() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var service = new RefreshTokenCleanupService(_scopeFactoryMock.Object, _loggerMock.Object); + var cts = new CancellationTokenSource(); + + // Act + await service.StartAsync(cts.Token); + await Task.Delay(100, TestContext.Current.CancellationToken); + await cts.CancelAsync(); + + try + { + await service.StopAsync(CancellationToken.None); + } + catch (OperationCanceledException) + { + // Expected + } + + // Assert + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + _tokenServiceMock.Verify(x => x.CleanupExpiredTokensAsync(), Times.Never); + VerifyNoOtherCalls(); + } + + [Fact] + public async Task ExecuteAsync_ShouldNotCleanupBeforeInterval_WhenAuthIsEnabled() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(true); + var service = new RefreshTokenCleanupService(_scopeFactoryMock.Object, _loggerMock.Object); + var cts = new CancellationTokenSource(); + + // Act - start and cancel quickly (before the 24h interval elapses) + await service.StartAsync(cts.Token); + await Task.Delay(100, TestContext.Current.CancellationToken); + await cts.CancelAsync(); + + try + { + await service.StopAsync(CancellationToken.None); + } + catch (OperationCanceledException) + { + // Expected + } + + // Assert - cleanup should not have been called yet (interval is 24h) + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + _tokenServiceMock.Verify(x => x.CleanupExpiredTokensAsync(), Times.Never); + VerifyNoOtherCalls(); + } + + [Fact] + public async Task ExecuteAsync_ShouldStopGracefully_WhenCancelled() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(true); + var service = new RefreshTokenCleanupService(_scopeFactoryMock.Object, _loggerMock.Object); + var cts = new CancellationTokenSource(); + + // Act + await service.StartAsync(cts.Token); + await Task.Delay(50, TestContext.Current.CancellationToken); + await cts.CancelAsync(); + + // Assert - should not throw unexpected exceptions + var act = () => service.StopAsync(CancellationToken.None); + await act.Should().NotThrowAsync(); + } + + [Fact] + public async Task ExecuteAsync_ShouldHandleCleanupException() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(true); + _tokenServiceMock + .Setup(x => x.CleanupExpiredTokensAsync()) + .ThrowsAsync(new InvalidOperationException("Database error")); + + // Use a real scope factory that returns fresh scopes so we can trigger cleanup + var scopeMock = new Mock(); + var scopedProviderMock = new Mock(); + scopeMock.Setup(x => x.ServiceProvider).Returns(scopedProviderMock.Object); + scopedProviderMock + .Setup(x => x.GetService(typeof(IEnvironmentProvider))) + .Returns(_environmentProviderMock.Object); + scopedProviderMock + .Setup(x => x.GetService(typeof(ITokenService))) + .Returns(_tokenServiceMock.Object); + _scopeFactoryMock.Setup(x => x.CreateScope()).Returns(scopeMock.Object); + + var service = new RefreshTokenCleanupService(_scopeFactoryMock.Object, _loggerMock.Object); + var cts = new CancellationTokenSource(); + + // Act - start and cancel quickly + await service.StartAsync(cts.Token); + await Task.Delay(50, TestContext.Current.CancellationToken); + await cts.CancelAsync(); + + // Assert - service should not crash from the exception + var act = () => service.StopAsync(CancellationToken.None); + await act.Should().NotThrowAsync(); + } + + [Fact] + public async Task ExecuteAsync_ShouldCheckAuthEnabled_InScopedContext() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var service = new RefreshTokenCleanupService(_scopeFactoryMock.Object, _loggerMock.Object); + var cts = new CancellationTokenSource(); + + // Act + await service.StartAsync(cts.Token); + await Task.Delay(50, TestContext.Current.CancellationToken); + await cts.CancelAsync(); + + try + { + await service.StopAsync(CancellationToken.None); + } + catch (OperationCanceledException) + { + // Expected + } + + // Assert - should have created a scope to check auth status + _scopeFactoryMock.Verify(x => x.CreateScope(), Times.Once); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + } +} diff --git a/BoardGameTracker.Tests/Extensions/LogLevelExtensionsTests.cs b/BoardGameTracker.Tests/Extensions/LogLevelExtensionsTests.cs index 478778e3..b83465e2 100644 --- a/BoardGameTracker.Tests/Extensions/LogLevelExtensionsTests.cs +++ b/BoardGameTracker.Tests/Extensions/LogLevelExtensionsTests.cs @@ -6,6 +6,7 @@ namespace BoardGameTracker.Tests.Extensions; +[Collection("EnvironmentVariables")] public class LogLevelExtensionsTests : IDisposable { private readonly string _originalLogLevel; diff --git a/BoardGameTracker.Tests/Filters/AuthDisabledFilterTests.cs b/BoardGameTracker.Tests/Filters/AuthDisabledFilterTests.cs new file mode 100644 index 00000000..100a1ed4 --- /dev/null +++ b/BoardGameTracker.Tests/Filters/AuthDisabledFilterTests.cs @@ -0,0 +1,169 @@ +using System.Collections.Generic; +using BoardGameTracker.Api.Infrastructure; +using BoardGameTracker.Core.Configuration.Interfaces; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Moq; +using Xunit; + +namespace BoardGameTracker.Tests.Filters; + +public class AuthDisabledFilterTests +{ + private readonly Mock _environmentProviderMock; + private readonly AuthDisabledFilter _filter; + + public AuthDisabledFilterTests() + { + _environmentProviderMock = new Mock(); + _filter = new AuthDisabledFilter(_environmentProviderMock.Object); + } + + private void VerifyNoOtherCalls() + { + _environmentProviderMock.VerifyNoOtherCalls(); + } + + private static ActionExecutingContext CreateContext(string path) + { + var httpContext = new DefaultHttpContext(); + httpContext.Request.Path = new PathString(path); + + var actionContext = new ActionContext( + httpContext, + new RouteData(), + new ActionDescriptor()); + + return new ActionExecutingContext( + actionContext, + new List(), + new Dictionary(), + new object()); + } + + [Fact] + public void OnActionExecuting_ShouldNotSetResult_WhenAuthIsEnabled() + { + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(true); + var context = CreateContext("/api/games"); + + _filter.OnActionExecuting(context); + + context.Result.Should().BeNull(); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + VerifyNoOtherCalls(); + } + + [Fact] + public void OnActionExecuting_ShouldSetConflictResult_WhenAuthIsDisabledAndPathIsLogin() + { + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var context = CreateContext("/api/auth/login"); + + _filter.OnActionExecuting(context); + + var conflictResult = context.Result.Should().BeOfType().Subject; + conflictResult.Value.Should().Be("Authentication is disabled. This endpoint is not available."); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + VerifyNoOtherCalls(); + } + + [Fact] + public void OnActionExecuting_ShouldNotSetResult_WhenAuthIsDisabledAndPathEndsWithStatus() + { + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var context = CreateContext("/api/auth/status"); + + _filter.OnActionExecuting(context); + + context.Result.Should().BeNull(); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + VerifyNoOtherCalls(); + } + + [Fact] + public void OnActionExecuting_ShouldNotSetResult_WhenAuthIsDisabledAndPathEndsWithStatusUppercase() + { + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var context = CreateContext("/api/auth/STATUS"); + + _filter.OnActionExecuting(context); + + context.Result.Should().BeNull(); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + VerifyNoOtherCalls(); + } + + [Fact] + public void OnActionExecuting_ShouldSetConflictResult_WhenAuthIsDisabledAndPathIsEmpty() + { + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var context = CreateContext("/"); + + _filter.OnActionExecuting(context); + + var conflictResult = context.Result.Should().BeOfType().Subject; + conflictResult.Value.Should().Be("Authentication is disabled. This endpoint is not available."); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + VerifyNoOtherCalls(); + } + + [Fact] + public void OnActionExecuted_ShouldDoNothing() + { + var httpContext = new DefaultHttpContext(); + var actionContext = new ActionContext( + httpContext, + new RouteData(), + new ActionDescriptor()); + + var context = new ActionExecutedContext( + actionContext, + new List(), + new object()); + + _filter.OnActionExecuted(context); + + context.Result.Should().BeNull(); + VerifyNoOtherCalls(); + } + + [Theory] + [InlineData("/api/auth/status")] + [InlineData("/api/auth/STATUS")] + [InlineData("/api/auth/Status")] + [InlineData("/status")] + public void OnActionExecuting_ShouldNotSetResult_WhenAuthIsDisabledAndPathEndsWithStatusCaseInsensitive(string path) + { + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var context = CreateContext(path); + + _filter.OnActionExecuting(context); + + context.Result.Should().BeNull(); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + _environmentProviderMock.Invocations.Clear(); + } + + [Theory] + [InlineData("/api/games")] + [InlineData("/api/auth/login")] + [InlineData("/api/auth/register")] + [InlineData("/api/settings")] + public void OnActionExecuting_ShouldSetConflictResult_WhenAuthIsDisabledAndVariousNonStatusPaths(string path) + { + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + var context = CreateContext(path); + + _filter.OnActionExecuting(context); + + var conflictResult = context.Result.Should().BeOfType().Subject; + conflictResult.Value.Should().Be("Authentication is disabled. This endpoint is not available."); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + _environmentProviderMock.Invocations.Clear(); + } +} diff --git a/BoardGameTracker.Tests/Infrastructure/AuthDisabledExtensionsTests.cs b/BoardGameTracker.Tests/Infrastructure/AuthDisabledExtensionsTests.cs new file mode 100644 index 00000000..541e3056 --- /dev/null +++ b/BoardGameTracker.Tests/Infrastructure/AuthDisabledExtensionsTests.cs @@ -0,0 +1,85 @@ +using System; +using BoardGameTracker.Api.Infrastructure; +using BoardGameTracker.Core.Configuration.Interfaces; +using FluentAssertions; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Xunit; + +namespace BoardGameTracker.Tests.Infrastructure; + +public class AuthDisabledExtensionsTests +{ + private readonly Mock _environmentProviderMock; + private readonly Mock _appBuilderMock; + + public AuthDisabledExtensionsTests() + { + _environmentProviderMock = new Mock(); + + var serviceProviderMock = new Mock(); + serviceProviderMock + .Setup(x => x.GetService(typeof(IEnvironmentProvider))) + .Returns(_environmentProviderMock.Object); + + _appBuilderMock = new Mock(); + _appBuilderMock + .Setup(x => x.ApplicationServices) + .Returns(serviceProviderMock.Object); + } + + private void VerifyNoOtherCalls() + { + _environmentProviderMock.VerifyNoOtherCalls(); + } + + [Fact] + public void UseAuthDisabledMiddleware_ShouldRegisterMiddleware_WhenAuthIsDisabled() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(false); + _appBuilderMock + .Setup(x => x.Use(It.IsAny>())) + .Returns(_appBuilderMock.Object); + + // Act + var result = _appBuilderMock.Object.UseAuthDisabledMiddleware(); + + // Assert + result.Should().BeSameAs(_appBuilderMock.Object); + _appBuilderMock.Verify(x => x.Use(It.IsAny>()), Times.Once); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + VerifyNoOtherCalls(); + } + + [Fact] + public void UseAuthDisabledMiddleware_ShouldNotRegisterMiddleware_WhenAuthIsEnabled() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(true); + + // Act + var result = _appBuilderMock.Object.UseAuthDisabledMiddleware(); + + // Assert + result.Should().BeSameAs(_appBuilderMock.Object); + _appBuilderMock.Verify(x => x.Use(It.IsAny>()), Times.Never); + _environmentProviderMock.Verify(x => x.AuthEnabled, Times.Once); + VerifyNoOtherCalls(); + } + + [Fact] + public void UseAuthDisabledMiddleware_ShouldReturnSameAppBuilder() + { + // Arrange + _environmentProviderMock.Setup(x => x.AuthEnabled).Returns(true); + + // Act + var result = _appBuilderMock.Object.UseAuthDisabledMiddleware(); + + // Assert + result.Should().BeSameAs(_appBuilderMock.Object); + } +} diff --git a/BoardGameTracker.Tests/Services/EnvironmentProviderTests.cs b/BoardGameTracker.Tests/Services/EnvironmentProviderTests.cs index 3537b8d0..c5ca1704 100644 --- a/BoardGameTracker.Tests/Services/EnvironmentProviderTests.cs +++ b/BoardGameTracker.Tests/Services/EnvironmentProviderTests.cs @@ -6,6 +6,7 @@ namespace BoardGameTracker.Tests.Services; +[Collection("EnvironmentVariables")] public class EnvironmentProviderTests : IDisposable { private readonly EnvironmentProvider _environmentProvider; @@ -20,10 +21,12 @@ public EnvironmentProviderTests() ["ENVIRONMENT"] = Environment.GetEnvironmentVariable("ENVIRONMENT"), ["PORT"] = Environment.GetEnvironmentVariable("PORT"), ["STATISTICS"] = Environment.GetEnvironmentVariable("STATISTICS"), - ["LOGLEVEL"] = Environment.GetEnvironmentVariable("LOGLEVEL") + ["STATISTICS_ENABLED"] = Environment.GetEnvironmentVariable("STATISTICS_ENABLED"), + ["LOGLEVEL"] = Environment.GetEnvironmentVariable("LOGLEVEL"), + ["AUTH_ENABLED"] = Environment.GetEnvironmentVariable("AUTH_ENABLED"), + ["JWT_SECRET"] = Environment.GetEnvironmentVariable("JWT_SECRET") }; - // Clear both environment variables for clean test state Environment.SetEnvironmentVariable("ASPNETCORE_ENVIRONMENT", null); Environment.SetEnvironmentVariable("ENVIRONMENT", null); } @@ -226,4 +229,55 @@ public void IsDevelopment_ShouldReturnFalse_WhenProductionEnvironment() result.Should().BeFalse(); } + + [Fact] + public void AuthEnabled_ShouldReturnTrue_WhenNotSet() + { + Environment.SetEnvironmentVariable("AUTH_ENABLED", null); + + _environmentProvider.AuthEnabled.Should().BeTrue(); + } + + [Theory] + [InlineData("true", true)] + [InlineData("True", true)] + [InlineData("TRUE", true)] + [InlineData("false", false)] + [InlineData("False", false)] + [InlineData("FALSE", false)] + [InlineData("yes", true)] + [InlineData("no", true)] + [InlineData("0", true)] + [InlineData("1", true)] + [InlineData("something", true)] + public void AuthEnabled_ShouldOnlyReturnFalse_WhenExplicitlySetToFalse(string value, bool expected) + { + Environment.SetEnvironmentVariable("AUTH_ENABLED", value); + + _environmentProvider.AuthEnabled.Should().Be(expected); + } + + [Fact] + public void JwtSecret_ShouldReturnNull_WhenNotSet() + { + Environment.SetEnvironmentVariable("JWT_SECRET", null); + + _environmentProvider.JwtSecret.Should().BeNull(); + } + + [Fact] + public void JwtSecret_ShouldReturnValue_WhenSet() + { + Environment.SetEnvironmentVariable("JWT_SECRET", "my-secret-key"); + + _environmentProvider.JwtSecret.Should().Be("my-secret-key"); + } + + [Fact] + public void JwtSecret_ShouldReturnNull_WhenSetToEmpty() + { + Environment.SetEnvironmentVariable("JWT_SECRET", ""); + + _environmentProvider.JwtSecret.Should().BeNull(); + } } diff --git a/README.md b/README.md index cc1622d2..abc63e30 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,14 @@ GitHub release + + CI + - Build + Deploy + + + Security Scan Coverage diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..7421047f --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,22 @@ +# Security Policy + +## Supported Versions + +The latest released version receives security fixes. +Older versions are not actively patched. + +## Reporting a Vulnerability + +Please **do not** open a public GitHub issue for security vulnerabilities. + +Use GitHub's private vulnerability reporting: +https://github.com/mregni/BoardGameTracker/security/advisories/new + +You will receive a response within 72 hours. A disclosure timeline will be +agreed — typically 90 days — before any public disclosure. + +## What to Expect + +- Acknowledgement within 72 hours +- Status update within 7 days +- Credit in release notes if desired diff --git a/boardgametracker.client/package-lock.json b/boardgametracker.client/package-lock.json index 88ee5376..7122e59d 100644 --- a/boardgametracker.client/package-lock.json +++ b/boardgametracker.client/package-lock.json @@ -7101,9 +7101,9 @@ "license": "MIT" }, "node_modules/axios": { - "version": "1.13.5", - "resolved": "https://registry.npmjs.org/axios/-/axios-1.13.5.tgz", - "integrity": "sha512-cz4ur7Vb0xS4/KUN0tPWe44eqxrIu31me+fbang3ijiNscE129POzipJJA6zniq2C/Z6sJCjMimjS8Lc/GAs8Q==", + "version": "1.13.6", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.13.6.tgz", + "integrity": "sha512-ChTCHMouEe2kn713WHbQGcuYrr6fXTBiu460OTwWrWob16g1bXn4vtz07Ope7ewMozJAnEquLk5lWQWtBig9DQ==", "license": "MIT", "dependencies": { "follow-redirects": "^1.15.11", @@ -7928,9 +7928,9 @@ } }, "node_modules/flatted": { - "version": "3.3.3", - "resolved": "https://registry.npmjs.org/flatted/-/flatted-3.3.3.tgz", - "integrity": "sha512-GX+ysw4PBCz0PzosHDepZGANEuFCMLrnRTiEy9McGjmkCQYwRq4A/X786G/fjM/+OjsWSU1ZrY5qyARZmO/uwg==", + "version": "3.4.2", + "resolved": "https://registry.npmjs.org/flatted/-/flatted-3.4.2.tgz", + "integrity": "sha512-PjDse7RzhcPkIJwy5t7KPWQSZ9cAbzQXcafsetQoD7sOJRQlGikNbx7yZp2OotDnJyrDcbyRq3Ttb18iYOqkxA==", "dev": true, "license": "ISC" }, diff --git a/boardgametracker.client/src/hooks/usePermissions.test.ts b/boardgametracker.client/src/hooks/usePermissions.test.ts new file mode 100644 index 00000000..f9ec62c7 --- /dev/null +++ b/boardgametracker.client/src/hooks/usePermissions.test.ts @@ -0,0 +1,111 @@ +import { renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { usePermissions } from "./usePermissions"; + +const mockHasRole = vi.fn<(role: string) => boolean>(); +let mockAuthStatus: { authEnabled: boolean } | null = null; + +vi.mock("@/hooks/useAuth", () => ({ + useAuth: (selector: (state: { hasRole: typeof mockHasRole; authStatus: typeof mockAuthStatus }) => unknown) => + selector({ hasRole: mockHasRole, authStatus: mockAuthStatus }), +})); + +describe("usePermissions", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAuthStatus = { authEnabled: true }; + mockHasRole.mockReturnValue(false); + }); + + describe("when auth is disabled", () => { + beforeEach(() => { + mockAuthStatus = { authEnabled: false }; + }); + + it("should grant admin regardless of role", () => { + const { result } = renderHook(() => usePermissions()); + + expect(result.current.isAdmin).toBe(true); + expect(result.current.canWrite).toBe(true); + expect(result.current.canManageSettings).toBe(true); + }); + + it("should grant admin even when hasRole returns false", () => { + mockHasRole.mockReturnValue(false); + const { result } = renderHook(() => usePermissions()); + + expect(result.current.isAdmin).toBe(true); + expect(result.current.canWrite).toBe(true); + expect(result.current.canManageSettings).toBe(true); + }); + }); + + describe("when auth is enabled", () => { + describe("with Admin role", () => { + beforeEach(() => { + mockHasRole.mockImplementation((role) => role === "Admin"); + }); + + it("should grant all permissions", () => { + const { result } = renderHook(() => usePermissions()); + + expect(result.current.isAdmin).toBe(true); + expect(result.current.canWrite).toBe(true); + expect(result.current.canManageSettings).toBe(true); + }); + }); + + describe("with User role", () => { + beforeEach(() => { + mockHasRole.mockImplementation((role) => role === "User"); + }); + + it("should grant canWrite but not admin permissions", () => { + const { result } = renderHook(() => usePermissions()); + + expect(result.current.isAdmin).toBe(false); + expect(result.current.canWrite).toBe(true); + expect(result.current.canManageSettings).toBe(false); + }); + }); + + describe("with no roles", () => { + it("should deny all permissions", () => { + const { result } = renderHook(() => usePermissions()); + + expect(result.current.isAdmin).toBe(false); + expect(result.current.canWrite).toBe(false); + expect(result.current.canManageSettings).toBe(false); + }); + }); + + describe("with both Admin and User roles", () => { + beforeEach(() => { + mockHasRole.mockReturnValue(true); + }); + + it("should grant all permissions", () => { + const { result } = renderHook(() => usePermissions()); + + expect(result.current.isAdmin).toBe(true); + expect(result.current.canWrite).toBe(true); + expect(result.current.canManageSettings).toBe(true); + }); + }); + }); + + describe("when authStatus is null", () => { + beforeEach(() => { + mockAuthStatus = null; + }); + + it("should treat null authStatus as auth disabled", () => { + const { result } = renderHook(() => usePermissions()); + + expect(result.current.isAdmin).toBe(true); + expect(result.current.canWrite).toBe(true); + expect(result.current.canManageSettings).toBe(true); + }); + }); +}); diff --git a/boardgametracker.client/src/hooks/usePermissions.ts b/boardgametracker.client/src/hooks/usePermissions.ts index de0837d2..3d538c93 100644 --- a/boardgametracker.client/src/hooks/usePermissions.ts +++ b/boardgametracker.client/src/hooks/usePermissions.ts @@ -2,8 +2,11 @@ import { useAuth } from "@/hooks/useAuth"; export const usePermissions = () => { const hasRole = useAuth((s) => s.hasRole); - const isAdmin = hasRole("Admin"); - const isUser = hasRole("User"); + const authStatus = useAuth((s) => s.authStatus); + const authDisabled = !authStatus?.authEnabled; + + const isAdmin = authDisabled || hasRole("Admin"); + const isUser = authDisabled || hasRole("User"); return { isAdmin, diff --git a/boardgametracker.client/src/models/Auth/Auth.ts b/boardgametracker.client/src/models/Auth/Auth.ts index 28e6c6e6..db59803b 100644 --- a/boardgametracker.client/src/models/Auth/Auth.ts +++ b/boardgametracker.client/src/models/Auth/Auth.ts @@ -24,7 +24,6 @@ export interface OidcProvider { export interface AuthStatus { authEnabled: boolean; - bypassEnabled: boolean; } export interface ProfileResponse { diff --git a/boardgametracker.client/src/routes/-components/BottomNav.tsx b/boardgametracker.client/src/routes/-components/BottomNav.tsx index a49c55b2..42274d19 100644 --- a/boardgametracker.client/src/routes/-components/BottomNav.tsx +++ b/boardgametracker.client/src/routes/-components/BottomNav.tsx @@ -1,4 +1,4 @@ -import { Link, useNavigate, useRouterState } from "@tanstack/react-router"; +import { Link, useRouterState } from "@tanstack/react-router"; import { cx } from "class-variance-authority"; import { useState } from "react"; import { useTranslation } from "react-i18next"; @@ -18,7 +18,6 @@ export const BottomNav = () => { const routerState = useRouterState(); const currentPath = routerState.location.pathname; const { user, isAuthenticated, authStatus, logout } = useAuth(); - const navigate = useNavigate(); const handleMoreClick = () => { setShowMoreMenu(!showMoreMenu); @@ -34,10 +33,9 @@ export const BottomNav = () => { mobileVisible: true, }); - const showAuth = authStatus?.authEnabled && !authStatus.bypassEnabled; + const showAuth = authStatus?.authEnabled; const handleLogout = async () => { await logout(); - navigate({ to: "/login" }); }; return ( diff --git a/boardgametracker.client/src/routes/-components/Sidebar.tsx b/boardgametracker.client/src/routes/-components/Sidebar.tsx index 0d406007..d95ff47c 100644 --- a/boardgametracker.client/src/routes/-components/Sidebar.tsx +++ b/boardgametracker.client/src/routes/-components/Sidebar.tsx @@ -1,4 +1,3 @@ -import { useNavigate } from "@tanstack/react-router"; import LogOut from "@/assets/icons/log-out.svg?react"; import User from "@/assets/icons/user.svg?react"; import { BgtIconButton } from "@/components/BgtIconButton/BgtIconButton"; @@ -9,15 +8,13 @@ import { useMenuInfo } from "../-hooks/useMenuInfo"; import { VersionCard } from "./VersionCard"; export const Sidebar = () => { - const navigate = useNavigate(); const { counts, versionInfo, menuItems } = useMenuInfo(); const { user, isAuthenticated, authStatus, logout } = useAuth(); - const showAuth = authStatus?.authEnabled && !authStatus.bypassEnabled; + const showAuth = authStatus?.authEnabled; const handleLogout = async () => { await logout(); - navigate({ to: "/login" }); }; return ( diff --git a/boardgametracker.client/src/routes/__root.test.tsx b/boardgametracker.client/src/routes/__root.test.tsx new file mode 100644 index 00000000..6100ac82 --- /dev/null +++ b/boardgametracker.client/src/routes/__root.test.tsx @@ -0,0 +1,252 @@ +import type { FC, ReactNode } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { render, screen, waitFor } from "@/test/test-utils"; + +const mocks = vi.hoisted(() => ({ + navigate: vi.fn(), + useMatch: vi.fn(), + useLocation: vi.fn(), + getEnvironmentCall: vi.fn(), + initSentry: vi.fn(), + authState: {} as { + isAuthenticated: boolean; + authStatus: { authEnabled: boolean } | null; + fetchAuthStatus: ReturnType; + }, + captured: {} as { Root: FC; Error: FC<{ error: Error; reset: () => void }> }, +})); + +vi.mock("@tanstack/react-router", () => ({ + createRootRouteWithContext: () => (config: { component: FC; errorComponent: FC }) => { + mocks.captured.Root = config.component; + mocks.captured.Error = config.errorComponent; + return config; + }, + Outlet: () =>
, + useMatch: (...args: unknown[]) => mocks.useMatch(...args), + useNavigate: () => mocks.navigate, + useLocation: () => mocks.useLocation(), +})); + +vi.mock("@/hooks/useAuth", () => ({ + useAuth: () => mocks.authState, +})); + +vi.mock("@/services/settingsService", () => ({ + getEnvironmentCall: () => mocks.getEnvironmentCall(), +})); + +vi.mock("@/utils/sentry", () => ({ + initSentry: () => mocks.initSentry(), +})); + +vi.mock("react-error-boundary", () => ({ + ErrorBoundary: ({ children }: { children: ReactNode }) => <>{children}, +})); + +vi.mock("@/components/ErrorBoundary/ErrorFallback", () => ({ + ErrorFallback: ({ error }: { error: Error }) =>
{error.message}
, +})); + +vi.mock("@/components/NotFound/NotFound", () => ({ + NotFound: () =>
, +})); + +vi.mock("./-components/Sidebar", () => ({ + Sidebar: () =>
, +})); + +vi.mock("./-components/BottomNav", () => ({ + BottomNav: () =>
, +})); + +import "./__root"; + +describe("RootComponent", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.authState = { + isAuthenticated: false, + authStatus: { authEnabled: false }, + fetchAuthStatus: vi.fn().mockResolvedValue(undefined), + }; + mocks.useMatch.mockReturnValue(null); + mocks.useLocation.mockReturnValue({ pathname: "/", searchStr: "" }); + mocks.getEnvironmentCall.mockResolvedValue({ enableStatistics: false }); + }); + + describe("Loading state", () => { + it("should show loading spinner while auth is being checked", () => { + mocks.authState.fetchAuthStatus = vi.fn(() => new Promise(() => {})); + const { container } = render(); + + expect(container.querySelector(".animate-spin")).toBeInTheDocument(); + expect(screen.queryByTestId("sidebar")).not.toBeInTheDocument(); + expect(screen.queryByTestId("outlet")).not.toBeInTheDocument(); + }); + + it("should show layout after auth check completes", async () => { + render(); + + await waitFor(() => { + expect(screen.getByTestId("sidebar")).toBeInTheDocument(); + }); + }); + + it("should show layout even if auth check fails", async () => { + mocks.authState.fetchAuthStatus = vi.fn().mockRejectedValue(new Error("fail")); + render(); + + await waitFor(() => { + expect(screen.getByTestId("sidebar")).toBeInTheDocument(); + }); + }); + }); + + describe("Auth redirect", () => { + beforeEach(() => { + mocks.authState.authStatus = { authEnabled: true }; + mocks.authState.isAuthenticated = false; + }); + + it("should redirect to /login when auth is enabled and not authenticated", async () => { + render(); + + await waitFor(() => { + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/login", + search: { redirect: "/" }, + }); + }); + }); + + it("should include current path with search string as redirect param", async () => { + mocks.useLocation.mockReturnValue({ pathname: "/settings", searchStr: "?tab=general" }); + render(); + + await waitFor(() => { + expect(mocks.navigate).toHaveBeenCalledWith({ + to: "/login", + search: { redirect: "/settings?tab=general" }, + }); + }); + }); + + it("should not redirect when already on /login", async () => { + mocks.useLocation.mockReturnValue({ pathname: "/login", searchStr: "" }); + render(); + + await waitFor(() => { + expect(screen.getByTestId("sidebar")).toBeInTheDocument(); + }); + expect(mocks.navigate).not.toHaveBeenCalled(); + }); + + it("should not redirect on bare route", async () => { + mocks.useMatch.mockReturnValue({}); + render(); + + await waitFor(() => { + expect(screen.getByTestId("outlet")).toBeInTheDocument(); + }); + expect(mocks.navigate).not.toHaveBeenCalled(); + }); + + it("should not redirect when auth is disabled", async () => { + mocks.authState.authStatus = { authEnabled: false }; + render(); + + await waitFor(() => { + expect(screen.getByTestId("sidebar")).toBeInTheDocument(); + }); + expect(mocks.navigate).not.toHaveBeenCalled(); + }); + + it("should not redirect when authenticated", async () => { + mocks.authState.isAuthenticated = true; + render(); + + await waitFor(() => { + expect(screen.getByTestId("sidebar")).toBeInTheDocument(); + }); + expect(mocks.navigate).not.toHaveBeenCalled(); + }); + }); + + describe("Layout", () => { + it("should render bare layout when on bare route", async () => { + mocks.useMatch.mockReturnValue({}); + render(); + + await waitFor(() => { + expect(screen.getByTestId("outlet")).toBeInTheDocument(); + }); + expect(screen.queryByTestId("sidebar")).not.toBeInTheDocument(); + expect(screen.queryByTestId("bottom-nav")).not.toBeInTheDocument(); + }); + + it("should render full layout with sidebar, outlet, and bottom nav", async () => { + render(); + + await waitFor(() => { + expect(screen.getByTestId("sidebar")).toBeInTheDocument(); + }); + expect(screen.getByTestId("outlet")).toBeInTheDocument(); + expect(screen.getByTestId("bottom-nav")).toBeInTheDocument(); + }); + }); + + describe("Sentry initialization", () => { + it("should initialize sentry when statistics enabled and auth disabled", async () => { + mocks.getEnvironmentCall.mockResolvedValue({ enableStatistics: true }); + render(); + + await waitFor(() => { + expect(mocks.initSentry).toHaveBeenCalled(); + }); + }); + + it("should initialize sentry when statistics enabled and authenticated", async () => { + mocks.authState.authStatus = { authEnabled: true }; + mocks.authState.isAuthenticated = true; + mocks.getEnvironmentCall.mockResolvedValue({ enableStatistics: true }); + render(); + + await waitFor(() => { + expect(mocks.initSentry).toHaveBeenCalled(); + }); + }); + + it("should not initialize sentry when statistics disabled", async () => { + mocks.getEnvironmentCall.mockResolvedValue({ enableStatistics: false }); + render(); + + await waitFor(() => { + expect(screen.getByTestId("sidebar")).toBeInTheDocument(); + }); + expect(mocks.initSentry).not.toHaveBeenCalled(); + }); + + it("should not call environment API when not authenticated and auth enabled", async () => { + mocks.authState.authStatus = { authEnabled: true }; + mocks.authState.isAuthenticated = false; + render(); + + await waitFor(() => { + expect(mocks.navigate).toHaveBeenCalled(); + }); + expect(mocks.getEnvironmentCall).not.toHaveBeenCalled(); + }); + }); + + describe("RouterErrorComponent", () => { + it("should render ErrorFallback with error props", () => { + const error = new Error("test error"); + const reset = vi.fn(); + render(); + + expect(screen.getByTestId("error-fallback")).toBeInTheDocument(); + expect(screen.getByText("test error")).toBeInTheDocument(); + }); + }); +}); diff --git a/boardgametracker.client/src/routes/__root.tsx b/boardgametracker.client/src/routes/__root.tsx index 957170ba..a41fabff 100644 --- a/boardgametracker.client/src/routes/__root.tsx +++ b/boardgametracker.client/src/routes/__root.tsx @@ -50,15 +50,14 @@ function RootComponent() { useEffect(() => { if (!authChecked || !authStatus) return; - // If auth is enabled and not bypassed, redirect unauthenticated users to login - if (authStatus.authEnabled && !authStatus.bypassEnabled && !isAuthenticated && !isBare) { - const currentPath = location.pathname + location.search; + if (authStatus.authEnabled && !isAuthenticated && !isBare && location.pathname !== "/login") { + const currentPath = location.pathname + location.searchStr; navigate({ to: "/login", search: { redirect: currentPath } }); } }, [authChecked, authStatus, isAuthenticated, isBare, navigate, location]); useEffect(() => { - if (!isAuthenticated) return; + if (!isAuthenticated && authStatus?.authEnabled) return; getEnvironmentCall() .then((env) => { @@ -67,7 +66,7 @@ function RootComponent() { } }) .catch(() => {}); - }, [isAuthenticated]); + }, [isAuthenticated, authStatus]); if (!authChecked) { return ( diff --git a/boardgametracker.client/src/routes/_bare/rsvp.tsx b/boardgametracker.client/src/routes/_bare/rsvp.tsx index 42ee77af..8f9548d1 100644 --- a/boardgametracker.client/src/routes/_bare/rsvp.tsx +++ b/boardgametracker.client/src/routes/_bare/rsvp.tsx @@ -35,8 +35,7 @@ function RsvpPage() { const { gameNight, isLoading, submitRsvp, isSubmitting, isSubmitted, submittedPlayerName, submittedState } = useRsvpData(linkId); - const requiresAuth = - settings?.rsvpAuthenticationEnabled && authStatus?.authEnabled && !authStatus?.bypassEnabled && !isAuthenticated; + const requiresAuth = settings?.rsvpAuthenticationEnabled && authStatus?.authEnabled && !isAuthenticated; if (requiresAuth) { return ( diff --git a/boardgametracker.client/src/routes/settings/-components/SettingsSidebar.test.tsx b/boardgametracker.client/src/routes/settings/-components/SettingsSidebar.test.tsx new file mode 100644 index 00000000..88afc019 --- /dev/null +++ b/boardgametracker.client/src/routes/settings/-components/SettingsSidebar.test.tsx @@ -0,0 +1,157 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { render, screen, userEvent } from "@/test/test-utils"; + +import { type SettingsCategory, SettingsSidebar } from "./SettingsSidebar"; + +let mockAuthStatus: { authEnabled: boolean } | null = null; + +vi.mock("@/hooks/useAuth", () => ({ + useAuth: (selector: (state: { authStatus: typeof mockAuthStatus }) => unknown) => + selector({ authStatus: mockAuthStatus }), +})); + +const defaultProps = { + activeCategory: "general" as SettingsCategory, + onCategoryChange: vi.fn(), + canManageSettings: true, +}; + +describe("SettingsSidebar", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAuthStatus = { authEnabled: true }; + }); + + describe("Rendering categories", () => { + it("should render all categories when canManageSettings is true and auth is enabled", () => { + render(); + + expect(screen.getByText("settings:sidebar.general.title")).toBeInTheDocument(); + expect(screen.getByText("settings:sidebar.shelf-of-shame.title")).toBeInTheDocument(); + expect(screen.getByText("settings:sidebar.game-nights.title")).toBeInTheDocument(); + expect(screen.getByText("settings:sidebar.advanced.title")).toBeInTheDocument(); + expect(screen.getByText("settings:sidebar.account.title")).toBeInTheDocument(); + }); + + it("should render category descriptions", () => { + render(); + + expect(screen.getByText("settings:sidebar.general.description")).toBeInTheDocument(); + expect(screen.getByText("settings:sidebar.shelf-of-shame.description")).toBeInTheDocument(); + }); + + it("should render categories as buttons", () => { + render(); + + const buttons = screen.getAllByRole("button"); + expect(buttons).toHaveLength(5); + }); + }); + + describe("Filtering categories", () => { + it("should hide non-account categories when canManageSettings is false", () => { + render(); + + expect(screen.queryByText("settings:sidebar.general.title")).not.toBeInTheDocument(); + expect(screen.queryByText("settings:sidebar.shelf-of-shame.title")).not.toBeInTheDocument(); + expect(screen.queryByText("settings:sidebar.game-nights.title")).not.toBeInTheDocument(); + expect(screen.queryByText("settings:sidebar.advanced.title")).not.toBeInTheDocument(); + }); + + it("should still show account tab when canManageSettings is false and auth is enabled", () => { + render(); + + expect(screen.getByText("settings:sidebar.account.title")).toBeInTheDocument(); + }); + + it("should hide account tab when auth is disabled", () => { + mockAuthStatus = { authEnabled: false }; + render(); + + expect(screen.queryByText("settings:sidebar.account.title")).not.toBeInTheDocument(); + }); + + it("should hide account tab when authStatus is null", () => { + mockAuthStatus = null; + render(); + + expect(screen.queryByText("settings:sidebar.account.title")).not.toBeInTheDocument(); + }); + + it("should show only account tab when canManageSettings is false and auth is enabled", () => { + render(); + + const buttons = screen.getAllByRole("button"); + expect(buttons).toHaveLength(1); + expect(screen.getByText("settings:sidebar.account.title")).toBeInTheDocument(); + }); + + it("should show no categories when canManageSettings is false and auth is disabled", () => { + mockAuthStatus = { authEnabled: false }; + render(); + + expect(screen.queryAllByRole("button")).toHaveLength(0); + }); + }); + + describe("Active category styling", () => { + it("should apply active styles to the active category", () => { + render(); + + const buttons = screen.getAllByRole("button"); + expect(buttons[0]).toHaveClass("bg-primary/20"); + }); + + it("should not apply active styles to inactive categories", () => { + render(); + + const buttons = screen.getAllByRole("button"); + expect(buttons[1]).not.toHaveClass("bg-primary/20"); + expect(buttons[1]).toHaveClass("text-white/70"); + }); + + it("should apply active styles to a different category", () => { + render(); + + const buttons = screen.getAllByRole("button"); + expect(buttons[3]).toHaveClass("bg-primary/20"); + }); + }); + + describe("Category change handler", () => { + it("should call onCategoryChange when a category is clicked", async () => { + const user = userEvent.setup(); + const onCategoryChange = vi.fn(); + render(); + + await user.click(screen.getByText("settings:sidebar.shelf-of-shame.title")); + + expect(onCategoryChange).toHaveBeenCalledWith("shelf-of-shame"); + }); + + it("should call onCategoryChange with correct id for each category", async () => { + const user = userEvent.setup(); + const onCategoryChange = vi.fn(); + render(); + + await user.click(screen.getByText("settings:sidebar.game-nights.title")); + expect(onCategoryChange).toHaveBeenCalledWith("game-nights"); + + await user.click(screen.getByText("settings:sidebar.advanced.title")); + expect(onCategoryChange).toHaveBeenCalledWith("advanced"); + + await user.click(screen.getByText("settings:sidebar.account.title")); + expect(onCategoryChange).toHaveBeenCalledWith("account"); + }); + + it("should call onCategoryChange when clicking the already active category", async () => { + const user = userEvent.setup(); + const onCategoryChange = vi.fn(); + render(); + + await user.click(screen.getByText("settings:sidebar.general.title")); + + expect(onCategoryChange).toHaveBeenCalledWith("general"); + }); + }); +}); diff --git a/boardgametracker.client/src/routes/settings/-components/SettingsSidebar.tsx b/boardgametracker.client/src/routes/settings/-components/SettingsSidebar.tsx index 42f05552..a95947b7 100644 --- a/boardgametracker.client/src/routes/settings/-components/SettingsSidebar.tsx +++ b/boardgametracker.client/src/routes/settings/-components/SettingsSidebar.tsx @@ -2,6 +2,7 @@ import { cx } from "class-variance-authority"; import { useTranslation } from "react-i18next"; import { BgtText } from "@/components/BgtText/BgtText"; +import { useAuth } from "@/hooks/useAuth"; export type SettingsCategory = "general" | "shelf-of-shame" | "game-nights" | "advanced" | "account"; @@ -47,11 +48,16 @@ interface Props { export const SettingsSidebar = ({ activeCategory, onCategoryChange, canManageSettings }: Props) => { const { t } = useTranslation(); + const authStatus = useAuth((s) => s.authStatus); + const showAccountTab = authStatus?.authEnabled; return (