Skip to content

ACHOO-146: AuthZ#29

Closed
jbickar wants to merge 21 commits intoACHOO-130-node-samlfrom
ACHOO-146
Closed

ACHOO-146: AuthZ#29
jbickar wants to merge 21 commits intoACHOO-130-node-samlfrom
ACHOO-146

Conversation

@jbickar
Copy link
Contributor

@jbickar jbickar commented Dec 11, 2025

NOT READY FOR REVIEW

  • (Edit the above to reflect status)

Summary

  • TL;DR - what's this PR for?

Review By (Date)

  • When does this need to be reviewed by?

Criticality

  • How critical is this PR on a 1-10 scale? Also see Severity Assessment.
  • E.g., it affects one site, or every site and product?

Review Tasks

Setup tasks and/or behavior to test

  1. Check out this branch
  2. Navigate to...
  3. Verify...

Front End Validation

  • Is the markup using the appropriate semantic tags and passes HTML validation?
  • Cross-browser testing has been performed?
  • Automated accessibility scans performed?
  • Manual accessibility tests performed?
  • Design is approved by @ user?

Backend / Functional Validation

Code

  • Are the naming conventions following our standards?
  • Does the code have sufficient inline comments?
  • Is there anything in this code that would be hidden or hard to discover through the UI?
  • Are there any code smells?
  • Are tests provided? eg (unit, behat, or codeception)

Code security

General

  • Is there anything included in this PR that is not related to the problem it is trying to solve?
  • Is the approach to the problem appropriate?

Affected Projects or Products

  • Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

  • JIRA ticket(s)
  • Other PRs
  • Any other contextual information that might be helpful (e.g., description of a bug that this PR fixes, new functionality that it adds, etc.)
  • Anyone who should be notified? (@mention them here)

Resources

Copilot AI review requested due to automatic review settings December 11, 2025 00:02
@vercel
Copy link

vercel bot commented Dec 11, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
churro Ready Ready Preview, Comment Dec 17, 2025 0:22am

@jbickar jbickar added the ai-generated Code in this PR was partially or wholly generated by AI label Dec 11, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a two-tier authorization system for the CHURRO application, adding both authentication and authorization controls on top of the existing SAML SSO integration. The implementation provides global access via eduPersonEntitlement attributes and per-application access via SUNet ID mappings.

Key Changes:

  • Middleware-level authorization for routes (dashboard and application pages)
  • API authorization wrapper for protecting API endpoints
  • Authorization utility functions for checking access permissions

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 21 comments.

Show a summary per file
File Description
middleware.ts Implements route-level authorization checks for dashboard and application pages, enforcing access control before page rendering
lib/auth-utils.ts Adds authorization utility functions including global access checks, per-application access validation, and dashboard access control
lib/api-auth.ts New file providing API authorization wrapper and helper functions for protecting API routes with authentication/authorization
app/api/acquia/applications/route.ts Wraps existing API route with authorization check to require authentication before accessing applications data
app/api/acquia/views/route.ts Wraps existing API route with authorization check to require authentication before accessing views metrics
app/api/acquia/visits/route.ts Wraps existing API route with authorization check to require authentication before accessing visits metrics
app/applications/page.tsx Converts to client component with authentication checks, loading states, and error handling for authorization failures
app/applications/[uuid]/page.tsx Adds authorization error handling to detect and display 403 responses with user-friendly error messages
app/auth/test/page.tsx Updates logout handler to support redirectTo parameter for improved test workflow
app/api/auth/logout/route.ts Adds support for redirectTo query parameter with validation to prevent open redirect vulnerabilities
docs/SAML.md Comprehensive documentation of the authorization system including configuration, components, and user experience
.github/copilot-instructions.md Updates AI assistance guidelines with authorization system architecture and usage patterns
README.md Adds authorization environment variables section to main documentation
.env.example Provides example configuration for global entitlements and per-app access mappings
.gitignore Adds .cache directory to ignored files
Comments suppressed due to low confidence (1)

app/api/acquia/views/route.ts:43

  • Inconsistent indentation. The if statement at line 28 should be indented at the same level as the previous if statement at line 20. This appears to be a result of wrapping the function with withApiAuthorization.
  if (!process.env.ACQUIA_API_KEY || !process.env.ACQUIA_API_SECRET) {
    console.error('❌ Missing required environment variables!');
    console.error('Available env vars:', Object.keys(process.env).filter(k => k.startsWith('ACQUIA')));
    return NextResponse.json(
      {
        error: 'Server configuration error: missing API credentials',
        envCheck: {
          ACQUIA_API_KEY: process.env.ACQUIA_API_KEY ? `${process.env.ACQUIA_API_KEY.substring(0, 8)}...` : 'missing',
          ACQUIA_API_SECRET: process.env.ACQUIA_API_SECRET ? 'present' : 'missing',
          ACQUIA_API_BASE_URL: process.env.ACQUIA_API_BASE_URL || 'missing',
          ACQUIA_AUTH_BASE_URL: process.env.ACQUIA_AUTH_BASE_URL || 'missing'
        }
      },
      { status: 500 }
    );
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jbickar
Copy link
Contributor Author

jbickar commented Jan 13, 2026

Implemented in 3.x

@jbickar jbickar closed this Jan 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-generated Code in this PR was partially or wholly generated by AI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant